tls, crypto: add ALPN Support #2564

shigeki · 2015-08-26T15:50:32Z

ALPN is added to tls according to RFC7301, which supersedes NPN.
When the server receives both NPN and ALPN extensions from the client, ALPN takes precedence over NPN and the server does not send NPN extension to the client. alpnProtocol in TLSSocket always returns false when no selected protocol exists by ALPN.
In https server, http/1.1 token is always set when no options.ALPNProtocols exists.

Here exist all 16x3 test cases of combination of ALPN and NPN in a server and a client. Tests shows that there are some inconsistent returns in NPN between the server and the client but they are not changed for compatibility.

NPN in Chrome will be deprecated in early 2016 as in http://blog.chromium.org/2015/02/hello-http2-goodbye-spdy-http-is_9.html so it is better to include ALPN support in Node 4.0.

R= @nodejs/crypto

indutny · 2015-08-26T18:40:10Z

src/node_crypto.cc

@@ -1154,6 +1163,12 @@ void SSLWrap<Base>::AddMethods(Environment* env, Handle<FunctionTemplate> t) {
  env->SetProtoMethod(t, "setNPNProtocols", SetNPNProtocols);
 #endif

+#ifdef TLSEXT_TYPE_application_layer_protocol_negotiation
+  NODE_SET_PROTOTYPE_METHOD(t, "getALPNNegotiatedProtocol",


What if we will always set and declare them, but just leave their implementations empty in case of absent ALPN support?

@shigeki I guess you can leave the #ifdef out. I remember from the last review that it looked like it wouldn't compile with ALPN disabled but maybe I saw that wrong.

I removed #ifdef here and fixed to have declaration to avoid compile errors and also changed NODE_SET_PROTOTYPE_METHOD macro into env->SetProtoMethod.

indutny · 2015-08-26T18:43:32Z

@shigeki this test is awesome, thanks! Some nits, otherwise looking good.

shigeki · 2015-08-27T03:41:29Z

@indutny Thanks very much for your quick review. I fixed this according your comments and I also found that Test12 was wrong to be duplicated from Test11 and fixed it to the test case of Server: NPN, Client: Nothing.
CI of https://jenkins-iojs.nodesource.com/job/node-test-commit-linux/315/ is all green.
I also made h2 server tests connected from Chrome(boringSSL) and Firefox(nss) with only ALPN and ALPN/NPN and it works fine. Unfortunately I don't have MS Edge so did not test it yet.

Can I land this?

indutny · 2015-08-27T03:57:01Z

lib/_tls_wrap.js

@@ -951,9 +962,10 @@ exports.connect = function(/* [port, host], options, cb */) {
                 options.host ||
                 (options.socket && options.socket._host) ||
                 'localhost',
-      NPN = {},
+      NPN = {}, ALPN = {},


Please put it on the next line.

Okay, I will fix this.

shigeki · 2015-08-28T20:36:32Z

The milestone was deleted not to block 4.0 release.

@indutny Sorry, I am in a vacation from today and will work on this on the next Saturday.

argon · 2015-10-09T21:56:42Z

What's the status of this? I may have a future dependency on http2 in a project and no ALPN may be a blocker.

shigeki · 2015-10-22T07:26:26Z

In Chrome, "Disable HTTP/2 over NPN (with OpenSSL)" https://codereview.chromium.org/1387363004 was landed today. It is better to include this in 5.0

I've rebased this against the latest master and CI is https://ci.nodejs.org/job/node-test-commit/912/ . Something wrong with tests environments is on freebsd and windows and plinux test failures are tick-processor.js and test-debug-args.js that are nothing to do with this PR. Otherwise tests are green.

@indutny Your comments are included in shigeki@9625a82. LGTM?

shigeki · 2015-10-22T07:39:46Z

@argon Sorry for being late. I've just update this. Thanks for patience.

argon · 2015-10-22T08:41:49Z

Thanks for the update!

bnoordhuis · 2015-10-22T09:29:49Z

doc/api/tls.markdown


 <!-- type=misc -->

-NPN (Next Protocol Negotiation) and SNI (Server Name Indication) are TLS
+ALPN(Application-Layer Protocol Negotiation Extension), NPN (Next


Space before (.

MylesBorins · 2017-01-14T01:01:24Z

@nodejs/crypto I attempted a backport of this but all the tests blew up. This is touching far too much code for me to comfortable backport. Would someone be able to get a one in next week?

shigeki · 2017-01-14T01:08:49Z

Okay, I can work it next week.

shigeki · 2017-01-16T05:16:07Z

@MylesBorins I've just submitted the PR of #10831 .

cherry-pick 802a2e7 from v6-staging. ALPN is added to tls according to RFC7301, which supersedes NPN. When the server receives both NPN and ALPN extensions from the client, ALPN takes precedence over NPN and the server does not send NPN extension to the client. alpnProtocol in TLSSocket always returns false when no selected protocol exists by ALPN. In https server, http/1.1 token is always set when no options.ALPNProtocols exists. PR-URL: #2564 Reviewed-By: Fedor Indutny <fedor@indutny.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

cherry-pick 7eee372 from v6-staging. This fix is to be consistent implementation with ALPN. Tow NPN protocol data in the persistent memebers move to hidden variables in the wrap object. PR-URL: #2564 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

cherry-pick 802a2e7 from v6-staging. ALPN is added to tls according to RFC7301, which supersedes NPN. When the server receives both NPN and ALPN extensions from the client, ALPN takes precedence over NPN and the server does not send NPN extension to the client. alpnProtocol in TLSSocket always returns false when no selected protocol exists by ALPN. In https server, http/1.1 token is always set when no options.ALPNProtocols exists. PR-URL: #2564 Reviewed-By: Fedor Indutny <fedor@indutny.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

cherry-pick 7eee372 from v6-staging. This fix is to be consistent implementation with ALPN. Tow NPN protocol data in the persistent memebers move to hidden variables in the wrap object. PR-URL: #2564 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

cherry-pick 802a2e7 from v6-staging. ALPN is added to tls according to RFC7301, which supersedes NPN. When the server receives both NPN and ALPN extensions from the client, ALPN takes precedence over NPN and the server does not send NPN extension to the client. alpnProtocol in TLSSocket always returns false when no selected protocol exists by ALPN. In https server, http/1.1 token is always set when no options.ALPNProtocols exists. PR-URL: #2564 Reviewed-By: Fedor Indutny <fedor@indutny.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

cherry-pick 7eee372 from v6-staging. This fix is to be consistent implementation with ALPN. Tow NPN protocol data in the persistent memebers move to hidden variables in the wrap object. PR-URL: #2564 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

Notable Changes: * child_process: add shell option to spawn() (cjihrig) #4598 * crypto: * add ALPN Support (Shigeki Ohtsu) #2564 * allow adding extra certs to well-known CAs (Sam Roberts) #9139 * deps: * v8: expose statistics about heap spaces (Ben Ripkens) #4463 * fs: add the fs.mkdtemp() function. (Florian MARGAINE) #5333 * process: * add `externalMemory` to `process` (Fedor Indutny) #9587 * add process.cpuUsage() (Patrick Mueller) #10796

Notable Changes: * child_process: add shell option to spawn() (cjihrig) #4598 * crypto: * add ALPN Support (Shigeki Ohtsu) #2564 * allow adding extra certs to well-known CAs (Sam Roberts) #9139 * deps: * v8: expose statistics about heap spaces (Ben Ripkens) #4463 * fs: add the fs.mkdtemp() function. (Florian MARGAINE) #5333 * process: * add `externalMemory` to `process` (Fedor Indutny) #9587 * add process.cpuUsage() (Patrick Mueller) #10796 PR-URL: #10973

Notable Changes: * child_process: add shell option to spawn() (cjihrig) nodejs/node#4598 * crypto: * add ALPN Support (Shigeki Ohtsu) nodejs/node#2564 * allow adding extra certs to well-known CAs (Sam Roberts) nodejs/node#9139 * deps: * v8: expose statistics about heap spaces (Ben Ripkens) nodejs/node#4463 * fs: add the fs.mkdtemp() function. (Florian MARGAINE) nodejs/node#5333 * process: * add `externalMemory` to `process` (Fedor Indutny) nodejs/node#9587 * add process.cpuUsage() (Patrick Mueller) nodejs/node#10796 Signed-off-by: Ilkka Myller <ilkka.myller@nodefield.com>

PR-URL: #54982 Refs: aa0308d Refs: 9010f5f Refs: 52a40e0 Refs: b3ef289 Refs: #2564 Refs: #25819 Refs: #27311 Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Moshe Atlow <moshe@atlow.co.il>

PR-URL: nodejs#54982 Refs: nodejs@aa0308d Refs: nodejs@9010f5f Refs: nodejs@52a40e0 Refs: nodejs@b3ef289 Refs: nodejs#2564 Refs: nodejs#25819 Refs: nodejs#27311 Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Moshe Atlow <moshe@atlow.co.il>

PR-URL: #54982 Refs: aa0308d Refs: 9010f5f Refs: 52a40e0 Refs: b3ef289 Refs: #2564 Refs: #25819 Refs: #27311 Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Moshe Atlow <moshe@atlow.co.il>

PR-URL: nodejs#54982 Refs: nodejs@aa0308d Refs: nodejs@9010f5f Refs: nodejs@52a40e0 Refs: nodejs@b3ef289 Refs: nodejs#2564 Refs: nodejs#25819 Refs: nodejs#27311 Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Moshe Atlow <moshe@atlow.co.il>

PR-URL: #54982 Backport-PR-URL: #56948 Refs: aa0308d Refs: 9010f5f Refs: 52a40e0 Refs: b3ef289 Refs: #2564 Refs: #25819 Refs: #27311 Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Moshe Atlow <moshe@atlow.co.il> PR-URL: #54897 Refs: #54295

shigeki added tls Issues and PRs related to the tls subsystem. semver-minor PRs that contain new features and should be released in the next minor version. labels Aug 26, 2015

indutny reviewed Aug 26, 2015
View reviewed changes

shigeki added this to the 4.0.0 milestone Aug 26, 2015

shigeki force-pushed the alpn_support branch from 65af1da to 0640d36 Compare August 27, 2015 03:02

indutny reviewed Aug 27, 2015
View reviewed changes

shigeki removed this from the 4.0.0 milestone Aug 28, 2015

rvagg force-pushed the master branch from 11c25c2 to ba02bd0 Compare September 6, 2015 11:55

shigeki force-pushed the alpn_support branch from 7b6d4f1 to 9625a82 Compare October 22, 2015 06:27

bnoordhuis reviewed Oct 22, 2015
View reviewed changes

bnoordhuis mentioned this pull request Nov 2, 2015

TLS ALPN? #3632

Closed

stevenvachon mentioned this pull request Nov 2, 2015

Is ALPN supported with this module on io.js? molnarg/node-http2#126

Closed

MylesBorins mentioned this pull request Apr 1, 2016

Backport of #5706 to v4.x #5983

Closed

MylesBorins added lts-agenda labels Jan 6, 2017

MylesBorins mentioned this pull request Jan 6, 2017

Potential Semver Minor Backports nodejs/Release#177

Closed

9 tasks

MylesBorins mentioned this pull request Jan 24, 2017

v4.8.0 proposal #10973

Merged

This was referenced Sep 12, 2024

doc: add documentation for process.features #54897

Merged

doc: fix history of process.features #54982

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tls, crypto: add ALPN Support #2564

tls, crypto: add ALPN Support #2564

shigeki commented Aug 26, 2015

indutny Aug 26, 2015

bnoordhuis Oct 25, 2015

shigeki Oct 26, 2015

indutny commented Aug 26, 2015

shigeki commented Aug 27, 2015

indutny Aug 27, 2015

shigeki Aug 27, 2015

shigeki commented Aug 28, 2015

argon commented Oct 9, 2015

shigeki commented Oct 22, 2015

shigeki commented Oct 22, 2015

argon commented Oct 22, 2015

bnoordhuis Oct 22, 2015

MylesBorins commented Jan 14, 2017

shigeki commented Jan 14, 2017

shigeki commented Jan 16, 2017

tls, crypto: add ALPN Support #2564

tls, crypto: add ALPN Support #2564

Conversation

shigeki commented Aug 26, 2015

indutny Aug 26, 2015

Choose a reason for hiding this comment

bnoordhuis Oct 25, 2015

Choose a reason for hiding this comment

shigeki Oct 26, 2015

Choose a reason for hiding this comment

indutny commented Aug 26, 2015

shigeki commented Aug 27, 2015

indutny Aug 27, 2015

Choose a reason for hiding this comment

shigeki Aug 27, 2015

Choose a reason for hiding this comment

shigeki commented Aug 28, 2015

argon commented Oct 9, 2015

shigeki commented Oct 22, 2015

shigeki commented Oct 22, 2015

argon commented Oct 22, 2015

bnoordhuis Oct 22, 2015

Choose a reason for hiding this comment

MylesBorins commented Jan 14, 2017

shigeki commented Jan 14, 2017

shigeki commented Jan 16, 2017