Re: [OAUTH-WG] Fwd: New Version Notification for draft-ietf-oauth-dpop-01.txt

[William Denniss <[e-mail]>]

Hi Brian, et. al,

I reviewed the document. Overall I think the concept is clearly
communicated, both the need, and how it is implemented. It was easy to
read, and concise. Great work!

Some comments and nits:

§ 1:
(nit) "DPoP" should be defined on first use in the body of the document, in
my opinion.

§ 3:

I don't follow this why the sentence "For confidential clients, refresh
tokens are bound to the "client_id", which is more flexible than binding it
to a particular public key." belongs in this explanation of the diagram. Is
it indicating that DPoP is not required for confidential clients? If the
sentence was deleted / moved elsewhere in the doc, would it alter the
meaning of the diagram?

I wonder if the diagram would be simpler with less optionality (e.g. public
/ confidential separated into 2 diagrams, if they behave differently), or
maybe just kept as 1 simple diagram, with any optionality described later.
Generally I think these kind of overview diagrams (which are extremely
helpful, and now I must go add one to my own draft) are best if they give
an overview of the protocol without trying to document the entire
specification, or justify why different decisions are made.

§ 4.1 figure 2

(nit) When showing a decoded JWT with the signature dropped, I think it's a
good idea to first present the full encoded JWT. Someone who is new to JWT
may not realize that this data has a signature (or even that it is encoded
at all), since it was dropped already, while showing the before/after can
make this clear. Not essential, I just think it would help readers. Having
this also sets the scene for later when encoded JWTs are used in example
request headers.

§ 4.1, below figure 2:

Regarding "Note: To keep DPoP simple to implement, only the HTTP method and
URI
   are signed in DPoP proofs."

I found this confusing as the entire JWT is signed, and the signature
covers the whole JWT and not just those 2 claims. I get that since the JWT
only meaningfully contains the HTTP method and URI that the statement is
effectively true, but I would rephrase. Something along the lines of:

"Of the HTTP headers in the request, only the HTTP method and URI are
included in the DPoP JWT, and therefore only these 2 headers of the request
are covered by the DPoP proof"

(nit) I would also drop the "To keep DPoP simple to implement" here, and
maybe move any discussion about the design trade-offs to the security
considerations (9.4).

§ 5

(nit) Expand "AS" on first use (or better: just say "authorization server"
and skip the acronym in this case, as "AS" is insider lingo IMO)

§ 5

Regarding "If a refresh token is issued to a public client"...

What about when it's issued to the confidential client? I think when
specific behavior is documented for one client type, it's probably good to
at least mention how the other types behave.

I think this could be clarified here. In fact, I wonder if the text I
highlighted earlier from § 3 might belong here.

§ 8

(nit) Suggest moving to an Appendix

§ 9.1

(nit) "up to a few seconds", I would just say "e.g. a few seconds in the
future". Seems odd to specify an upper bound on a non-exact number.

§ 9.4

" DPoP does not ensure the integrity of the payload or headers of
   requests.  The signature of DPoP proofs only contains the HTTP URI
   and method, but not, for example, the message body or other request
   headers."

Again, I find this an awkward phrase, "the signature of the DPoP proofs
only contains the HTTP URI and method", and I think it' better to talk
about the JWT only containing these claims, and the signature therefore
only signing those claims. Also, (nit) does a proof "contain" something, or
more accurately cover/sign something?

William




On Fri, May 1, 2020 at 12:04 PM Brian Campbell <bcampbell=
[e-mail]> wrote:

> I've pushed out a -01 revision of DPoP hopefully allowing folks enough
> time to read it before the interim meeting on Monday (apologies that it
> wasn't sooner but the edits took longer than expected or hoped). For ease
> of reference the changes in this revision are summarized below. There are,
> of course, still outstanding issues and discussion points that I hope to
> make some progress on during the interim meeting on Monday.
>
>    -01
>
>    *  Editorial updates
>    *  Attempt to more formally define the DPoP Authorization header
>       scheme
>    *  Define the 401/WWW-Authenticate challenge
>    *  Added "invalid_dpop_proof" error code for DPoP errors in token
>       request
>    *  Fixed up and added to the IANA section
>    *  Added "dpop_signing_alg_values_supported" authorization server
>       metadata
>    *  Moved the Acknowledgements into an Appendix and added a bunch of
>       names (best effort)
>
> ---------- Forwarded message ---------
> From: <internet-drafts@ietf..org <[e-mail]>>
> Date: Fri, May 1, 2020 at 12:24 PM
> Subject: New Version Notification for draft-ietf-oauth-dpop-01.txt
> To: Torsten Lodderstedt <[e-mail]>, David Waite <
> [e-mail]>, John Bradley <[e-mail]>, Brian
> Campbell <[e-mail]>, Daniel Fett <[e-mail]>,
> Michael Jones <[e-mail]>
>
>
>
> A new version of I-D, draft-ietf-oauth-dpop-01.txt
> has been successfully submitted by Brian Campbell and posted to the
> IETF repository.
>
> Name:           draft-ietf-oauth-dpop
> Revision:       01
> Title:          OAuth 2.0 Demonstration of Proof-of-Possession at the
> Application Layer (DPoP)
> Document date:  2020-05-01
> Group:          oauth
> Pages:          22
> URL:
> https://www.ietf.org/internet-drafts/draft-ietf-oauth-dpop-01.txt
> Status:         https://datatracker.ietf.org/doc/draft-ietf-oauth-dpop/
> Htmlized:       https://tools.ietf.org/html/draft-ietf-oauth-dpop-01
> Htmlized:
> https://datatracker.ietf.org/doc/html/draft-ietf-oauth-dpop
> Diff:           https://www.ietf.org/rfcdiff?url2=draft-ietf-oauth-dpop-01
>
> Abstract:
>    This document describes a mechanism for sender-constraining OAuth 2.0
>    tokens via a proof-of-possession mechanism on the application level.
>    This mechanism allows for the detection of replay attacks with access
>    and refresh tokens.
>
>
>
>
> Please note that it may take a couple of minutes from the time of
> submission
> until the htmlized version and diff are available at tools.ietf.org.
>
> The IETF Secretariat
>
>
>
> *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
> [e-mail]
> https://www.ietf.org/mailman/listinfo/oauth
>