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

[Benjamin Kaduk <[e-mail]>]

I would go ahead and put them in.  The blog post might get some
pushback, but I think there's plenty of precedent for academic
papers.

-Ben

On Wed, Apr 18, 2018 at 09:34:23AM -0600, Brian Campbell wrote:
> Thanks for the text, Neil. And the nit on the text, Ben. I'll include it in
> the next draft.
> 
> Ben, bit of a procedural question for you: can or should I include those
> references (https://www.cryptologie.net/article/374/common-x509-certificate-
> validationcreation-pitfalls/ & http://www.cs.utexas.edu/~
> shmat/shmat_ccs12.pdf) that Neil had with the text in the draft as
> informational? Or? I'm honestly not sure if it's okay to cite a blog post
> or university paper.
> 
> 
> <https://www.cryptologie.net/article/374/common-x509-certificate-validationcreation-pitfalls/>
> 
> 
> 
> 
> On Tue, Apr 17, 2018 at 8:13 AM, Benjamin Kaduk <[e-mail]> wrote:
> 
> > Picking nits, but maybe "established and well-tested X.509 library
> > (such as one used by an established TLS library)", noting that TLS
> > 1.3 has added a new protocol feature that allows for TLS and X.509
> > library capabilities to be separately indicated (as would be needed
> > if they were organizationally separate).
> >
> > -Ben
> >
> > On Tue, Apr 17, 2018 at 10:48:04AM +0100, Neil Madden wrote:
> > > OK, here’s a stab at a new security considerations section on X.509
> > parsing and validation:
> > >
> > > ---
> > > 5.3 X.509 Certificate Parsing and Validation Complexity
> > >
> > > Parsing and validation of X.509 certificates and certificate chains is
> > complex and implementation mistakes have previously exposed security
> > vulnerabilities. Complexities of validation include (but are not limited
> > to) [1][2][3]:
> > > - checking of Basic Constraints, basic and extended Key Usage
> > constraints, validity periods, and critical extensions;
> > > - handling of null-terminator bytes and non-canonical string
> > representations in subject names;
> > > - handling of wildcard patterns in subject names;
> > > - recursive verification of certificate chains and checking certificate
> > revocation.
> > > For these reasons, implementors SHOULD use an established and
> > well-tested TLS library for validation of X.509 certificate chains and
> > SHOULD NOT attempt to write their own X.509 certificate validation
> > procedures.
> > >
> > > [1] https://www.cryptologie.net/article/374/common-x509-certificate-
> > validationcreation-pitfalls/ <https://www.cryptologie.net/
> > article/374/common-x509-certificate-validationcreation-pitfalls/>
> > > [2] http://www.cs.utexas.edu/~shmat/shmat_ccs12.pdf <
> > http://www.cs.utexas.edu/~shmat/shmat_ccs12.pdf>
> > > [3] https://tools.ietf.org/html/rfc5280 <https://tools.ietf.org/html/
> > rfc5280>
> > >
> > > ---
> > >
> > > NB - this blog post [1] is the best concise summary of attacks I could
> > find. Most of these attacks have been published as Black Hat talks and I
> > can’t seem to find definitive references or good survey papers (beyond [2],
> > which is a little older).
> > >
> > > Let me know what you think,
> > >
> > > Neil
> > >
> > >
> > > > On 12 Apr 2018, at 20:42, Brian Campbell <[e-mail]>
> > wrote:
> > > >
> > > > Thanks Neil.
> > > >
> > > > Other than the potential metadata changes, which I'd like more WG
> > input on and may raise in a new thread, I think I've got enough to make
> > updates addressing your comments.  But please do send text for that
> > Security Considerations bit, if you come up with something.
> > > >
> > > > On Thu, Apr 12, 2018 at 3:03 AM, Neil Madden <
> > [e-mail] <mailto:[e-mail]>> wrote:
> > > > Hi Brian,
> > > >
> > > > Thanks for the detailed responses. Comments in line below (marked with
> > ***).
> > > >
> > > > Neil
> > > >
> > > > On Wednesday, Apr 11, 2018 at 9:47 pm, Brian Campbell <
> > [e-mail] <mailto:[e-mail]>> wrote:
> > > > 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] <mailto:[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 <
> > 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.
> > > >
> > > > ***
> > > > OK, that is reasonable. Given that the draft says SHOULD rather than
> > MUST for using this confirmation key method, I think it is currently
> > allowed to use either representation.
> > > >
> > > >
> > > >
> > > > 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/tls-extensiontype-values.xhtml#tls-extensiontype-values-3 <
> > https://www.iana.org/assignments/tls-extensiontype-
> > values/tls-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/draft-ietf-oauth-mtls-07#
> > section-2.1.1 <https://tools.ietf.org/html/draft-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.
> > > >
> > > > *** Thanks.
> > > >
> > > >
> > > >
> > > > 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.
> > > >
> > > > *** OK, that seems good to me.
> > > >
> > > >
> > > >
> > > > 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?
> > > >
> > > >  ***
> > > > To be honest, when I raised this it was because I didn’t really know
> > what the implications were. I’ve done some reading around and I think it
> > should all just work - both session resumption and PSK-based resumption
> > preserve the original client and server authentication context so we can
> > assume that any presented client cert is still valid for the resumed
> > session. So I think we can leave out any discussion of this and assume it
> > works as expected.
> > > >
> > > >
> > > >
> > > > 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/draft-ietf-oauth-pop-
> > architecture-08#section-6.2 <https://tools.ietf.org/html/
> > draft-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?
> > > >
> > > > *** Thanks. I agree this might cause compatibility issues. It is not a
> > big issue for us, but might cause some confusion.
> > > >
> > > >
> > > >
> > > > 6. The OAuth 2.0 PoP Architecture draft says (
> > https://tools.ietf.org/html/draft-ietf-oauth-pop-architecture-08#section-5
> > <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/
> > draft-ietf-oauth-mtls-07#section-3 <https://tools.ietf.org/html/
> > draft-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 <
> > 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-
> > distribution-03#section-5.1 <https://tools.ietf.org/html/
> > draft-ietf-oauth-pop-key-distribution-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.
> > > >
> > > > *** You are right, the text doesn’t preclude that. I am happy with
> > that solution. I suspect most people will deploy and be happy with reusing
> > a long-lived cert for every access token, so this may not matter in
> > practice.
> > > >
> > > >
> > > > 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.
> > > >
> > > > *** On reflection, I am going to withdraw this comment. As you say
> > there are other ways to correlate clients. The privacy issue would mainly
> > arise in the context of dynamic client registration e.g., a pattern I’ve
> > seen described where every instance of a mobile app does dynamic client
> > registration to avoid including credentials directly in the app bundle.
> > This would make clients one-to-one with users. But (a) those apps are
> > fairly unlikely to be using TLS certs, and (b) that is more of a privacy
> > consideration for dynamic client registration rather than this draft.
> > > >
> > > >
> > > >
> > > > 8. This is maybe a more general point, but RFC 6750 defines the
> > Authorization: Bearer scheme (https://tools.ietf.org/html/
> > rfc6750#section-2 <https://tools.ietf.org/html/rfc6750#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.
> > > >
> > > >  *** OK.
> > > >
> > > >
> > > > 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?
> > > >
> > > > *** I’ll try and come up with some text.
> > > >
> > > >
> > > >
> > > > 10. The PKI client authentication method (https://tools.ietf.org/html/
> > draft-ietf-oauth-mtls-07#section-2.1 <https://tools.ietf.org/html/
> > draft-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?
> > > >
> > > > *** I think you are right about this. I always assumed it was
> > symmetric (and I think it technically could work), but the spec only talks
> > about stapling in the server-side of the handshake.
> > > >
> > > > 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.
> > > >
> > > > *** Thanks.
> > > >
> > > >
> > > >
> > > > 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 <
> > 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.
> > > >
> > > > *** OK, that is an interesting observation. If either the client or AS
> > suspect the key has been compromised they can revoke the access token(s)
> > instead.
> > > >
> > > >
> > > >
> > > > 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?
> > > >
> > > > *** Yes please.
> > > >
> > > > 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]
> > <mailto:[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 <
> > 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] <mailto:[e-mail]>
> > > > > https://www.ietf.org/mailman/listinfo/oauth <
> > https://www.ietf.org/mailman/listinfo/oauth>
> > > >
> > > >
> > > > _______________________________________________
> > > > OAuth mailing list
> > > > [e-mail] <mailto:[e-mail]>
> > > > https://www.ietf.org/mailman/listinfo/oauth <
> > 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.
> > > >
> > > >
> > > > 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._