1berry/nodejs
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

crypto: make ALPN the same for OpenSSL 1.0.2 & 1.1.0

Browse Source 
This is kind of hairy. OpenSSL 1.0.2 ignored the return value and always
treated everything as SSL_TLSEXT_ERR_NOACK (so the comment was wrong and
Node was never sending a warning alert). OpenSSL 1.1.0 honors
SSL_TLSEXT_ERR_NOACK vs SSL_TLSEXT_ERR_FATAL_ALERT and treats everything
unknown as SSL_TLSEXT_ERR_FATAL_ALERT.

Since this is a behavior change (tests break too), start by aligning
everything on SSL_TLSEXT_ERR_NOACK. If sending no_application_protocol
is desirable in the future, this can by changed to
SSL_TLSEXT_ERR_FATAL_ALERT with whatever deprecation process is
appropriate.

However, note that, contrary to
https://rt.openssl.org/Ticket/Display.html?id=3463#txn-54498,
SSL_TLSEXT_ERR_FATAL_ALERT is *not* useful to a server with no fallback
protocol. Even if such mismatches were rejected, such a server must
*still* account for the fallback protocol case when the client does not
advertise ALPN at all. Thus this may not be worth bothering.

PR-URL: https://github.com/nodejs/node/pull/16130
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Rod Vagg <rod@vagg.org>
This commit is contained in:
David Benjamin 2017-09-23 12:44:24 -04:00 committed by Rod Vagg
parent d9b9229d98
commit 63c278959a
1 changed files with 6 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
src/node_crypto.cc
View File

@ -2502,20 +2502,12 @@ int SSLWrap<Base>::SelectALPNCallback(SSL* s,
unsigned alpn_protos_len = Buffer::Length(alpn_buffer); unsigned alpn_protos_len = Buffer::Length(alpn_buffer);
int status = SSL_select_next_proto(const_cast<unsigned char**>(out), outlen, int status = SSL_select_next_proto(const_cast<unsigned char**>(out), outlen,
alpn_protos, alpn_protos_len, in, inlen); alpn_protos, alpn_protos_len, in, inlen);
// According to 3.2. Protocol Selection of RFC7301, fatal
switch (status) { // no_application_protocol alert shall be sent but OpenSSL 1.0.2 does not
case OPENSSL_NPN_NO_OVERLAP: // support it yet. See
// According to 3.2. Protocol Selection of RFC7301, // https://rt.openssl.org/Ticket/Display.html?id=3463&user=guest&pass=guest
// fatal no_application_protocol alert shall be sent return status == OPENSSL_NPN_NEGOTIATED ? SSL_TLSEXT_ERR_OK
// but current openssl does not support it yet. See : SSL_TLSEXT_ERR_NOACK;
// https://rt.openssl.org/Ticket/Display.html?id=3463&user=guest&pass=guest
// Instead, we send a warning alert for now.
return SSL_TLSEXT_ERR_ALERT_WARNING;
case OPENSSL_NPN_NEGOTIATED:
return SSL_TLSEXT_ERR_OK;
default:
return SSL_TLSEXT_ERR_ALERT_FATAL;
}
} }
#endif // TLSEXT_TYPE_application_layer_protocol_negotiation #endif // TLSEXT_TYPE_application_layer_protocol_negotiation