Re: [OAUTH-WG] Shepherd Review of draft-ietf-oauth-security-topics-23

[Daniel Fett <[e-mail]>]

Hi Axel,

It is to be expected that not all AS will immediately upgrade to adhere 
to the security BCP after its release. So a client who wants to use PKCE 
may encounter AS that don't support it.

See also the discussion in 
https://mailarchive.ietf.org/arch/msg/oauth/ZiwEfenZZlboikXxBLes5ebPmBw/

-Daniel

Am 03.01.24 um 17:39 schrieb [e-mail]:
>
> Hi Daniel,
>
> there is also this sentence in this section 
> https://oauthstuff.github.io/draft-ietf-oauth-security-topics/draft-ietf-oauth-security-topics.html#name-authorization-code-grant 
> in a paragraph on it own.
>
> "Authorization servers MUST support PKCE [RFC7636 
> <https://oauthstuff.github.io/draft-ietf-oauth-security-topics/draft-ietf-oauth-security-topics.html#RFC7636>]."
>
> Why must a client "ensure" that the AS supports PKCE if the security 
> best practices say the AS MUST support PKCE?
>
> //Axel
>
> *From: *OAuth <[e-mail]> on behalf of Daniel Fett 
> <mail=[e-mail]>
> *Date: *Wednesday, 3. January 2024 at 14:01
> *To: *[e-mail] <[e-mail]>
> *Subject: *Re: [OAUTH-WG] Shepherd Review of 
> draft-ietf-oauth-security-topics-23
>
> Hi Axel,
>
> I would be happy to see OAuth move away from state as a CSRF 
> protection mechanism in the future, but there is not too much to be 
> gained from relying solely on PKCE right now. The main advantage is 
> that relying on PKCE incentivizes clients to properly manage the 
> session state in a cookie instead of relying on a parameter. Beyond 
> that, there's a small reduction in effort required by the client, 
> messages will be smaller messages, etc.. The disadvantage is that a 
> client puts CSRF protection in the hands of the AS. Therefore, we 
> chose the wording "ensure" to say that the client has be sure that the 
> AS actually implements PKCE correctly before relying on it. What that 
> means in the concrete instance is up to the client.
>
> Likewise, to your second point, I do not see enough of an advantage to 
> RECOMMEND relying solely on PKCE for CSRF protection.
>
> The main intention here is to open the door to rely on PKCE, e.g., in 
> closed ecosystems, ecosystems with in-depth conformance testing, 
> first-party applications and similar. This also helps to avoid a lot 
> of convoluted language telling client developers how to properly 
> choose, track, and check state values in profiles such as FAPI (and 
> the pitfalls when interpreting that language).
>
> -Daniel
>
> Am 02.01.24 um 10:31 schrieb [e-mail]:
>
>     Hi,
>
>     sorry for being late in the game.
>
>     I am not too happy with this section:
>
>     "Clients that have ensured that the authorization server supports
>     Proof Key for Code Exchange (PKCE, [RFC7636
>     <https://oauthstuff.github.io/draft-ietf-oauth-security-topics/draft-ietf-oauth-security-topics.html#RFC7636>])
>     MAY rely on the CSRF protection provided by PKCE."
>
>      1. Maybe a minor point that is due to not being a native speaker,
>         but the verb "ensure" seems too strong.
>         If the AZ states in its metadata, that it supports PKCE than
>         this is "ensurance" enough, right? The client does not have to
>         "ensure" the support by actually testing compliance, right?
>         I suggest rephrasing that to "*If**the authorization server
>         **states in its meta-data support for**Proof Key for Code
>         Exchange* (PKCE, [RFC7636
>         <https://oauthstuff.github.io/draft-ietf-oauth-security-topics/draft-ietf-oauth-security-topics.html#RFC7636>])
>         the client MAY rely on the CSRF protection provided by PKCE."
>      2. I suggest changing the "MAY" into a recommendation for all
>         OAuth2-based protocols. OIDC flows can easily support PKCE,
>         and new clients SHOULD use PKCE, I think.
>         Suggestion:
>         "If the authorization server states in its meta-data support
>         for Proof Key for Code Exchange (PKCE, [RFC7636
>         <https://oauthstuff.github.io/draft-ietf-oauth-security-topics/draft-ietf-oauth-security-topics.html#RFC7636>]),
>         it is RECOMMENDED the client relies on the CSRF protection
>         provided by PKCE."
>
>     Kind regards
>
>     Axel
>
>     *From: *OAuth <[e-mail]>
>     <mailto:[e-mail]> on behalf of Daniel Fett
>     <fett=[e-mail]>
>     <mailto:fett=[e-mail]>
>     *Date: *Thursday, 28. December 2023 at 14:38
>     *To: *[e-mail] <[e-mail]> <mailto:[e-mail]>
>     *Subject: *Re: [OAUTH-WG] Shepherd Review of
>     draft-ietf-oauth-security-topics-23
>
>     Hi Hannes,
>
>     thanks again for your feedback! It is incorporated in the editor's
>     copy now.
>
>     -
>     https://oauthstuff.github.io/draft-ietf-oauth-security-topics/draft-ietf-oauth-security-topics.html
>
>     - Diff to published version:
>     https://author-tools.ietf.org/api/iddiff?doc_1=draft-ietf-oauth-security-topics&url_2=https://oauthstuff.github.io/draft-ietf-oauth-security-topics/draft-ietf-oauth-security-topics.txt
>     <https://author-tools.ietf.org/api/iddiff?doc_1=draft-ietf-oauth-security-topics&url_2=https://oauthstuff.github.io/draft-ietf-oauth-security-topics/draft-ietf-oauth-security-topics.txt>
>
>     I plan to publish the next version once we have resolved the
>     discussion points from Roman's AD review.
>
>     -Daniel
>
>     Am 04.10.23 um 15:41 schrieb Tschofenig, Hannes:
>
>         Hi all,
>
>         here are some comments as part of my shepherd review of the
>         OAuth Security BCP.
>
>         First, I want to send a big "Thanks" to everyone in the group
>         for the work on this document and to the authors in
>         particular. It has taken us a while to come up with such an
>         impressive list of security recommendations for OAuth 2.0.
>
>         At this point in time my review comments can only be minor
>         given the amount of feedback this documents has already received.
>
>         Here are a few remarks.
>
>         I believe we should indicate that the specification updates
>         other OAuth RFCs. The obvious documents it updates are RFC
>         6749, RFC 6750 and RFC 6819.
>
>         You can set these "updates" in the template you are using.
>
>         In Section 1 you say:
>
>         "
>
>         It does not supplant the security advice given in
>
>         [RFC6749], [RFC6750], and [RFC6819], but complements those
>         documents.
>
>         "
>
>         In the subsequent paragraph you state that you "depreciate
>         some modes of operation".
>
>         I believe you are need to be clear about what you are doing in
>         relationship to these prior documents. It might also be useful
>         to say something about OAuth 2.1.
>
>         Expand abbreviations on first use. Example: "AS" and "PKCE" in
>         Section 2.1. The AS abbreviation is only expanded later in
>         Section 3. Decide whether you want to use abbreviations or
>         not. You mix them throughout the document without no reasons.
>
>         Listing the abbreviations in Section 1.2 may also be useful.
>         There are not that many abbreviations anyway.
>
>         I have wording suggestions for this paragraph:
>
>         FROM:
>
>         "
>
>         Authorization servers SHOULD use client authentication if
>         possible.
>
>            It is RECOMMENDED to use asymmetric (public-key based)
>         methods for
>
>         client authentication such as mTLS [RFC8705] or using signed JWTs
>
>         ("Private Key JWT") in accordance with [RFC7521] and [RFC7523] (in
>
>         [OpenID.Core] defined as the client authentication method
>
>         private_key_jwt).  When such methods for client authentication are
>
>            used, authorization servers do not need to store sensitive
>         symmetric
>
>            keys, making these methods more robust against a number of
>         attacks.
>
>         "
>
>         TO:
>
>         "
>
>         Authorization servers SHOULD enforce client authentication, if
>         possible.
>
>            It is RECOMMENDED to use asymmetric cryptography for
>
>         client authentication, such as mTLS [RFC8705] or using signed JWTs
>
>         ("Private Key JWT"), in accordance with [RFC7521] and
>         [RFC7523] (in
>
>         [OpenID.Core] defined as the client authentication method
>
>         private_key_jwt).  When asymmetric cryptography for client
>         authentication is
>
>            used, authorization servers do not need to store sensitive
>         symmetric
>
>            keys, making client authentication more robust against
>         leakage of keys.
>
>         "
>
>         (Note: For the reader it is always better if they are told
>         what attacks
>
>         are prevented rather than saying "a number of attacks". You
>         don't want the reader
>
>         to guess what you mean.)
>
>         Section 2 is a summary of what follows in Section 4. Maybe you
>         can make this explicit
>
>         either in the title of Section 2 or in the first paragraph of
>         Section 2.
>
>         Section 3.
>
>         You write:
>
>         "
>
>            These attackers conform to the attacker model that was used
>         in formal
>
>         analysis efforts for OAuth [arXiv.1601.01229].  This is a minimal
>
>         attacker model.  Implementers MUST take into account all possible
>
>            types of attackers in the environment in which their OAuth
>
>         implementations are expected to run.
>
>         "
>
>         When you say "these attackers" please clarify which attackers
>         you are talking about.
>
>         Prior to this paragraph you have just spoken about various
>         forms of network attackers.
>
>         Just before that you talked about network and web attackers.
>
>         Then, you introduce more attackers and you keep talking about
>         "this attacker model" and
>
>         "these attackers". Make it easier for the reader by referring
>         explictly which attackers
>
>         you are talking about in a specific paragraph.
>
>         Then, you conclude the section with a hint that there is an
>         even stronger attacker model.
>
>         As a reader I might want to know what this stronger attacker
>         model looks like and why you
>
>         do not consider it in this document.
>
>         Section 4.1.1:
>
>         You write:
>
>         "
>
>         Note: Vulnerabilities of this kind can also exist if the
>
>         authorization server handles wildcards properly.
>
>         "
>
>         I believe you are saying that the vulnerabilities caused by
>         incorrect redirect URI validation parsing when you refer to
>         "this kind".
>
>         I would also remove the "note"
>
>         Section 4.1.3:
>
>         You write:
>
>         "
>
>            * Servers on which callbacks are hosted MUST NOT expose open
>
>         redirectors (see Section 4.11).
>
>         "
>
>         Are you talking about authorization servers (which is what was
>         referenced in the paragraph before)?
>
>         Section 4.10.1: Sender-constrained Access Tokens
>
>         The text gives the reader the impression that the token
>         binding would be an option for developers to use.
>
>         I don't think that this is the case. I am particularly
>         referring to this sentence:
>
>         "
>
>            * *DPoP* ([I-D.ietf-oauth-dpop]): DPoP (Demonstration of
>         Proof-of-
>
>         Possession at the Application Layer) outlines an application-level
>
>         sender-constraining for access and refresh tokens that can be used
>
>               in cases where neither mTLS nor OAuth Token Binding (see
>         below)
>
>         are available.
>
>         "
>
>         I would change it to:
>
>         "
>
>            * *DPoP* ([I-D.ietf-oauth-dpop]): DPoP (Demonstration of
>         Proof-of-
>
>         Possession at the Application Layer) outlines an application-level
>
>         sender-constraining for access and refresh tokens that can be used
>
>               in cases where mTLS is not available.
>
>         "
>
>         I would then remove the subsequent text talking about old,
>         expired drafts.
>
>         Alternatively, you could move the text to the appendix.
>
>         Section 4.10.2: Audience Restricted Access Tokens
>
>         In the text you say:
>
>         "
>
>         Audience restriction essentially restricts access tokens to a
>
>         particular resource server.  The authorization server
>         associates the
>
>         access token with the particular resource server and the resource
>
>         server SHOULD verify the intended audience.
>
>         "
>
>         You have to put a MUST here. If the resource server does not
>         check the audience
>
>         restriction when using audience restricted access tokens then
>         you obviously do not
>
>         get the value from it. It is like using DPOP and not using the
>         proof-of-possession.
>
>         Likewise the SHOULD language in this sentence is also
>         questionable:
>
>         "
>
>         The client SHOULD tell the authorization server the intended
>         resource
>
>         server.  The proposed mechanism [RFC8707] could be used or by
>
>         encoding the information in the scope value.
>
>         "
>
>         If the client does not tell the authorization server what the
>         intended resource server
>
>         is then how should the authorization server know (unless in a
>         very limited setup).
>
>         Also the reference to RFC 8707 is a bit weak. We standardized
>         resource indicators: why not
>
>         recommend using it?
>
>         Section 4.10.3: The section heading is "Discussion: Preventing
>         Leakage via Metadata".
>
>         The content of the section is not really a discussion but
>         rather a description of why
>
>         this path has not been taken. I wonder whether it would be
>         better to move this section
>
>         to the appendix and then start the text by explaining why
>         other solutions have been used instead of this approach.
>
>         Section 4.11: I would put the definition about what an "open
>         redirector" is into the terminology section since you
>
>         are using the term already in earlier sections. Here is the
>         definition:
>
>         "
>
>         An open redirector is an endpoint that forwards a user’s
>
>         browser to an arbitrary URI obtained from a query parameter.
>
>         "
>
>         Typos/Wording:
>
>         FROM:
>
>         "
>
>         Afterwards, the updated the OAuth attacker model is presented.
>
>         "
>
>         TO:
>
>         "
>
>         Afterwards, the updated OAuth attacker model is presented.
>
>         "
>
>         Section 4.1:
>
>         "... wild ."
>
>         ^
>
>         Consider using the guidelines for inclusive language:
>
>         https://www.rfc-editor.org/part2/#inclusive_language
>         <https://www.rfc-editor.org/part2/#inclusive_language>
>
>         For example, "If the attacker is able to ... , **he** will
>         directly get access to ..."
>
>         Another example is "whitelisted".
>
>         Section 4.1.2: a wording suggestion.
>
>         FROM:
>
>         "
>
>         The attack
>
>         described here combines this behavior with the client as an open
>
>         redirector (see Section 4.11.1) in order to get access to access
>
>         tokens.
>
>         "
>
>         TO:
>
>         "
>
>         The attack
>
>         described here combines this behavior with the client as an open
>
>         redirector (see Section 4.11.1) to obtain access tokens.
>
>         "
>
>         Section 4.7.1: word missing
>
>         FROM:
>
>         "
>
>         PKCE provides robust protection against CSRF attacks even in
>         presence
>
>            of an that can read the authorization response (see
>         Attacker A3 in
>
>         Section 3).
>
>         "
>
>         TO:
>
>         "
>
>         PKCE provides robust protection against CSRF attacks even in
>         presence
>
>            of an attacker that can read the authorization response
>         (see Attacker A3 in
>
>         Section 3).
>
>         "
>
>         Section 4.18.2: capitalization
>
>         FROM:
>
>         "
>
>         Wildcard origins like "*" in postMessage MUST not be used as
>
>         attackers can use them to leak a victim's in-browser message to
>
>         malicious origins.
>
>         "
>
>         TO:
>
>         "
>
>         Wildcard origins like "*" in postMessage MUST NOT be used as
>
>         attackers can use them to leak a victim's in-browser message to
>
>         malicious origins.
>
>         "
>
>         You might also want to replace the short title
>         "oauth-security-topics" (which can be found on each page) with
>         something like "OAuth 2.0 Security BCP".
>
>         Ciao
>
>         Hanns
>
>         _______________________________________________
>
>         OAuth mailing list
>
>         [e-mail]
>
>         https://www.ietf.org/mailman/listinfo/oauth
>
>     -- 
>
>     Please use my new email address:[e-mail]
>
>
>
>     _______________________________________________
>
>     OAuth mailing list
>
>     [e-mail]
>
>     https://www.ietf.org/mailman/listinfo/oauth
>
>
> _______________________________________________
> OAuth mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/oauth