diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index d1aa1d20ad..d9c7832fd8 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h @@ -2926,7 +2926,8 @@ OPENSSL_EXPORT int SSL_set_alpn_protos(SSL *ssl, const uint8_t *protos, // SSL_CTX_set_alpn_select_cb sets a callback function on |ctx| that is called // during ClientHello processing in order to select an ALPN protocol from the -// client's list of offered protocols. +// client's list of offered protocols. |SSL_select_next_proto| is an optional +// utility function which may be useful in implementing this callback. // // The callback is passed a wire-format (i.e. a series of non-empty, 8-bit // length-prefixed strings) ALPN protocol list in |in|. To select a protocol, @@ -3090,7 +3091,8 @@ OPENSSL_EXPORT void SSL_CTX_set_next_protos_advertised_cb( // set to point to the selected protocol (which may be within |in|). The length // of the protocol name must be written into |*out_len|. The server's advertised // protocols are provided in |in| and |in_len|. The callback can assume that -// |in| is syntactically valid. +// |in| is syntactically valid. |SSL_select_next_proto| is an optional utility +// function which may be useful in implementing this callback. // // The client must select a protocol. It is fatal to the connection if this // callback returns a value other than |SSL_TLSEXT_ERR_OK|. @@ -3112,21 +3114,45 @@ OPENSSL_EXPORT void SSL_get0_next_proto_negotiated(const SSL *ssl, const uint8_t **out_data, unsigned *out_len); -// SSL_select_next_proto implements the standard protocol selection. It is -// expected that this function is called from the callback set by +// SSL_select_next_proto implements the standard protocol selection for either +// ALPN servers or NPN clients. It is expected that this function is called from +// the callback set by |SSL_CTX_set_alpn_select_cb| or // |SSL_CTX_set_next_proto_select_cb|. // -// |peer| and |supported| must be vectors of 8-bit, length-prefixed byte strings -// containing the peer and locally-configured protocols, respectively. The -// length byte itself is not included in the length. A byte string of length 0 -// is invalid. No byte string may be truncated. |supported| is assumed to be -// non-empty. -// -// This function finds the first protocol in |peer| which is also in -// |supported|. If one was found, it sets |*out| and |*out_len| to point to it -// and returns |OPENSSL_NPN_NEGOTIATED|. Otherwise, it returns +// |peer| and |supported| contain the peer and locally-configured protocols, +// respectively. This function finds the first protocol in |peer| which is also +// in |supported|. If one was found, it sets |*out| and |*out_len| to point to +// it and returns |OPENSSL_NPN_NEGOTIATED|. Otherwise, it returns // |OPENSSL_NPN_NO_OVERLAP| and sets |*out| and |*out_len| to the first // supported protocol. +// +// In ALPN, the server should only select protocols among those that the client +// offered. Thus, if this function returns |OPENSSL_NPN_NO_OVERLAP|, the caller +// should ignore |*out| and return |SSL_TLSEXT_ERR_ALERT_FATAL| from +// |SSL_CTX_set_alpn_select_cb|'s callback to indicate there was no match. +// +// In NPN, the client may either select one of the server's protocols, or an +// "opportunistic" protocol as described in Section 6 of +// draft-agl-tls-nextprotoneg-03. When this function returns +// |OPENSSL_NPN_NO_OVERLAP|, |*out| implicitly selects the first supported +// protocol for use as the opportunistic protocol. The caller may use it, +// ignore it and select a different opportunistic protocol, or ignore it and +// select no protocol (empty string). +// +// |peer| and |supported| must be vectors of 8-bit, length-prefixed byte +// strings. The length byte itself is not included in the length. A byte string +// of length 0 is invalid. No byte string may be truncated. |supported| must be +// non-empty; a caller that supports no ALPN/NPN protocols should skip +// negotiating the extension, rather than calling this function. If any of these +// preconditions do not hold, this function will return |OPENSSL_NPN_NO_OVERLAP| +// and set |*out| and |*out_len| to an empty buffer for robustness, but callers +// are not recommended to rely on this. An empty buffer is not a valid output +// for |SSL_CTX_set_alpn_select_cb|'s callback. +// +// WARNING: |*out| and |*out_len| may alias either |peer| or |supported| and may +// not be used after one of those buffers is modified or released. Additionally, +// this function is not const-correct for compatibility reasons. Although |*out| +// is a non-const pointer, callers may not modify the buffer though |*out|. OPENSSL_EXPORT int SSL_select_next_proto(uint8_t **out, uint8_t *out_len, const uint8_t *peer, unsigned peer_len, const uint8_t *supported, diff --git a/ssl/extensions.cc b/ssl/extensions.cc index ef42194442..e779d3dfa8 100644 --- a/ssl/extensions.cc +++ b/ssl/extensions.cc @@ -1519,16 +1519,19 @@ bool ssl_is_alpn_protocol_allowed(const SSL_HANDSHAKE *hs, } // Check that the protocol name is one of the ones we advertised. - CBS client_protocol_name_list = - MakeConstSpan(hs->config->alpn_client_proto_list), - client_protocol_name; - while (CBS_len(&client_protocol_name_list) > 0) { - if (!CBS_get_u8_length_prefixed(&client_protocol_name_list, - &client_protocol_name)) { + return ssl_alpn_list_contains_protocol(hs->config->alpn_client_proto_list, + protocol); +} + +bool ssl_alpn_list_contains_protocol(Span list, + Span protocol) { + CBS cbs = list, candidate; + while (CBS_len(&cbs) > 0) { + if (!CBS_get_u8_length_prefixed(&cbs, &candidate)) { return false; } - if (client_protocol_name == protocol) { + if (candidate == protocol) { return true; } } diff --git a/ssl/internal.h b/ssl/internal.h index 828e473c75..952ad48b3b 100644 --- a/ssl/internal.h +++ b/ssl/internal.h @@ -2242,6 +2242,11 @@ bool ssl_is_valid_alpn_list(Span in); bool ssl_is_alpn_protocol_allowed(const SSL_HANDSHAKE *hs, Span protocol); +// ssl_alpn_list_contains_protocol returns whether |list|, a serialized ALPN +// protocol list, contains |protocol|. +bool ssl_alpn_list_contains_protocol(Span list, + Span protocol); + // ssl_negotiate_alpn negotiates the ALPN extension, if applicable. It returns // true on successful negotiation or if nothing was negotiated. It returns false // and sets |*out_alert| to an alert on error. diff --git a/ssl/ssl_lib.cc b/ssl/ssl_lib.cc index c88e6a54c6..ee4b63e48d 100644 --- a/ssl/ssl_lib.cc +++ b/ssl/ssl_lib.cc @@ -2194,34 +2194,49 @@ int SSL_CTX_set_tlsext_servername_arg(SSL_CTX *ctx, void *arg) { int SSL_select_next_proto(uint8_t **out, uint8_t *out_len, const uint8_t *peer, unsigned peer_len, const uint8_t *supported, unsigned supported_len) { - const uint8_t *result; - int status; + *out = nullptr; + *out_len = 0; + + // Both |peer| and |supported| must be valid protocol lists, but |peer| may be + // empty in NPN. + auto peer_span = MakeConstSpan(peer, peer_len); + auto supported_span = MakeConstSpan(supported, supported_len); + if ((!peer_span.empty() && !ssl_is_valid_alpn_list(peer_span)) || + !ssl_is_valid_alpn_list(supported_span)) { + return OPENSSL_NPN_NO_OVERLAP; + } // For each protocol in peer preference order, see if we support it. - for (unsigned i = 0; i < peer_len;) { - for (unsigned j = 0; j < supported_len;) { - if (peer[i] == supported[j] && - OPENSSL_memcmp(&peer[i + 1], &supported[j + 1], peer[i]) == 0) { - // We found a match - result = &peer[i]; - status = OPENSSL_NPN_NEGOTIATED; - goto found; - } - j += supported[j]; - j++; + CBS cbs = peer_span, proto; + while (CBS_len(&cbs) != 0) { + if (!CBS_get_u8_length_prefixed(&cbs, &proto) || CBS_len(&proto) == 0) { + return OPENSSL_NPN_NO_OVERLAP; + } + + if (ssl_alpn_list_contains_protocol(MakeConstSpan(supported, supported_len), + proto)) { + // This function is not const-correct for compatibility with existing + // callers. + *out = const_cast(CBS_data(&proto)); + // A u8 length prefix will fit in |uint8_t|. + *out_len = static_cast(CBS_len(&proto)); + return OPENSSL_NPN_NEGOTIATED; } - i += peer[i]; - i++; } - // There's no overlap between our protocols and the peer's list. - result = supported; - status = OPENSSL_NPN_NO_OVERLAP; + // There's no overlap between our protocols and the peer's list. In ALPN, the + // caller is expected to fail the connection with no_application_protocol. In + // NPN, the caller is expected to opportunistically select the first protocol. + // See draft-agl-tls-nextprotoneg-04, section 6. + cbs = supported_span; + if (!CBS_get_u8_length_prefixed(&cbs, &proto) || CBS_len(&proto) == 0) { + return OPENSSL_NPN_NO_OVERLAP; + } -found: - *out = (uint8_t *)result + 1; - *out_len = result[0]; - return status; + // See above. + *out = const_cast(CBS_data(&proto)); + *out_len = static_cast(CBS_len(&proto)); + return OPENSSL_NPN_NO_OVERLAP; } void SSL_get0_next_proto_negotiated(const SSL *ssl, const uint8_t **out_data, diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc index b3c7e05036..30bc3ffc87 100644 --- a/ssl/ssl_test.cc +++ b/ssl/ssl_test.cc @@ -5454,17 +5454,52 @@ TEST(SSLTest, SelectNextProto) { (const uint8_t *)"\3ccc\2bb\1a", 9)); EXPECT_EQ(Bytes("a"), Bytes(result, result_len)); - // If there is no overlap, return the first local protocol. + // If there is no overlap, opportunistically select the first local protocol. + // ALPN callers should ignore this, but NPN callers may use this per + // draft-agl-tls-nextprotoneg-03, section 6. EXPECT_EQ(OPENSSL_NPN_NO_OVERLAP, SSL_select_next_proto(&result, &result_len, (const uint8_t *)"\1a\2bb\3ccc", 9, (const uint8_t *)"\1x\2yy\3zzz", 9)); EXPECT_EQ(Bytes("x"), Bytes(result, result_len)); + // The peer preference order may be empty in NPN. This should be treated as no + // overlap and continue to select an opportunistic protocol. EXPECT_EQ(OPENSSL_NPN_NO_OVERLAP, SSL_select_next_proto(&result, &result_len, nullptr, 0, (const uint8_t *)"\1x\2yy\3zzz", 9)); EXPECT_EQ(Bytes("x"), Bytes(result, result_len)); + + // Although calling this function with no local protocols is a caller error, + // it should cleanly return an empty protocol. + EXPECT_EQ( + OPENSSL_NPN_NO_OVERLAP, + SSL_select_next_proto(&result, &result_len, + (const uint8_t *)"\1a\2bb\3ccc", 9, nullptr, 0)); + EXPECT_EQ(Bytes(""), Bytes(result, result_len)); + + // Syntax errors are similarly caller errors. + EXPECT_EQ( + OPENSSL_NPN_NO_OVERLAP, + SSL_select_next_proto(&result, &result_len, (const uint8_t *)"\4aaa", 4, + (const uint8_t *)"\1a\2bb\3ccc", 9)); + EXPECT_EQ(Bytes(""), Bytes(result, result_len)); + EXPECT_EQ(OPENSSL_NPN_NO_OVERLAP, + SSL_select_next_proto(&result, &result_len, + (const uint8_t *)"\1a\2bb\3ccc", 9, + (const uint8_t *)"\4aaa", 4)); + EXPECT_EQ(Bytes(""), Bytes(result, result_len)); + + // Protocols in protocol lists may not be empty. + EXPECT_EQ(OPENSSL_NPN_NO_OVERLAP, + SSL_select_next_proto(&result, &result_len, + (const uint8_t *)"\0\2bb\3ccc", 8, + (const uint8_t *)"\1a\2bb\3ccc", 9)); + EXPECT_EQ(OPENSSL_NPN_NO_OVERLAP, + SSL_select_next_proto(&result, &result_len, + (const uint8_t *)"\1a\2bb\3ccc", 9, + (const uint8_t *)"\0\2bb\3ccc", 8)); + EXPECT_EQ(Bytes(""), Bytes(result, result_len)); } // The client should gracefully handle no suitable ciphers being enabled.