Thanks Brian and Justin! I will work on the shepherd writeup.
Regards, Rifaat On Wed, Aug 10, 2022 at 4:22 PM Brian Campbell <bcampb...@pingidentity.com> wrote: > Thanks Justin! > > I've just published a -11 draft > <https://datatracker.ietf.org/doc/draft-ietf-oauth-dpop/11/> with these > changes (also listed below copied from the doc history section). Rifaat, I > believe all the shepherd review comments have now been addressed. > > > -11 > > * Updates addressing outstanding shepherd review comments per side > meeting discussions at IETF 114 > * Added more explanation of the PAR considerations > * Added parenthetical remark "(such as ES256)" to Signature > Algorithms subsection > * Added more explanation for ath > * Added a reference to RFC8725 in mention of explicit JWT typing > > On Fri, Aug 5, 2022 at 2:50 PM Justin Richer <jric...@mit.edu> wrote: > >> Also, I wanted to take a moment to respond specifically to something in >> the shepherd’s review: >> >> > • The DPoP Proof contains a hash of the Access Token, and the Access >> Token contains a hash of the public key in the DPoP Proof. Why do you need >> both? Would one of these be sufficient? >> >> The access token is not guaranteed to “contain” a hash of the public key >> — that’s only if you’re using JWTs and if you’re using the confirmation >> method specified in here. You are completely free to use non-JWT tokens >> with introspection or use some other means to look up the right keys >> associated with the token. >> >> The other aspect, which I hope is covered by the text in the PR, is that >> it’s about binding it in both directions. Binding the key to the token >> means that you can’t use the token without presenting the key. That’s the >> fundamental problem of proof-of-possession that DPoP is solving. Binding >> the token to the proof means that you can’t use the proof without the >> token. This is a part of making sure you’re binding the proof to the >> request as-presented. It also means that you can’t take a >> token-less-but-signed call to the RS and add a token value that you can’t >> generate a signature for and have it work. >> >> — Justin >> >> >> On Aug 5, 2022, at 1:00 PM, Justin Richer <jric...@mit.edu> wrote: >> >> I think Brian’s just in Danial about this one, but we’ll let it slide. >> >> And I have in fact gone through and added text about ATH: >> >> https://github.com/danielfett/draft-dpop/pull/168 >> >> I opted to add this in the introduction of the section about using access >> tokens instead of a separate security consideration because it’s really >> fundamental to the token use here. >> >> — Justin >> >> >> On Jul 27, 2022, at 7:11 PM, Brian Campbell < >> bcampbell=40pingidentity....@dmarc.ietf.org> wrote: >> >> I need to make one more apology - this time for the incorrect spelling of >> Dr. Fett's name (should be Daniel not Danial). My apologies. >> >> On Wed, Jul 27, 2022 at 6:43 PM Brian Campbell < >> bcampb...@pingidentity.com> wrote: >> >>> Thanks Rifaat and others for the vibrant* discussions about the DPoP >>> draft in the side meeting yesterday. >>> >>> I thought it'd be appropriate to share/reiterate the three action items >>> we'd agreed on during the meeting (as I remember anyway): >>> >>> 1. Justin to review the text about why we have the AT hash and >>> either create a PR adding additional motivations or say that what we have >>> is already sufficient >>> 2. Danial to add some text to further explain decisions with respect >>> to PAR >>> 3. Brian (aka me) to add a parenthetical remark to the Signature >>> Algorithms subsection listing 'ES256' >>> >>> PR's for the latter two are here >>> <https://github.com/danielfett/draft-dpop/pull/165> and here >>> <https://github.com/danielfett/draft-dpop/pull/166> respectively. And >>> yes, this message is, at least in part, a passive-aggressive reminder to >>> Justin about #1. >>> >>> The slides that I used to try and help guide the discussions are >>> attached. They are admittedly rather suboptimal but I'm including them for >>> the sake of transparency (and because they have a couple of photos). >>> >>> >>> * my apologies for being overly vibrant at times >>> >>> >>> >>> >>> On Wed, Jul 6, 2022 at 4:32 PM Brian Campbell < >>> bcampb...@pingidentity.com> wrote: >>> >>>> Thanks Rifaat! >>>> I will make those changes in the document source and come to Philly >>>> prepared to discuss the other items. One of the side meetings seems like a >>>> good forum for that, good idea. >>>> >>>> On Tue, Jul 5, 2022 at 11:14 AM Rifaat Shekh-Yusef < >>>> rifaat.s.i...@gmail.com> wrote: >>>> >>>>> Thanks Brian! >>>>> See my replies inline below. >>>>> >>>>> >>>>> On Thu, Jun 30, 2022 at 6:52 PM Brian Campbell < >>>>> bcampb...@pingidentity.com> wrote: >>>>> >>>>>> Thanks for shepherding Rifaat. And apologies for the slow reply. My >>>>>> attempts at answering questions and responding to comments are inline >>>>>> below. >>>>>> >>>>>> >>>>>> On Fri, Jun 3, 2022 at 11:55 AM Rifaat Shekh-Yusef < >>>>>> rifaat.s.i...@gmail.com> wrote: >>>>>> >>>>>>> The following is my review as a document shepherd: >>>>>>> >>>>>>> Section 4.3 >>>>>>> Last sentence >>>>>>> Since the document uses “SHOULD”, this implies that there are some >>>>>>> valid cases where this is not needed. >>>>>>> Should a text be added to explain when this is not needed? >>>>>>> >>>>>> >>>>>> >>>>>> What about giving a bit more context about why they should? Changing >>>>>> that sentence to say, "To reduce the likelihood of false negatives, >>>>>> servers >>>>>> SHOULD employ Syntax-Based Normalization (Section 6.2.2 >>>>>> <https://rfc-editor.org/rfc/rfc3986> of [RFC3986]) and Scheme-Based >>>>>> Normalization (Section 6.2.2 <https://rfc-editor.org/rfc/rfc3986> of >>>>>> [RFC3986]) before comparing the htu claim." And also maybe changing >>>>>> it to a little "should". >>>>>> >>>>>> Yes, that works. >>>>> I suggest keeping it as "SHOULD" to encourage implementers to use >>>>> this, unless they have a really good reason not to. >>>>> >>>>> >>>>> >>>>>> >>>>>> >>>>>>> >>>>>>> Section 6.1 >>>>>>> >>>>>>> 1. First sentence - what is the reason for using “SHOULD”, >>>>>>> instead of “MUST” in this case? >>>>>>> >>>>>>> >>>>>> >>>>>> Good question. I think it was a bit of carryover from OAuth in >>>>>> general not strictly defining access token format or content. And wanting >>>>>> to not encroach on that. But that's kinda covered/allowed for in general >>>>>> by >>>>>> Section 6 already. And Section 6.2 is effectively the same as 6.1 but for >>>>>> introspection and it doesn't use "SHOULD". I think the “SHOULD” in >>>>>> the first sentence of 6.1 should be removed thereby making it an >>>>>> implicit must - like "when using JWT, this is how it is". That would >>>>>> align >>>>>> with the way it's described for introspection. Also leaves some room for >>>>>> hash algorithm agility via a new confirmation method member (described in >>>>>> https://www.ietf.org/archive/id/draft-ietf-oauth-dpop-09.html#name-access-token-and-public-key) >>>>>> without going against a "MUST" >>>>>> >>>>>> >>>>> I am fine with removing the "SHOULD" to make it an implicit must. >>>>> >>>>> >>>>> >>>>>> >>>>>> >>>>>>> >>>>>>> 1. The DPoP Proof contains a hash of the Access Token, and the >>>>>>> Access Token contains a hash of the public key in the DPoP Proof. >>>>>>> >>>>>>> Why do you need both? Would one of these be sufficient? >>>>>>> >>>>>> >>>>>> >>>>>> The latter (AT containing a hash of the public key in the DPoP Proof) >>>>>> is needed and largely sufficient for the main goals of binding the AT to >>>>>> a >>>>>> key held by the client. The former (DPoP Proof containing a hash of >>>>>> the AT) was added later via very rough WG consensus - it can prevent some >>>>>> esoteric swapping of tokens that I never really understood to be honest >>>>>> and >>>>>> also limits the impact of using maliciously precomputed and exfiltrated >>>>>> proofs ( >>>>>> https://www.ietf.org/archive/id/draft-ietf-oauth-dpop-09.html#section-2-6 >>>>>> talks about it a bit). Use of the nonce mechanism, which was added >>>>>> to the draft even later, also (and better) protects against precomputed >>>>>> and exfiltrated proofs. The value of the AT hash in the proof seems >>>>>> somewhat questionable. To me anyway. But removing it at this point is >>>>>> potentially problematic due to inertia, existing >>>>>> implementations/deployments, rough WG consensus, and more. >>>>>> >>>>>> >>>>> I think that at least a text is needed to justify this, and explain >>>>> the " it can prevent some esoteric swapping of tokens" issue. >>>>> Maybe we can discuss this during one of the side meetings in Philly. >>>>> >>>>> >>>>> >>>>>> >>>>>> >>>>> >>>>>>> Section 7.1 >>>>>>> >>>>>>> 1. “if the request does not include valid credentials or does >>>>>>> not contain an access token sufficient for access, the server can >>>>>>> respond with a challenge to the client to provide DPoP authentication >>>>>>> information.” >>>>>>> >>>>>>> >>>>>>> Should the “can” be replaced with a “SHOULD”? >>>>>>> >>>>>> >>>>>> >>>>>> FWIW, there was some discussion around that sentence that included >>>>>> some pushback on dropping the "can". >>>>>> https://github.com/danielfett/draft-dpop/issues/119 and >>>>>> https://github.com/danielfett/draft-dpop/pull/122 have the >>>>>> conversation. I'm rather hesitant to try and change it after all that. >>>>>> >>>>>> >>>>>> >>>>> Ok >>>>> >>>>> >>>>> >>>>>> >>>>>>> >>>>>>> 1. Also, I think it would be clearer if you can explicitly state >>>>>>> what the authorization server should do when it does not challenge >>>>>>> the >>>>>>> client, which I am assuming will be something along the lines of: >>>>>>> “the >>>>>>> authorization server issues an error response per Section 5.2 of >>>>>>> RFC6749“ >>>>>>> >>>>>>> >>>>>> >>>>>> The section in question is about protected resource access so >>>>>> anything about the authorization server wouldn't be appropriate there. >>>>>> The >>>>>> protected resource / RS always has the option to simply fail the request >>>>>> and can do that however it sees fit. I'm not sure how to state that in >>>>>> the >>>>>> document text. Or if anything should be stated, honestly. >>>>>> >>>>>> Ok >>>>> >>>>> >>>>> >>>>>> >>>>>> >>>>>>> >>>>>>> Section 7.2 >>>>>>> >>>>>>> 1. “Specifically, such a protected resource MUST reject a >>>>>>> DPoP-bound access token received as a bearer token per [RFC6750 >>>>>>> >>>>>>> <https://www.ietf.org/archive/id/draft-ietf-oauth-dpop-08.html#RFC6750> >>>>>>> ].” >>>>>>> >>>>>>> >>>>>>> I think that I understand what you are trying to say with this >>>>>>> sentence, but the way the sentence reads is confusing to me. >>>>>>> I am assuming what you are trying to say is something along the >>>>>>> lines of “a dpop protected resource must reject a request that provides >>>>>>> a >>>>>>> bearer token”. Is that correct? If so, can you please rephrase the >>>>>>> sentence >>>>>>> to make it clearer? >>>>>>> >>>>>> >>>>>> >>>>>> That's not quite correct. That paragraph >>>>>> <https://www.ietf.org/archive/id/draft-ietf-oauth-dpop-09.html#section-7.2-1> >>>>>> (copied below) is attempting say that a protected resource that will >>>>>> accept either "Authorization: Bearer <bearer token>" or "Authorization: >>>>>> DPoP <dpop-bound token>" is required to reject a request that uses the >>>>>> Bearer scheme with a DPoP-bound access token. This is to prevent >>>>>> downgraded usage of a bound access token without demonstrating possession >>>>>> of the key to which it is bound. >>>>>> >>>>>> "Protected resources simultaneously supporting both the DPoP and >>>>>> Bearer schemes need to update how evaluation of bearer tokens is >>>>>> performed to prevent downgraded usage of a DPoP-bound access token. >>>>>> Specifically, such a protected resource MUST reject a DPoP-bound access >>>>>> token received as a bearer token per [RFC6750 >>>>>> <https://www.ietf.org/archive/id/draft-ietf-oauth-dpop-09.html#RFC6750> >>>>>> ]." >>>>>> >>>>>> Got it. >>>>> >>>>> >>>>>> >>>>>> >>>>>>> >>>>>>> >>>>>>> 1. “A protected resource that supports only [RFC6750 >>>>>>> >>>>>>> <https://www.ietf.org/archive/id/draft-ietf-oauth-dpop-08.html#RFC6750>] >>>>>>> and is unaware of DPoP would most presumably accept a DPoP-bound >>>>>>> access >>>>>>> token as a bearer token” >>>>>>> >>>>>>> >>>>>>> Wouldn't such a resource server check the value of the >>>>>>> WWW-Authenticate header to make sure it contains the Bearer scheme, >>>>>>> which >>>>>>> means that the request is most likely to be declined? >>>>>>> >>>>>> >>>>>> >>>>>> What that is trying to say is that a protected resource that only >>>>>> does or knows about the RFC6750 Bearer scheme ("Authorization: Bearer >>>>>> <token>") will almost certainly accept a bound access token sent via the >>>>>> Bearer scheme. >>>>>> >>>>>> Ok >>>>> >>>>> >>>>>> >>>>>> >>>>>>> >>>>>>> Section 10.1 >>>>>>> Why define two different mechanisms to achieve the same thing? >>>>>>> This seems to add complexity without an obvious benefit. >>>>>>> >>>>>> >>>>>> >>>>>> This is a bit of a tricky area. The benefit with PAR is the direct >>>>>> request from client to AS, which allows for an actual DPoP proof to be >>>>>> used >>>>>> for the eventual binding of the authorization code to the key. Also the >>>>>> client doesn't have to do the JWK hash in that case. Whereas the normal >>>>>> authorization request is indirect via the browser and just a hash of the >>>>>> key is given for the code binding with the dpop_jkt parameter. And the >>>>>> client has to compute the hash. But PAR is just an alternative way to >>>>>> pass >>>>>> the authorization parameters (like dpop_jkt) so it's kinda awkward to use >>>>>> things together like this. >>>>>> https://github.com/danielfett/draft-dpop/issues/103 and >>>>>> https://github.com/danielfett/draft-dpop/pull/111 have some >>>>>> discussion around this but there was some in person talk too so that's >>>>>> not >>>>>> complete. >>>>>> >>>>>> I don't love that there's two different mechanisms here. But it's >>>>>> what we were able to come up with given all the factors. Certainly open >>>>>> to >>>>>> considering improvements but am pretty much at a loss of what that might >>>>>> be. >>>>>> >>>>>> Let's discuss this during one of the side meetings in Philly >>>>> >>>>> >>>>> >>>>>> >>>>>> >>>>>>> >>>>>>> Section 11.6 >>>>>>> Should the algorithms be explicitly called out? Or at least >>>>>>> reference a document that calls out such algorithms? >>>>>>> >>>>>> >>>>>> >>>>>> There isn't a single such document and it's not necessarily a static >>>>>> list of algorithms. I was about to say we could point to the JOSE >>>>>> alg registry but glancing again at it >>>>>> https://www.iana.org/assignments/jose/jose.xhtml#web-signature-encryption-algorithms >>>>>> and I suspect that'd confuse more than help. We could perhaps list >>>>>> some/many of the algs with the qualification that it's not an exclusive >>>>>> or >>>>>> complete list? But I'm not sure how useful that would be, to be honest. >>>>>> >>>>>> If you do not specify any algorithm, how do you ensure interop? >>>>> I think this is worth a discussion in Philly >>>>> >>>>> >>>>> >>>>>> >>>>>> >>>>>>> >>>>>>> Section 11.7 >>>>>>> Why is OAuth Token Binding included? >>>>>>> >>>>>> >>>>>> >>>>>> Yeah, that doesn't make sense to encourage its use because it's not a >>>>>> viable thing to use. OAuth Token Binding should be removed from Section >>>>>> 11.7. Was that what you were getting at? That particular text in the >>>>>> paragraph that mentions token binding has been in the draft for a long >>>>>> time >>>>>> and honestly never made a lot of sense to me. So I could envision >>>>>> removing >>>>>> more. But that's maybe more than you were aiming for. >>>>>> >>>>>> I am actually in favor of removing it >>>>> >>>>> >>>>> >>>>>> >>>>>>> >>>>>>> Section 11.8 >>>>>>> Why not include algorithm agility to make sure the mechanism is >>>>>>> ready to allow for more secure algorithms in the future? >>>>>>> >>>>>> >>>>>> Algorithm agility is a whole can of worms that can be accomplished in >>>>>> different ways with different amounts of added complexity and potential >>>>>> vulnerabilities and issues of interop and MTI. Section 11.8 describes how >>>>>> DPoP allows for algorithm agility (without using the exact words) by >>>>>> suggesting that new dpop binding cnf method and/or AT hash claim be >>>>>> defined using a "better" hash algorithm if/when the need arises (OAuth >>>>>> 2.0 Mutual-TLS takes a similar approach FWIW). The intent of doing >>>>>> it that way was to keep things as simple as possible in the spec right >>>>>> now >>>>>> without completely closing the door on future needs. . >>>>>> >>>>>> This is another topic that is worth a discussion in Philly. >>>>> >>>>> Regards, >>>>> Rifaat >>>>> >>>>> >>>>> >>>>>> >>>>>> *CONFIDENTIALITY NOTICE: This email may contain confidential and >>>>>> privileged material for the sole use of the intended recipient(s). Any >>>>>> review, use, distribution or disclosure by others is strictly prohibited. >>>>>> If you have received this communication in error, please notify the >>>>>> sender >>>>>> immediately by e-mail and delete the message and any file attachments >>>>>> from >>>>>> your computer. Thank you.* >>>>> >>>>> >> *CONFIDENTIALITY NOTICE: This email may contain confidential and >> privileged material for the sole use of the intended recipient(s). Any >> review, use, distribution or disclosure by others is strictly prohibited. >> If you have received this communication in error, please notify the sender >> immediately by e-mail and delete the message and any file attachments from >> your computer. Thank you.*_______________________________________________ >> 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 >> >> >> > *CONFIDENTIALITY NOTICE: This email may contain confidential and > privileged material for the sole use of the intended recipient(s). Any > review, use, distribution or disclosure by others is strictly prohibited. > If you have received this communication in error, please notify the sender > immediately by e-mail and delete the message and any file attachments from > your computer. Thank you.*
_______________________________________________ OAuth mailing list OAuth@ietf.org https://www.ietf.org/mailman/listinfo/oauth
