OAuth 2.0 Demonstrating Proof of Possession (DPoP)
draft-ietf-oauth-dpop-16
Yes
Roman Danyliw
No Objection
Erik Kline
Jim Guichard
(Andrew Alston)
(John Scudder)
Note: This ballot was opened for revision 14 and is now closed.
Roman Danyliw
Yes
Erik Kline
No Objection
Jim Guichard
No Objection
Paul Wouters
No Objection
Comment
(2023-04-12 for -14)
Sent
Thanks for the clear specification. While I agree with Ben Schwartz comment in the secdir review that the term "nonce" is wrong in the document, and that it should really be called "cookie", I think it is too late in the game to change this.
Éric Vyncke
No Objection
Comment
(2023-04-10 for -14)
Sent
Thank you for the work put into this document. Please find below some non-blocking COMMENT points, and some nits. Special thanks to Rifaat Shekh-Yusef for the shepherd's detailed write-up including the WG consensus (and the author count) even if the justification of the intended status is rather light. I hope that this review helps to improve the document, Regards, -éric # COMMENTS (non blocking) ## Section 1 Should there be a reference to OAuth ? s/The mechanism described herein /The mechanism specified herein / ? as it is proposed standard Adding a short description of SPA would be useful, or simply remove this reference ? # NITS (non blocking / cosmetic) ## Section 2 ` Properly audience restricting access tokens can prevent such misuse` is difficult to parse ## Section 4.1 s/repeated below for ease of reference/repeated below in figure 3 for ease of reference/ ? ## Section 4.2 s/MUST NOT be none or an identifier for a symmetric algorithm (MAC)/MUST NOT be 'none' or an identifier for a symmetric algorithm/ ## Section 6.1 `JSON Web Tokens (JWT)` the JWT acronym has already been defined.
Andrew Alston Former IESG member
No Objection
No Objection
(for -15)
Not sent
John Scudder Former IESG member
No Objection
No Objection
(for -14)
Not sent
Lars Eggert Former IESG member
(was Discuss)
No Objection
No Objection
(2023-04-12 for -14)
Sent for earlier
# GEN AD review of draft-ietf-oauth-dpop-14 CC @larseggert ## Comments ### Section 9, paragraph 5 ``` only at the issuing server. Developers should also take care to not confuse DPoP nonces with the OpenID Connect [OpenID.Core] ID Token nonce. ``` Could this ambiguity not be avoided by using a different term/claim? ### Too many authors The document has six authors, which exceeds the recommended author limit. Has the sponsoring AD agreed that this is appropriate? ### Missing references No reference entries found for these items, which were mentioned in the text: `[IANA.OAuth.Parameters]`. ### DOWNREFs DOWNREF `[RFC8792]` from this Proposed Standard to Informational `RFC8792`. (For IESG discussion. It seems this DOWNREF was not mentioned in the Last Call and also seems to not appear in the DOWNREF registry.) ### Inclusive language Found terminology that should be reviewed for inclusivity; see https://www.rfc-editor.org/part2/#inclusive_language for background and more guidance: * Term `native`; alternatives might be `built-in`, `fundamental`, `ingrained`, `intrinsic`, `original` * Term `blindly`; alternatives might be `visually impaired`, `unmindful of`, `unconcerned about`, `negligent of`, `unaware`, `uncomprehending`, `unaware`, `uncritical`, `unthinking`, `hasty`, `blocked`, `opaque` ## Nits All comments below are about very minor potential issues that you may choose to address in some way - or ignore - as you see fit. Some were flagged by automated tools (via https://github.com/larseggert/ietf-reviewtool), so there will likely be some false positives. There is no need to let me know what you did with these suggestions. ### JSON ``` { "error": "use_dpop_nonce" ^ Expecting ',' delimiter "error_description": }``` ### Outdated references Document references `draft-ietf-oauth-security-topics-21`, but `-22` is the latest available revision. ### URLs These URLs in the document can probably be converted to HTTPS: * http://www.iana.org/assignments/jwt * http://openid.net/specs/openid-connect-core-1_0.html ## Notes This review is in the ["IETF Comments" Markdown format][ICMF], You can use the [`ietf-comments` tool][ICT] to automatically convert this review into individual GitHub issues. Review generated by the [`ietf-reviewtool`][IRT]. [ICMF]: https://github.com/mnot/ietf-comments/blob/main/format.md [ICT]: https://github.com/mnot/ietf-comments [IRT]: https://github.com/larseggert/ietf-reviewtool
Murray Kucherawy Former IESG member
No Objection
No Objection
(2023-04-12 for -14)
Sent
Most of the SHOULDs here seem unsupported to me, in the sense that I'm not clear what interoperability breaks if I decide not to do what it says. Some prose about that would be helpful to include. I know this isn't the first OAUTH document I've reviewed, but I still find it strange that claim names are not quoted or in all-caps or something. In prose, they just look like typos to me.
Robert Wilton Former IESG member
No Objection
No Objection
(2023-04-12 for -14)
Sent
Hi, Thanks for your work on this document. I found it pleasant to read and informative. I have no substantive comments. Regards, Rob
Warren Kumari Former IESG member
No Objection
No Objection
(2023-04-11 for -14)
Sent
Thank you for writing this; I found it a fascinating and informative read. I don't have any particularly substantive comments, but I do have some nits and similar to hopefully further improve the document. 1: "These stolen artifacts can later be used together independent of the client application to access protected resources." -- I found this really hard to parse. I think that some of it is the "used together independent" formulation - adding a comma would help, but I think just dropping "together" works even better (it does say "artifacts" in plural, so that's already covered?) 2: "Properly audience restricting access tokens can prevent such misuse" - I think that it would be helpful to reword this, or find a reference for "audience restricting" 3: Might it be worth adding a reference for XSS? I'm guessing that the audience will already be familiar, but if not, https://owasp.org/www-community/attacks/xss/ ? 4: Question: Why is the Nonce optional? Perhaps I missed it, but I was unable to find any discussion (I was expecting something in Sec 8,9 or 10) providing some reason why a server might not use a nonce (the closest I found was "The logic through which the server makes that determination is out of scope of this document.", so I'm guessing that there *is* a reason, but... )
Zaheduzzaman Sarker Former IESG member
No Objection
No Objection
(2023-04-12 for -14)
Not sent
Thanks for working on this specification. My review of this document did not identified any transport protocol related issues.