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

Assign the peerCertificate before Node.js closes the connection #3568

Closed
wants to merge 1 commit into from
Closed

Assign the peerCertificate before Node.js closes the connection #3568

wants to merge 1 commit into from

Conversation

Andrewiski
Copy link

The kind of change this PR does introduce

  • a bug fix
  • a new feature
  • an update to the documentation
  • a code change that improves performance
  • other

Current behaviour

socket.client.request.client.getPeerCertificate() always returns null

New behaviour

socket.client.peerCertificate is populated before the underlying TLS is closed so it is available
on socket.on("connection")

I belive this is the result of a change in node tls functionality.

nodejs/node@eff8c3e

Other information (e.g. related issues)

#3567

@dasilvacontin
Copy link

This might be hard to merge unless it can easily be tested, or a test is provided.

I also worry whether this takes into account when a session is reused, like discussed in this issue (nodejs/node#3940).

@Andrewiski
Copy link
Author

Andrewiski commented May 26, 2020

Since this is used for client security auth its like having the connection is the certificate is no longer valid the client will need to get a new one and recreate the connection anyways,

Are you asking that I provide a test? This is a fix for something that did work but because of a node.js change the getPeerCertificate goes away after initial https socket connection. All we are doing is making it live for the life of the connections. I suppose if we were trying to save memory like they did we would only make it live during the socket.io connect event then remove it from memory.

if (socket.client.request.client.getPeerCertificate() !== null) {
console.log("Test Passed");
}else{
console.log("Test Failed");
}

@darrachequesne
Copy link
Member

Closed due to inactivity, please reopen if needed.

@Andrewiski
Copy link
Author

I assume from the response no one merged my pull request into the main branch so going forward there is no way to view a client certificate on a socket.io connection to determine who the clinet is? IE want to use TLS client certificates for client auth. Currently all you know is the cert was signed by the CA not which cert or who is connecting.

@darrachequesne
Copy link
Member

@Andrewiski that's a valid concern actually, please see my answer here: #3567 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants