Re: [OAUTH-WG] OAuth 2.1-03 - WG adoption?

[Sascha Preibisch <[e-mail]>]

Hello Dick!

Unless the two typos that I have mentioned should be updated beforehand ,
no, I do not.

Thank you,
Sascha

On Tue, 7 Jul 2020 at 16:36, Dick Hardt <[e-mail]> wrote:

> Thanks Sascha and Vladimir for the feedback!
>
> Sascha: did you have a concern with the document being adopted by the WG?
>
> ᐧ
>
> On Tue, Jul 7, 2020 at 4:04 PM Sascha Preibisch <[e-mail]>
> wrote:
>
>> Hi all!
>>
>> Here is the rest of my feedback. At the end I also included a list of
>> typos and the summary of changes that I have found between RFC 6739
>> and the current draft.
>>
>> Regards,
>> Sascha
>>
>> Section 2.3. Client Authentication
>> -------------------------
>>
>> Draft and current:
>> - both documents contain this description: "... the authorization
>> server (e.g., password, public/private key pair)"
>> - since the client usually uses a 'client secret', maybe this could be
>> worded as such:
>> - suggestion: "... the authorization server (e.g., client secret,
>> public/private key pair)"
>>
>> Draft:
>>
>> "The authorization server MAY establish a client authentication method
>> with public clients, which converts them to credentialed clients.
>> However, the authorization server MUST NOT rely on credentialed
>> client authentication for the purpose of identifying the client."
>>
>> - Does this mean that credentialed clients are as trustworthy/ not
>> trustworthy as public clients?
>>
>> Draft:
>>
>> "Clients in possession of a client password, also known as a client
>> secret, ..."
>>
>> - Maybe this could simply be changed to:
>> "Clients in possession of a client secret ..."
>>
>> Section 3.1.2.2 Registration Requirements
>> -------------------------
>>
>> Draft:
>>
>> "Lack of requiring registration of redirect URIs enables an attacker
>> to use the authorization endpoint as an open redirector as described
>> in <a href="#section-9.18">Section 9.18</a>."
>>
>> - is that still required since redirect_uris have to be pre-registered
>> now?
>>
>> Section 4.1. Authorization Code Grant
>> -------------------------
>>
>> Draft:
>> - Figure 3, step (1), does not include 'code_challenge_method' Is that
>> intentionally?
>> - I am suggesting to include it to avoid potential questions and
>> confusion. It could listed as 'optional' as 'scope' is
>> - In addition, when referencing parameters, they should be spelled as
>> used in the protocol, i.e.: 'code_challenge' instead of
>> 'code_challenge'
>>
>> Section 4.1.1 Authorization Request
>> -------------------------
>>
>> Draft:
>>
>> "Clients use a unique secret per authorization request to protect .... "
>>
>> - It would be less confusing if this section would start of with "PKCE
>> is required"
>> - Introducing a '... unique secret per ...' sounds like something very
>> new although this is referencing PKCE
>> - Suggestion (along the lines of):
>> "Clients MUST leverage PKCE per authorization request to protect ..."
>>
>> Section 4.1.2.1 Error Response
>> -------------------------
>>
>> Draft:
>>
>> "An AS MUST reject requests without a "code_challenge" from public
>> clients, and MUST reject such requests from other clients  unless
>> there is reasonable assurance that ..."
>>
>> - These statements are the ones that cause discussions between
>> developers and/ or third parties
>> - ' ... unless ...' is very difficult to grasp, even when looking into
>> section 9.8
>> - I would suggest to make it required
>>
>> Section 5.1 Successful Response
>> -------------------------
>>
>> Draft and current:
>>
>> - both documents describe the refresh_token response parameter and
>> describe it as such:
>> "OPTIONAL.  The refresh token, which can be used to obtain new access
>> tokens using the same authorization grant as described in <a
>> href="#section-6">Section 6</a>"
>>
>> - As an enhancement, I suggest this update:
>> "OPTIONAL.  The refresh token, which can be used to obtain new access
>> tokens using the grant type "refresh_token" as described in <a
>> href="#section-6">Section 6</a>"
>>
>> Section 6. Refreshing an Access Token
>> -------------------------
>>
>> Draft:
>>
>> Authorization servers SHOULD determine, based on a risk assessment,
>> whether to issue refresh tokens to a certain client.  If the
>> authorization server decides not to issue refresh tokens, the client
>> MAY refresh access tokens by utilizing other grant types, such as the
>> authorization code grant type.  In such a case, the authorization
>> server may utilize cookies and persistent grants to optimize the user
>> experience.
>>
>> - That paragraph sounds like a general advice for web developers and
>> should appear in an appendix for my taste
>> - ' ... based on risk assessment ... ' may exclude any implementation
>> that does not have such capabilities
>>
>> ===
>>
>> Draft:
>>
>> - this section includes this statement:
>> "Confidential or credentialed clients MUST authenticate with the
>> authorization server ..."
>>
>> - section 2.3 includes this statement and makes me wonder how
>> confident an authorization server can be when working with
>> 'credentialed' clients':
>> "However, the authorization server MUST NOT rely on credentialed
>> client authentication for the purpose of identifying the client."
>>
>> - Any clarification, I would say about the client type 'credentialed'
>> in general, would be helpful
>>
>> -------------------------
>> Typos:
>> -------------------------
>>
>> Section 2.1. Client Types
>> -------------------------
>>
>> Draft:
>>
>> "credentialed":  Clients that have credentials and their identity has
>> been not been confirmed by the AS are designated as "credentialed
>> clients"
>>
>> - I believe it should be:
>>
>> "credentialed":  Clients that have credentials and their identity has
>> not been confirmed by the AS are designated as "credentialed clients"
>>
>> Section 3.2.1 Client Authentication
>> -------------------------
>>
>> Draft:
>>
>> "Confidential or credentialed clients client MUST authenticate with..."
>>
>> - I believe it should be:
>> "Confidential or credentialed clients MUST authenticate with..."
>>
>> -------------------------
>> Summary of changes between draft and current:
>> -------------------------
>>
>> - no more implicit
>> - no more response_type=token
>> - no more ropc
>> - no more redirect code 307
>> - no more open redirect_uri
>> - new client type 'credentialed'
>> - must use PKCE (with few exceptions)
>> - AS must provide a way to show their support for 'code_challenge_method'
>>
>> - refresh token should expire
>> - description for client type 'confidential' got updated
>> - clients should not be able to choose their client_id
>> - no reference to 'mac' token profile anymore
>> - section 7.2 details on Bearer token
>> - resource server must include 'WWW-Authenticate: Bearer
>> realm="example"' header for failing authorization
>> - extended list of security threats
>> - discussion on native apps removed
>> - recommended bindings between access_token and resource_server
>> - recommended refresh_token rotation or sender-constraints
>> - recommended to use '127.0.0.1' instead of 'localhost'
>>
>> On Tue, 7 Jul 2020 at 15:21, Vladimir Dzhuvinov <[e-mail]>
>> wrote:
>> >
>> > I find 03 well structured, well written and it shows that a lot of
>> thought and work has gone into it.
>> >
>> > If this is a formal call for adoption - I support it.
>> >
>> >
>> > - defined new client type - credentialed clients - a client that has
>> credentials, but the AS has not confirmed the identity of the client.
>> Confidential clients have had their identity confirmed by the AS. We talked
>> about changing the names of confidential and public, but thought that would
>> be confusing. This new definition cleans up the text substantially.
>> >
>> > I understand why this new client type was introduced. For the reader
>> who is not familiar with the recent OAuth RFCs and drafts - I suspect this
>> can still be confusing. There will likely be questions -- Why does this
>> difference between confidential and credentialed matter? What is a concrete
>> example of a credentialed client?
>> >
>> > Also, people will likely ask themselves - what does the confirmation of
>> a (credentialed) client's identity by the AS actually mean and do? (section
>> 2.1)
>> >
>> >
>> >    Authorization servers SHOULD consider the level of confidence in a
>> >    client's identity when deciding whether they allow such a client
>> >    access to more critical functions, such as the Client Credentials
>> >    grant type.
>> >
>> > Again, normative text that relies on the implementer being able to
>> assign levels of confidence in the client's identity, but is not
>> immediately obvious how to go about this.
>> >
>> >
>> > There is mention in 9.1 about "enlisting the resource owner to confirm
>> identity" and "if there is a web application whose developer's identity was
>> verified...". But this talk about client identity is secondary and happens
>> in the context of client authentication.
>> >
>> > Perhaps it will make sense to promote the discussion about identity to
>> a new 9.x section "Client identity" or "Client Identification", before
>> "Client Authentication". Addressing the topics what client identity is, why
>> does it matter (especially for security), and the "confirmation by the AS".
>> Then proceed with "Client Authentication" which now is freed to focus on
>> the credentials / auth methods aspects.
>> >
>> >    Such credentials are either issued by the
>> >    authorization server or registered by the developer of the client
>> >    with the authorization server.
>> >
>> > Credentials (public key) can also be registered by a client performing
>> dynamic registration (section 2.1)
>> >
>> >
>> > Vladimir
>> >
>> > _______________________________________________
>> > OAuth mailing list
>> > [e-mail]
>> > https://www.ietf.org/mailman/listinfo/oauth
>>
>> _______________________________________________
>> OAuth mailing list
>> [e-mail]
>> https://www.ietf.org/mailman/listinfo/oauth
>>
>