Subject: [OAUTH-WG] Benjamin Kaduk's No Objection on draft-ietf-oauth-jwsreq-32: (with COMMENT)

From: Benjamin Kaduk via Datatracker <[email-masked]>
Date: Tue, 06 Apr 2021 11:39:04 -0700

Benjamin Kaduk has entered the following ballot position for
draft-ietf-oauth-jwsreq-32: No Objection

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-jwsreq/

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

Thank you for the new security considerations text in Sections 10.7 and
10.8 since I last reviewed; it does a great job capturing my comments
(and the current deployed reality).

Thanks also to Watson Ladd for the latest secdir review and the authors
for their responses to it.

Section 6.2

   The Authorization Server MUST validate the signature of the JSON Web
   Signature [RFC7515] signed Request Object.  The signature MUST be
   validated using a key associated with the client and the algorithm
   specified in the "alg" Header Parameter.  [...]

The last version I reviewed had some language tying the algorithm used
for verification back to a registration (and I commented that perhaps a
registration might not always exist).  This language seems to set the
recipient up to blindly use the "alg" header parameter, even if it's
"none", and we should probably have some hedging language here (I see we
do have language elsewhere that bans "none" specifically)...

                                             If a "kid" Header Parameter
   is present, the key identified MUST be the key used, and MUST be a
   key associated with the client.  Algorithm verification MUST be
   performed, as specified in Sections 3.1 and 3.2 of [RFC8725].

... and maybe that would just take the form of swapping the order of
these two sentences, now that I keep reading.

Section 10.2

Do any of the procedures listed for steps (c) and/or (d) need to be
performed in step (e) as well?  (In particular, the requirements on the
returned URI seem like they would still apply, and possibly the need for
client authentication.

Section 10.3

Nat's response at
https://mailarchive.ietf.org/arch/msg/oauth/hB_ON_BR0fDf3NSDFcFo5MwbZ2Y/ to my
previous review suggested that there might be value in future work that
specifies the linkage across these endpoints to try to address the
cross-phase attacks discussed in [FETT].  However, the paragraph that I
had commented on (that mentions "an extension specification") has since
been removed, and I failed to track down why just from a quick
mailarchive search.  There may well have been a good reason for removing
it (and the reference to [FETT]), so please help refresh my memory if
that's the case.

Section 10.4

   The introduction of "request_uri" introduces several attack
   possibilities.  Consult the security considerations in Section 7 of
   RFC3986 [RFC3986] for more information regarding risks associated
   with URIs.

My previous comments had mentioned that because of time skew the
dereferenced request URI might actually have the contents of a different
request than the one intended, and Nat's response at
https://mailarchive.ietf.org/arch/msg/oauth/hB_ON_BR0fDf3NSDFcFo5MwbZ2Y/
pointed out that OIDC actually does use a nonce that would prevent such
an occurrence.  Hopefully Nat did get a chance to think about whether
there was anything else that we should mention in this document, on this
topic.  ("There isn't anything else to mention here" is a fine answer; I
just wanted to close the loop on that thread, since I didn't find one in
the mail archive.)

Section 11.1

nit: s/TFP/trusted third-party service/ (multiple instances), since we
stopped using "Trust Framework Provider" in the main body.

Sction 14.1

Not your fault, but BCP 195 now includes both RFC 7525 and RFC 8996 --
thank you for referencing it as BCP 195!  I expect the RFC Editor will
catch the new reference if you don't want to figure out how to notate it
properly.