Skip to content

Commit 7873826

Browse files
davidbengibfahn
authored andcommitted
crypto: make ALPN the same for OpenSSL 1.0.2 & 1.1.0
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: #16130 Backport-PR-URL: #18622 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Rod Vagg <[email protected]>
1 parent 92ec6f6 commit 7873826

File tree

1 file changed

+6
-14
lines changed

1 file changed

+6
-14
lines changed

src/node_crypto.cc

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2505,20 +2505,12 @@ int SSLWrap<Base>::SelectALPNCallback(SSL* s,
25052505
unsigned alpn_protos_len = Buffer::Length(alpn_buffer);
25062506
int status = SSL_select_next_proto(const_cast<unsigned char**>(out), outlen,
25072507
alpn_protos, alpn_protos_len, in, inlen);
2508-
2509-
switch (status) {
2510-
case OPENSSL_NPN_NO_OVERLAP:
2511-
// According to 3.2. Protocol Selection of RFC7301,
2512-
// fatal no_application_protocol alert shall be sent
2513-
// but current openssl does not support it yet. See
2514-
// https://rt.openssl.org/Ticket/Display.html?id=3463&user=guest&pass=guest
2515-
// Instead, we send a warning alert for now.
2516-
return SSL_TLSEXT_ERR_ALERT_WARNING;
2517-
case OPENSSL_NPN_NEGOTIATED:
2518-
return SSL_TLSEXT_ERR_OK;
2519-
default:
2520-
return SSL_TLSEXT_ERR_ALERT_FATAL;
2521-
}
2508+
// According to 3.2. Protocol Selection of RFC7301, fatal
2509+
// no_application_protocol alert shall be sent but OpenSSL 1.0.2 does not
2510+
// support it yet. See
2511+
// https://rt.openssl.org/Ticket/Display.html?id=3463&user=guest&pass=guest
2512+
return status == OPENSSL_NPN_NEGOTIATED ? SSL_TLSEXT_ERR_OK
2513+
: SSL_TLSEXT_ERR_NOACK;
25222514
}
25232515
#endif // TLSEXT_TYPE_application_layer_protocol_negotiation
25242516

0 commit comments

Comments
 (0)