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

Cannot read property 'getALPNNegotiatedProtocol' of null #24658

Closed
petoknm opened this issue Nov 26, 2018 · 12 comments
Closed

Cannot read property 'getALPNNegotiatedProtocol' of null #24658

petoknm opened this issue Nov 26, 2018 · 12 comments
Labels
https Issues or PRs related to the https subsystem. tls Issues and PRs related to the tls subsystem.

Comments

@petoknm
Copy link

petoknm commented Nov 26, 2018

Version: v11.2.0
Platform: Linux xps15 4.19.2-arch1-1-ARCH #1 SMP PREEMPT Tue Nov 13 21:16:19 UTC 2018 x86_64 GNU/Linux
Subsystem: https, tls

I am trying to get https client certificate authentication to work but I get the following error:

_tls_wrap.js:620
  this.alpnProtocol = this._handle.getALPNNegotiatedProtocol();
                                   ^

TypeError: Cannot read property 'getALPNNegotiatedProtocol' of null
    at TLSSocket._finishInit (_tls_wrap.js:620:36)
    at TLSWrap.onhandshakedone (_tls_wrap.js:101:9)

code:

const https = require('https');
const fs = require('fs');

// Some valid paths to CA files
const crtPath = process.env.CA_CERT_PATH || "/var/run/secrets/certs/ca.crt";
const keyPath = process.env.CA_CERT_KEY || "/var/run/secrets/certs/ca.key";

const options = {
  key: fs.readFileSync(keyPath),
  cert: fs.readFileSync(crtPath),
  requestCert: true,
};

https.createServer(options, (req, res) => {
  res.writeHead(200);
  res.end("hello world\n");
}).listen(8443);

when I set requestCert: false in the options it works fine, but I need the client to present a certificate, thus need the requestCert: true.

Edit:
Adding rejectUnauthorized: false makes it work. But I still think that it should not throw an error when rejecting unauthorized clients.

@Trott
Copy link
Member

Trott commented Nov 26, 2018

@indutny @nodejs/http @nodejs/crypto

@petoknm
Copy link
Author

petoknm commented Nov 26, 2018

Adding options.ca equal to the same thing as options.cert also fixes it... According to the docs:

"For self-signed certificates, the certificate is its own CA, and must be provided."

So I guess it makes sense that it was failing, but it would be nice to fail when I'm creating the server, not during connection handling, and with a more sensible error message.
Thanks :)

@Trott Trott added tls Issues and PRs related to the tls subsystem. https Issues or PRs related to the https subsystem. labels Nov 26, 2018
@bnoordhuis
Copy link
Member

Are you getting that exception on the server side or the client side (i.e., when trying to connect to the server you started)?

As to the exception itself, it looks like the connection is already dead (and this._handle === null) by the time the onhandshakedone callback is invoked.

Should be easy to fix with an if (this._handle === null) return; guard.

it would be nice to fail when I'm creating the server

Probably hard to do because there are combinations of options where that condition isn't an error.

With the guard in place, you should get a 'tlsClientError' event on the server instance.

@petoknm
Copy link
Author

petoknm commented Nov 27, 2018

The exception is on the server side

@sam-github
Copy link
Contributor

@petoknm Your example code lacks a client. I added one, see below, and cannot reproduce. Could you provide a complete example?

const https = require('https');
const fs = require('fs');

// Some valid paths to CA files
const crtPath = process.env.CA_CERT_PATH || "server.crt";
const keyPath = process.env.CA_CERT_KEY || "server.key";

const options = {
  key: fs.readFileSync(keyPath),
  cert: fs.readFileSync(crtPath),
  requestCert: true,
};

https.createServer(options, (req, res) => {
  res.writeHead(200);
  res.end("hello world\n");
}).listen(0)
.on('listening', function() {
  const port =  this.address().port;

  console.log('server on port', port);
  console.log('node version', process.versions.node);

  https.get({
    key: fs.readFileSync(keyPath),
    cert: fs.readFileSync(crtPath),
    port,
  }).on('error', (err) => {
    console.error('client err', err);
  });
})
.on('tlsClientError', (err) => {
  console.error('server tlsClientError:', err);
})
;

Output, as expected the server resets the connection because it can't find a trusted CA for the client.

server on port 37479
node version 11.2.0
server tlsClientError: { Error: socket hang up
    at TLSSocket.onSocketClose (_tls_wrap.js:737:23)
    at TLSSocket.emit (events.js:187:15)
    at _handle.close (net.js:616:12)
    at Socket.done (_tls_wrap.js:384:7)
    at Object.onceWrapper (events.js:273:13)
    at Socket.emit (events.js:182:13)
    at TCP._handle.close (net.js:616:12) code: 'ECONNRESET' }
client err { Error: Client network socket disconnected before secure TLS connection was established                                                                          
    at TLSSocket.onConnectEnd (_tls_wrap.js:1159:19)
    at Object.onceWrapper (events.js:273:13)
    at TLSSocket.emit (events.js:187:15)
    at endReadableNT (_stream_readable.js:1098:12)
    at process.internalTickCallback (internal/process/next_tick.js:72:19)
  code: 'ECONNRESET',
  path: null,
  host: 'localhost',
  port: 37479,
  localAddress: undefined }

@petoknm
Copy link
Author

petoknm commented Nov 28, 2018

Well, I cant exactly reproduce it using https.request(), but I can with the following code and a curl command:

code:

const https = require('https');
const fs = require('fs');

const options = {
  cert: fs.readFileSync('server.crt'),
  key: fs.readFileSync('server.key'),
  requestCert: true
};

const server = https.createServer(options, (req, res) => {
  res.writeHead(200);
  res.end("hello world\n");
});

server.listen(8443);

server.on('listening', () => {
  const port =  server.address().port;

  console.log('server on port', port);
  console.log('node version', process.versions.node);
});

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

request:

$ curl https://localhost:8443/ -kv -E client.crt.pem --key client.key.pem 
*   Trying ::1...
* TCP_NODELAY set
* Connected to localhost (::1) port 8443 (#0)
* ALPN, offering h2
* ALPN, offering http/1.1
* successfully set certificate verify locations:
*   CAfile: /etc/ssl/certs/ca-certificates.crt
  CApath: none
* TLSv1.3 (OUT), TLS handshake, Client hello (1):
* TLSv1.3 (IN), TLS handshake, Server hello (2):
* TLSv1.3 (IN), TLS handshake, [no content] (0):
* TLSv1.3 (IN), TLS handshake, Encrypted Extensions (8):
* TLSv1.3 (IN), TLS handshake, [no content] (0):
* TLSv1.3 (IN), TLS handshake, Request CERT (13):
* TLSv1.3 (IN), TLS handshake, [no content] (0):
* TLSv1.3 (IN), TLS handshake, Certificate (11):
* TLSv1.3 (IN), TLS handshake, [no content] (0):
* TLSv1.3 (IN), TLS handshake, CERT verify (15):
* TLSv1.3 (IN), TLS handshake, [no content] (0):
* TLSv1.3 (IN), TLS handshake, Finished (20):
* TLSv1.3 (OUT), TLS change cipher, Change cipher spec (1):
* TLSv1.3 (OUT), TLS handshake, [no content] (0):
* TLSv1.3 (OUT), TLS handshake, Certificate (11):
* TLSv1.3 (OUT), TLS handshake, [no content] (0):
* TLSv1.3 (OUT), TLS handshake, CERT verify (15):
* TLSv1.3 (OUT), TLS handshake, [no content] (0):
* TLSv1.3 (OUT), TLS handshake, Finished (20):
* SSL connection using TLSv1.3 / TLS_AES_256_GCM_SHA384
* ALPN, server accepted to use http/1.1
* Server certificate: [redacted]
*  SSL certificate verify result: self signed certificate (18), continuing anyway.
* TLSv1.3 (OUT), TLS app data, [no content] (0):
> GET / HTTP/1.1
> Host: localhost:8443
> User-Agent: curl/7.62.0
> Accept: */*
> 
* OpenSSL SSL_read: SSL_ERROR_SYSCALL, errno 104
* Closing connection 0
* TLSv1.3 (OUT), TLS alert, [no content] (0):
curl: (56) OpenSSL SSL_read: SSL_ERROR_SYSCALL, errno 104

output:

$ node app.js
server on port 8443
node version 11.2.0
_tls_wrap.js:620
  this.alpnProtocol = this._handle.getALPNNegotiatedProtocol();
                                   ^

TypeError: Cannot read property 'getALPNNegotiatedProtocol' of null
    at TLSSocket._finishInit (_tls_wrap.js:620:36)
    at TLSWrap.onhandshakedone (_tls_wrap.js:101:9)

@petoknm
Copy link
Author

petoknm commented Nov 28, 2018

Upgraded to 11.3.0, issue still remains, even after PR #18987

@bnoordhuis
Copy link
Member

After some investigation I'm reasonably sure it's caused by the use of TLSv1.3. Can you check whether it works for you when you add --tlsv1.2 to the curl command?

@petoknm
Copy link
Author

petoknm commented Dec 11, 2018

Indeed, using TLSv1.2 seems fine

$ curl https://localhost:8443/ --tls-max 1.2 -kv -E server.crt --key server.key
*   Trying ::1...
* TCP_NODELAY set
* Connected to localhost (::1) port 8443 (#0)
* ALPN, offering h2
* ALPN, offering http/1.1
* successfully set certificate verify locations:
*   CAfile: /etc/ssl/certs/ca-certificates.crt
  CApath: none
* TLSv1.2 (OUT), TLS handshake, Client hello (1):
* TLSv1.2 (IN), TLS handshake, Server hello (2):
* TLSv1.2 (IN), TLS handshake, Certificate (11):
* TLSv1.2 (IN), TLS handshake, Server key exchange (12):
* TLSv1.2 (IN), TLS handshake, Request CERT (13):
* TLSv1.2 (IN), TLS handshake, Server finished (14):
* TLSv1.2 (OUT), TLS handshake, Certificate (11):
* TLSv1.2 (OUT), TLS handshake, Client key exchange (16):
* TLSv1.2 (OUT), TLS handshake, CERT verify (15):
* TLSv1.2 (OUT), TLS change cipher, Change cipher spec (1):
* TLSv1.2 (OUT), TLS handshake, Finished (20):
* OpenSSL SSL_connect: SSL_ERROR_SYSCALL in connection to localhost:8443 
* Closing connection 0
curl: (35) OpenSSL SSL_connect: SSL_ERROR_SYSCALL in connection to localhost:8443
$ node app.js 
server on port 8443
node version 11.3.0
server tlsClientError: { Error: socket hang up
    at TLSSocket.onSocketClose (_tls_wrap.js:737:23)
    at TLSSocket.emit (events.js:187:15)
    at _handle.close (net.js:616:12)
    at Socket.done (_tls_wrap.js:384:7)
    at Object.onceWrapper (events.js:273:13)
    at Socket.emit (events.js:182:13)
    at TCP._handle.close (net.js:616:12) code: 'ECONNRESET' }

@shigeki
Copy link
Contributor

shigeki commented Dec 12, 2018

  • TLSv1.3 (IN), TLS handshake, Server hello (2):

This is very strange because the node TLS server returns TLS1.3 ServerHello. We have not supported OpenSSL-1.1.1 and TLS1.3 yet.

@petknm Are you using Node.js built with openssl shared library of OpenSSL-1.1.1? Please give us the output result of process.versions.openssl.

@petoknm
Copy link
Author

petoknm commented Dec 13, 2018

> process.versions.openssl
'1.1.1'

I'm running node from the arch linux packages, so the configuration and build arguments are in this PKGBUILD file: https://git.archlinux.org/svntogit/community.git/tree/trunk/PKGBUILD?h=packages/nodejs&id=27476317f60e8e7a74aba2107fd9dbcd14a64257

@sam-github
Copy link
Contributor

@petoknm Can you report to Arch that openssl 1.1.1 is not similar enough to openssl 1.1.0 to be a drop-in replacement, at least not for Node.js?

At least, not unless we do #25024, pulled from work on getting 1.1.1 support, see #18770 and sam-github@0b140c2 from https://github.com/sam-github/node/tree/update_openssl1.1.1a

Trott pushed a commit to sam-github/node that referenced this issue Dec 17, 2018
Several secureProtocol strings allow any supported TLS version as the
maximum, but our maximum supported protocol version is TLSv1.2 even if
someone configures a build against an OpenSSL that supports TLSv1.3.

Fixes: nodejs#24658
MylesBorins pushed a commit that referenced this issue Dec 25, 2018
Several secureProtocol strings allow any supported TLS version as the
maximum, but our maximum supported protocol version is TLSv1.2 even if
someone configures a build against an OpenSSL that supports TLSv1.3.

Fixes: #24658

PR-URL: #25024
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
refack pushed a commit to refack/node that referenced this issue Jan 14, 2019
Several secureProtocol strings allow any supported TLS version as the
maximum, but our maximum supported protocol version is TLSv1.2 even if
someone configures a build against an OpenSSL that supports TLSv1.3.

Fixes: nodejs#24658

PR-URL: nodejs#25024
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
BethGriggs pushed a commit that referenced this issue Feb 12, 2019
Several secureProtocol strings allow any supported TLS version as the
maximum, but our maximum supported protocol version is TLSv1.2 even if
someone configures a build against an OpenSSL that supports TLSv1.3.

Fixes: #24658

PR-URL: #25024
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
BethGriggs pushed a commit that referenced this issue Feb 20, 2019
Several secureProtocol strings allow any supported TLS version as the
maximum, but our maximum supported protocol version is TLSv1.2 even if
someone configures a build against an OpenSSL that supports TLSv1.3.

Fixes: #24658

PR-URL: #25024
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
rvagg pushed a commit that referenced this issue Feb 28, 2019
Several secureProtocol strings allow any supported TLS version as the
maximum, but our maximum supported protocol version is TLSv1.2 even if
someone configures a build against an OpenSSL that supports TLSv1.3.

Fixes: #24658

PR-URL: #25024
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
BaochengSu added a commit to BaochengSu/node that referenced this issue Oct 22, 2020
Ported from
OpenSUSE:nodejs8-8.17.0-lp152.147.1:openssl_1_1_1.patch

Original commit message:

Backport OpenSSL 1.1.1 support, mostly be disabling TLS 1.3
Upstream commits:

commit 8dd8033
Author: Shigeki Ohtsu <ohtsu@ohtsu.org>
Date:   Wed Sep 12 17:34:24 2018 +0900

    tls: workaround handshakedone in renegotiation

    `SSL_CB_HANDSHAKE_START` and `SSL_CB_HANDSHAKE_DONE` are called
    sending HelloRequest in OpenSSL-1.1.1.
    We need to check whether this is in a renegotiation state or not.

    Backport-PR-URL: nodejs#26270
    PR-URL: nodejs#25381
    Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
    Reviewed-By: Shigeki Ohtsu <ohtsu@ohtsu.org>

commit 161dca7
Author: Sam Roberts <vieuxtech@gmail.com>
Date:   Wed Nov 28 14:11:18 2018 -0800

    tls: re-define max supported version as 1.2

    Several secureProtocol strings allow any supported TLS version as the
    maximum, but our maximum supported protocol version is TLSv1.2 even if
    someone configures a build against an OpenSSL that supports TLSv1.3.

    Fixes: nodejs#24658

    PR-URL: nodejs#25024
    Reviewed-By: Richard Lau <riclau@uk.ibm.com>
    Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
    Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
    Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

Partial port, remain compatible with 1.0.2:

commit 970ce14
Author: Shigeki Ohtsu <ohtsu@ohtsu.org>
Date:   Wed Mar 14 14:26:55 2018 +0900

    crypto: remove deperecated methods of TLS version

    All version-specific methods were deprecated in OpenSSL 1.1.0 and
    min/max versions explicitly need to be set.
    This still keeps comptatible with JS and OpenSSL-1.0.2 APIs for now.

    crypto, constants: add constant of OpenSSL-1.1.0

    Several constants for OpenSSL-1.1.0 engine were removed and renamed in
    OpenSSL-1.1.0. This added one renamed constant in order to have a
    compatible feature with that of OpenSSL-1.0.2.
    Other missed or new constants in OpenSSL-1.1.0 are not yet added.

    crypto,tls,constants: remove OpenSSL1.0.2 support

    This is semver-majar change so that we need not to have
    compatibilities with older versions.

    Fixes: nodejs#4270
    PR-URL: nodejs#19794
    Reviewed-By: James M Snell <jasnell@gmail.com>
    Reviewed-By: Rod Vagg <rod@vagg.org>
    Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

Signed-off-by: Su Baocheng <baocheng.su@siemens.com>
BaochengSu added a commit to BaochengSu/node that referenced this issue Jul 14, 2022
Ported from
OpenSUSE:nodejs8-8.17.0-lp152.147.1:openssl_1_1_1.patch

Original commit message:

Backport OpenSSL 1.1.1 support, mostly be disabling TLS 1.3
Upstream commits:

commit 8dd8033
Author: Shigeki Ohtsu <ohtsu@ohtsu.org>
Date:   Wed Sep 12 17:34:24 2018 +0900

    tls: workaround handshakedone in renegotiation

    `SSL_CB_HANDSHAKE_START` and `SSL_CB_HANDSHAKE_DONE` are called
    sending HelloRequest in OpenSSL-1.1.1.
    We need to check whether this is in a renegotiation state or not.

    Backport-PR-URL: nodejs#26270
    PR-URL: nodejs#25381
    Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
    Reviewed-By: Shigeki Ohtsu <ohtsu@ohtsu.org>

commit 161dca7
Author: Sam Roberts <vieuxtech@gmail.com>
Date:   Wed Nov 28 14:11:18 2018 -0800

    tls: re-define max supported version as 1.2

    Several secureProtocol strings allow any supported TLS version as the
    maximum, but our maximum supported protocol version is TLSv1.2 even if
    someone configures a build against an OpenSSL that supports TLSv1.3.

    Fixes: nodejs#24658

    PR-URL: nodejs#25024
    Reviewed-By: Richard Lau <riclau@uk.ibm.com>
    Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
    Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
    Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

Partial port, remain compatible with 1.0.2:

commit 970ce14
Author: Shigeki Ohtsu <ohtsu@ohtsu.org>
Date:   Wed Mar 14 14:26:55 2018 +0900

    crypto: remove deperecated methods of TLS version

    All version-specific methods were deprecated in OpenSSL 1.1.0 and
    min/max versions explicitly need to be set.
    This still keeps comptatible with JS and OpenSSL-1.0.2 APIs for now.

    crypto, constants: add constant of OpenSSL-1.1.0

    Several constants for OpenSSL-1.1.0 engine were removed and renamed in
    OpenSSL-1.1.0. This added one renamed constant in order to have a
    compatible feature with that of OpenSSL-1.0.2.
    Other missed or new constants in OpenSSL-1.1.0 are not yet added.

    crypto,tls,constants: remove OpenSSL1.0.2 support

    This is semver-majar change so that we need not to have
    compatibilities with older versions.

    Fixes: nodejs#4270
    PR-URL: nodejs#19794
    Reviewed-By: James M Snell <jasnell@gmail.com>
    Reviewed-By: Rod Vagg <rod@vagg.org>
    Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

Signed-off-by: Su Baocheng <baocheng.su@siemens.com>
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

Successfully merging a pull request may close this issue.

5 participants