Re: [OAUTH-WG] DPoP - Document Shepherd Review

[Rifaat Shekh-Yusef]

Thanks Brian and Justin!

I will work on the shepherd writeup.

Regards,
 Rifaat


On Wed, Aug 10, 2022 at 4:22 PM Brian Campbell 
wrote:

> Thanks Justin!
>
> I've just published a -11 draft
>  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  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  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
>>>  and here
>>>  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 

Re: [OAUTH-WG] DPoP - Document Shepherd Review

[Brian Campbell]

Thanks Justin!

I've just published a -11 draft
 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  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  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 
> 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
>>  and here
>>  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 
>> 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 

Re: [OAUTH-WG] DPoP - Document Shepherd Review

[Justin Richer]

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 
mailto: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 
mailto: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 
mailto: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 and 
here 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 
mailto: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 
mailto:rifaat.s.i...@gmail.com>> wrote:
Thanks Brian!
See my replies inline below.


On Thu, Jun 30, 2022 at 6:52 PM Brian Campbell 
mailto: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 
mailto: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 of [RFC3986]) and Scheme-Based 
Normalization (Section 6.2.2 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 

Re: [OAUTH-WG] DPoP - Document Shepherd Review

[Justin Richer]

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 
mailto: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 
mailto: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 and 
here 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 
mailto: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 
mailto:rifaat.s.i...@gmail.com>> wrote:
Thanks Brian!
See my replies inline below.


On Thu, Jun 30, 2022 at 6:52 PM Brian Campbell 
mailto: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 
mailto: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 of [RFC3986]) and Scheme-Based 
Normalization (Section 6.2.2 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 

Re: [OAUTH-WG] DPoP - Document Shepherd Review

[Rifaat Shekh-Yusef]

Brian,

Thanks for all the hard work that you put into this important document.
We do not mind it when you are vibrant; we know that you always have good
intentions 

Regards,
 Rifaat



On Wed, Jul 27, 2022 at 6:44 PM Brian Campbell 
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
>  and here
>  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 
> 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
  of [RFC3986]) and Scheme-Based
 Normalization (Section 6.2.2  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 (
 

Re: [OAUTH-WG] DPoP - Document Shepherd Review

[Daniel Fett]

Apologies accepted! :-)

Am 28.07.22 um 01:11 schrieb Brian Campbell:
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 
 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
 and here
 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
 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
 wrote:

Thanks Brian!
See my replies inline below.


On Thu, Jun 30, 2022 at 6:52 PM Brian Campbell
 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
 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
 of [RFC3986]) and
Scheme-Based Normalization (Section 6.2.2
 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.




2.

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 

Re: [OAUTH-WG] DPoP - Document Shepherd Review

[Brian Campbell]

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 
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
>  and here
>  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 
> 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
  of [RFC3986]) and Scheme-Based
 Normalization (Section 6.2.2  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 (
 

Re: [OAUTH-WG] DPoP - Document Shepherd Review

[Brian Campbell]

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

> Thanks Brian!
> See my replies inline below.
>
>
> On Thu, Jun 30, 2022 at 6:52 PM Brian Campbell 
> 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
>>  of [RFC3986]) and Scheme-Based
>> Normalization (Section 6.2.2  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 

Re: [OAUTH-WG] DPoP - Document Shepherd Review

[Rifaat Shekh-Yusef]

Thanks Brian!
See my replies inline below.


On Thu, Jun 30, 2022 at 6:52 PM Brian Campbell 
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
>  of [RFC3986]) and Scheme-Based
> Normalization (Section 6.2.2  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 

Re: [OAUTH-WG] DPoP - Document Shepherd Review

[Brian Campbell]

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 
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
 of [RFC3986]) and Scheme-Based
Normalization (Section 6.2.2  of [
RFC3986]) before comparing the htu claim." And also maybe changing it to a
little "should".



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



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



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



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



>
> Section 7.2
>
>1.
>
>“Specifically, such a protected resource MUST reject a DPoP-bound
>access token received as a bearer token per [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

(copied below) is attempting say that a protected resource that 

[OAUTH-WG] DPoP - Document Shepherd Review

[Rifaat Shekh-Yusef]

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?

Section 6.1

   1.

   First sentence - what is the reason for using “SHOULD”, instead of
   “MUST” in this case?



   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?

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”?


   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“


Section 7.2

   1.

   “Specifically, such a protected resource MUST reject a DPoP-bound access
   token received as a bearer token per [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?


   1.

   “A protected resource that supports only [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?

Section 10.1

Why define two different mechanisms to achieve the same thing?

This seems to add complexity without an obvious benefit.

Section 11.6

Should the algorithms be explicitly called out? Or at least reference a
document that calls out such algorithms?

Section 11.7

Why is OAuth Token Binding included?

Section 11.8

Why not include algorithm agility to make sure the mechanism is ready to
allow for more secure algorithms in the future?


Regards,
 Rifaat
___
OAuth mailing list
OAuth@ietf.org
https://www.ietf.org/mailman/listinfo/oauth