Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose whether a TLS server requested a client certificate #29882

Closed
pimterry opened this issue Oct 8, 2019 · 6 comments
Closed

Expose whether a TLS server requested a client certificate #29882

pimterry opened this issue Oct 8, 2019 · 6 comments
Labels
feature request Issues that request new features to be added to Node.js. tls Issues and PRs related to the tls subsystem. wontfix Issues that will not be fixed.

Comments

@pimterry
Copy link
Member

pimterry commented Oct 8, 2019

Is your feature request related to a problem? Please describe.

I'm making TLS connections to various servers, and for both successful & failing connections I want to know whether the servers requested a client certificate.

For context: my app is an MITM proxy, and I want to be able to warn the downstream user when they might want to configure a client certificate.

Describe the solution you'd like

I'd like a way to know whether the server requested a client cert during the handshake. This is data that's received by node (or at least, by OpenSSL) alongside the initial hello, but it's not exposed in any way that I can see.

There's also probably other handshake metadata that would be interesting to expose too, which might be worth considering in future, but nothing that's immediately useful to me right this second.

A handshakeStarted event on sockets that exposes the data received from the server for inspection would work, for example, or alternatively a tlsSocket.clientCertificateRequested boolean would be fine too (as long as its available regardless of subsequent errors).

Describe alternatives you've considered

As far as I can tell, there's no way to do this right now other than manually collecting the entire TLS handshake data myself and parsing it from scratch in JS. That sounds like an major undertaking, and significant duplication of work since node is clearly parsing this data already.

For servers that outright reject connections without certificates it might be possible to infer the cause from the error details, but that's a limited case and not reliable. Discussed on SO too, with no useful result as yet: https://stackoverflow.com/questions/58283656/how-to-tell-if-a-tls-server-requested-a-client-certificate

@addaleax addaleax added feature request Issues that request new features to be added to Node.js. tls Issues and PRs related to the tls subsystem. labels Oct 8, 2019
@addaleax
Copy link
Member

addaleax commented Oct 8, 2019

@nodejs/crypto

@bnoordhuis
Copy link
Member

There are a couple of issues here that make it less straightforward than what's requested:

  1. It's not safe to call into openssl unchecked during the handshake.
  2. The handshake is not a single event but a sequence of events.
  3. There isn't necessarily a single handshake; renegotiation starts a new handshake over the same connection.

onhandshakestart() and onhandshakedone() in lib/_tls_wrap.js track the handshake.

  • onhandshakestart() probably isn't interesting to you because nothing is known at that point.
  • The time window between the two is when you can't call into openssl.
  • onhandshakedone() is basically synonymous to the socket.on('secure') event.

Adding a socket.clientCertificateRequested property isn't out of the question but, crucially, you'd only see it after the handshake completes and the secure connection has been established.

@pimterry
Copy link
Member Author

pimterry commented Oct 8, 2019

Adding a socket.clientCertificateRequested property isn't out of the question but, crucially, you'd only see it after the handshake completes and the secure connection has been established.

Would it be possible to see it on successful handshakes and also after error events? That's how socket.servername works for example (#27759) - it's set as soon as the SNI content comes in, so you can read it in either case.

If onhandshakedone() isn't called and onerror() is called instead, is it still unsafe to call into openssl, and is there any useful way to inspect handshake data from there?

Being able to read it only in successful cases would still be useful to me, but both cases is definitely better.

@bnoordhuis
Copy link
Member

Would it be possible to see it on successful handshakes and also after error events?

Up to a point. If the handshake never gets to the ServerHello stage, there's no way to tell. Session resumption or session tickets also seem to prevent node/openssl from knowing whether there was originally a client certificate request.

I made a quick attempt using SSL_CTX_set_cert_cb() and SSL_CTX_set_client_cert_cb() but it fails more often than not. You're welcome to give it a go yourself, test/parallel/test-tls-client-auth.js is a good way to test.

@bnoordhuis
Copy link
Member

I've poked at this some more and I don't see a good, reliable way to make it work. I'm going to close this out as a wontfix but you're still welcome to send a PR if you can make it work.

I toyed with the idea of implementing a custom ServerHello parser like we do for ClientHellos (to perform async session resumption, something openssl doesn't support) but I'm not convinced the complexity/payoff trade-off is worth it for just this particular feature.

@bnoordhuis bnoordhuis added the wontfix Issues that will not be fixed. label Oct 10, 2019
@pimterry
Copy link
Member Author

Yeah ok, fair enough. It's a shame, but this makes sense and I'll survive without it. Thanks for trying anyway!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js. tls Issues and PRs related to the tls subsystem. wontfix Issues that will not be fixed.
Projects
None yet
Development

No branches or pull requests

3 participants