Barry Leiba has entered the following ballot position for draft-ietf-oauth-spop-12: Discuss
When responding, please keep the subject line intact and reply to all email addresses included in the To and CC lines. (Feel free to cut this introductory paragraph, however.) Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html for more information about IESG DISCUSS and COMMENT positions. The document, along with other ballot positions, can be found here: https://datatracker.ietf.org/doc/draft-ietf-oauth-spop/ ---------------------------------------------------------------------- DISCUSS: ---------------------------------------------------------------------- After getting John's explanation of the background and details of the attack (and the different communications paths involved, I still would prefer that we build this protocol as a more robust one (with only S256 and not "plain"), but I understand the reasons to have it this way, and I think it will be acceptable with some changes to the text to make the situation clearer. To that end, I suggest this: In the introduction, I think the explanation of Figure 1 (the fourth paragraph) should be changed to make it clear what communication paths are used in each step, and where the vulnerable piece is. Something like this: OLD In step (1) the native app running on the end device, such as a smart phone, issues an authorization request via the browser/operating system, which then gets forwarded to the OAuth 2.0 authorization server in step (2). The authorization server returns the authorization code in step (3). The malicious app is able to observe the authorization code in step (4) since it is registered to the custom URI scheme used by the legitimate app. This allows the attacker to reguest and obtain an access token in step (5) and step (6), respectively. NEW In step (1) the native app running on the end device, such as a smart phone, issues an authorization request via the browser/operating system. The request includes a URI by which the response will be returned, and that uses a custom URI scheme. Step (1) happens through a secure API that cannot be intercepted. The request then gets forwarded to the OAuth 2.0 authorization server in step (2). Because OAuth requires the use of TLS, this communication is protected by TLS, and also cannot be intercepted. The authorization server returns the authorization code over the same TLS connection in step (3). In step (4), the Authorization Code is returned to the requester via the URI that was provided in step (1). A malicious app that has been designed to attack this native app has previously registered itself as a handler for the custom URI scheme, and is now able to observe the Authorization Code in step (4). This allows the attacker to request and obtain an access token in steps (5) and (6). END That (or something like it) makes it clear that only one step is vulnerable, and explicitly tells us that the other communication paths are already protected. In the list of pre-conditions, I suggest changing (1) to make "another application" be "a legitimate application that uses OAuth". *** IMPORTANT *** I am still puzzled by this, in pre-condition (4), which seems to contradict what John said and what I proposed above: 4) The attacker (via the installed app) is able to observe responses from the authorization endpoint. As a more sophisticated attack scenario the attacker is also able to observe requests (in addition to responses) to the authorization endpoint. Can you please explain the "more sophisticated attack", and how the attacker can observe the request? Because "plain" will NOT work in such a situation. I suggest changing the last paragraph in the Introduction like this: OLD To mitigate this attack, this extension utilizes a dynamically created cryptographically random key called 'code verifier'. A unique code verifier is created for every authorization request and its transformed value, called 'code challenge', is sent to the authorization server to obtain the authorization code. NEW To mitigate this attack, this extension utilizes a dynamically created cryptographically random key called 'code verifier'. A unique code verifier is created for every authorization request and its transformed value, called 'code challenge', is sent to the authorization server to obtain the authorization code. This transmission is sent through a secure API, and cannot be intercepted. END I know it repeats what was said in the explanation above, but it's an important enough point to merit repetition. I suggest changing Section 4.4.1 like this (because of a misuse of "MAY" that I missed before): OLD If the client is capable of using "S256", it MUST use "S256", as "S256" is Mandatory To Implement (MTI) on the server. Clients MAY use "plain" only if they cannot support "S256" for some technical reason and knows that the server supports "plain". NEW If the client is capable of using "S256", it MUST use "S256", as "S256" is Mandatory To Implement (MTI) on the server. Clients are permitted to use "plain" only if they cannot support "S256" for some technical reason and know that the server supports "plain". END Finally, I suggest changing this in Section 7.2: OLD "S256" method protects against eavesdroppers observing or intercepting the "code_challenge". If the "plain" method is used, there is a chance that it will be observed by the attacker on the device. The use of "S256" protects against it. NEW The "S256" method protects against eavesdroppers observing or intercepting the "code_challenge", because the challenge cannot be used without the verifier. With the "plain" method, the challenge can be directly used during the verification step, so "plain" does not protect against interception of the initial request. Because of this, "plain" SHOULD NOT be used, and exists only for compatibility with deployed implementations where the request path is already protected. The "plain" method MUST NOT be used in new implementations. END With that set of changes, or something like them (feel free to re-write, and certainly correct anything I got wrong!), I will clear this DISCUSS and we can move ahead. If you find anything fundamentally messed up in any of that, please do discuss it with me, and let's sort it out. And, again, thanks for the explanation that made this all clear. ---------------------------------------------------------------------- COMMENT: ---------------------------------------------------------------------- Putting quotation marks around "code_verifier" and "code_challenge" in the formulas is confusing: it makes it look as if you're putting in those strings themselves, rather than the values of the variables. It's probably unlikely that anyone would make that mistake, but I nevertheless suggest removing the quotation marks when you mean to refer to the values. -- Section 2 -- There is no real distinction between STRING and ASCII(STRING), because STRING is already defined to be ASCII. Using "ASCII(xxx)" only adds clutter, and a suggest removing it. So, for example, these last two comments would make changes such as this one: OLD BASE64URL-ENCODE(SHA256(ASCII("code_verifier" ))) == "code_challenge" NEW BASE64URL-ENCODE(SHA256(code_verifier)) == code_challenge END -- Section 4.3 -- I would very strongly prefer that S256 be the default, not "plain"... or that the method be required, with no default. I understand that this makes existing implementations break. -- Section 4.4 -- The word "code" is used for too many things, and "Authorization Code" is already the right name for what we're talking about here. I suggest that in both the section title and body you use that term, to make it clear what you mean by the "code". Similarly, in Section 4.5 please say "code_verifier" rather than "secret". (Nat plans to make these changes.) -- Section 4.4.1 -- Please expand "PKCE", which is, at the moment, only expanded in the document title. (Nat plans to make a change for this.) -- Section 5 -- The SHOULD in the first paragraph is wrong. You already have a MAY covering the general behavior. You should just take out the "SHOULD", and just say that severs supporting backwards compatibility revert to the normal OAuth protocol. (Nat plans to make a change for this.) -- Section 6.2 -- I have the same comment here as in the other OAuth document: please shift the focus away from telling IANA how to handle tracking of the expert review, and make the mailing list something that the designated expert(s) keep track of. Also, please give more instructions to the DEs about what they should consider when they're evaluating a request (for example, should they approve all requests, or are there criteria they should apply?). For the first, here's a text change that I suggest we move toward for this sort of thing: OLD <most of Section 6.2> NEW Additional code_challenge_method types for use with the authorization endpoint are registered using the Specification Required policy [RFC5226], which includes review of the request by one or more Designated Experts. The DEs will ensure there is at least a two-week review of the request on the oauth-ext-rev...@ietf.org mailing list, and that any discussion on that list converges before they respond to the request. To allow for the allocation of values prior to publication, the Designated Expert(s) may approve registration once they are satisfied that an acceptable specification will be published. Discussion on the oauth-ext-rev...@ietf.org mailing list should use an appropriate subject, such as "Request for PKCE code_challenge_method: example"). The Designated Expert(s) should consider the discussion on the mailing list, as well as <<these other things>> when evaluating registration requests. Denials should include an explanation and, if applicable, suggestions as to how to make the request successful. END (Nat plans to make a change for this.) -- Section 7.2 -- For the first paragraph, after discussion with the author I suggest this: NEW Clients MUST NOT downgrade to "plain" after trying the S256 method. Because servers are required to support S256, an error when S256 is presented can only mean that the server does not support PKCE at all. Otherwise, such an error could be indicative of a MITM attacker trying a downgrade attack. END _______________________________________________ OAuth mailing list OAuth@ietf.org https://www.ietf.org/mailman/listinfo/oauth