Hi Mike, Also inline...
On Thu, Apr 08, 2021 at 04:45:15AM +0000, Mike Jones wrote: > Thanks for your review, Ben. We've published > https://tools.ietf.org/html/draft-ietf-oauth-jwsreq-33 to address your and > other IESG comments. > > Responses are inline below, prefixed by "Mike>". > > -----Original Message----- > From: OAuth <oauth-boun...@ietf.org> On Behalf Of Benjamin Kaduk via > Datatracker > Sent: Tuesday, April 6, 2021 11:39 AM > To: The IESG <i...@ietf.org> > Cc: oauth@ietf.org; oauth-cha...@ietf.org; draft-ietf-oauth-jws...@ietf.org > Subject: Benjamin Kaduk's No Objection on draft-ietf-oauth-jwsreq-32: (with > COMMENT) > > 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. > > Mike> Glad to hear 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. > > Mike> Order swapped, per your suggestion. > > 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. > > Mike> Having re-read (c), (d), and (e), I don't think so. The returned URI > in (e) can be an opaque identifier that is not a URL, so the URL checking > logic wouldn't apply. Okay. Thanks for taking a second look. > 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. > > Mike> It was removed as suggested by Watson Ladd in the discussion that > resulted from his SecDir review. Ah! Thanks for refreshing my memory. > 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.) > > Mike> OpenID Connect requests using some response_type values include a nonce > and those using others don't. Thinking about it some more, I don't think > there's any particular risks about using these URLs that are sufficiently > different than those of other URLs that they're worth mentioning here. Okay, as before, thanks for thinking through it again. -Ben > Section 11.1 > > nit: s/TFP/trusted third-party service/ (multiple instances), since we > stopped using "Trust Framework Provider" in the main body. > > Mike> Thanks for the catch - corrected! > > 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. > > Mike> Thanks for the heads-up. I'll plan to ask the RFC Editor what the > right way to do this is. > > _______________________________________________ > OAuth mailing list > OAuth@ietf.org > https://www.ietf.org/mailman/listinfo/oauth > > Thanks again! > -- Mike > _______________________________________________ OAuth mailing list OAuth@ietf.org https://www.ietf.org/mailman/listinfo/oauth
