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 <bcampb...@pingidentity.com> 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 <neil.mad...@forgerock.com > > <mailto:neil.mad...@forgerock.com>> 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 > > <bcampb...@pingidentity.com <mailto:bcampb...@pingidentity.com>> 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 <neil.mad...@forgerock.com > > <mailto:neil.mad...@forgerock.com>> 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 <rifaat.i...@gmail.com > > > <mailto:rifaat.i...@gmail.com>> 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 > > > OAuth@ietf.org <mailto:OAuth@ietf.org> > > > https://www.ietf.org/mailman/listinfo/oauth > > > <https://www.ietf.org/mailman/listinfo/oauth> > > > > > > _______________________________________________ > > OAuth mailing list > > OAuth@ietf.org <mailto:OAuth@ietf.org> > > 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 > OAuth@ietf.org > https://www.ietf.org/mailman/listinfo/oauth _______________________________________________ OAuth mailing list OAuth@ietf.org https://www.ietf.org/mailman/listinfo/oauth
