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

new tls.TLSSocket issue #31305

Open
leor-gh opened this issue Jan 10, 2020 · 6 comments
Open

new tls.TLSSocket issue #31305

leor-gh opened this issue Jan 10, 2020 · 6 comments
Labels
confirmed-bug Issues with confirmed bugs. doc Issues and PRs related to the documentations. tls Issues and PRs related to the tls subsystem.

Comments

@leor-gh
Copy link

leor-gh commented Jan 10, 2020

  • Version: 10.x+
  • Platform: All
  • Subsystem: tls

When I try to create a secure connection over an existing stream, implementing something similar to STARTTLS, the suggested way is to use new tls.TLSSocket().

I also want to use mutual certification authentication but the tlsSocket.authorized is never set to true for the Server even when the client certificate is signed correctly by the expected CA. From the _tls_wrap.js code, it seems this flag is only set for the server when the underlying stream is an actual socket.

On the client side things are working as expected, as the client side connection is created with tls.connect().

It used to be that the deprecated pair returned by tls.createSecurePair() has access to the internal SSL object, which I can use verifyError() to check the validity of the client certificate. But this hidden feature has also been removed in recent versions.

What is the correct approach to mca for "upgraded" connection?

@sam-github
Copy link
Contributor

This use case is supported, but not documented.

I tried to address that deficiency with #10846, but it stalled.

Maybe I should try again, but in the meantime, check out that PR and see if helps.

@sam-github sam-github added the doc Issues and PRs related to the documentations. label Jan 10, 2020
@leor-gh
Copy link
Author

leor-gh commented Jan 10, 2020

Thank you for the response.

I ran into this issue when I was writing the code the previous few days, and it was only that I found out I needed to use 'secure' instead of 'secureConnection' on the server to receive the proper event.

Accordingly, it seems that the ssl object was moved to the tlsSocket class itself.

I did a quick test, and was indeed able to locate the object on the server side once the handshake is complete and the 'secure' event is emitted. However, calling verifyError() returns 'Error: unable to get issuer certificate' even though the client cert supplied is trusted by the CA.

I wrote some test code on v8.12 (where both interfaces are available), and test both API styles, with the same tlsSecureConetxt() options. The interface using pair.ssl can successfully validate the client cert, but not the tlsSocket.ssl interface.

I also tested the same code on v12.13 for the tlsSocket.ssl interface and got the same error.

If tlsSocket.ssl is a supported API albeit undocumented, then there is indeed a bug in the tlsSocket interface. I could be just not knowing how to set things up though, given there is no documentation.

@leor-gh
Copy link
Author

leor-gh commented Jan 11, 2020

Sorry, my bad.

requestCert option needs to be set in the new TLSSocket call, not as an option to createSecureContext(). There is no problem with the tlsSocket.ssl interface.

So this is just merely a documentation issue.

@sam-github sam-github added confirmed-bug Issues with confirmed bugs. tls Issues and PRs related to the tls subsystem. labels Jan 11, 2020
@simllll
Copy link
Contributor

simllll commented Feb 14, 2020

Regarding tls and crypt, one question that popped up.. anyone an idea? Is it a bug that tlssocket does not fire keylog event? Or is this just not supported (yet)? https://stackoverflow.com/questions/60232165/ssl-export-keying-material-in-node-js

Just want to shed some light on this before opening a bug report / feature request. Thanks

@sam-github
Copy link
Contributor

keylog is not supported on v10.x, that's why it isn't in the docs: https://nodejs.org/docs/latest-v10.x/api/tls.html

@jasnell
Copy link
Member

jasnell commented Dec 31, 2020

#36711

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug Issues with confirmed bugs. doc Issues and PRs related to the documentations. tls Issues and PRs related to the tls subsystem.
Projects
None yet
Development

No branches or pull requests

4 participants