Skip to content

Commit

Permalink
[alpn] Remove grpc-exp experimental ALPN protocol. (grpc#34876)
Browse files Browse the repository at this point in the history
This fixes grpc#21619. This experimental ALPN protocol has already been removed from the other gRPC stacks.

Closes grpc#34876

COPYBARA_INTEGRATE_REVIEW=grpc#34876 from matthewstevenson88:remove-grpc-exp 1cb9d08
PiperOrigin-RevId: 592080195
  • Loading branch information
matthewstevenson88 authored and copybara-github committed Dec 19, 2023
1 parent 6d35000 commit af36847
Show file tree
Hide file tree
Showing 7 changed files with 24 additions and 55 deletions.
2 changes: 1 addition & 1 deletion src/core/ext/transport/chttp2/alpn/alpn.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
15 changes: 2 additions & 13 deletions test/core/handshake/client_ssl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -153,27 +153,19 @@ static int alpn_select_cb(SSL* /*ssl*/, const uint8_t** out, uint8_t* out_len,
*out_len = static_cast<uint8_t>(
strlen(reinterpret_cast<const char*>(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<const char*>(in);
const char* in_end = inp + in_len;
while (inp < in_end) {
const size_t length = static_cast<size_t>(*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;
Expand Down Expand Up @@ -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<char*>("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<char*>("h2")));

// TODO(gtcooke94) Figure out why test is failing with OpenSSL and fix it.
Expand Down
4 changes: 2 additions & 2 deletions test/core/handshake/readahead_handshaker_server_ssl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down
9 changes: 3 additions & 6 deletions test/core/handshake/server_ssl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion test/core/security/security_connector_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
28 changes: 14 additions & 14 deletions test/core/security/tls_security_connector_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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,
Expand Down
19 changes: 1 addition & 18 deletions test/core/transport/chttp2/alpn_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down

0 comments on commit af36847

Please sign in to comment.