Subject: [OAUTH-WG] draft-ietf-oauth-spop-04 comments

From: Bill Mills <[email-masked]>
Date: Thu, 13 Nov 2014 22:01:22 +0000 (UTC)

This draft has crypto agility problems.

4.3 code_challenge_method default to 'plain' should be noted as supporting legacy clients.  This section should specify that S256 is MTI rather than leaving it to 4.4.1.

I'd be much happier if this section said something like, "The server MUST support a code_challenge_method other than 'plain', as such S256 is MTI as of the publication of this document.  The server MAY implement a code_challenge_method other than S256, and if it does then implementing a mechanism for the client to discover the supported code_challenge_method is RECOMMENDED."

4.4.1 requires that S256 be used even if it also supports something other than 'plain'.

7.2 is too tightly coupled to S256.  Instead it should say something like "Clients that support anything other than the 'plain'  algorithm MUST NOT attempt using 'plain' or fall back to it if they are returned an "unsupported_spop_transform" error by the server.

This spec should probably also register preferred_code_challenge_method as an extension in the OAuth error registryto allow minimal discovery/debugging, and with the above will solve most of the agility problems.

On the naming thing....  How about a title such as "Code Interception Protection for the OAuth Authorization Code Grant" and in the intro section make the last sentence "This specification describes a symmetric proof of possession mechanism that acts as a control against this threat.".  I think this removes the POP/SPOP confusion and says all the same stuff.

-bill

---

From: Bill Mills <[email-masked]>
Date: Thu, 13 Nov 2014 22:39:40 +0000 (UTC)

I understand the intent of:
    S256  "code_challenge" = BASE64URL(SHA256("code_verifier"))
but I think what it really says is take the base64url encoding of the sha256 hash of the constant string "code_verifier".  Are those quotes really supposed to be there?
-bill