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

getPeerCertificate() only works once on the same host #3940

Closed
f0zi opened this issue Nov 20, 2015 · 7 comments
Closed

getPeerCertificate() only works once on the same host #3940

f0zi opened this issue Nov 20, 2015 · 7 comments
Labels
https Issues or PRs related to the https subsystem. tls Issues and PRs related to the tls subsystem.

Comments

@f0zi
Copy link

f0zi commented Nov 20, 2015

Requesting from the same host twice using https will only return the certificate on the first connection.

var req = https.request(url);
req.once('socket', function(socket) {
    socket.once('secureConnect', function() {
        var cert = socket.getPeerCertificate(); // detailed or not does not seem to matter
        // cert is ok the first time, empty the second time.
    });
});

I would expect to get back the cert every time even if the connection is reused.

I'm caching it now but it's a bad workaround as other code could be using https.request to connect to the same host without using the cache which means that my code will never see the cert.

Also caching the cert means I'm relying on the assumption that the fact that I did not get a cert this time means the cert in my cache still applies and is valid.

@mscdex mscdex added the tls Issues and PRs related to the tls subsystem. label Nov 20, 2015
@jasnell jasnell added the https Issues or PRs related to the https subsystem. label Nov 21, 2015
@jasnell
Copy link
Member

jasnell commented Nov 21, 2015

/cc @nodejs/http

@indutny
Copy link
Member

indutny commented Nov 21, 2015

The session is resumed the next time, so this info is no longer available. It is actually a pretty good question on how it should work.

@nodejs/crypto : what are your thoughts on this? Should we store the cert in the session cache?

@shigeki
Copy link
Contributor

shigeki commented Nov 21, 2015

If we store the cert, we need to check its revocation at every connection even if it is a short time.
To get a certificate in every request from the connection, I think it is better to make it be full handshake not resumption.
IIRC, using no agent is one idea. Or is it better to add a new options to disable a session cache?

@indutny
Copy link
Member

indutny commented Nov 21, 2015

@shigeki technically, the session should expire if the cert has expired. I mean, if the server has made session timeout way too high, it is its fault, and not node.js.

I'm definitely +1 for an option to disable it, however I think that the particular request code should not depend on agent configuration.

@f0zi could using socket.isSessionReused() be an option for you? If the return value is true, then there generally is no certificate available.

@f0zi
Copy link
Author

f0zi commented Nov 23, 2015

@indutny Not really, if I don't see the cert I have to abort the connection. Since I'm using this from inside node-fetch I can't easily restart the request internally (I tried tlsSocket.renegotiate but that just seems to abort the fetch request with "closed socket"). So I'll have client code doing fetch() dealing with TLS connections. And at that point, if I see this correctly (I have not tried this), I would have to deal with replacing the global agent to make it not reuse the connection.

Note that I would be ok with reusing the request as long as it's the same domain (as sent with SNI). If it is talking to the same host but for a different domain as requested it would have to request the right cert from the host.

Would it be possible to limit a request to reusing a session to the domain indicated by SNI or trigger a renegotiation otherwise?

indutny added a commit to indutny/io.js that referenced this issue Dec 22, 2015
https requests with different SNI values should not be sent over the
same connection, even if the `host` is the same. Server may want to
present different certificate or route the incoming TLS connection
differently, depending on the received servername extension.

Fix: nodejs#3940
@indutny
Copy link
Member

indutny commented Dec 22, 2015

@f0zi SNI should be indeed accounting when doing https requests. Filed a PR to fix this issue: #4389, thanks for pointing out!

Regarding the cert presence, I think that this is just a documentation bug. The cert simply cannot be obtained if the session was reused, but it should be safe to assume that the connection is valid and secure if the session is reused.

Cheers!

@indutny indutny closed this as completed Dec 22, 2015
indutny added a commit that referenced this issue Dec 22, 2015
https requests with different SNI values should not be sent over the
same connection, even if the `host` is the same. Server may want to
present different certificate or route the incoming TLS connection
differently, depending on the received servername extension.

Fix: #3940
PR-URL: #4389
Reviewed-By: Ben Noordhuis <[email protected]>
Fishrock123 pushed a commit to Fishrock123/node that referenced this issue Jan 6, 2016
https requests with different SNI values should not be sent over the
same connection, even if the `host` is the same. Server may want to
present different certificate or route the incoming TLS connection
differently, depending on the received servername extension.

Fix: nodejs#3940
PR-URL: nodejs#4389
Reviewed-By: Ben Noordhuis <[email protected]>
MylesBorins pushed a commit that referenced this issue Jan 13, 2016
https requests with different SNI values should not be sent over the
same connection, even if the `host` is the same. Server may want to
present different certificate or route the incoming TLS connection
differently, depending on the received servername extension.

Fix: #3940
PR-URL: #4389
Reviewed-By: Ben Noordhuis <[email protected]>
MylesBorins pushed a commit that referenced this issue Jan 19, 2016
https requests with different SNI values should not be sent over the
same connection, even if the `host` is the same. Server may want to
present different certificate or route the incoming TLS connection
differently, depending on the received servername extension.

Fix: #3940
PR-URL: #4389
Reviewed-By: Ben Noordhuis <[email protected]>
scovetta pushed a commit to scovetta/node that referenced this issue Apr 2, 2016
https requests with different SNI values should not be sent over the
same connection, even if the `host` is the same. Server may want to
present different certificate or route the incoming TLS connection
differently, depending on the received servername extension.

Fix: nodejs#3940
PR-URL: nodejs#4389
Reviewed-By: Ben Noordhuis <[email protected]>
@lacksfish
Copy link

lacksfish commented Jun 8, 2017

So I've been struggling with this as well and here is how I get a fresh cert and fingerprint every time.

let options = {
    hostname: url,
    port: port,
    method: 'GET',
    headers: {
        'User-Agent': 'Node/https'
    },
    //disable session caching   (ノ°Д°)ノ︵ ┻━┻
    agent: new https.Agent({
        maxCachedSessions: 0
    })
};

let req = https.request(options);

req.on('error', (e) => {
    if(e.code === "ECONNRESET") {/* I don't care about these */}
    else console.error(e);
});

req.on('socket', function(socket) {
    socket.on('secureConnect', function() {
        let cert = socket.getPeerCertificate();
        callback(cert);
        return req.abort();
    });
});

callback(cert) returns the certificate each time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
https Issues or PRs related to the https subsystem. tls Issues and PRs related to the tls subsystem.
Projects
None yet
Development

No branches or pull requests

7 participants
@mscdex @indutny @jasnell @shigeki @lacksfish @f0zi and others