[OAUTH-WG] Barry Leiba's Discuss on draft-ietf-oauth-spop-12: (with DISCUSS and COMMENT)

["Barry Leiba" <[e-mail]>]

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:
----------------------------------------------------------------------

How does "plain" do anything at all to mitigate this attack?  Wouldn't
anyone who could snag the grant also be able to snag the code verifier as
well?  Why is "plain" even here?


----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

General:
I would think that this mechanism should never be a substitute for using
TLS, and that this document should be explicit about that, and should say
that the proper mitigation for situations where TLS can be used... is to
use TLS.  Is there a reason we should NOT say that?

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 --
I don't understand the distinction between STRING and ASCII(STRING).  Can
you please explain it?

-- Section 4.3 --
If "plain" does stay, why on Earth is it the default?  Even if just for
form's sake, shouldn't S256 be the default?

-- Section 4.4 --
The word "code" is used for too many things, and "Authorization Grant" 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".

-- Section 4.4.1 --
Please expand "PKCE", which is, at the moment, only expanded in the
document title.

-- 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.

-- 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 [e-mail] 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 [e-mail] 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

-- Section 7.2 --
Please rewrite the first paragraph.  Please do not leave it for the RFC
Editor, as they may inadvertently get it technically wrong when they try.