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

[Daniel Fett <[e-mail]>]

Am 03.01.24 um 20:10 schrieb [e-mail]:
>
> Why not RECOMMEND PKCE for CSRF protection, instead of that "MAY"?
>
That's the case already, PKCE is RECOMMENDED/REQUIRED for clients (which 
includes the CSRF protection).

It is just that omitting "state" is a MAY, because "(to) rely on the 
CSRF protection provided by PKCE" effectively means that.

-Daniel


> And in some cases MUST (verb) PKCE?
>
> //Axel
>
> *From: *Justin Richer <[e-mail]>
> *Date: *Wednesday, 3. January 2024 at 19:53
> *To: *Nennker, Axel <[e-mail]>
> *Cc: *mail=[e-mail] 
> <mail=[e-mail]>, [e-mail] <[e-mail]>
> *Subject: *Re: [OAUTH-WG] Shepherd Review of 
> draft-ietf-oauth-security-topics-23
>
> On Jan 3, 2024, at 12:30 PM, [e-mail] wrote:
>
>     The email discussion triggered me jumping into the discussion.
>
>     Also, I am looking into this from a Camara PoV.
>
>     https://github.com/camaraproject/IdentityAndConsentManagement
>     <https://github.com/camaraproject/IdentityAndConsentManagement>
>
>     Camara is about to define what is a MUST for authorization servers
>     etc and we are taking FAPI and the OAuth2 security best practices
>     as input.
>
>     So, when we write our own security profile in Camara, we are
>     probably going to copy the language"Authorization servers MUST
>     support PKCE [RFC7636
>     <https://oauthstuff.github.io/draft-ietf-oauth-security-topics/draft-ietf-oauth-security-topics.html#RFC7636>]."
>
>     We then have no problem requiring clients to use PKCE.
>
>     Coming back to draft-ietf-oauth-security-topics-23. That
>     document's language feels like it would really, really likes to
>     require PKCE support, but then it does not go there.
>
>     "Public clients MUST use PKCE"
>
>     "For confidential clients, the use of PKCE [RFC7636
>     <https://oauthstuff.github.io/draft-ietf-oauth-security-topics/draft-ietf-oauth-security-topics.html#RFC7636>]
>     is RECOMMENDED"
>
>     "Authorization servers MUST support PKCE [RFC7636
>     <https://oauthstuff.github.io/draft-ietf-oauth-security-topics/draft-ietf-oauth-security-topics.html#RFC7636>]."
>
>     "Although PKCE was designed as a mechanism to protect native apps,
>     this advice applies to all kinds of OAuth clients, including web
>     applications."
>
> I read this as pretty clear advice — the AS has to support it, but 
> doesn’t universally require it. Public clients have to use it, so an 
> AS could reasonably require it automatically for public clients. 
> Confidential clients can use it, and it’s a good idea, but it would be 
> surprising for an AS to enforce it on a confidential client. Of 
> course, even today, an AS can decide that any particular client has to 
> use PKCE for whatever reason, so we’re not that different in the 
> "maybe" space than we are now.
>
> The following two lines are talking about different things:
>
>
>
>     I still think that the MAY in 2.1
>
>     "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."
>
> This is not about the client using or not using PKCE — it’s about the 
> client relying on PKCE for CSRF protection, which was previously 
> relegated to state and nonce parameters alone.
>
>
>
>     and the MUST in 2.1.1
>
>     "Authorization servers MUST support PKCE [RFC7636
>     <https://oauthstuff.github.io/draft-ietf-oauth-security-topics/draft-ietf-oauth-security-topics.html#RFC7636>]."
>
> This just means it has to be available at the AS for anyone to use, 
> and, as above, possibly required for some circumstances.
>
>
>
>     do not go well together.
>
>     //Axel
>
>  — Justin
>
>
>
>     *From:*OAuth <[e-mail]
>     <mailto:[e-mail]>> on behalf of Daniel Fett
>     <mail=[e-mail]
>     <mailto:mail=[e-mail]>>
>     *Date:*Wednesday, 3. January 2024 at 17:48
>     *To:*[e-mail] <mailto:[e-mail]><[e-mail]
>     <mailto:[e-mail]>>
>     *Subject:*Re: [OAUTH-WG] Shepherd Review of
>     draft-ietf-oauth-security-topics-23
>
>     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
>     inhttps://mailarchive.ietf.org/arch/msg/oauth/ZiwEfenZZlboikXxBLes5ebPmBw/
>     <https://mailarchive.ietf.org/arch/msg/oauth/ZiwEfenZZlboikXxBLes5ebPmBw/>
>
>     -Daniel
>
>     Am 03.01.24 um 17:39 [e-mail]
>     <mailto:[e-mail]>:
>
>         Hi Daniel,
>
>         there is also this sentence in this
>         sectionhttps://oauthstuff.github.io/draft-ietf-oauth-security-topics/draft-ietf-oauth-security-topics.html#name-authorization-code-grant
>         <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]>
>         <mailto:[e-mail]>on behalf of Daniel
>         Fett<mail=[e-mail]>
>         <mailto:mail=[e-mail]>
>         *Date:*Wednesday, 3. January 2024 at 14:01
>         *To:*[e-mail] <mailto:[e-mail]><[e-mail]>
>         <mailto:[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 [e-mail]
>         <mailto:[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
>                 clientMAY 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:
>                 "Ifthe authorization serverstates in its meta-data
>                 support forProof 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 relieson 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]
>             <mailto:[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
>             <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]  <mailto:[e-mail]>
>
>                 https://www.ietf.org/mailman/listinfo/oauth  <https://www.ietf.org/mailman/listinfo/oauth>
>
>             -- 
>
>             Please use my new email address:[e-mail]  <mailto:[e-mail]>
>
>             _______________________________________________
>
>             OAuth mailing list
>
>             [e-mail]  <mailto:[e-mail]>
>
>             https://www.ietf.org/mailman/listinfo/oauth  <https://www.ietf.org/mailman/listinfo/oauth>
>
>         _______________________________________________
>
>         OAuth mailing list
>
>         [e-mail]  <mailto:[e-mail]>
>
>         https://www.ietf.org/mailman/listinfo/oauth  <https://www.ietf.org/mailman/listinfo/oauth>
>
>     _______________________________________________
>     OAuth mailing list
>     [e-mail] <mailto:[e-mail]>
>     https://www.ietf.org/mailman/listinfo/oauth
>     <https://www.ietf.org/mailman/listinfo/oauth>
>
>
> _______________________________________________
> OAuth mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/oauth