Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Allow retrieval of client certificate #175

Closed
bpfoster opened this Issue Jun 17, 2013 · 23 comments

Comments

7 participants
Contributor

bpfoster commented Jun 17, 2013

When using client auth with 2-way SSL, present the client certificate for custom authorization code to use. The result should be something similar to the call of SSLEngine.getSession().getPeerCertificateChain()

Originally posted in the users group at https://groups.google.com/forum/?fromgroups#!topic/finaglers/j4xrFRF1MDI

Contributor

mariusae commented Jun 17, 2013

A proposed API: make SSL transports a special type, with retrievable certificates. Listeners can then match on this:

transport match {
  case SSLTransport(cert) => …
  case _ => …
}
Contributor

danielpcox commented Oct 17, 2013

How would this allow an RPC method implementation access to the client cert, say in a custom Filter?

Contributor

danielpcox commented Oct 18, 2013

I ask because I have the same requirement as @bpfoster, and I was interested in contributing this improvement. I'm having some trouble figuring out the flow, though.

mbk commented Oct 24, 2013

👍 form me on this. It would also allow for easy SSL debugging on the "higher" layer, i.e. catching handshake exceptions, wrap them in json, and send them back.

Ping?

Contributor

stevegury commented Aug 21, 2014

@delitescere sorry for the absence of updates, but this has been unprioritized on our side, but we would gladly accept pull request (and/or give guidance about that).

Contributor

bpfoster commented Nov 4, 2014

@stevegury I'd like to start revisiting this. I can see how to retrieve the certificate from within a ChannelHandler, but am not finding a clean way to get it to the service/filter layer. Do you think anyone could provide some guidance on that?

Contributor

danielpcox commented Jul 17, 2015

Does this commit on develop: 3d6f236 and the fact that Thrift RPC is now supported over TLS close this issue?

Contributor

mosesn commented Jul 17, 2015

I think so? Have you done this by any chance :P

Contributor

danielpcox commented Jul 17, 2015

Not yet, but it's on my list. :) I'll give it a shot next week and let you know.

Contributor

danielpcox commented Jul 27, 2015

Make that the next three weeks... I'm pretty distracted at the moment. @mosesn would you be able to try it out? At the moment, I expect to be able to circle back around and confirm that this solves our problems by the end of next week, but anybody with some time to kill right now could pull develop and try extracting a client certificate over TLS via Thrift and HTTP.

I think you'd just have to write a little demo ping server that uses TLS and grabs some attribute out of the client cert, and then write a client to connect to it with any certificate.

Contributor

mosesn commented Jul 27, 2015

End of next week is fine, I don't think anyone needs this more urgently than you do (and if they do, they can run their own tests :P)

Contributor

danielpcox commented Aug 21, 2015

OK, I'm finally getting around to this now. I think I'm misunderstanding something fundamental, though. How do I actually get ahold of the Transport from inside a service's request handler? How can I do anything with the client's incoming certificate information so as to affect the response?

Also, as an aside, is the builder pattern deprecated now? I've been operating under the assumption that it's the idiomatic way to construct a Finagle client or server, but maybe there's no way to do what I want to with a ServerBuilder?

Contributor

mosesn commented Aug 21, 2015

Looks like you should be able to call Transport.peerCertificate on the serverside.

I don't think it's officially deprecated yet, but we're gently encouraging folks to use the finagle-6 style APIs.

Contributor

danielpcox commented Aug 28, 2015

I'm sorry Moses, I still don't understand. How do I get a handle on the Transport object from within a server implementation? I've read all the documentation and I still only know how to do the most common things. I'm implementing a Thrift IDL-defined interface, and I don't see how the transport object could be injected at this point.

Contributor

mosesn commented Aug 28, 2015

Have you tried calling Transport.peerCertificate? Or is your question about how to invoke the method? Transport is an object, so it's available globally.

The actual code would look like:

val cert = Transport.peerCertificate
Contributor

danielpcox commented Aug 28, 2015

How can something available globally present me with the certificate for an individual request like this? Is it a thread-local binding or something? How can that be safe? Is each request handled by a single thread and vice-versa, and then all local-bindings cleared for the next request to be processed on that thread?

This must be the key thing I was missing. Does finagle expose many things this way? Is this in the documentation somewhere and I just missed it?

Contributor

mosesn commented Aug 28, 2015

It doesn't expose many things this way, but it does for a few things, like Traces. It's a context that gets threaded through your execution context when you do things like Future#map. Most users don't ever need to use the Local mechanism directly (we consider it an expert feature), so we don't document it explicitly. It should "just work" in the majority of cases.

Contributor

danielpcox commented Sep 1, 2015

Transport.peerCertificate seems to be None at this point in my server implementation. Am I doing anything obviously wrong here? https://github.com/DecipherNow/finagle-tls/blob/master/src/main/scala/com/twitter/finagle/example/tls/TLSServer.scala#L23

Thanks Moses for your patience.

Contributor

bpfoster commented Sep 2, 2015

@danielpcox

  • The big issue is the tls() method in Finagle does not expose a way to configure the engine to want or need client authentication. The default is for neither, so the server will never even prompt the client to present a certificate. You're going to need to create your own engine and use newSslEngine() instead of tls(), and specify whether you want or need client auth in the engine. For example:
val server = ServerBuilder()
      .codec(ThriftServerFramedCodec())
      .newSslEngine(() => {
          val engine: SSLEngine = createSslContext().createSSLEngine()
          engine.setNeedClientAuth(true)
          engine
       })
      .bindTo(new InetSocketAddress(8080))
      .name("TLSServer").build(service)

The finagle native OpenSSL bindings don't currently support specifying client auth modes, so I ended up creating a javax.net.ssl.SSLContext (similar to the Client) to get an SSLEngine. In theory performance may be worse than via native.

  • Your sample certificate has expired so I couldn't get a successful handshake with it. Generating a new one worked.

With those 2 in mind, I was able to then run your code and get back what looks like a good response:
BeautifulDogResponse(O=Internet Widgits Pty Ltd,ST=Some-State,C=AU,true)

  • When you move off the old API, I believe you can configure TLS similar to
Thrift.server.configured(Transport.TLSServerEngine(Some(ssl.newEngine)))
Contributor

danielpcox commented Sep 2, 2015

Thank you @bpfoster, I really appreciate this. Moses helped me earlier today with the new API, and my experiments are here: https://github.com/DecipherNow/finagle-tls/blob/new-api/src/main/scala/com/twitter/finagle/example/tls/TLSServer.scala

The certificate is expired... That explains why the client was hanging. Bah. I forked this project from someone else and didn't check how old it was.

I'll try to catch up to you tomorrow. :) Thanks again.

Contributor

danielpcox commented Sep 2, 2015

All right, I was able to confirm that these changes provide access to the client certificate with the right incantations, though I was never able to get it working with the new API. I'll file a separate issue about that.

Contributor

mosesn commented Sep 2, 2015

Rad, then I'll mark it as solved :]

@mosesn mosesn closed this Sep 2, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment