diff --git a/src/core/lib/security/security_connector/ssl_utils.cc b/src/core/lib/security/security_connector/ssl_utils.cc index 664f3a18e61..2465496a2d3 100644 --- a/src/core/lib/security/security_connector/ssl_utils.cc +++ b/src/core/lib/security/security_connector/ssl_utils.cc @@ -67,6 +67,9 @@ static const char* cipher_suites = nullptr; // All cipher suites for default are compliant with HTTP2. GPR_GLOBAL_CONFIG_DEFINE_STRING( grpc_ssl_cipher_suites, + "TLS_AES_128_GCM_SHA256:" + "TLS_AES_256_GCM_SHA384:" + "TLS_CHACHA20_POLY1305_SHA256:" "ECDHE-ECDSA-AES128-GCM-SHA256:" "ECDHE-ECDSA-AES256-GCM-SHA384:" "ECDHE-RSA-AES128-GCM-SHA256:" diff --git a/src/core/tsi/ssl_transport_security.cc b/src/core/tsi/ssl_transport_security.cc index dda3f3c3d4a..3e6f0bb9dd2 100644 --- a/src/core/tsi/ssl_transport_security.cc +++ b/src/core/tsi/ssl_transport_security.cc @@ -1410,6 +1410,28 @@ static void ssl_handshaker_destroy(tsi_handshaker* self) { gpr_free(impl); } +static tsi_result ssl_bytes_remaining(tsi_ssl_handshaker* impl, + unsigned char** bytes_remaining, + size_t* bytes_remaining_size) { + if (impl == nullptr || bytes_remaining == nullptr || + bytes_remaining_size == nullptr) { + return TSI_INVALID_ARGUMENT; + } + tsi_result result = TSI_OK; + size_t counter = 0; + size_t bytes_in_ssl_buffer = BIO_pending(SSL_get_rbio(impl->ssl)); + if (bytes_in_ssl_buffer == 0) return TSI_OK; + *bytes_remaining = static_cast(gpr_malloc(bytes_in_ssl_buffer)); + int read_success = 1; + while (read_success > 0 && counter < bytes_in_ssl_buffer) { + read_success = + BIO_read(SSL_get_rbio(impl->ssl), *bytes_remaining + counter, 1); + if (read_success == 1) counter += 1; + } + *bytes_remaining_size = counter; + return result; +} + static tsi_result ssl_handshaker_next( tsi_handshaker* self, const unsigned char* received_bytes, size_t received_bytes_size, const unsigned char** bytes_to_send, @@ -1450,9 +1472,10 @@ static tsi_result ssl_handshaker_next( if (ssl_handshaker_get_result(impl) == TSI_HANDSHAKE_IN_PROGRESS) { *handshaker_result = nullptr; } else { - size_t unused_bytes_size = received_bytes_size - bytes_consumed; - const unsigned char* unused_bytes = - unused_bytes_size == 0 ? nullptr : received_bytes + bytes_consumed; + unsigned char* unused_bytes = nullptr; + size_t unused_bytes_size = 0; + status = ssl_bytes_remaining(impl, &unused_bytes, &unused_bytes_size); + if (status != TSI_OK) return status; status = ssl_handshaker_result_create(impl, unused_bytes, unused_bytes_size, handshaker_result); if (status == TSI_OK) { @@ -1805,8 +1828,11 @@ tsi_result tsi_create_ssl_client_handshaker_factory_with_options( return TSI_INVALID_ARGUMENT; } -#if defined(OPENSSL_NO_TLS1_2_METHOD) || OPENSSL_API_COMPAT >= 0x10100000L +#if OPENSSL_VERSION_NUMBER >= 0x10100000 + // TODO(mattstev): Allow user to set min/max TLS version. + // https://github.com/grpc/grpc/issues/22403 ssl_context = SSL_CTX_new(TLS_method()); + SSL_CTX_set_min_proto_version(ssl_context, TLS1_2_VERSION); #else ssl_context = SSL_CTX_new(TLSv1_2_method()); #endif @@ -1969,8 +1995,11 @@ tsi_result tsi_create_ssl_server_handshaker_factory_with_options( for (i = 0; i < options->num_key_cert_pairs; i++) { do { -#if defined(OPENSSL_NO_TLS1_2_METHOD) || OPENSSL_API_COMPAT >= 0x10100000L +#if OPENSSL_VERSION_NUMBER >= 0x10100000 + // TODO(mattstev): Allow user to set min/max TLS version. + // https://github.com/grpc/grpc/issues/22403 impl->ssl_contexts[i] = SSL_CTX_new(TLS_method()); + SSL_CTX_set_min_proto_version(impl->ssl_contexts[i], TLS1_2_VERSION); #else impl->ssl_contexts[i] = SSL_CTX_new(TLSv1_2_method()); #endif diff --git a/src/csharp/Grpc.IntegrationTesting/SslCredentialsTest.cs b/src/csharp/Grpc.IntegrationTesting/SslCredentialsTest.cs index 4c1189320fa..c7ff3216ec8 100644 --- a/src/csharp/Grpc.IntegrationTesting/SslCredentialsTest.cs +++ b/src/csharp/Grpc.IntegrationTesting/SslCredentialsTest.cs @@ -246,8 +246,10 @@ namespace Grpc.IntegrationTesting private void CheckRejected() { var ex = Assert.Throws(() => client.UnaryCall(new SimpleRequest { ResponseSize = 10 })); - Assert.AreEqual(StatusCode.Unavailable, ex.Status.StatusCode); - } + if (ex.Status.StatusCode != StatusCode.Unavailable & ex.Status.StatusCode != StatusCode.Unknown) { + Assert.Fail("Expect status to be either Unavailable or Unknown"); + } + } private async Task CheckAuthContextIsPopulated() { diff --git a/src/python/grpcio_tests/tests/unit/_server_ssl_cert_config_test.py b/src/python/grpcio_tests/tests/unit/_server_ssl_cert_config_test.py index a0948600a25..6129db62742 100644 --- a/src/python/grpcio_tests/tests/unit/_server_ssl_cert_config_test.py +++ b/src/python/grpcio_tests/tests/unit/_server_ssl_cert_config_test.py @@ -161,8 +161,15 @@ class _ServerSSLCertReloadTest( else: with self.assertRaises(grpc.RpcError) as exception_context: client_stub.UnUn(request) - self.assertEqual(exception_context.exception.code(), - grpc.StatusCode.UNAVAILABLE) + # If TLS 1.2 is used, then the client receives an alert message + # before the handshake is complete, so the status is UNAVAILABLE. If + # TLS 1.3 is used, then the client receives the alert message after + # the handshake is complete, so the TSI handshaker returns the + # TSI_PROTOCOL_FAILURE result. This result does not have a + # corresponding status code, so this yields an UNKNOWN status. + self.assertTrue( + exception_context.exception.code() in [ + grpc.StatusCode.UNAVAILABLE, grpc.StatusCode.UNKNOWN]) def _do_one_shot_client_rpc(self, expect_success, diff --git a/test/core/handshake/client_ssl.cc b/test/core/handshake/client_ssl.cc index daeebee269c..3e7f5120c26 100644 --- a/test/core/handshake/client_ssl.cc +++ b/test/core/handshake/client_ssl.cc @@ -161,7 +161,9 @@ static void server_thread(void* arg) { // Set the cipher list to match the one expressed in // src/core/tsi/ssl_transport_security.c. const char* cipher_list = - "ECDHE-RSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-SHA256:ECDHE-RSA-AES256-" + "TLS_AES_128_GCM_SHA256:TLS_AES_256_GCM_SHA384:TLS_CHACHA20_POLY1305_" + "SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-SHA256:ECDHE-RSA-" + "AES256-" "SHA384:ECDHE-RSA-AES256-GCM-SHA384"; if (!SSL_CTX_set_cipher_list(ctx, cipher_list)) { ERR_print_errors_fp(stderr); diff --git a/test/core/handshake/server_ssl_common.cc b/test/core/handshake/server_ssl_common.cc index 1493094e3c3..d1bc7b1a596 100644 --- a/test/core/handshake/server_ssl_common.cc +++ b/test/core/handshake/server_ssl_common.cc @@ -203,8 +203,9 @@ bool server_ssl_test(const char* alpn_list[], unsigned int alpn_list_len, // Set the cipher list to match the one expressed in // src/core/tsi/ssl_transport_security.c. const char* cipher_list = - "ECDHE-RSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-SHA256:ECDHE-RSA-AES256-" - "SHA384:ECDHE-RSA-AES256-GCM-SHA384"; + "TLS_AES_128_GCM_SHA256:TLS_AES_256_GCM_SHA384:TLS_CHACHA20_POLY1305_" + "SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-SHA256:ECDHE-RSA-" + "AES256-SHA384:ECDHE-RSA-AES256-GCM-SHA384"; if (!SSL_CTX_set_cipher_list(ctx, cipher_list)) { ERR_print_errors_fp(stderr); gpr_log(GPR_ERROR, "Couldn't set server cipher list."); diff --git a/test/core/tsi/ssl_transport_security_test.cc b/test/core/tsi/ssl_transport_security_test.cc index 8c4e736fc8b..6d2bc5e535c 100644 --- a/test/core/tsi/ssl_transport_security_test.cc +++ b/test/core/tsi/ssl_transport_security_test.cc @@ -317,10 +317,14 @@ static void ssl_test_check_handshaker_peers(tsi_test_fixture* fixture) { GPR_ASSERT(ssl_fixture->key_cert_lib != nullptr); ssl_key_cert_lib* key_cert_lib = ssl_fixture->key_cert_lib; tsi_peer peer; - bool expect_success = + bool expect_server_success = !(key_cert_lib->use_bad_server_cert || (key_cert_lib->use_bad_client_cert && ssl_fixture->force_client_auth)); - if (expect_success) { + // In TLS 1.3, the client-side handshake succeeds even if the client sends a + // bad certificate. In such a case, the server would fail the TLS handshake + // and send an alert to the client as the first application data message. + bool expect_client_success = !key_cert_lib->use_bad_server_cert; + if (expect_client_success) { GPR_ASSERT(tsi_handshaker_result_extract_peer( ssl_fixture->base.client_result, &peer) == TSI_OK); check_session_reusage(ssl_fixture, &peer); @@ -338,7 +342,7 @@ static void ssl_test_check_handshaker_peers(tsi_test_fixture* fixture) { } else { GPR_ASSERT(ssl_fixture->base.client_result == nullptr); } - if (expect_success) { + if (expect_server_success) { GPR_ASSERT(tsi_handshaker_result_extract_peer( ssl_fixture->base.server_result, &peer) == TSI_OK); check_session_reusage(ssl_fixture, &peer);