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

[Brian Campbell <[e-mail]>]

Thanks Neil.  I do appreciate your review and feedback (even if it takes me
a nontrivial amount of time to reply). I've attempted to respond to things
inline below.

On Mon, May 4, 2020 at 5:51 AM Neil Madden <[e-mail]>
wrote:

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

That's fair. It was subjectively true to me as I wrote it but I take your
point.


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

The “stronger methods" was preceded with "potentially" in hopes of toning
down the strength of the statement about strength somewhat. But yes it
could be reworded to take away any comparative language.



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

Agree that mobile/native use-cases deserve a mention.


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

Daniel is way way way more adept than I when it comes to threat and
attacker modeling. He also penned the original text of the draft, much of
which is still present. In trying not to step on his toes and also be
mindful of the disparity in our competency, I'm really hoping that he can
do an overhaul of the threat model, objectives, security considerations,
etc. to address this and similar commentary in a more meaningful way. The
inclusion of BREACH seems worthwhile, thanks for the pointer/suggestion. I
think I'd also like to see some discussion of mitigations for unforeseen
"stuff happens" kinds of threats. Like the cloudflare parser bug of a few
years ago that leaked data including tokens and cookies. The specifics of
that sort of thing probably don't belong in a document like this. But bound
tokens would be protected against reuse when leaked like that or via other
many unforeseen ways.



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

The intended intent of the document is that refresh tokens be DPoP bound
only when issued to public clients. RFC6749 already requires that an AS
bind refresh tokens to the client to which they were issued and that
confidential clients (those having established authentication credentials
with the AS) authenticate to the AS when presenting a refresh token. Thus,
refresh tokens issued to confidential clients are already
sender-constrained to the client and its credentials.


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

Yes. That wasn't meant to imply that other mechanisms weren't possible.
Rather just to highlight the cases where defined interop between AS and RS
is in scope of this doc. We'll look at rewording this so as not to imply
exclusivity.


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

I'm thinking that that bullet point should be removed from this
introductory part and hopefully those questions are or will be addressed
elsewhere in the document. But the idea is that a client will always supply
a DPoP proof in the token request and issued access tokens are bound to the
proof's public key. For a public client, it has to be a proof for the same
key for the duration of the refresh token (b/c the RT is bound). A
confidential client can send a DPoP proof for a different key to
effectively rotate the key pair.


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

Hrm. Section 4 here is trying to define the DPoP header field. I guess that
needs some work/improvement in the document. Section 6 defines the DPoP
Authorization Scheme, which is related to but different from the header
field.


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

There are a number of different ways the various bits of information could
flow. And there can be advantages/disadvantages depending on other things
like opaque v.s. structured ATs and caching and more.

As such, I don't believe there's necessarily a right way to do things. But
the model we've tended towards here and are pretty happy with is that the
proof is sent the same way to both the AS and RS. And that proof contains
the key to which tokens can be bound or such a binding can be checked.



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

Maybe worth mentioning but seems more like a security consideration thing.



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

I'll look to add such a reference and better explain here that verifying
the signature and claims of the DPoP proof JWT is what provides (some
reasonable level) of assurance that the caller holds the private key
corresponding to the public key in the proof. But not more than that on
it's own.


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

There are a variety of different ways this particular cat could have been
skinned. As I mentioned previously this model where the proof is the same
regardless of where it's used is the method of skinning that's kind of
emerged as the preferred. Of course there are some trade offs and
implications with whatever approach is taken. There's some historical
discussion that includes this in
https://github.com/danielfett/draft-dpop/issues/26 and maybe gives some
more insight into how it got to the way it is now.

That's not to try and say that what's been done is the one true way. But
that it's one way of many, each of which has pluses and minuses, and is
where things have coalesced.


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

Good point.


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

Also a good point.


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

The DPoP header syntax is supposed to be defined in the preceding section
4. Will work on improving that section to make that more clear. Section 6
is about protected resource access and the new authorization header scheme
and www-authenticate challenge.



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

Interesting, I hadn't looked at the text that way.  The "public client" was
included there more as a reminder that DPoP-bound refresh tokens would only
be issued to public clients. But I now see how you've interpreted that text
and can't unsee it. Will delete the “by a public client" there.


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

I had assumed that was more or less self evident. But having bugs raised
for that suggests it's a lot less than more. So yeah, maybe such a mention
is worthwhile.



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

I don't think it happened on the mailing list but it's been among the
various approaches discussed. There's still the need for the additional
header at the token endpoint for the proof or I suppose it could be moved
into a new parameter. But, like I've said, there are trade offs with the
different approaches and, while there's nothing necessarily bad about this
one, it's not where we've mostly coalesced. Of course, we the WG could
collectively come to some rough consensus to revisit the approach. But I'm
(I hope understandably) not terribly eager to do so.


>
> (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 token68 is for the access token, which isn't necessarily a JWT. And
token68 was intentionally keeping with RFC6750 where an access token may
contain those other characters.


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

The JWT AT there is the encoded version of the one whose payload is shown
in sec 7 to illustrate confirmation claim. And the jkt value is the hash of
the jwk in the DPoP proof key in the same example. So while I acknowledge
the potential confusion from two JWTs here, there was reasoning behind it.
And I want to keep it. Perhaps an additional example with a short
reference-based AT using some of the less popular toke68 chars and maybe
along with an additional example in sec 7 showing introspection.


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

I opted to use a shorter name for the default and only hash. It's a
departure from the x5t stuff but didn't seem necessary. Some history can be
seen here https://github.com/danielfett/draft-dpop/issues/52 FWIW



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

It can't really because there's no WWW-Authenticate challenge (other than
Basic) coming from the token endpoint and it's not accepting the DPoP
authorization scheme so it would be nonsensical to challenge for one.


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

Okay, we can work on the wording here towards that end. I don't think it'll
go so far as to recommend it but can reword so that it doesn't imply
incompatibility.


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

Thanks for that note as a good reason to keep jti even if the requirements
on checking it are relaxed.


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

Maybe but it seems a bit unnecessary in this context.


>
> [1]:
> https://blog.hboeck.de/archives/900-Generating-CRIME-safe-CSRF-Tokens.html
>
> [2]: https://tools.ietf.org/html/rfc7230#section-3.1.1
>
> Cheers,
>
> Neil
>
>
> On 1 May 2020, at 20:03, 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
>
>
>

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