diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index 462e4ce2..921ae2a0 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h @@ -2626,35 +2626,21 @@ OPENSSL_EXPORT void SSL_get0_next_proto_negotiated(const SSL *ssl, * expected that this function is called from the callback set by * |SSL_CTX_set_next_proto_select_cb|. * - * The protocol data is assumed to be a vector 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. + * |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. * - * The current, but experimental algorithm for selecting the protocol is: - * - * 1) If the server doesn't support NPN then this is indicated to the - * callback. In this case, the client application has to abort the connection - * or have a default application level protocol. - * - * 2) If the server supports NPN, but advertises an empty list then the - * client selects the first protocol in its list, but indicates via the - * API that this fallback case was enacted. - * - * 3) Otherwise, the client finds the first protocol in the server's list - * that it supports and selects this protocol. This is because it's - * assumed that the server has better information about which protocol - * a client should use. - * - * 4) If the client doesn't support any of the server's advertised - * protocols, then this is treated the same as case 2. - * - * It returns either |OPENSSL_NPN_NEGOTIATED| if a common protocol was found, or - * |OPENSSL_NPN_NO_OVERLAP| if the fallback case was reached. */ + * 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. */ OPENSSL_EXPORT int SSL_select_next_proto(uint8_t **out, uint8_t *out_len, - const uint8_t *server, - unsigned server_len, - const uint8_t *client, - unsigned client_len); + const uint8_t *peer, unsigned peer_len, + const uint8_t *supported, + unsigned supported_len); #define OPENSSL_NPN_UNSUPPORTED 0 #define OPENSSL_NPN_NEGOTIATED 1 diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c index f56abbe2..6273e000 100644 --- a/ssl/ssl_lib.c +++ b/ssl/ssl_lib.c @@ -1678,32 +1678,31 @@ int SSL_CTX_set_tlsext_servername_arg(SSL_CTX *ctx, void *arg) { return 1; } -int SSL_select_next_proto(uint8_t **out, uint8_t *out_len, - const uint8_t *server, unsigned server_len, - const uint8_t *client, unsigned client_len) { - unsigned int i, j; +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 = OPENSSL_NPN_UNSUPPORTED; + int status; - /* For each protocol in server preference order, see if we support it. */ - for (i = 0; i < server_len;) { - for (j = 0; j < client_len;) { - if (server[i] == client[j] && - OPENSSL_memcmp(&server[i + 1], &client[j + 1], server[i]) == 0) { + /* 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 = &server[i]; + result = &peer[i]; status = OPENSSL_NPN_NEGOTIATED; goto found; } - j += client[j]; + j += supported[j]; j++; } - i += server[i]; + i += peer[i]; i++; } - /* There's no overlap between our protocols and the server's list. */ - result = client; + /* There's no overlap between our protocols and the peer's list. */ + result = supported; status = OPENSSL_NPN_NO_OVERLAP; found: diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc index 2bf6eeb7..b53c93a0 100644 --- a/ssl/ssl_test.cc +++ b/ssl/ssl_test.cc @@ -1497,6 +1497,7 @@ TEST(SSLTest, SessionDuplication) { } static void ExpectFDs(const SSL *ssl, int rfd, int wfd) { + EXPECT_EQ(rfd, SSL_get_fd(ssl)); EXPECT_EQ(rfd, SSL_get_rfd(ssl)); EXPECT_EQ(wfd, SSL_get_wfd(ssl)); @@ -3454,6 +3455,49 @@ TEST(SSLTest, SSL3Method) { EXPECT_EQ(SSL_R_NO_SUPPORTED_VERSIONS_ENABLED, ERR_GET_REASON(err)); } +TEST(SSLTest, SelectNextProto) { + uint8_t *result; + uint8_t result_len; + + // If there is an overlap, it should be returned. + EXPECT_EQ(OPENSSL_NPN_NEGOTIATED, + SSL_select_next_proto(&result, &result_len, + (const uint8_t *)"\1a\2bb\3ccc", 9, + (const uint8_t *)"\1x\1y\1a\1z", 8)); + EXPECT_EQ(Bytes("a"), Bytes(result, result_len)); + + EXPECT_EQ(OPENSSL_NPN_NEGOTIATED, + SSL_select_next_proto(&result, &result_len, + (const uint8_t *)"\1a\2bb\3ccc", 9, + (const uint8_t *)"\1x\1y\2bb\1z", 9)); + EXPECT_EQ(Bytes("bb"), Bytes(result, result_len)); + + EXPECT_EQ(OPENSSL_NPN_NEGOTIATED, + SSL_select_next_proto(&result, &result_len, + (const uint8_t *)"\1a\2bb\3ccc", 9, + (const uint8_t *)"\1x\1y\3ccc\1z", 10)); + EXPECT_EQ(Bytes("ccc"), Bytes(result, result_len)); + + // Peer preference order takes precedence over local. + EXPECT_EQ(OPENSSL_NPN_NEGOTIATED, + SSL_select_next_proto(&result, &result_len, + (const uint8_t *)"\1a\2bb\3ccc", 9, + (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. + 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)); + + 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)); +} + // TODO(davidben): Convert this file to GTest properly. TEST(SSLTest, AllTests) { if (!TestSSL_SESSIONEncoding(kOpenSSLSession) ||