[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