From af36847fb50356545521e8dc3d6471dd265d9ecd Mon Sep 17 00:00:00 2001 From: Matthew Stevenson Date: Mon, 18 Dec 2023 19:49:57 -0800 Subject: [PATCH] [alpn] Remove grpc-exp experimental ALPN protocol. (#34876) This fixes #21619. This experimental ALPN protocol has already been removed from the other gRPC stacks. Closes #34876 COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/34876 from matthewstevenson88:remove-grpc-exp 1cb9d084eaa11b3b0c6ddfdfa8afab2bcbc654d0 PiperOrigin-RevId: 592080195 --- src/core/ext/transport/chttp2/alpn/alpn.cc | 2 +- test/core/handshake/client_ssl.cc | 15 ++-------- .../readahead_handshaker_server_ssl.cc | 4 +-- test/core/handshake/server_ssl.cc | 9 ++---- test/core/security/security_connector_test.cc | 2 +- .../security/tls_security_connector_test.cc | 28 +++++++++---------- test/core/transport/chttp2/alpn_test.cc | 19 +------------ 7 files changed, 24 insertions(+), 55 deletions(-) diff --git a/src/core/ext/transport/chttp2/alpn/alpn.cc b/src/core/ext/transport/chttp2/alpn/alpn.cc index f02249c0b8a79..432f2c06076ba 100644 --- a/src/core/ext/transport/chttp2/alpn/alpn.cc +++ b/src/core/ext/transport/chttp2/alpn/alpn.cc @@ -25,7 +25,7 @@ #include "src/core/lib/gpr/useful.h" // in order of preference -static const char* const supported_versions[] = {"grpc-exp", "h2"}; +static const char* const supported_versions[] = {"h2"}; int grpc_chttp2_is_alpn_version_supported(const char* version, size_t size) { size_t i; diff --git a/test/core/handshake/client_ssl.cc b/test/core/handshake/client_ssl.cc index 45e39c1517606..d26b6a5fca902 100644 --- a/test/core/handshake/client_ssl.cc +++ b/test/core/handshake/client_ssl.cc @@ -153,27 +153,19 @@ static int alpn_select_cb(SSL* /*ssl*/, const uint8_t** out, uint8_t* out_len, *out_len = static_cast( strlen(reinterpret_cast(alpn_preferred))); - // Validate that the ALPN list includes "h2" and "grpc-exp", that "grpc-exp" - // precedes "h2". - bool grpc_exp_seen = false; + // Validate that the ALPN list includes "h2". bool h2_seen = false; const char* inp = reinterpret_cast(in); const char* in_end = inp + in_len; while (inp < in_end) { const size_t length = static_cast(*inp++); - if (length == strlen("grpc-exp") && strncmp(inp, "grpc-exp", length) == 0) { - grpc_exp_seen = true; - EXPECT_FALSE(h2_seen); - } if (length == strlen("h2") && strncmp(inp, "h2", length) == 0) { h2_seen = true; - EXPECT_TRUE(grpc_exp_seen); } inp += length; } EXPECT_EQ(inp, in_end); - EXPECT_TRUE(grpc_exp_seen); EXPECT_TRUE(h2_seen); return SSL_TLSEXT_ERR_OK; @@ -400,10 +392,7 @@ static bool client_ssl_test(char* server_alpn_preferred) { } TEST(ClientSslTest, MainTest) { - // Handshake succeeeds when the server has grpc-exp as the ALPN preference. - ASSERT_TRUE(client_ssl_test(const_cast("grpc-exp"))); - // Handshake succeeeds when the server has h2 as the ALPN preference. This - // covers legacy gRPC servers which don't support grpc-exp. + // Handshake succeeeds when the server has h2 as the ALPN preference. ASSERT_TRUE(client_ssl_test(const_cast("h2"))); // TODO(gtcooke94) Figure out why test is failing with OpenSSL and fix it. diff --git a/test/core/handshake/readahead_handshaker_server_ssl.cc b/test/core/handshake/readahead_handshaker_server_ssl.cc index f09dff35ce4b0..8acb7120f1096 100644 --- a/test/core/handshake/readahead_handshaker_server_ssl.cc +++ b/test/core/handshake/readahead_handshaker_server_ssl.cc @@ -85,8 +85,8 @@ TEST(HandshakeServerWithReadaheadHandshakerTest, MainTest) { }); grpc_init(); - const char* full_alpn_list[] = {"grpc-exp", "h2"}; - ASSERT_TRUE(server_ssl_test(full_alpn_list, 2, "grpc-exp")); + const char* full_alpn_list[] = {"h2"}; + ASSERT_TRUE(server_ssl_test(full_alpn_list, 1, "h2")); CleanupSslLibrary(); grpc_shutdown(); } diff --git a/test/core/handshake/server_ssl.cc b/test/core/handshake/server_ssl.cc index 36575005dc6ff..3e006d41d42a7 100644 --- a/test/core/handshake/server_ssl.cc +++ b/test/core/handshake/server_ssl.cc @@ -24,17 +24,14 @@ #include "test/core/util/test_config.h" TEST(ServerSslTest, MainTest) { - // Handshake succeeeds when the client supplies the standard ALPN list. - const char* full_alpn_list[] = {"grpc-exp", "h2"}; - ASSERT_TRUE(server_ssl_test(full_alpn_list, 2, "grpc-exp")); // Handshake succeeeds when the client supplies only h2 as the ALPN list. This // covers legacy gRPC clients which don't support grpc-exp. const char* h2_only_alpn_list[] = {"h2"}; ASSERT_TRUE(server_ssl_test(h2_only_alpn_list, 1, "h2")); // Handshake succeeds when the client supplies superfluous ALPN entries and - // also when h2 precedes gprc-exp. - const char* extra_alpn_list[] = {"foo", "h2", "bar", "grpc-exp"}; - ASSERT_TRUE(server_ssl_test(extra_alpn_list, 4, "h2")); + // also when h2 is included. + const char* extra_alpn_list[] = {"foo", "h2", "bar"}; + ASSERT_TRUE(server_ssl_test(extra_alpn_list, 3, "h2")); // Handshake fails when the client uses a fake protocol as its only ALPN // preference. This validates the server is correctly validating ALPN // and sanity checks the server_ssl_test. diff --git a/test/core/security/security_connector_test.cc b/test/core/security/security_connector_test.cc index e7e5d47aa5a3d..cb781d1fd7e68 100644 --- a/test/core/security/security_connector_test.cc +++ b/test/core/security/security_connector_test.cc @@ -718,7 +718,7 @@ static void test_default_ssl_roots(void) { static void test_peer_alpn_check(void) { #if TSI_OPENSSL_ALPN_SUPPORT tsi_peer peer; - const char* alpn = "grpc"; + const char* alpn = "h2"; const char* wrong_alpn = "wrong"; // peer does not have a TSI_SSL_ALPN_SELECTED_PROTOCOL property. ASSERT_EQ(tsi_construct_peer(1, &peer), TSI_OK); diff --git a/test/core/security/tls_security_connector_test.cc b/test/core/security/tls_security_connector_test.cc index 6323fe2bc091b..837b707740992 100644 --- a/test/core/security/tls_security_connector_test.cc +++ b/test/core/security/tls_security_connector_test.cc @@ -392,7 +392,7 @@ TEST_F(TlsSecurityConnectorTest, tsi_peer peer; GPR_ASSERT(tsi_construct_peer(2, &peer) == TSI_OK); GPR_ASSERT(tsi_construct_string_peer_property(TSI_SSL_ALPN_SELECTED_PROTOCOL, - "grpc", strlen("grpc"), + "h2", strlen("h2"), &peer.properties[0]) == TSI_OK); GPR_ASSERT(tsi_construct_string_peer_property_from_cstring( TSI_X509_SUBJECT_COMMON_NAME_PEER_PROPERTY, "foo.bar.com", @@ -428,7 +428,7 @@ TEST_F(TlsSecurityConnectorTest, tsi_peer peer; GPR_ASSERT(tsi_construct_peer(2, &peer) == TSI_OK); GPR_ASSERT(tsi_construct_string_peer_property(TSI_SSL_ALPN_SELECTED_PROTOCOL, - "grpc", strlen("grpc"), + "h2", strlen("h2"), &peer.properties[0]) == TSI_OK); GPR_ASSERT(tsi_construct_string_peer_property_from_cstring( TSI_X509_SUBJECT_COMMON_NAME_PEER_PROPERTY, "foo.bar.com", @@ -581,7 +581,7 @@ TEST_F(TlsSecurityConnectorTest, tsi_peer peer; GPR_ASSERT(tsi_construct_peer(2, &peer) == TSI_OK); GPR_ASSERT(tsi_construct_string_peer_property(TSI_SSL_ALPN_SELECTED_PROTOCOL, - "grpc", strlen("grpc"), + "h2", strlen("h2"), &peer.properties[0]) == TSI_OK); GPR_ASSERT(tsi_construct_string_peer_property_from_cstring( TSI_X509_SUBJECT_COMMON_NAME_PEER_PROPERTY, "foo.bar.com", @@ -617,7 +617,7 @@ TEST_F(TlsSecurityConnectorTest, tsi_peer peer; GPR_ASSERT(tsi_construct_peer(2, &peer) == TSI_OK); GPR_ASSERT(tsi_construct_string_peer_property(TSI_SSL_ALPN_SELECTED_PROTOCOL, - "grpc", strlen("grpc"), + "h2", strlen("h2"), &peer.properties[0]) == TSI_OK); GPR_ASSERT(tsi_construct_string_peer_property_from_cstring( TSI_X509_SUBJECT_COMMON_NAME_PEER_PROPERTY, "foo.bar.com", @@ -655,7 +655,7 @@ TEST_F(TlsSecurityConnectorTest, tsi_peer peer; GPR_ASSERT(tsi_construct_peer(7, &peer) == TSI_OK); GPR_ASSERT(tsi_construct_string_peer_property(TSI_SSL_ALPN_SELECTED_PROTOCOL, - "grpc", strlen("grpc"), + "h2", strlen("h2"), &peer.properties[0]) == TSI_OK); GPR_ASSERT(tsi_construct_string_peer_property_from_cstring( TSI_X509_SUBJECT_COMMON_NAME_PEER_PROPERTY, "foo.bar.com", @@ -703,7 +703,7 @@ TEST_F(TlsSecurityConnectorTest, tsi_peer peer; GPR_ASSERT(tsi_construct_peer(7, &peer) == TSI_OK); GPR_ASSERT(tsi_construct_string_peer_property(TSI_SSL_ALPN_SELECTED_PROTOCOL, - "grpc", strlen("grpc"), + "h2", strlen("h2"), &peer.properties[0]) == TSI_OK); GPR_ASSERT(tsi_construct_string_peer_property_from_cstring( TSI_X509_SUBJECT_COMMON_NAME_PEER_PROPERTY, "foo.com", @@ -761,8 +761,8 @@ TEST_F(TlsSecurityConnectorTest, tsi_peer peer; EXPECT_EQ(tsi_construct_peer(2, &peer), TSI_OK); EXPECT_EQ( - tsi_construct_string_peer_property(TSI_SSL_ALPN_SELECTED_PROTOCOL, "grpc", - strlen("grpc"), &peer.properties[0]), + tsi_construct_string_peer_property(TSI_SSL_ALPN_SELECTED_PROTOCOL, "h2", + strlen("h2"), &peer.properties[0]), TSI_OK); EXPECT_EQ(tsi_construct_string_peer_property_from_cstring( TSI_X509_VERIFIED_ROOT_CERT_SUBECT_PEER_PROPERTY, @@ -1012,7 +1012,7 @@ TEST_F(TlsSecurityConnectorTest, tsi_peer peer; GPR_ASSERT(tsi_construct_peer(2, &peer) == TSI_OK); GPR_ASSERT(tsi_construct_string_peer_property(TSI_SSL_ALPN_SELECTED_PROTOCOL, - "grpc", strlen("grpc"), + "h2", strlen("h2"), &peer.properties[0]) == TSI_OK); GPR_ASSERT(tsi_construct_string_peer_property_from_cstring( TSI_X509_SUBJECT_COMMON_NAME_PEER_PROPERTY, "foo.bar.com", @@ -1043,7 +1043,7 @@ TEST_F(TlsSecurityConnectorTest, tsi_peer peer; GPR_ASSERT(tsi_construct_peer(2, &peer) == TSI_OK); GPR_ASSERT(tsi_construct_string_peer_property(TSI_SSL_ALPN_SELECTED_PROTOCOL, - "grpc", strlen("grpc"), + "h2", strlen("h2"), &peer.properties[0]) == TSI_OK); GPR_ASSERT(tsi_construct_string_peer_property_from_cstring( TSI_X509_SUBJECT_COMMON_NAME_PEER_PROPERTY, "foo.bar.com", @@ -1078,7 +1078,7 @@ TEST_F(TlsSecurityConnectorTest, tsi_peer peer; GPR_ASSERT(tsi_construct_peer(2, &peer) == TSI_OK); GPR_ASSERT(tsi_construct_string_peer_property(TSI_SSL_ALPN_SELECTED_PROTOCOL, - "grpc", strlen("grpc"), + "h2", strlen("h2"), &peer.properties[0]) == TSI_OK); GPR_ASSERT(tsi_construct_string_peer_property_from_cstring( TSI_X509_SUBJECT_COMMON_NAME_PEER_PROPERTY, "foo.bar.com", @@ -1111,7 +1111,7 @@ TEST_F(TlsSecurityConnectorTest, tsi_peer peer; GPR_ASSERT(tsi_construct_peer(2, &peer) == TSI_OK); GPR_ASSERT(tsi_construct_string_peer_property(TSI_SSL_ALPN_SELECTED_PROTOCOL, - "grpc", strlen("grpc"), + "h2", strlen("h2"), &peer.properties[0]) == TSI_OK); GPR_ASSERT(tsi_construct_string_peer_property_from_cstring( TSI_X509_SUBJECT_COMMON_NAME_PEER_PROPERTY, "foo.bar.com", @@ -1150,8 +1150,8 @@ TEST_F(TlsSecurityConnectorTest, tsi_peer peer; EXPECT_EQ(tsi_construct_peer(2, &peer), TSI_OK); EXPECT_EQ( - tsi_construct_string_peer_property(TSI_SSL_ALPN_SELECTED_PROTOCOL, "grpc", - strlen("grpc"), &peer.properties[0]), + tsi_construct_string_peer_property(TSI_SSL_ALPN_SELECTED_PROTOCOL, "h2", + strlen("h2"), &peer.properties[0]), TSI_OK); EXPECT_EQ(tsi_construct_string_peer_property_from_cstring( TSI_X509_VERIFIED_ROOT_CERT_SUBECT_PEER_PROPERTY, diff --git a/test/core/transport/chttp2/alpn_test.cc b/test/core/transport/chttp2/alpn_test.cc index b638714ff3b9e..a21ed75514f25 100644 --- a/test/core/transport/chttp2/alpn_test.cc +++ b/test/core/transport/chttp2/alpn_test.cc @@ -26,29 +26,12 @@ TEST(AlpnTest, TestAlpnSuccess) { ASSERT_TRUE(grpc_chttp2_is_alpn_version_supported("h2", 2)); - ASSERT_TRUE(grpc_chttp2_is_alpn_version_supported("grpc-exp", 8)); } TEST(AlpnTest, TestAlpnFailure) { ASSERT_FALSE(grpc_chttp2_is_alpn_version_supported("h2-155", 6)); ASSERT_FALSE(grpc_chttp2_is_alpn_version_supported("h1-15", 5)); -} - -// First index in ALPN supported version list of a given protocol. Returns a -// value one beyond the last valid element index if not found. -static size_t alpn_version_index(const char* version, size_t size) { - size_t i; - for (i = 0; i < grpc_chttp2_num_alpn_versions(); ++i) { - if (!strncmp(version, grpc_chttp2_get_alpn_version_index(i), size)) { - return i; - } - } - return i; -} - -TEST(AlpnTest, TestAlpnGrpcBeforeH2) { - // grpc-exp is preferred over h2. - ASSERT_LT(alpn_version_index("grpc-exp", 8), alpn_version_index("h2", 2)); + ASSERT_FALSE(grpc_chttp2_is_alpn_version_supported("grpc-exp", 8)); } int main(int argc, char** argv) {