node tls,http2: send fatal alert on ALPN mismatch

To comply with RFC 7301, make TLS servers send a fatal alert during the TLS handshake if both the client and the server are configured to use ALPN and if the server does not support any of the protocols advertised by the client.

It is expected that a server will have a list of protocols that it supports, in preference order, and will only select a protocol if the client supports it. In that case, the server SHOULD select the most highly preferred protocol that it supports and that is also advertised by the client. In the event that the server supports no protocols that the client advertises, then the server SHALL respond with a fatal "no_application_protocol" alert.
enum {
    no_application_protocol(120),
    (255)
} AlertDescription;

This affects HTTP/2 servers. Until now, applications could intercept the 'unknownProtocol' event when the client either did not advertise any protocols or if the list of protocols advertised by the client did not include HTTP/2 (or HTTP/1.1 if allowHTTP1 was true). With this change, only the first case can be handled, and the 'unknownProtocol' event will not be emitted in the second case because the TLS handshake fails and no secure connection is established.

I am marking this as semver-major because it changes existing behavior in a potentially breaking way.

@nodejs/http2 It seems that the HTTP/2 server implementation has a few tricks up its sleeve for when ALPN does not match (switching to HTTP/1.1, sending an informational HTTP/1.0 message, destroying the connection...). Please review these changes carefully.

Jul 28 '22 21:07 tniessen

Review requested:

[ ] @nodejs/crypto
[ ] @nodejs/http
[ ] @nodejs/http2
[ ] @nodejs/net
[ ] @nodejs/tsc

Jul 28 '22 21:07 nodejs-github-bot

CI: https://ci.nodejs.org/job/node-test-pull-request/45727/

Jul 29 '22 14:07 nodejs-github-bot

Commit Queue failed

- Loading data for nodejs/node/pull/44031
✔  Done loading data for nodejs/node/pull/44031
----------------------------------- PR info ------------------------------------
Title      tls,http2: send fatal alert on ALPN mismatch (#44031)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     tniessen:tls-http2-alpn-fatal -> nodejs:main
Labels     tls, crypto, c++, semver-major, security, http2, author ready, needs-ci, commit-queue-squash
Commits    1
 - tls,http2: send fatal alert on ALPN mismatch
Committers 1
 - Tobias Nießen 
PR-URL: https://github.com/nodejs/node/pull/44031
Reviewed-By: Paolo Insogna 
Reviewed-By: Matteo Collina 
Reviewed-By: Rafael Gonzaga 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/44031
Reviewed-By: Paolo Insogna 
Reviewed-By: Matteo Collina 
Reviewed-By: Rafael Gonzaga 
--------------------------------------------------------------------------------
   ℹ  This PR was created on Thu, 28 Jul 2022 21:56:01 GMT
   ✔  Approvals: 3
   ✔  - Paolo Insogna (@ShogunPanda): https://github.com/nodejs/node/pull/44031#pullrequestreview-1055520310
   ✔  - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/44031#pullrequestreview-1055567487
   ✔  - Rafael Gonzaga (@RafaelGSS): https://github.com/nodejs/node/pull/44031#pullrequestreview-1055638949
   ✖  semver-major requires at least 2 TSC approvals
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2022-07-29T14:21:54Z: https://ci.nodejs.org/job/node-test-pull-request/45727/
- Querying data for job/node-test-pull-request/45727/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu

https://github.com/nodejs/node/actions/runs/2762177314

Jul 29 '22 17:07 nodejs-github-bot

cc @nodejs/tsc since this is semver-major.

Jul 29 '22 19:07 tniessen

CitGM: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/2976/

Aug 02 '22 16:08 tniessen

ping @nodejs/tsc @jasnell

Aug 06 '22 16:08 tniessen

Cheers @mcollina but I'm afraid two approvals from one TSC member don't count as two TSC approvals 😄

another ping @nodejs/tsc

Aug 12 '22 19:08 tniessen

Landed in 77def91bf9a2abd6a19514b57e3dc08f3e3f0fc6

Aug 13 '22 07:08 nodejs-github-bot

node node copied to clipboard

tls,http2: send fatal alert on ALPN mismatch

node
node copied to clipboard