From: "Manger, James" <James.H.Manger@team.telstra.com>
To: "oauth@ietf.org" <oauth@ietf.org>
Date: Thu, 5 Feb 2015 12:43:41 +1100
Thread-Topic: [OAUTH-WG] I-D Action: draft-ietf-oauth-spop-09.txt
Thread-Index: AdBA1AEYXq7rjRewS0OREYk72TcMvgAAIblA
Message-ID: <255B9BB34FB7D647A506DC292726F6E12851EBA8C3@WSMSG3153V.srv.dir.telstra.com>
References: <20150204234040.19482.87437.idtracker@ietfa.amsl.com>
In-Reply-To: <20150204234040.19482.87437.idtracker@ietfa.amsl.com>
Accept-Language: en-US, en-AU
Content-Language: en-US
acceptlanguage: en-US, en-AU
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Archived-At: <http://mailarchive.ietf.org/arch/msg/oauth/WyCAKDnFcE8IobtaTETUoc3N64c>
Subject: Re: [OAUTH-WG] I-D Action: draft-ietf-oauth-spop-09.txt
Precedence: list

>     Title           : Proof Key for Code Exchange by OAuth Public Clients
> 	Filename        : draft-ietf-oauth-spop-09.txt
> https://tools.ietf.org/html/draft-ietf-oauth-spop-09


Some nits on this draft:

1. 42 chars.
The lower limit of 42 chars for code_verifier: is not mentioned in prose (j=
ust the upper limit); is too high (128-bits=3D22-chars is sufficient); and =
doesn't correspond to 256-bits (BASE64URL-ENCODE(32 bytes) gives 43 chars, =
not 42).

2.=20
Quotes around "code_verifier" and "code_challenge" in prose are okay, thoug=
h not really necessary as the underscore is enough to distinguish them as t=
echnical labels. Quotes around these terms in formula is bad as it looks li=
ke the formula applies to the 13 or 14 chars of the label. The quoting is a=
lso used inconsistently.
Suggestion: remove all quotes around "code_verifier" and "code_challenge" i=
n prose and formula.
For example, change ASCII("code_verifier") to ASCII(code_verifier).

3.
Two ways to check code_verifier are given in appendix B, whereas only one o=
f these is mentioned in section 4.6.
  SHA256(verifier) =3D=3D=3D B64-DECODE(challenge)
  B64-ENCODE(SHA256(verifier)) =3D=3D=3D challenge

I suggest only mentioning the 2nd (change 4.6 to use the 2nd, and drop the =
1st from appendix B). It is simpler to mention only one. It also means base=
64url-decoding is never done, and doesn't need to be mentioned in the spec.


4.
Expand "MTI" to "mandatory to implement".

P.S. Suggesting code challenge method names not exceed 8 chars to be compac=
t is a bit perverse given the field holding these values has the long name =
"code_challenge_method" ;)

--
James Manger