Re: [OAUTH-WG] WGLC on draft-ietf-oauth-mtls-07

[Brian Campbell <[e-mail]>]

Thanks for the review and feedback, Neil. I apologize for my being slow to
respond. As I said to Justin recently
<https://mailarchive.ietf.org/arch/msg/oauth/cNmk8fSuxp37L-z8Rvr6_EnyCug>,
I've been away from things for a while. Also there's a lot here to get
through so took me some time.

It looks like John touched on some of your comments but not all. I'll try
and reply to them as best I can inline below.


On Thu, Mar 29, 2018 at 9:18 AM, Neil Madden <[e-mail]>
wrote:

> Hi,
>
> I have reviewed this draft and have a number of comments, below. ForgeRock
> have not yet implemented this draft, but there is interest in implementing
> it at some point. (Disclaimer: We have no firm commitments on this at the
> moment, I do not speak for ForgeRock, etc).
>
> 1. https://tools.ietf.org/html/draft-ietf-oauth-mtls-07#section-3.1
> defines a new confirmation method “x5t#S256”. However, there is already a
> confirmation method “jwk” that can contain a JSON Web Key, which itself can
> contain a “x5t#S526” claim with exactly the same syntax and semantics. The
> draft proposes:
>
>         { “cnf”: { “x5t#S256”: “…” } }
>
> but you can already do:
>
>         { “cnf”: { “jwk”: { … , “x5t#S256”: “…” } } }
>
> If the intent is just to save some space and avoid the mandatory fields of
> the existing JWK types, maybe this would be better addressed by defining a
> new JWK type which only has a thumbprint? e.g., { “kty”: “x5t”, “x5t#S256”:
> “…” }.
>

The intent of the x5t#S256 confirmation method was to be space efficient
and straightforward while utilizing the framework and registry that RFC
7800 gives.  Even a new JWK type like that would still use more space. And
I'd argue that the new confirmation method is considerably more
straightforward than registering a new JWK type (and the implications that
would have on JWK implementations in general) in order to use the existing
"jwk" confirmation method.



>
> 2. I find the naming “mutual TLS” and “mTLS” a bit of a misnomer: it’s
> really only the client authentication that we are interested here, and the
> fact that the server also authenticates with a certificate is not hugely
> relevant to this particular spec (although it is to the overall security of
> OAuth). Also, TLS defines non-certificate based authentication mechanisms
> (e.g. TLS-SRP extension for password authenticated key exchange, PSK for
> pre-shared key authentication) and even non-X.509 certificate types (
> https://www.iana.org/assignments/tls-extensiontype-values/t
> ls-extensiontype-values.xhtml#tls-extensiontype-values-3). I’d prefer
> that the draft explicitly referred to “X.509 Client Certificate
> Authentication” rather than mutual TLS, and changed identifiers like
> ‘tls_client_auth’ (https://tools.ietf.org/html/d
> raft-ietf-oauth-mtls-07#section-2.1.1) to something more explicit like
> ‘tls_x509_pki_client_auth’.
>
> This is especially confusing in section 3 on sender constrained access
> tokens, as there are two different servers involved: the AS and the
> protected resource server, but there is no “mutual” authentication between
> them, only between each of them and the client.
>

Choosing names and terminology is difficult and the "right" wording is
often subjective. I believe that the current wording sufficiently conveys
what is going on in the draft to most readers. Most readers thus far seem
to agree. There is some text now that does say that the mutual auth in the
draft is in fact X.509 client cert authn but, in the next revision, I'll
look for other opportunities where it could be stated more clearly.



>
> 3. The draft links to the TLS 1.2 RFC, while the original OAuth 2.0 RFC
> only specifies TLS 1.0. Is the intention that TLS 1.2+ is required? The
> wording in Section 5.1 doesn’t seem clear if this could also be used with
> TLS 1.0 or 1.1, or whether it is only referring to future TLS versions.
>

The reference to BCP 195 (which unfortunately the original OAuth 2.0 RFC
doesn't have because it didn't exist then) is meant to account for changing
versions and recommendations around TLS. Currently that BCP says TLS 1.2 is
a must and suggests against 1.1 & 1.0 but doesn't outright prohibit them.



>
> 4. It might be useful to have a discussion for implementors of whether TLS
> session resumption (and PSK in TLS 1.3) and/or renegotiation impact the use
> of client certificates, if at all?
>

That might well be useful but I don't myself know what it would say. I've
(maybe naively) figured those are deployment details that will just work
out. Perhaps you could propose some text around such a discussion that the
WG could consider?



>
> 5. Section 3 defines sender-constrained access tokens in terms of the
> confirmation key claims (e.g., RFC 7800 for JWT). However, the OAuth 2.0
> Pop Architecture draft defines sender constraint and key confirmation as
> different things (https://tools.ietf.org/html/d
> raft-ietf-oauth-pop-architecture-08#section-6.2). The draft should decide
> which of those it is implementing and if sender constraint is intended,
> then reusing the confirmation key claims seems misleading. (I think this
> mTLS draft is doing key confirmation so should drop the language about
> sender constrained tokens).
>

I will say again that choosing names and terminology is difficult...

Although I must admit that I started using "sender constrained" somewhat
indiscriminately at first and it's just sort of stuck. At the time I was
trying to incorporate bits of draft-sakimura-oauth-jpop in a way that might
help bring on and keep the authors of that draft onboard with this mtls
draft. In retrospect it looks like I did that part wrong anyway. But that
was the thinking at the time and the history, for whatever it's worth. More
recently, Nat was requesting that "sender constrained" be included in the
title. So there's that too.

In defense of my mistake, however, if there's a line between "sender
constrained" and "key confirmation" tokens, it's a bit of a fuzzy line. And
I'd argue that what this draft is doing pretty close to the line.

But ultimately I think you are right that what this mtls draft is doing
with access tokens is more accurately described with the key confirmation
term.

So, yes, the draft should probably drop (or at least minimize) the language
about sender constrained. I'll do that in the next draft version, barring
big objections from the WG.

The tricky thing with making that change is that there a client and server
metadata parameters with the name "mutual_tls_sender_constrained_access_tokens"
that should probably also change. But that would be a breaking change (of
sorts anyway), which shouldn't be taken lightly at this stage. I feel that
some explicit okays from the WG are needed before doing so (rough consensus
stye) . Any WG members want to weigh in here and help get a "sense of the
group" concerning changing those metadata names?



>
> 6. The OAuth 2.0 PoP Architecture draft says (
> https://tools.ietf.org/html/draft-ietf-oauth-pop-architecture-08#section-5
> ):
>
>          Strong, fresh session keys:
>
>                 Session keys MUST be strong and fresh.  Each session
> deserves an
>                 independent session key, i.e., one that is generated
> specifically
>                 for the intended use.  In context of OAuth this means that
> keying
>                 material is created in such a way that can only be used by
> the
>                 combination of a client instance, protected resource, and
>                 authorization scope.
>
>
> However, the mTLS draft section 3 (https://tools.ietf.org/html/d
> raft-ietf-oauth-mtls-07#section-3) says:
>
>         The client makes protected resource requests as described in
>         [RFC6750], however, those requests MUST be made over a mutually
>         authenticated TLS connection using the same certificate that was
> used
>         for mutual TLS at the token endpoint.
>
> These two statements are contradictory: the OAuth 2.0 PoP architecture
> effectively requires a fresh key-pair to be used for every access token
> request, whereas this draft proposes reusing the same long-lived client
> certificate for every single access token and every resource server.
>
> In the self-signed case (and even in the CA case, with a bit of work -
> e.g., https://www.vaultproject.io/docs/secrets/pki/index.html) it is
> perfectly possible for the client to generate a fresh key-pair for each
> access token and include the certificate on the token request (e.g., as per
> https://tools.ietf.org/html/draft-ietf-oauth-pop-key-distrib
> ution-03#section-5.1 - in which case an appropriate “alg” value should
> probably be described). This should probably at least be an option.
>

This draft doesn't necessarily seek to align with the (long expired) PoP
architecture draft.  Rather it is aiming to provide a pragmatic solution
for PoP style access tokens and OAuth client auth using mTLS client
certificates.

That said, with the current draft, it's certainly possible for a client to
update its cert more frequently, even so far as using a new one for each
access token. The details of how that would work will vary some based on
the token endpoint authentication method. But it's not precluded.



>
> 7. The use of a single client certificate with every resource server (RS)
> should be called out in a Privacy Considerations section, as it allows
> correlation of activity.
>

Practically speaking the access tokens being presented likely already have
correlatable info in them about the client as well as the user. I don't
know that there's much of a (new) concern in reality. If you feel
this concern is unique and import enough though, perhaps there's some text
you'd like to propose for a Privacy Considerations section that the WG
could consider? I mean, I guess it doesn't hurt to mention it but I would
like to avoid overstating the issue.



>
> 8. This is maybe a more general point, but RFC 6750 defines the
> Authorization: Bearer scheme (https://tools.ietf.org/html/r
> fc6750#section-2) for a client to communicate it’s access token to the RS
> in a standard way. As sender-constrained access tokens are not strictly
> bearer tokens any more, should this draft also register a new scheme for
> that? Should there be a generic PoP scheme?
>

The thinking and general consensus (in this draft as well as the OAuth
token binding work) has been that continuing to use the RFC 6750 scheme
while putting the "not strictly bearer" stuff in (or referenced by) the
token itself will be easier on deployment and implementation. And better
for adoption as a result. I believe some early implementation work has
borne that out too.



>
> 9. The Security Considerations should really make some mention of the long
> history of attacks against X.509 certificate chain validation, e.g. failure
> to check the “CA” bit in the basic constraints, errors in parsing DNs, etc.
> It should be strongly suggested to use an existing TLS library to perform
> these checks rather than implementing your own checks. This relates to
> Justin’s comments around DN parsing and normalisation.
>

Suggesting to use an existing TLS library is certainly sound advice and I
sort of felt is implied in the draft. But saying so more
strongly/explicitly might be worthwhile.  And pointing to historical
reasons to do so would probably be good too.  Could you propose a new
Security Considerations section or maybe augmentation of §5.2 with that
content?



>
> 10. The PKI client authentication method (https://tools.ietf.org/html/d
> raft-ietf-oauth-mtls-07#section-2.1) makes no mention at all of
> certificate revocation and how to handle checking for that (CRLs, OCSP -
> with stapling?). Neither does the Security Considerations. If this is a
> detail to be agreed between then AS and the CA (or just left up to the AS
> TLS stack) then that should perhaps be made explicit. Again, there are
> privacy considerations with some of these mechanisms, as OCSP requests are
> typically sent in the clear (plain HTTP) and so allow an observer to see
> which clients are connecting to which AS.
>

I didn't think that a TLS client could do OCSP stapling?

That aside, revocation checking (how and even if) is something that's at
the discretion of the AS. I can add something in §2.1 to say that more
explicitly.



> 11. The same comment applies to how the protected resource checks for
> revocation of the certificate presented during sender constrained access
> token usage. Should the RS make its own revocation checks based on the
> information in the certificate presented, or should it trust the
> certificate while the access token is still valid? If the latter case, is
> the AS responsible for revoking any access tokens whose certificate have
> been revoked (if so, should it be doing an OCSP call on every token
> introspection request, and should the RS be passing on the
> certificate/serial number on that request)? If the Client request uses OCSP
> Stapling (https://en.wikipedia.org/wiki/OCSP_stapling) how can the RS
> verify the signature on that if it does not have a separate trust
> relationship with the CA already?
>

The draft effectively uses cert mtls at the RS as a proof-of-possession
method only and not as authentication. So revocation checking isn't really
applicable. In specific deployment situations, I suppose an RS could check
revocation. But that'd be a deployment decision for the RS that's beyond
the scope of this draft.



> 12. The use of only SHA-256 fingerprints means that the security strength
> of the sender-constrained access tokens is limited by the collision
> resistance of SHA-256 - roughly “128-bit security" - without a new
> specification for a new thumbprint algorithm. An implication of this is
> that is is fairly pointless for the protected resource TLS stack to ever
> negotiate cipher suites/keys with a higher level of security. In more
> crystal ball territory, if a practical quantum computer becomes a
> possibility within the lifetime of this spec, then the expected collision
> resistance of SHA-256 would drop quadratically, allowing an attacker to
> find a colliding certificate in ~2^64 effort. If we are going to pick just
> one thumbprint hash algorithm, I would prefer we pick SHA-512.
>

The idea behind haveing just one thumbprint hash algorithm was to keep
things simple. And SHA-256 seems good enough for the reasonably foreseeable
future (and space aware). Also a new little spec to register a different
hash algorithm, should the need arise, didn't seem particularity onerous.

That was the thinking anyway. Maybe it is too short sighted though?

I do think SHA-256 should stay regardless.

But the draft could also define SHA-512 (and maybe others). What do you and
WG folks think about that?

It would probably then be useful for the metadata in §3.3 and §3.4 to
change from just boolean values to something to convey what hash alg/cnf
method the client expects and the list of what the server supports. That's
maybe something that should be done anyway. That'd be a breaking change to
the metadata. But there's already another potential breaking change
identified earlier in this message. So maybe it's worth doing...

How do folks feel about making this kind of change?






> Cheers,
>
> Neil
>
>
> > On 19 Mar 2018, at 22:34, Rifaat Shekh-Yusef <[e-mail]>
> wrote:
> >
> > All,
> >
> > As discussed during the meeting today, we are starting a WGLC on the
> MTLS document:
> > https://tools.ietf.org/html/draft-ietf-oauth-mtls-07
> >
> > Please, review the document and provide feedback on any issues you see
> with the document.
> >
> > The WGLC will end in two weeks, on April 2, 2018.
> >
> > Regards,
> >  Rifaat and Hannes
> >
> > _______________________________________________
> > OAuth mailing list
> > [e-mail]
> > https://www.ietf.org/mailman/listinfo/oauth
>
>
> _______________________________________________
> 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._