Some review comments:

Section 1:
I think terms like “relatively simple” are subjective and should be left out. I 
don’t think the machinery of JWS signature verification (and associated 
security issues) is necessarily simple at all.

“stronger methods … such as [RFC8705] or [token binding]”
I would dispute that these are actually stronger methods in the context of an 
SPA, putting aside the usability issues. As has been discussed before, TLS is a 
shared context in a web browser - the same TLS connection will be reused for 
all requests, regardless of the origin they come from. The situation is better 
for cert-bound ATs because you need the CORS Access-Control-Allow-Credentials 
to be set to make a cross-origin request with a client certificate, but as I 
describe below there are also other attacks against TLS that don’t apply to 
DPoP (and would be a good reason to adopt DPoP). 

I think this section should also mention mobile use-cases, as IMO the case for 
DPoP is much stronger in those environments. Many of our customers are looking 
for a PoP solution for mobile and have rejected mTLS for various reasons (which 
have previously been discussed on this list, e.g. incompatible middleboxes). 
DPoP is potentially really nice for these environments as they often have a 
nice secure element available to store the private key and are much less 
susceptible to XSS and so on, but may still worry about a malicious RS. IoT 
applications may also be a use-case as requests are often relayed over multiple 
hops and different transport protocols, so TLS cannot be used for end-to-end 
security (although in that case you probably want a full request signing 
approach).

Section 2:
I think the “Main Objective” in section 2 could be improved. The vast majority 
of the mechanisms described in the draft do nothing to address this main 
objective, and are in fact intended to prevent DPoP proof replay which is 
relegated to a “secondary objective” in section 9. Only the “htu” claim really 
has anything at all to do with the main objective. I think some of these 
secondary objectives, especially the anti-replay one, should be brought forward 
to section 2 to better justify what follows.

It has been mentioned several times on this list that an audience restriction 
in the AT would also solve this main objective, and the response has generally 
been that adding audience restrictions is hard in practice - e.g. due to the AS 
not knowing where the AT is intended to be used. I think it would be useful to 
explicitly add some discussion of this, and perhaps a comparison with things 
like resource indicators that presumably could solve at least some of the 
issues with audience restrictions.

I think there *are* significant advantages of DPoP that address real-world 
attacks that cannot be solved in any other way currently. For example, I was 
reading [1] recently about CRIME/BREACH attacks against CSRF tokens and it 
occurred to me that exactly the same kind of attack is a real threat to access 
tokens in a browser and that DPoP is an effective mitigation against it. To 
summarise, these attacks exploit vulnerabilities in TLS when combined with 
compression (either in TLS itself or HTTP compression) and can allow an 
attacker to steal bearer tokens or cookies after observing enough requests with 
the same token (and being able to influence some other content in the same 
request/response). One recommended mitigation is to randomize tokens so that 
they are different on every request, which thwarts the attack. Although DPoP 
doesn’t directly randomize the AT the AT is useless without a DPoP proof and a 
unique “jti” claim ensures that the DPoP proof is randomized and so not 
recoverable through this attack. Given that BREACH is largely unmitigated in 
practice, due to the usefulness of compression, adding a description of this 
attack to the justification would IMO significantly improve the rationale for 
DPoP.

Section 3:
The second bullet point of step (B) isn’t clear - is it implying that refresh 
tokens for confidential clients shouldn’t be DPoP-bound? Must not?

Step (C) implies that only JWT-structured ATs or token introspection are 
supported for communicating the public key. Presumably deployers are free to 
use another secure mechanism if they wish?

The last bullet point about requiring a DPoP proof for refresh tokens is 
ambiguous about what happens in the case of confidential clients if the refresh 
token is not DPoP-bound. Do they not supply a DPoP proof at all (and it reuses 
the original one), or must they supply a new one? Can they supply a *different* 
DPoP proof to e.g. rotate their key pair?

Section 4:
Maybe a forward reference to section 6 when mentioning the DPoP header field, 
as that’s where it is defined.

4.1:
I’m not keen about including the full “jwk” in every DPoP proof. It never 
changes for the life of the AT and adds considerable overhead to the size of 
the proof. If the RS is using token introspection (with opaque ATs) then it 
would be much more efficient to return the JWK in the token introspection 
response and let the RS cache that. Alternatively the AS could publish the JWK 
under a URL on behalf of the client and include a “jku” header instead.

In the last paragraph, is it worth explicitly calling out things like GraphQL 
as not suitable for DPoP due to the fact that they reuse the same URI and HTTP 
method for all requests?

4.2:
I feel like this section should contain a forward reference to section 6, 
otherwise it appears to suggest that verifying the signature and claims is 
sufficient - without the crucial step of ensuring that the JWK matches the 
actual access token. The potential for implementers to get this wrong is 
another reason to prefer not including the JWK directly in the DPoP proof but 
instead have it retrieved from the access token (either as a cnf or from token 
introspection). That makes it much harder to validate a proof that is unrelated 
to the AT.

Step 6 - HTTP methods are case-sensitive [2] so this check should also be.

Step 7 - It might be worth explicitly clarifying that “HTTP URI” means HTTPS.

Section 5:
Can we move the definition of the DPoP header syntax (currently in section 6) 
into its own section before here?

In the last paragraph delete “by a public client":

   When a DPoP-bound refresh token is used at the token endpoint by a
   public client, the AS MUST ensure that the DPoP proof contains the
   same public key as the one the refresh token is bound to.  The access
   token issued MUST be bound to the public key contained in the DPoP
   proof.

If a DPoP-bound RT is used then the AS should verify the DPoP proof matches the 
one bound to the RT regardless of which client presented it. (Otherwise a 
confidential client could steal RTs from a public client and circumvent the 
DPoP binding).

Is it worth explicitly mentioning at some point that the AS *shouldn’t* try to 
enforce any DPoP binding at the introspection endpoint? We’ve actually had bugs 
raised from people believing that we should enforce this for mTLS, not 
realising the chicken-and-egg situation that would create.

Section 6:
I may have missed it, but has there been any discussion of using a parameter on 
the Authorization header rather than an entirely separate header? For example:

Authorization: DPoP at=…,proof=…

Appendix A of RFC 7235 describes the use of token68 syntax for “legacy” 
authentication schemes, so I assume the authparam syntax is preferred now and 
this would avoid adding another header (but it does mean that header might get 
very large if the AT is also a JWT, potentially exceeding size limits in 
servers/proxies).

(It’s also weird to explicitly define the syntax as token68 even though we know 
it's a JWT and will never contain some of those chars).

The example uses a JWT AT, which is needlessly confusing. I think using a short 
reference-based AT in this example would both reduce the size and also make 
clear that the DPoP proof goes in the DPoP header not the Authorization header.

Section 7:
As mentioned earlier, I’d prefer that the full JWK be included in the “cnf” 
rather than a hash, but if we’re going to use a hash can we be explicit and use 
“jkt#S256” please?

Section 8:
Is the AS also allowed to return a WWW-Authenticate challenge with an algs 
attribute if the client sends a proof with an unsupported algorithm?

Section 9:
DPoP is not incompatible with TLS-based methods, and has advantages over them. 
I think the wording here should leave open the idea that you could actually use 
both.

9.1:
This would be a good place to mention BREACH as an example of how a DPoP proof 
(and AT) might leak, despite only being sent over a direct HTTPS channel. Note 
though that adding a random jti is an effective defence against this even if 
the server doesn’t check it.

9.2:
Maybe mention that clients should avoid reusing the same key for other JWTs too?

[1]: https://blog.hboeck.de/archives/900-Generating-CRIME-safe-CSRF-Tokens.html 
<https://blog.hboeck.de/archives/900-Generating-CRIME-safe-CSRF-Tokens.html> 
[2]: https://tools.ietf.org/html/rfc7230#section-3.1.1 
<https://tools.ietf.org/html/rfc7230#section-3.1.1>

Cheers,

Neil


> On 1 May 2020, at 20:03, Brian Campbell 
> <bcampbell=40pingidentity....@dmarc.ietf.org> 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-dra...@ietf..org <mailto:internet-dra...@ietf.org>>
> Date: Fri, May 1, 2020 at 12:24 PM
> Subject: New Version Notification for draft-ietf-oauth-dpop-01.txt
> To: Torsten Lodderstedt <tors...@lodderstedt.net 
> <mailto:tors...@lodderstedt.net>>, David Waite <da...@alkaline-solutions.com 
> <mailto:da...@alkaline-solutions.com>>, John Bradley <ve7...@ve7jtb.com 
> <mailto:ve7...@ve7jtb.com>>, Brian Campbell <bcampb...@pingidentity.com 
> <mailto:bcampb...@pingidentity.com>>, Daniel Fett <m...@danielfett.de 
> <mailto:m...@danielfett.de>>, Michael Jones <m...@microsoft.com 
> <mailto:m...@microsoft.com>>
> 
> 
> 
> 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 
> <https://www.ietf.org/internet-drafts/draft-ietf-oauth-dpop-01.txt>
> Status:         https://datatracker.ietf.org/doc/draft-ietf-oauth-dpop/ 
> <https://datatracker.ietf.org/doc/draft-ietf-oauth-dpop/>
> Htmlized:       https://tools.ietf.org/html/draft-ietf-oauth-dpop-01 
> <https://tools.ietf.org/html/draft-ietf-oauth-dpop-01>
> Htmlized:       https://datatracker.ietf.org/doc/html/draft-ietf-oauth-dpop 
> <https://datatracker.ietf.org/doc/html/draft-ietf-oauth-dpop>
> Diff:           https://www.ietf.org/rfcdiff?url2=draft-ietf-oauth-dpop-01 
> <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 
> <http://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
> OAuth@ietf.org
> https://www.ietf.org/mailman/listinfo/oauth

_______________________________________________
OAuth mailing list
OAuth@ietf.org
https://www.ietf.org/mailman/listinfo/oauth

Reply via email to