Re: [OAUTH-WG] I-D Action: draft-ietf-oauth-spop-09.txt

2015-02-05 10:43 GMT+09:00 Manger, James <James.H.Manger@team.telstra.com>:

> >     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
> (just the upper limit); is too high (128-bits=22-chars is sufficient); and
> doesn't correspond to 256-bits (BASE64URL-ENCODE(32 bytes) gives 43 chars,
> not 42).
>

Thanks for pointing out.

>
> 2.
> Quotes around "code_verifier" and "code_challenge" in prose are okay,
> though not really necessary as the underscore is enough to distinguish them
> as technical labels. Quotes around these terms in formula is bad as it
> looks like the formula applies to the 13 or 14 chars of the label. The
> quoting is also used inconsistently.
> Suggestion: remove all quotes around "code_verifier" and "code_challenge"
> in prose and formula.
> For example, change ASCII("code_verifier") to ASCII(code_verifier).
>

They are actually put in by the tools automagically.
In XML, it is <spanx style="verb"> </spanx>, and if HTML is compiled from
it, it will appear in fixed width type.
However, the xml2txt converter at the tools.ietf.org does convert them to
quoted strings.
We have also found other nits due to the tools and trying to figure out
what to do.
It may end up modifying the text to avoid those tools issues.

>
>
> 3.
> Two ways to check code_verifier are given in appendix B, whereas only one
> of these is mentioned in section 4.6.
>   SHA256(verifier) === B64-DECODE(challenge)
>   B64-ENCODE(SHA256(verifier)) === 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
> base64url-decoding is never done, and doesn't need to be mentioned in the
> spec.
>

Good point.

>
>
> 4.
> Expand "MTI" to "mandatory to implement".
>

Will do.

>
> P.S. Suggesting code challenge method names not exceed 8 chars to be
> compact is a bit perverse given the field holding these values has the long
> name "code_challenge_method" ;)
>

Yup.

>
> --
> James Manger
>
> _______________________________________________
> OAuth mailing list
> OAuth@ietf.org
> https://www.ietf.org/mailman/listinfo/oauth
>

-- 
Nat Sakimura (=nat)
Chairman, OpenID Foundation
http://nat.sakimura.org/
@_nat_en