[OAUTH-WG] Proposed changes to draft-ietf-oauth-dpop-02

2020-12-02 Thread Denis
I have reviewed the whole draft and you will find comments below 
starting with five editorials comments. Every other comment is numbered.


Let us start with five typos where there is a duplication of the word "the":

Page 4:

XXS vulnerabilities also allow an attacker to execute code in the 
context of the browser-based client application and maliciously use a 
token indirectly through the _the_ client.


Page 11:

The example response in Figure 5 included a refresh token, which the 
client can use to obtain a new access token when the _the_ previous one 
expires.


Page 11:

Refreshing an access token is a token request using the "refresh_token" 
grant type made to the _the_ authorization server’s token endpoint.


Page 13:

Resource servers MUST be able to reliably identify whether an access 
token is bound using DPoP and ascertain sufficient information about the 
public key to which
the token is bound in order to verify the binding with respect to the 
_the_ presented DPoP proof (see Section 7.1).


Page 13:

Resource servers supporting DPoP MUST ensure that the _the_ public key 
from the DPoP proof matches the pubic key to which the access token is 
bound.



1. Section 1. Introduction

The text states:

 The value of the header is a JWT [RFC7519] that enables the 
authorization server to bind issued tokens to the public part of the 
client’s key pair.


A client may use different key pairs.

Change :

"the client’s key pair"

into :

" a client’s key pair".

2. Section 2.Objectives

The text states:

    The primary aim of DPoP is to prevent unauthorized or illegitimate 
parties from using leaked or stolen access tokens by binding a token to 
a public key
    upon issuance and requiring that the client demonstrate possession 
of the corresponding private key when using the token.


The objective needs to be described in terms of what ,the mechanism does 
and not yet at this time for which reasons.


Change into:

 The aim of DPoP is first to require a client to demonstrate 
possession of a public key when requesting an access token or a refresh 
token to an AS
 where the AS will then bind that public key upon a token issuance 
and, then to demonstrate the possession of a public key included into a 
token

 when presenting a refresh token to an AS or an access token to a RS.


3. The next sentence looks odd as far as the English grammar is considered.

    This constrains the legitimate sender of the token to only the 
party with access to the private key and gives the server receiving the 
token added assurances

    that the sender is legitimately authorized to use it.

With the proposed change above, this sentence looks unnecessary and 
could be deleted.


4. Section 3 Concepts Page 5

In order to make crystal clear that they are to different DPoP Proofs, 
(DPoP Proof 1) and (DPoP Proof 2) should be shown on the figure as 
proposed below:


*+++---+
||--(A)-- Token Request --->||
| Client |(DPoP Proof 1)| Authorization |
|||Server|
||<-(B)-- DPoP-bound Access Token --||
||(token_type=DPoP)+---+
||
||
||+---+
||--(C)-- DPoP-bound Access Token ->||
||(DPoP Proof 2)|Resource|
|||Server|
||<-(D)-- Protected Resource ---||
||+---+
++
Figure 1: Basic DPoP Flow
*
5. The text states:

 The basic steps of an OAuth flow with DPoP are shown in Figure 1:

 *(A) In the Token Request, the client sends an authorization grant 
(e.g., an authorization code, refresh token, etc.) to the authorization 
server
    in order to obtain an access token (and potentially a refresh 
token).The client attaches a DPoP proof to the request in an HTTP header.


At the end of the sentence, add : (DPoP Proof 2).


6. The text states:

 *(C) To use the access token the client has to prove possession of 
the private key by, again, adding a header to the request that carries 
the DPoP proof.


Change into:

 *(C) To use the access token the client has to prove possession of 
the private key by using a header to the request that carries another 
DPoP proof (DPoP Proof 2).



7. Section 4.DPoP Proof JWTs

The text states:

 A valid DPoP proof demonstrates to the server that the client 
holds the private key that was used to sign the JWT.


For avoiding misunderstandings, it would be better to say that this 
applies to DPoP Proof JWTs and that it applicable for both ASs and RSs.


Proposed change:

 A valid DPoP proof demonstrates to a server, (i.e. an AS or a RS), 
that the client holds the private key that was used to sign a DPoP Proof 
JWT.


8. The text states:

 This enables authorization servers to bind issued tokens to the 
corresponding public key (as described in Section 5) and for resource 
servers to verify
 the key-binding of tokens that it receives (see Section 7.1), 
which prevents said tokens from being used by any entity that does not 
have access to the private 

Re: [OAUTH-WG] Proposed changes to draft-ietf-oauth-dpop-02

2020-12-08 Thread Brian Campbell
attempts at replies are inline

On Wed, Dec 2, 2020 at 8:42 AM Denis  wrote:

> I have reviewed the whole draft and you will find comments below starting
> with five editorials comments. Every other comment is numbered.
> Let us start with five typos where there is a duplication of the word
> "the":
>

Will fix, thank you.

1. Section 1. Introduction
>
> The text states:
>
>  The value of the header is a JWT [RFC7519] that enables the
> authorization server to bind issued tokens to the public part of the
> client’s key pair.
>
> A client may use different key pairs.
>
> Change :
>
> "the client’s key pair"
>
> into :
>
> " a client’s key pair".
>

Okay, sure.

2. Section 2.  Objectives
>
> The text states:
>
> The primary aim of DPoP is to prevent unauthorized or illegitimate
> parties from using leaked or stolen access tokens by binding a token to a
> public key
> upon issuance and requiring that the client demonstrate possession of
> the corresponding private key when using the token.
>
> The objective needs to be described in terms of what ,the mechanism does
> and not yet at this time for which reasons.
>
> Change into:
>
>  The aim of DPoP is first to require a client to demonstrate
> possession of a public key when requesting an access token or a refresh
> token to an AS
>  where the AS will then bind that public key upon a token issuance
> and, then to demonstrate the possession of a public key included into a
> token
>  when presenting a refresh token to an AS or an access token to a RS.
>
I'm of the opinion that the current text is fine.

3. The next sentence looks odd as far as the English grammar is considered.
>
> This constrains the legitimate sender of the token to only the party
> with access to the private key and gives the server receiving the token
> added assurances
> that the sender is legitimately authorized to use it.
>
> With the proposed change above, this sentence looks unnecessary and could
> be deleted.
>

Grammar has admittedly never been my strong suit but, even after rereading
the text a few times, I don't see reason to change it. Perhaps the RFC
editor will help out later in the process, if need be.


4. Section 3 Concepts Page 5
>
> In order to make crystal clear that they are to different DPoP Proofs,
> (DPoP Proof 1) and (DPoP Proof 2) should be shown on the figure as proposed
> below:
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
> *++  +---+
> ||--(A)-- Token Request --->|   | |
> Client |(DPoP Proof 1)| Authorization |
> ||  | Server|
> ||<-(B)-- DPoP-bound Access Token --|   |
> ||(token_type=DPoP) +---+
> || || ||
> +---+ ||--(C)-- DPoP-bound Access Token
> ->|   | ||(DPoP Proof
> 2)|Resource   | |
> |  | Server| |
> |<-(D)-- Protected Resource ---|   | |
> |  +---+ ++
>   Figure 1: Basic DPoP Flow *
> 5. The text states:
>
>  The basic steps of an OAuth flow with DPoP are shown in Figure 1:
>
>  *  (A) In the Token Request, the client sends an authorization grant
> (e.g., an authorization code, refresh token, etc.) to the authorization
> server
> in order to obtain an access token (and potentially a refresh
> token).  The client attaches a DPoP proof to the request in an HTTP
> header.
>
> At the end of the sentence, add : (DPoP Proof 2).
>

I don't think having the proofs labeled with 1 & 2 detracts more than it
adds. And that would be the wrong one in the context of that sentence, if
they were labeled.


>
>
> 6. The text states:
>
>  *  (C) To use the access token the client has to prove possession of
> the private key by, again, adding a header to the request that carries the
> DPoP proof.
>
> Change into:
>
>  *  (C) To use the access token the client has to prove possession of
> the private key by using a header to the request that carries another DPoP
> proof (DPoP Proof 2).
>

Will add/adjust some text to try and make it more clear that the proof is
specific and unique to that request.


>
> 7. Section 4.  DPoP Proof JWTs
>
> The text states:
>
>  A valid DPoP proof demonstrates to the server that the client holds
> the private key that was used to sign the JWT.
>
> For avoiding misunderstandings, it would be better to say that this
> applies to DPoP Proof JWTs and that it applicable for both ASs and RSs.
>
> Proposed change:
>
>  A valid DPoP proof demonstrates to a server, (i.e. an AS or a RS),
> that the client holds the private key that was used to sign a DPoP Proof
> JWT.
>

What other server could there