Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit21958e4

Browse files
committed
tls,http2: send fatal alert on ALPN mismatch
To comply with RFC 7301, make TLS servers send a fatal alert during theTLS handshake if both the client and the server are configured to useALPN and if the server does not support any of the protocols advertisedby the client.This affects HTTP/2 servers. Until now, applications could intercept the'unknownProtocol' event when the client either did not advertise anyprotocols or if the list of protocols advertised by the client did notinclude HTTP/2 (or HTTP/1.1 if allowHTTP1 was true). With this change,only the first case can be handled, and the 'unknownProtocol' event willnot be emitted in the second case because the TLS handshake fails and nosecure connection is established.
1 parent0616eaf commit21958e4

File tree

7 files changed

+107
-19
lines changed

7 files changed

+107
-19
lines changed

‎doc/api/http2.md

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2275,6 +2275,11 @@ a given number of milliseconds set using `http2secureServer.setTimeout()`.
22752275

22762276
<!-- YAML
22772277
added: v8.4.0
2278+
changes:
2279+
- version: REPLACEME
2280+
pr-url: https://github.com/nodejs/node/pull/44031
2281+
description: This event will only be emitted if the client did not transmit
2282+
an ALPN extension during the TLS handshake.
22782283
-->
22792284

22802285
*`socket` {stream.Duplex}
@@ -2284,6 +2289,15 @@ negotiate an allowed protocol (i.e. HTTP/2 or HTTP/1.1). The event handler
22842289
receives the socket for handling. If no listener is registered for this event,
22852290
the connection is terminated. A timeout may be specified using the
22862291
`'unknownProtocolTimeout'` option passed to[`http2.createSecureServer()`][].
2292+
2293+
In earlier versions of Node.js, this event would be emitted if`allowHTTP1` is
2294+
`false` and, during the TLS handshake, the client either does not send an ALPN
2295+
extension or sends an ALPN extension that does not include HTTP/2 (`h2`). Newer
2296+
versions of Node.js only emit this event if`allowHTTP1` is`false` and the
2297+
client does not send an ALPN extension. If the client sends an ALPN extension
2298+
that does not include HTTP/2 (or HTTP/1.1 if`allowHTTP1` is`true`), the TLS
2299+
handshake will fail and no secure connection will be established.
2300+
22872301
See the[Compatibility API][].
22882302

22892303
####`server.close([callback])`

‎doc/api/tls.md

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -683,8 +683,8 @@ is set to describe how authorization failed. Depending on the settings
683683
of the TLS server, unauthorized connections may still be accepted.
684684

685685
The`tlsSocket.alpnProtocol` property is a string that contains the selected
686-
ALPN protocol. When ALPN has no selected protocol,`tlsSocket.alpnProtocol`
687-
equals`false`.
686+
ALPN protocol. When ALPN has no selected protocol because the client or the
687+
server did not send an ALPN extension,`tlsSocket.alpnProtocol`equals`false`.
688688

689689
The`tlsSocket.servername` property is a string containing the server name
690690
requested via SNI.
@@ -2012,6 +2012,11 @@ where `secureSocket` has the same API as `pair.cleartext`.
20122012
<!-- YAML
20132013
added: v0.3.2
20142014
changes:
2015+
- version: REPLACEME
2016+
pr-url: https://github.com/nodejs/node/pull/44031
2017+
description: If `ALPNProtocols` is set, incoming connections that send an
2018+
ALPN extension with no supported protocols are terminated with
2019+
a fatal `no_application_protocol` alert.
20152020
- version: v12.3.0
20162021
pr-url: https://github.com/nodejs/node/pull/27665
20172022
description: The `options` parameter now supports `net.createServer()`

‎lib/internal/http2/core.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3055,7 +3055,7 @@ function connectionListener(socket) {
30553055
// going on in a format that they *might* understand.
30563056
socket.end('HTTP/1.0 403 Forbidden\r\n'+
30573057
'Content-Type: text/plain\r\n\r\n'+
3058-
'Unknown ALPN Protocol, expected `h2` to be available.\n'+
3058+
'Missing ALPN Protocol, expected `h2` to be available.\n'+
30593059
'If this is a HTTP request: The server was not '+
30603060
'configured with the `allowHTTP1` option or a '+
30613061
'listener for the `unknownProtocol` event.\n');

‎src/crypto/crypto_tls.cc

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -246,13 +246,13 @@ int SelectALPNCallback(
246246
in,
247247
inlen);
248248

249-
//According to 3.2. Protocol Selection of RFC7301, fatal
250-
//no_application_protocol alert shall be sent but OpenSSL 1.0.2 does not
251-
//support it yet. See
252-
//https://rt.openssl.org/Ticket/Display.html?id=3463&user=guest&pass=guest
253-
return status == OPENSSL_NPN_NEGOTIATED
254-
? SSL_TLSEXT_ERR_OK
255-
: SSL_TLSEXT_ERR_NOACK;
249+
//Previous versions of Node.js returned SSL_TLSEXT_ERR_NOACK if no protocol
250+
//match was found. This would neither cause a fatal alert nor would it result
251+
//in a useful ALPN response as part of the Server Hello message.
252+
//We now return SSL_TLSEXT_ERR_ALERT_FATAL in that case as per Section 3.2
253+
// of RFC 7301, which causes a fatal no_application_protocol alert.
254+
return status == OPENSSL_NPN_NEGOTIATED ? SSL_TLSEXT_ERR_OK
255+
: SSL_TLSEXT_ERR_ALERT_FATAL;
256256
}
257257

258258
intTLSExtStatusCallback(SSL* s,void* arg) {

‎test/parallel/test-http2-https-fallback.js

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ function onSession(session, next) {
126126
const{ port}=server.address();
127127
constorigin=`https://localhost:${port}`;
128128

129-
constcleanup=countdown(3,()=>server.close());
129+
constcleanup=countdown(4,()=>server.close());
130130

131131
// HTTP/2 client
132132
connect(
@@ -149,12 +149,22 @@ function onSession(session, next) {
149149

150150
functiontestWrongALPN(){
151151
// Incompatible ALPN TLS client
152-
lettext='';
153152
tls(Object.assign({ port,ALPNProtocols:['fake']},clientOptions))
153+
.on('error',common.mustCall((err)=>{
154+
strictEqual(err.code,'ECONNRESET');
155+
cleanup();
156+
testNoALPN();
157+
}));
158+
}
159+
160+
functiontestNoALPN(){
161+
// TLS client does not send an ALPN extension
162+
lettext='';
163+
tls(Object.assign({ port},clientOptions))
154164
.setEncoding('utf8')
155165
.on('data',(chunk)=>text+=chunk)
156166
.on('end',common.mustCall(()=>{
157-
ok(/UnknownALPNProtocol,expected`h2`tobeavailable/.test(text));
167+
ok(/MissingALPNProtocol,expected`h2`tobeavailable/.test(text));
158168
cleanup();
159169
}));
160170
}

‎test/parallel/test-http2-server-unknown-protocol.js

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ const fixtures = require('../common/fixtures');
88
if(!common.hasCrypto)
99
common.skip('missing crypto');
1010

11+
constassert=require('assert');
1112
consth2=require('http2');
1213
consttls=require('tls');
1314

@@ -18,16 +19,29 @@ const server = h2.createSecureServer({
1819
allowHalfOpen:true
1920
});
2021

21-
server.on('connection',(socket)=>{
22+
server.on('secureConnection',common.mustCall((socket)=>{
2223
socket.on('close',common.mustCall(()=>{
2324
server.close();
2425
}));
25-
});
26+
}));
2627

2728
server.listen(0,function(){
29+
// If the client does not send an ALPN connection, and the server has not been
30+
// configured with allowHTTP1, then the server should destroy the socket
31+
// after unknownProtocolTimeout.
2832
tls.connect({
2933
port:server.address().port,
3034
rejectUnauthorized:false,
31-
ALPNProtocols:['bogus']
3235
});
36+
37+
// If the client sends an ALPN extension that does not contain 'h2', the
38+
// server should send a fatal alert to the client before a secure connection
39+
// is established at all.
40+
tls.connect({
41+
port:server.address().port,
42+
rejectUnauthorized:false,
43+
ALPNProtocols:['bogus']
44+
}).on('error',common.mustCall((err)=>{
45+
assert.strictEqual(err.code,'ECONNRESET');
46+
}));
3347
});

‎test/parallel/test-tls-alpn-server-client.js

Lines changed: 48 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ if (!common.hasCrypto)
55
common.skip('missing crypto');
66

77
constassert=require('assert');
8+
const{ spawn}=require('child_process');
89
consttls=require('tls');
910
constfixtures=require('../common/fixtures');
1011

@@ -68,7 +69,7 @@ function Test1() {
6869
},{
6970
ALPNProtocols:['c','b','e'],
7071
},{
71-
ALPNProtocols:['first-priority-unsupported','x','y'],
72+
ALPNProtocols:['x','y','c'],
7273
}];
7374

7475
runTest(clientsOptions,serverOptions,function(results){
@@ -82,8 +83,8 @@ function Test1() {
8283
client:{ALPN:'b'}});
8384
// Nothing is selected by ALPN
8485
checkResults(results[2],
85-
{server:{ALPN:false},
86-
client:{ALPN:false}});
86+
{server:{ALPN:'c'},
87+
client:{ALPN:'c'}});
8788
// execute next test
8889
Test2();
8990
});
@@ -161,6 +162,50 @@ function Test4() {
161162
{server:{ALPN:false},
162163
client:{ALPN:false}});
163164
});
165+
166+
TestFatalAlert();
167+
}
168+
169+
functionTestFatalAlert(){
170+
constserver=tls.createServer({
171+
ALPNProtocols:['foo'],
172+
key:loadPEM('agent2-key'),
173+
cert:loadPEM('agent2-cert')
174+
},common.mustNotCall());
175+
176+
server.listen(0,serverIP,common.mustCall(()=>{
177+
const{ port}=server.address();
178+
179+
// The Node.js client will just report ECONNRESET because the connection
180+
// is severed before the TLS handshake completes.
181+
tls.connect({
182+
host:serverIP,
183+
port,
184+
rejectUnauthorized:false,
185+
ALPNProtocols:['bar']
186+
},common.mustNotCall()).on('error',common.mustCall((err)=>{
187+
assert.strictEqual(err.code,'ECONNRESET');
188+
189+
// OpenSSL's s_client should output the TLS alert number, which is 120
190+
// for the 'no_application_protocol' alert.
191+
const{ opensslCli}=common;
192+
if(opensslCli){
193+
constaddr=`${serverIP}:${port}`;
194+
letstderr='';
195+
spawn(opensslCli,['s_client','--alpn','bar',addr],{
196+
stdio:['ignore','ignore','pipe']
197+
}).stderr
198+
.setEncoding('utf8')
199+
.on('data',(chunk)=>stderr+=chunk)
200+
.on('close',common.mustCall(()=>{
201+
assert.match(stderr,/SSLalertnumber120/);
202+
server.close();
203+
}));
204+
}else{
205+
server.close();
206+
}
207+
}));
208+
}));
164209
}
165210

166211
Test1();

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp