From 78738266d6f73b343083c8fbc920b848b2135810 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Sat, 23 Sep 2017 12:44:24 -0400 Subject: [PATCH] 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: https://github.com/nodejs/node/pull/16130 Backport-PR-URL: https://github.com/nodejs/node/pull/18622 Reviewed-By: Ben Noordhuis Reviewed-By: Rod Vagg --- src/node_crypto.cc | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/src/node_crypto.cc b/src/node_crypto.cc index d4a90313bdc11d..ff219c5df7893a 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -2505,20 +2505,12 @@ int SSLWrap::SelectALPNCallback(SSL* s, unsigned alpn_protos_len = Buffer::Length(alpn_buffer); int status = SSL_select_next_proto(const_cast(out), outlen, alpn_protos, alpn_protos_len, in, inlen); - - switch (status) { - case OPENSSL_NPN_NO_OVERLAP: - // According to 3.2. Protocol Selection of RFC7301, - // fatal no_application_protocol alert shall be sent - // but current openssl does not support it yet. See - // 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; - } + // According to 3.2. Protocol Selection of RFC7301, fatal + // no_application_protocol alert shall be sent but OpenSSL 1.0.2 does not + // support it yet. See + // https://rt.openssl.org/Ticket/Display.html?id=3463&user=guest&pass=guest + return status == OPENSSL_NPN_NEGOTIATED ? SSL_TLSEXT_ERR_OK + : SSL_TLSEXT_ERR_NOACK; } #endif // TLSEXT_TYPE_application_layer_protocol_negotiation