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

TLS 'secureConnect' event fires twice in case of TLS renegotiation #54362

Open
mxschmitt opened this issue Aug 13, 2024 · 0 comments
Open

TLS 'secureConnect' event fires twice in case of TLS renegotiation #54362

mxschmitt opened this issue Aug 13, 2024 · 0 comments
Labels
tls Issues and PRs related to the tls subsystem.

Comments

@mxschmitt
Copy link

mxschmitt commented Aug 13, 2024

Version

22.6.0

Platform

Microsoft Windows NT 10.0.22631.0 x64

also reproducible under:

Darwin 23.6.0 Darwin Kernel Version 23.6.0: Mon Jul 29 21:14:30 PDT 2024; root:xnu-10063.141.2~1/RELEASE_ARM64_T6000 arm64

Subsystem

tls

What steps will reproduce the bug?

Generate the certs and then run the server.js and then the client.js.

mkdir certs
cd certs
openssl req -newkey rsa:2048 -nodes -keyout key.pem -x509 -days 365 -out cert.pem -subj "/C=US/ST=State/L=City/O=Organization/OU=Unit/CN=localhost"
openssl req -newkey rsa:2048 -nodes -keyout client_key.pem -out client_req.pem -subj "/C=US/ST=State/L=City/O=Organization/OU=Unit/CN=client"
openssl x509 -req -in client_req.pem -CA cert.pem -CAkey key.pem -CAcreateserial -out client_cert.pem -days 365  -subj "/C=US/ST=State/L=City/O=Organization/OU=Unit/CN=client"
// client.js
const tls = require('tls');
const fs = require('fs');

const tlsSocket = tls.connect({
    host: '127.0.0.1',
    port: 8443,
    servername: 'localhost',
    rejectUnauthorized: false,
    key: fs.readFileSync('certs/client_key.pem'),
    cert: fs.readFileSync('certs/client_cert.pem'),
});
tlsSocket.on('secureConnect', () => {
    console.log('secureConnect event fired!');
    tlsSocket.write('hi');
});
tlsSocket.on('data', (data) => {
    console.log(`Received: ${data.toString().trim()}`);
});
// server.js
const tls = require('tls');
const fs = require('fs');

const options = {
    key: fs.readFileSync('certs/key.pem'),
    cert: fs.readFileSync('certs/cert.pem'),
    requestCert: false,
    rejectUnauthorized: false,
    secureProtocol: 'TLSv1_2_method', // Force TLS 1.2
};

const server = tls.createServer(options, (socket) => {
    console.log(`Secure connection established using ${socket.getProtocol()}`);
    socket.on('data', (data) => {
        console.log('Received:', data.toString());
    });
    socket.write('\n_renegotiating\n');
    socket.renegotiate({
        requestCert: true,
        rejectUnauthorized: true
    }, (err) => {
        if (err) {
            console.error('Renegotiation error:', err);
        } else {
            console.log('Renegotiation successful');
            socket.end('\n_renegotiated\n');
        }
    });
});

server.listen(8443, () => {
    console.log('Server listening on port 8443');
});

server.on('tlsClientError', (err) => {
    console.error('TLS Client Error:', err);
});

How often does it reproduce? Is there a required condition?

  • TLS 1.2.
  • server re-negotiates the TLS connections with client certificates.

What is the expected behavior? Why is that the expected behavior?

secureConnect events fires once.

What do you see instead?

secureConnect event fires multiple times.

Additional information

  • This got surfaced when using IIS on Windows Server. Windows Server does use negotiation when using client-certificates.

  • When passing a callback to tls.connect (which is similar to secureConnect this won't surface, since its using once() internally.

Logs:

env➜  playwright git:(main) ✗ NODE_DEBUG=tls node client.js
TLS 25844: client _init handle? true
TLS 25844: client initRead handle? true buffered? false
TLS 25844: client _start handle? true connecting? false requestOCSP? false
TLS 25844: client onhandshakedone
TLS 25844: client _finishInit handle? true alpn false servername localhost
TLS 25844: client emit secureConnect. rejectUnauthorized: false, authorizationError: DEPTH_ZERO_SELF_SIGNED_CERT
secureConnect event fired! Error
    at TLSSocket.<anonymous> (/Users/maxschmitt/Developer/playwright/client.js:13:47)
    at TLSSocket.emit (node:events:520:28)
    at TLSSocket.onConnectSecure (node:_tls_wrap:1716:8)
    at TLSSocket.emit (node:events:520:28)
    at TLSSocket._finishInit (node:_tls_wrap:1078:8)
    at ssl.onhandshakedone (node:_tls_wrap:864:12)
Received: _renegotiating
TLS 25844: client onhandshakedone
TLS 25844: client _finishInit handle? true alpn false servername localhost
TLS 25844: client emit secureConnect. rejectUnauthorized: false, authorizationError: DEPTH_ZERO_SELF_SIGNED_CERT
secureConnect event fired! Error
    at TLSSocket.<anonymous> (/Users/maxschmitt/Developer/playwright/client.js:13:47)
    at TLSSocket.emit (node:events:520:28)
    at TLSSocket.onConnectSecure (node:_tls_wrap:1716:8)
    at TLSSocket.emit (node:events:520:28)
    at TLSSocket._finishInit (node:_tls_wrap:1078:8)
    at ssl.onhandshakedone (node:_tls_wrap:864:12)
@RedYetiDev RedYetiDev added the tls Issues and PRs related to the tls subsystem. label Aug 13, 2024
mxschmitt added a commit to microsoft/playwright that referenced this issue Aug 14, 2024
Certain https servers like Microsoft IIS aka. TLS servers do the TLS
renegotiation after the TLS handshake. This ends up in two
`'secureConnect'` events due to an upstream Node.js bug:
nodejs/node#54362

Drive-by: Move other listeners like `'close'` / `'end'` to `once()` as
well.

Relates #32004
mxschmitt added a commit to microsoft/playwright that referenced this issue Aug 14, 2024
…enegotiation

Certain https servers like Microsoft IIS aka. TLS servers do the TLS
renegotiation after the TLS handshake. This ends up in two
`'secureConnect'` events due to an upstream Node.js bug:
nodejs/node#54362

Drive-by: Move other listeners like `'close'` / `'end'` to `once()` as
well.

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

No branches or pull requests

2 participants