Commit a5e7255
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
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Rod Vagg <rod@vagg.org>1 parent 8c29420 commit a5e7255
1 file changed
Lines changed: 6 additions & 14 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -2509,20 +2509,12 @@ int SSLWrap<Base>::SelectALPNCallback(SSL* s, | |||
| 2509 | 2509 | unsigned alpn_protos_len = Buffer::Length(alpn_buffer); | |
| 2510 | 2510 | int status = SSL_select_next_proto(const_cast<unsigned char**>(out), outlen, | |
| 2511 | 2511 | alpn_protos, alpn_protos_len, in, inlen); | |
| 2512 | - | ||
| 2513 | - switch (status) { | ||
| 2514 | - case OPENSSL_NPN_NO_OVERLAP: | ||
| 2515 | - // According to 3.2. Protocol Selection of RFC7301, | ||
| 2516 | - // fatal no_application_protocol alert shall be sent | ||
| 2517 | - // but current openssl does not support it yet. See | ||
| 2518 | - // https://rt.openssl.org/Ticket/Display.html?id=3463&user=guest&pass=guest | ||
| 2519 | - // Instead, we send a warning alert for now. | ||
| 2520 | - return SSL_TLSEXT_ERR_ALERT_WARNING; | ||
| 2521 | - case OPENSSL_NPN_NEGOTIATED: | ||
| 2522 | - return SSL_TLSEXT_ERR_OK; | ||
| 2523 | - default: | ||
| 2524 | - return SSL_TLSEXT_ERR_ALERT_FATAL; | ||
| 2525 | - } | ||
| 2512 | + // According to 3.2. Protocol Selection of RFC7301, fatal | ||
| 2513 | + // no_application_protocol alert shall be sent but OpenSSL 1.0.2 does not | ||
| 2514 | + // support it yet. See | ||
| 2515 | + // https://rt.openssl.org/Ticket/Display.html?id=3463&user=guest&pass=guest | ||
| 2516 | + return status == OPENSSL_NPN_NEGOTIATED ? SSL_TLSEXT_ERR_OK | ||
| 2517 | + : SSL_TLSEXT_ERR_NOACK; | ||
| 2526 | 2518 | } | |
| 2527 | 2519 | #endif // TLSEXT_TYPE_application_layer_protocol_negotiation | |
| 2528 | 2520 | ||
0 commit comments