22-chars (128 bits) as a lower limit seems just fine for this case. "ccm" works for me but I don't feel strongly about it either way.
On Thu, Feb 5, 2015 at 9:49 AM, John Bradley <ve7...@ve7jtb.com> wrote: > Inline > > > > On Feb 4, 2015, at 10:43 PM, Manger, James < > james.h.man...@team.telstra.com> wrote: > > > >> 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). > > In my editors draft I fixed the 43 octet base64url encoding of 32bytes. I > originally had 43 but it got changed at some point > > Is there working group feedback on making the lower limit clear in the > prose and if so what should it be? 22-chars (128 bits) or 43 char (256 > bits)? > > > > > > 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). > > > > I am going to leave this for a later formatting cleanup at the moment, I > need to find a good style compromise that works with rfcmarkup. > > > 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. > > > Yes when I added the example I realized that the normative text was the > more complicated way to do the comparison. > > I will go back and refactor the main text to talk about the simpler > comparison and drop the base64url-decode references. > > > > 4. > > Expand "MTI" to "mandatory to implement". > > Done in editors draft. > > > > 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" ;) > > On the topic of the parameter name "code_challange_method", James has > a point in that it is a bit long. > > We could shorten it to "ccm". If we want to change the name sooner is > better than later. > > It is that balance between compactness and clear parameter names for > developers, that we keep running into. > > I don't know that encouraging longer parameter values is the best > direction. > > Feedback please > > John B. > > > > -- > > James Manger > > > > _______________________________________________ > > OAuth mailing list > > OAuth@ietf.org > > https://www.ietf.org/mailman/listinfo/oauth > > > _______________________________________________ > OAuth mailing list > OAuth@ietf.org > https://www.ietf.org/mailman/listinfo/oauth > >
_______________________________________________ OAuth mailing list OAuth@ietf.org https://www.ietf.org/mailman/listinfo/oauth
