TLS client certificate support

[jroper]

@guillaumebort and @sadache This pull request is based on #340 by @bblfish

I'm not sure if you've been following that discussion, so I'll give a summary here. The great thing about this feature is that unlike client certificate support in other frameworks (mod_ssl, tomcat etc), this gives the application code full control over when client certificates are required and how to deal with them, rather than it being a static entry in a config file on a per directory basis. Furthermore, the client certificate request is completely asynchronous, which is necessary because it may involve the user selecting a certificate when prompted (who knows how long that may take, they may decide to submit a helpdesk ticket to get a certificate etc etc etc then finally select it), something that I'm sure no other framework provides.

There is a big discussion over the use of setWantClientAuth or setNeedClientAuth. The first one is far more desirable, since it gives the application a chance to render a meaningful error page if client certificates are not supplied, or have resources that can be accessed both with and without client certificates. However, due to a bug that I found in the JDK that I've mentioned in the docs, this will sometimes result (it depends on the client, different browsers show different behaviour) in the server not sending a certificate request. To deal with this, I've added the required boolean parameter to the certs method, and given it no default value. In future, when the JDK has fixed the issue, we could change this to have a default value of false, without breaking backwards compatibility.

I've implemented tests for this, both testing the Scala API and the Java API, as well as done some serious testing against various web browsers.

Please review and comment.

[guillaumebort]

Yes I followed the conversation and it looks good to me. It doesn't add any parametrized type to the RequestHeader trait, right?

Please wait for a review of @sadache and if he doesn't find any problem, please merge it.

Thanks!

[bblfish]

The parametrized types was a bad solution (and never really one I took that seriously) to an important problem - but I gave that up a while ago. The issue of knowing what level a certificate has been validated at is still an important one, but not a blocking one. So I opened a new issue on Lighthouse for it: passing certificate-validation-level info to the client. Sorry, I brought it up here: I was not sure if not solving that issue was what was blocking progress on this issue here - as I got very little feedback.

[pk11]

As far as I can tell this looks good.

[jroper]

@bblfish The reason I chose List is that Lists are far nicer to work with in Java, there is no reason why it can't be array. And the reason I replaced scala.collection.IndexedSeq with Seq is that IndexedSeq is not really anything more than a marker trait to say that this Seq supports random access in constant time, it doesn't change anything about what you can do with the Seq, and I didn't see any reason to expose that implementation detail. We are still using an IndexedSeq in the implementation.

I would guess that most Scala code that wanted to pass the list to other APIs would be passing it to the same Java APIs that accept arrays, in which case, it may be better to return Array[Certificate] from the Scala API as well, save all the conversions between Array and Scala collection classes. But I've got no idea about real world use cases of certificates to know which people are more likely to do... are people more likely to do something with the certificates themselves, or hand it off to an API that only supports Certificate[]?

@sadache Just waiting on a review from you about this before merging it.

[bblfish]

I guess most applications just get the first certificate. Those chains should not be that long anyway - and if they were that should be cause to refuse the connection further down. In scala it is certainly much nicer to work with Seq or IndexedSeq in my experience, and I think a conversion back to Array from IndexedSeq is virtually at no cost. But I think usage will tell, and it is also an issue that may be better discussed as part of Ticket 787

[bblfish]

Hi, I have taken @jroper 's improvements have

• updated them to the latest master branch
• added some flexibility in the creation of TrustManagers - so that we can experiment a bit
  ( compare with Tomcat TLS setup )

see: https://github.com/bblfish/Play20/commits/TLS

[jroper]

@bblfish We have talked about this feature a lot on the Play team, and we have decided that we will target it's inclusion in Play 2.2. The reason for not targeting Play 2.1 is that previously, RequestHeader could be treated as being just a value, it was immutable, it could be copied, passed to multiple threads, even serialised/deserialised to be handled remotely etc. This added functionality to it that would break that, and before we go ahead with that, we wanted to get a better understanding of the consequences, and investigate whether there are other alternatives. That's not going to be feasible for 2.1.

[pk11]

considering @jroper's comment, I am closing this for now.

[bblfish]

I am living near Paris, so if you'd like me to help out a bit more closely we can also meet some time.

[marklister]

Based on James' comment above can we set the milestone to 2.2 and reopen?

Has anyone in the Play team had any thoughts on how to do this and retain immutability of the RequestHeader?

[jroper]

I'm still concerned about adding functionality like this to RequestHeader, particularly as we want to ensure that going forward we are not tied to Netty, and it is possible that other frameworks won't make this so simple.

There are three separate use cases that we want to fulfil here, one is the ability to specify a client certificate trust store, the other is retrieve existing client certificates, the third is the ability to asynchronously renegotiate an SSL handshake, this time getting a client certificate. The first we should definitely do, the second is probably necessary in order to make the first useful, and the third is quite separate. I think we should provide support for the first and second.

As for renegotiating asynchronously, I'm particularly concerned that the main use case it seeks to implement is hindered by this bug in the JDK:

https://bugs.openjdk.java.net/show_bug.cgi?id=100281

Which they say is probably not possible to fix (or maybe they just don't want to fix it). If we can't guarantee consistent behaviour here using want auth, then we need to set need auth, and that leaves us in a similar situation (a confusing browser specific SSL error) as having need auth set without renegotiation.

Perhaps somehow giving access to the ssl context and a method to trigger an asynchronous handshake would be a better approach here, giving the user the full power to do what they want with SSL, and therefore they can also decide what they want to do about the JDK bugs.

One way to implement this would be to have pluggable action types (currently we have EssentialAction and WebSocketAction). Plugins would be implemented specific to the underlying HTTP container (eg currently Netty, in future servlet/spray/whatever). This way a third party plugin could allow creating an SSLAction, it might have the signature SSLSupport => EssentialAction, and would allow this to be done in a way that didn't impact on Plays current architecture. But we'll have to do a lot more thinking about that.

[geeksville]

So from reading this thread, am I correct that TLS client cert access is now removed from the 2.2 release? (i.e. not just removed from inclusion in 2.1)

[reggert]

I'd just like to point out that lack of client certificate support is a non-starter for organizations (esp. government agencies) that use PKI for authentication in their web applications/services as a matter of policy. It's not merely a "nice to have", as they will not be able to use the Play Framework at all without it.

[jroper]

I'd just like to point out that it is considered best practice (and we on the Play team recommend this) that you offload SSL handling to another server, for example, an nginx server running on the local machine. Not only are the SSL implementations in these servers faster, since they use openssl which is known to be faster than Java's SSL engine, but they are also better understood by operations teams in terms of how to configure them with SSL. And with those, you can certainly require client certificates, and you can also pass client certificate information to Play in headers for authentication purposes.

So there is nothing stopping you from using PKI authentication with Play Framework. That means this issue is just a "nice to have", it just means that you don't need to offload SSL to another server. For those that follow our recommended best practices for SSL handling and are already using PKI authentication with their Play application, the feature is completely irrelevant.

[bblfish]

@geeksville @reggert I am maintaining a branch of Play2 that does have client certificates working.
https://github.com/stample/Play20/
It would of course be nice to have official support for it from the Play team. ( I'll put a new pull request out for the new version when I have updated it.)

In any case the issue remains that you need a way to request the client certificate from the client and a way of doing so that would allow you to do TLS-renegotiation. Currently there is no way to get the client certificate using nginx or any other system. To work on getting nginx to do client certificate renegotiation from Play would be a lot more work. It would be very useful to have the abstraction that would cover both cases and I opened a bug report on that.

Note that the criticism of TLS as not user friendly is partly because the TLS-renegotiation aspects have not been used effectively by developers. The branch I am maintaining gives full control to the developer on this issue.

[bblfish]

@geeksville @reggert check out pull request #1761 for TLS support on 2.2.2

[NathanC]

@jroper I realize I'm resurrecting a dead thread, but wanted to see if there has been any change in this. If not, do you have any material detailing the practice of offloading TLS to another server? I'm trying to get information on the client cert while handling a request.

[wsargent]

@PainterAndHacker There hasn't been any change to this. Offloading TLS is something you'd do as part of setting up a front end HTTP server -- see https://www.playframework.com/documentation/2.4.x/HTTPServer for more details (there are TLS details in comments, but bettercrypto.org will have the definitive secure config)