[Openid-specs-heart] Feedback on the HEART OAuth and OpenID Connect profiles from Mike Jones

Debbie Bucci debbucci at gmail.com
Sun May 8 10:35:18 UTC 2016


Thanks much Mike for the through read and feedback.
On May 8, 2016 1:28 AM, "Mike Jones" <Michael.Jones at microsoft.com> wrote:

> The 27 review feedback items below are based upon a complete read of the
> HEART OAuth and OpenID Connect profiles during the Implementer’s Draft
> review follows.  I believe that all the comments equally apply to the
> current working group drafts.  I’ve used links to the Implementer’s Drafts
> below since these are stable references to immutable versions.  They are
> intended to make the profiles clearer, more easily understood, more easily
> implemented, and more secure.
>
>
>
> *OAuth Profile - http://openid.net/specs/openid-heart-oauth2-1_0-ID1.html
> <http://openid.net/specs/openid-heart-oauth2-1_0-ID1.html>*
>
>
>
> http://openid.net/specs/openid-heart-oauth2-1_0-ID1.html#rfc.section.2.1.1
>
> 2.1.1.
> <http://openid.net/specs/openid-heart-oauth2-1_0-ID1.html#rfc.section.2.1.1> Full
> Client with User Delegation
> <http://openid.net/specs/openid-heart-oauth2-1_0-ID1.html#FullClient>
>
>
>
> (1)  Why is this called a “full client”?  Wouldn’t a more descriptive name
> be something more aligned with OAuth naming, like “Client using
> Authorization Code Flow” or with OpenID Connect naming, like “Basic Client”?
>
>
>
> (2)  Especially in light of the OAuth mix-up attacks, wouldn’t it be
> better for these clients to use, or at least be allowed to use,
> “response_type=id_token code”, rather than be required to use
> “response_type=code”?
>
>
>
> (3)  What is the “unique key” that is RECOMMENDED used for, how is it
> used, and what restrictions are there on the key type?  For instance, is it
> assumed to be an asymmetric key?  Are there required or recommended
> algorithms to use, such as “RS256”?  The spec should say more about this in
> the interests of both interoperability and clarity.
>
>
>
> (4)  What are the implications of not having the “unique key” (not
> following the RECOMMENDED)?  Why is this RECOMMENDED, rather than
> REQUIRED?  For one thing, not having this key would seem to conflict with
> the requirement in 2.2 to use private_key_jwt client authentication.
>
>
>
> http://openid.net/specs/openid-heart-oauth2-1_0-ID1.html#rfc.section.2.1.2
>
> 2.1.2.
> <http://openid.net/specs/openid-heart-oauth2-1_0-ID1.html#rfc.section.2.1.2> Browser-embedded
> Client with User Delegation
> <http://openid.net/specs/openid-heart-oauth2-1_0-ID1.html#BrowserClient>
>
>
>
> (5)  Why is this called “Browser-embedded Client” when this flow could
> also be used by non-browser applications?  Why not follow the OAuth and
> Connect naming conventions and call this an “Implicit Client”?
>
>
>
> http://openid.net/specs/openid-heart-oauth2-1_0-ID1.html#rfc.section.2.1.3
>
> 2.1.3.
> <http://openid.net/specs/openid-heart-oauth2-1_0-ID1.html#rfc.section.2.1.3> Direct
> Access Client
> <http://openid.net/specs/openid-heart-oauth2-1_0-ID1.html#DirectClient>
>
>
>
> (6)  What is the use case behind the decision for the profile to include
> use of the Client Credentials flow, especially when this bypasses user
> involvement and user consent?  Is including this actually necessary?  Many
> servers don’t support this, by design.
>
>
>
> http://openid.net/specs/openid-heart-oauth2-1_0-ID1.html#rfc.section.2.2
>
> 2.2.
> <http://openid.net/specs/openid-heart-oauth2-1_0-ID1.html#rfc.section.2.2> Requests
> to the Token Endpoint
> <http://openid.net/specs/openid-heart-oauth2-1_0-ID1.html#RequestsToTokenEndpoint>
>
>
>
> (7) The text “MAY use other asymmetric signature methods listed in the
> JSON Web Algorithms (JWA
> <http://openid.net/specs/openid-heart-oauth2-1_0-ID1.html#RFC7518>
> [RFC7518]) specification” should actually refer to the IANA JSON Web
> Signature and Encryption Algorithms registry at
> http://www.iana.org/assignments/jose/jose.xhtml#web-signature-encryption-algorithms
> – not the JWA spec.
>
>
>
> (8)  The paragraph including “Authorization servers MAY require some
> clients to additionally authenticate using mutual Transport Layer Security
> (TLS) authentication” is problematic in several ways.  First, how does
> the server communicate to the client that mutual TLS is required?  How is
> the client TLS key registered with the server?  Is support for mutual TLS
> required of servers?  Of clients?  If it’s not required, how is interop
> assured when it is used?
>
>
>
> http://openid.net/specs/openid-heart-oauth2-1_0-ID1.html#rfc.section.3
>
> 3.
> <http://openid.net/specs/openid-heart-oauth2-1_0-ID1.html#rfc.section.3> Client
> Registration
> <http://openid.net/specs/openid-heart-oauth2-1_0-ID1.html#ClientRegistration>
>
>
>
> (9)  The profile requires that “each client instance MUST receive a
> unique client identifier from the authorization server”.  This isn’t how
> most OAuth deployments work today – particular for native applications.  Is
> this significant departure from existing practice truly necessary or is
> this just a preference?  If it’s necessary, the specification should
> clearly say why.  If it’s not truly necessary, this requirement should be
> downgraded to being a recommendation.
>
>
>
> http://openid.net/specs/openid-heart-oauth2-1_0-ID1.html#rfc.section.3.2
>
> 3.2.
> <http://openid.net/specs/openid-heart-oauth2-1_0-ID1.html#rfc.section.3.2> Dynamic
> Registration
> <http://openid.net/specs/openid-heart-oauth2-1_0-ID1.html#DynamicRegistration>
>
>
>
> (10)  The profile requires the use of Dynamic Registration.  This is
> another significant departure from existing practice.  Like the previous
> comment, this requirement either needs to be justified as being essential
> or downgraded to being a recommendation.
>
>
>
> (11)  The profile indicates that servers MAY accept software statements.
> How does the server communicate to clients that it supports software
> statements or not?  How does it communicate whether their use by clients is
> required or not?  How does it communicate to developers and clients what
> kinds of software statements will be accepted and rejected?
>
>
>
> (12)  The profile requires that authorization servers “MUST indicate to
> the end user that such a statement was used in the client's registration”.
> This seems beyond silly, as most normal people would have no clue what such
> a message means or how they should respond to it (or the lack of it).
> Requiring another “Click OK” barrier in the UI that people won’t understand
> and will just click through won’t help either usability or security.
>
>
>
> http://openid.net/specs/openid-heart-oauth2-1_0-ID1.html#rfc.section.4
>
> 4.
> <http://openid.net/specs/openid-heart-oauth2-1_0-ID1.html#rfc.section.4> OAuth
> 2.0 Server Profile
> <http://openid.net/specs/openid-heart-oauth2-1_0-ID1.html#ServerProfile>
>
>
>
> (13)  The profile requires that “The authorization server MUST limit each
> registered client (identified by a client ID) to a single grant type only”.
> This may or may not be fine but doesn’t match existing practice for many
> implementations and the requirement isn’t justified in the specification.
> At a minimum, clear rationale for this requirement needs to be included.
>
>
>
> http://openid.net/specs/openid-heart-oauth2-1_0-ID1.html#rfc.section.4.1
>
> 4.1.
> <http://openid.net/specs/openid-heart-oauth2-1_0-ID1.html#rfc.section.4.1>
> Discovery
> <http://openid.net/specs/openid-heart-oauth2-1_0-ID1.html#Discovery>
>
>
>
> (14)  The profile requires support for the introspection endpoint.  This
> unnecessarily increases the implementation footprint of both clients and
> servers, increases run-time cost of common operations, and is a significant
> deviation from most deployments.  If, for instance, the UMA profile
> requires introspection for some reason, add it there, but do not require
> HEART OAuth or Connect to support introspection.  This must be removed from
> the OAuth profile.
>
>
>
> (15)  The profile requires support for the revocation endpoint.  This
> unnecessarily increases the implementation footprint of both clients and
> servers and is a significant deviation from most deployments.  If, for
> instance, the UMA profile requires revocation for some reason, add it
> there, but do not require HEART OAuth or Connect to support revocation.
> This must be removed from the OAuth profile.
>
>
>
> (16)  The profile states “It is RECOMMENDED that servers provide cache
> information through HTTP headers and make the cache valid for at least one
> week.”  How does this interact with the need to immediately rotate keys
> upon key compromise?  Is this saying that a compromised key must remain
> valid for at least a week?  If this isn’t implied by this recommendation,
> please explain what is actually implied for clients and servers.
>
>
>
> http://openid.net/specs/openid-heart-oauth2-1_0-ID1.html#rfc.section.4.2
>
> 4.2.
> <http://openid.net/specs/openid-heart-oauth2-1_0-ID1.html#rfc.section.4.2> JWT
> Bearer Tokens
> <http://openid.net/specs/openid-heart-oauth2-1_0-ID1.html#JWTBearerTokens>
>
>
>
> (17)  The spec is unclear what kinds of bearer tokens are being referred
> to here.  Is it saying that access tokens must be JWTs?  Is it saying that
> refresh tokens must be JWTs?  This is certainly a reasonable implementation
> choice but the specification does not clearly state why this is a
> requirement.  Please say why bearer tokens must be JWTs.
>
>
>
> (18)  The profile says that an audience may be included.  Why is the
> audience not required?  What are the implications of sending and accepting
> bearer tokens that aren’t audience?
>
>
>
> (19)  The profile requires including “azp” in JWT bearer tokens.  It’s now
> widely held by the OpenID Connect working group that “azp” is both
> underspecified and its inclusion in OpenID Connect was a mistake.  (See
> https://bitbucket.org/openid/connect/issues/973/core-2-3137-azp-claim-underspecified-and
> - Core 2 / 3.1.3.7 - azp claim underspecified and overreaching.)  Please
> remove all uses of “azp” from the HEART specifications.  If the client ID
> needs to be conveyed, an appropriate new well-specified claim should be
> defined and used for this purpose.
>
>
>
> http://openid.net/specs/openid-heart-oauth2-1_0-ID1.html#rfc.section.5
>
> 5.
> <http://openid.net/specs/openid-heart-oauth2-1_0-ID1.html#rfc.section.5> Requests
> to the Protected Resource
> <http://openid.net/specs/openid-heart-oauth2-1_0-ID1.html#RequestsToProtectedResource>
>
>
>
> (20)  The profile says that resources may support the query-parameter
> parameter method from RFC 6750.  This is a terrible security mistake.
> Change the text to say that “Resources MUST NOT support the query-parameter
> parameter method from RFC 6750”.
>
>
>
> *OpenID Connect Profile -
> http://openid.net/specs/openid-heart-openid-connect-1_0-ID1.html
> <http://openid.net/specs/openid-heart-openid-connect-1_0-ID1.html>*
>
>
>
>
> http://openid.net/specs/openid-heart-openid-connect-1_0-ID1.html#rfc.section.2
>
> 2.
> <http://openid.net/specs/openid-heart-openid-connect-1_0-ID1.html#rfc.section.2> ID
> Tokens
> <http://openid.net/specs/openid-heart-openid-connect-1_0-ID1.html#IDTokens>
>
>
>
> (21)  The profile says that “The ID Token MUST expire and SHOULD have an
> active lifetime no longer than five minutes”.  While this expiration time
> may be a reasonable implementation choice, the specification is silent on
> why this reasonably short expiration time is a requirement.  Please either
> justify this requirement or downgrade this text to a non-normative
> discussion on criteria for choosing appropriate ID Token lifetimes.
>
>
>
>
> http://openid.net/specs/openid-heart-openid-connect-1_0-ID1.html#rfc.section.3
>
> 3.
> <http://openid.net/specs/openid-heart-openid-connect-1_0-ID1.html#rfc.section.3> UserInfo
> Endpoint
> <http://openid.net/specs/openid-heart-openid-connect-1_0-ID1.html#UserInfoEndpoint>
>
>
>
> (22)  The profile requires supporting signed UserInfo responses.  This may
> be a reasonable thing for the profile to require but the specification
> fails to motivate this requirement.  Please do so.
>
>
>
> (23)  The profile talks about encrypted UserInfo responses but doesn’t say
> whether support for these is required for clients or servers, or why.
> Please do so.
>
>
>
>
> http://openid.net/specs/openid-heart-openid-connect-1_0-ID1.html#rfc.section.4
>
> 4.
> <http://openid.net/specs/openid-heart-openid-connect-1_0-ID1.html#rfc.section.4> Request
> Objects
> <http://openid.net/specs/openid-heart-openid-connect-1_0-ID1.html#RequestObjects>
>
>
>
> (24)  The profile requires that servers support request objects.  This may
> be a reasonable thing for the profile to require but the specification
> fails to motivate this requirement.  Please do so.
>
>
>
>
> http://openid.net/specs/openid-heart-openid-connect-1_0-ID1.html#rfc.section.5
>
> 5.
> <http://openid.net/specs/openid-heart-openid-connect-1_0-ID1.html#rfc.section.5> Authentication
> Context
> <http://openid.net/specs/openid-heart-openid-connect-1_0-ID1.html#AuthenticationContext>
>
>
>
> (25)  The profile uses URIs for FICAM LOA numbers as “acr” values.  This
> seems short-signed and limiting, as these values are specific to the United
> States and not international in nature.  These must be changed to
> appropriate international equivalents – especially in coordination with
> other working groups, such as MODRNA.
>
>
>
> (26)  Remove the requirement to support the introspection endpoint from
> this profile, for the same reasons as in (14).
>
>
>
> (27)  Remove the requirement to support the revocation endpoint from this
> profile, for the same reasons as in (15).
>
>
>
>                                                           Thanks all,
>
>                                                           -- Mike]
>
>
>
> _______________________________________________
> Openid-specs-heart mailing list
> Openid-specs-heart at lists.openid.net
> http://lists.openid.net/mailman/listinfo/openid-specs-heart
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openid.net/pipermail/openid-specs-heart/attachments/20160508/8abb2302/attachment.html>


More information about the Openid-specs-heart mailing list