From 274840e4d304b094d1d4711d4b9572866221a4ed Mon Sep 17 00:00:00 2001 From: jiangtaoli2016 Date: Wed, 9 May 2018 14:42:48 -0700 Subject: [PATCH] Migrate SSL_transport_security TSI to new TSI handshaker API --- BUILD | 2 - CMakeLists.txt | 2 - Makefile | 3 - build.yaml | 2 - config.m4 | 1 - config.w32 | 1 - gRPC-C++.podspec | 1 - gRPC-Core.podspec | 3 - grpc.gemspec | 2 - grpc.gyp | 1 - package.xml | 2 - .../lib/http/httpcli_security_connector.cc | 4 +- .../security_connector/security_connector.cc | 7 +- src/core/tsi/ssl_transport_security.cc | 314 ++++++++++++------ src/core/tsi/transport_security_adapter.cc | 242 -------------- src/core/tsi/transport_security_adapter.h | 41 --- src/core/tsi/transport_security_interface.h | 2 + src/python/grpcio/grpc_core_dependencies.py | 1 - test/core/tsi/ssl_transport_security_test.cc | 13 +- tools/doxygen/Doxyfile.core.internal | 2 - .../generated/sources_and_headers.json | 3 - 21 files changed, 225 insertions(+), 424 deletions(-) delete mode 100644 src/core/tsi/transport_security_adapter.cc delete mode 100644 src/core/tsi/transport_security_adapter.h diff --git a/BUILD b/BUILD index 80b40338c34..c35a7d65e1c 100644 --- a/BUILD +++ b/BUILD @@ -1625,11 +1625,9 @@ grpc_cc_library( name = "tsi_interface", srcs = [ "src/core/tsi/transport_security.cc", - "src/core/tsi/transport_security_adapter.cc", ], hdrs = [ "src/core/tsi/transport_security.h", - "src/core/tsi/transport_security_adapter.h", "src/core/tsi/transport_security_interface.h", ], language = "c++", diff --git a/CMakeLists.txt b/CMakeLists.txt index d99a71e0783..190e2dd465b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1150,7 +1150,6 @@ add_library(grpc third_party/nanopb/pb_decode.c third_party/nanopb/pb_encode.c src/core/tsi/transport_security.cc - src/core/tsi/transport_security_adapter.cc src/core/ext/transport/chttp2/client/insecure/channel_create.cc src/core/ext/transport/chttp2/client/insecure/channel_create_posix.cc src/core/ext/transport/chttp2/client/authority.cc @@ -1569,7 +1568,6 @@ add_library(grpc_cronet third_party/nanopb/pb_decode.c third_party/nanopb/pb_encode.c src/core/tsi/transport_security.cc - src/core/tsi/transport_security_adapter.cc src/core/ext/transport/chttp2/client/insecure/channel_create.cc src/core/ext/transport/chttp2/client/insecure/channel_create_posix.cc src/core/ext/transport/chttp2/client/authority.cc diff --git a/Makefile b/Makefile index 3225478720e..ba765f0af6e 100644 --- a/Makefile +++ b/Makefile @@ -3540,7 +3540,6 @@ LIBGRPC_SRC = \ third_party/nanopb/pb_decode.c \ third_party/nanopb/pb_encode.c \ src/core/tsi/transport_security.cc \ - src/core/tsi/transport_security_adapter.cc \ src/core/ext/transport/chttp2/client/insecure/channel_create.cc \ src/core/ext/transport/chttp2/client/insecure/channel_create_posix.cc \ src/core/ext/transport/chttp2/client/authority.cc \ @@ -3959,7 +3958,6 @@ LIBGRPC_CRONET_SRC = \ third_party/nanopb/pb_decode.c \ third_party/nanopb/pb_encode.c \ src/core/tsi/transport_security.cc \ - src/core/tsi/transport_security_adapter.cc \ src/core/ext/transport/chttp2/client/insecure/channel_create.cc \ src/core/ext/transport/chttp2/client/insecure/channel_create_posix.cc \ src/core/ext/transport/chttp2/client/authority.cc \ @@ -24107,7 +24105,6 @@ src/core/tsi/ssl/session_cache/ssl_session_cache.cc: $(OPENSSL_DEP) src/core/tsi/ssl/session_cache/ssl_session_openssl.cc: $(OPENSSL_DEP) src/core/tsi/ssl_transport_security.cc: $(OPENSSL_DEP) src/core/tsi/transport_security.cc: $(OPENSSL_DEP) -src/core/tsi/transport_security_adapter.cc: $(OPENSSL_DEP) src/core/tsi/transport_security_grpc.cc: $(OPENSSL_DEP) src/cpp/client/cronet_credentials.cc: $(OPENSSL_DEP) src/cpp/client/secure_credentials.cc: $(OPENSSL_DEP) diff --git a/build.yaml b/build.yaml index 930236eb7dd..1bc2adbb8c7 100644 --- a/build.yaml +++ b/build.yaml @@ -1080,11 +1080,9 @@ filegroups: - name: tsi_interface headers: - src/core/tsi/transport_security.h - - src/core/tsi/transport_security_adapter.h - src/core/tsi/transport_security_interface.h src: - src/core/tsi/transport_security.cc - - src/core/tsi/transport_security_adapter.cc deps: - gpr secure: true diff --git a/config.m4 b/config.m4 index df06259606a..ee2aca4fa53 100644 --- a/config.m4 +++ b/config.m4 @@ -321,7 +321,6 @@ if test "$PHP_GRPC" != "no"; then third_party/nanopb/pb_decode.c \ third_party/nanopb/pb_encode.c \ src/core/tsi/transport_security.cc \ - src/core/tsi/transport_security_adapter.cc \ src/core/ext/transport/chttp2/client/insecure/channel_create.cc \ src/core/ext/transport/chttp2/client/insecure/channel_create_posix.cc \ src/core/ext/transport/chttp2/client/authority.cc \ diff --git a/config.w32 b/config.w32 index f60a5b746d7..abca8e22f26 100644 --- a/config.w32 +++ b/config.w32 @@ -297,7 +297,6 @@ if (PHP_GRPC != "no") { "third_party\\nanopb\\pb_decode.c " + "third_party\\nanopb\\pb_encode.c " + "src\\core\\tsi\\transport_security.cc " + - "src\\core\\tsi\\transport_security_adapter.cc " + "src\\core\\ext\\transport\\chttp2\\client\\insecure\\channel_create.cc " + "src\\core\\ext\\transport\\chttp2\\client\\insecure\\channel_create_posix.cc " + "src\\core\\ext\\transport\\chttp2\\client\\authority.cc " + diff --git a/gRPC-C++.podspec b/gRPC-C++.podspec index fe082ef3af8..29b79e0b014 100644 --- a/gRPC-C++.podspec +++ b/gRPC-C++.podspec @@ -308,7 +308,6 @@ Pod::Spec.new do |s| 'src/core/tsi/alts/handshaker/handshaker.pb.h', 'src/core/tsi/alts/handshaker/transport_security_common.pb.h', 'src/core/tsi/transport_security.h', - 'src/core/tsi/transport_security_adapter.h', 'src/core/tsi/transport_security_interface.h', 'src/core/ext/transport/chttp2/client/authority.h', 'src/core/ext/transport/chttp2/client/chttp2_connector.h', diff --git a/gRPC-Core.podspec b/gRPC-Core.podspec index f6374ebfcd5..ce6ff76806a 100644 --- a/gRPC-Core.podspec +++ b/gRPC-Core.podspec @@ -318,7 +318,6 @@ Pod::Spec.new do |s| 'src/core/tsi/alts/handshaker/handshaker.pb.h', 'src/core/tsi/alts/handshaker/transport_security_common.pb.h', 'src/core/tsi/transport_security.h', - 'src/core/tsi/transport_security_adapter.h', 'src/core/tsi/transport_security_interface.h', 'src/core/ext/transport/chttp2/client/authority.h', 'src/core/ext/transport/chttp2/client/chttp2_connector.h', @@ -737,7 +736,6 @@ Pod::Spec.new do |s| 'src/core/tsi/alts/handshaker/handshaker.pb.c', 'src/core/tsi/alts/handshaker/transport_security_common.pb.c', 'src/core/tsi/transport_security.cc', - 'src/core/tsi/transport_security_adapter.cc', 'src/core/ext/transport/chttp2/client/insecure/channel_create.cc', 'src/core/ext/transport/chttp2/client/insecure/channel_create_posix.cc', 'src/core/ext/transport/chttp2/client/authority.cc', @@ -898,7 +896,6 @@ Pod::Spec.new do |s| 'src/core/tsi/alts/handshaker/handshaker.pb.h', 'src/core/tsi/alts/handshaker/transport_security_common.pb.h', 'src/core/tsi/transport_security.h', - 'src/core/tsi/transport_security_adapter.h', 'src/core/tsi/transport_security_interface.h', 'src/core/ext/transport/chttp2/client/authority.h', 'src/core/ext/transport/chttp2/client/chttp2_connector.h', diff --git a/grpc.gemspec b/grpc.gemspec index bb40a3ba027..2a66801e343 100644 --- a/grpc.gemspec +++ b/grpc.gemspec @@ -253,7 +253,6 @@ Gem::Specification.new do |s| s.files += %w( third_party/nanopb/pb_decode.h ) s.files += %w( third_party/nanopb/pb_encode.h ) s.files += %w( src/core/tsi/transport_security.h ) - s.files += %w( src/core/tsi/transport_security_adapter.h ) s.files += %w( src/core/tsi/transport_security_interface.h ) s.files += %w( src/core/ext/transport/chttp2/client/authority.h ) s.files += %w( src/core/ext/transport/chttp2/client/chttp2_connector.h ) @@ -675,7 +674,6 @@ Gem::Specification.new do |s| s.files += %w( third_party/nanopb/pb_decode.c ) s.files += %w( third_party/nanopb/pb_encode.c ) s.files += %w( src/core/tsi/transport_security.cc ) - s.files += %w( src/core/tsi/transport_security_adapter.cc ) s.files += %w( src/core/ext/transport/chttp2/client/insecure/channel_create.cc ) s.files += %w( src/core/ext/transport/chttp2/client/insecure/channel_create_posix.cc ) s.files += %w( src/core/ext/transport/chttp2/client/authority.cc ) diff --git a/grpc.gyp b/grpc.gyp index fff7c5380ad..5726719521e 100644 --- a/grpc.gyp +++ b/grpc.gyp @@ -481,7 +481,6 @@ 'third_party/nanopb/pb_decode.c', 'third_party/nanopb/pb_encode.c', 'src/core/tsi/transport_security.cc', - 'src/core/tsi/transport_security_adapter.cc', 'src/core/ext/transport/chttp2/client/insecure/channel_create.cc', 'src/core/ext/transport/chttp2/client/insecure/channel_create_posix.cc', 'src/core/ext/transport/chttp2/client/authority.cc', diff --git a/package.xml b/package.xml index 75a69931a49..ac9b520fb71 100644 --- a/package.xml +++ b/package.xml @@ -260,7 +260,6 @@ - @@ -682,7 +681,6 @@ - diff --git a/src/core/lib/http/httpcli_security_connector.cc b/src/core/lib/http/httpcli_security_connector.cc index 0b53d63e773..50078c37a17 100644 --- a/src/core/lib/http/httpcli_security_connector.cc +++ b/src/core/lib/http/httpcli_security_connector.cc @@ -32,7 +32,6 @@ #include "src/core/lib/security/transport/security_handshaker.h" #include "src/core/lib/slice/slice_internal.h" #include "src/core/tsi/ssl_transport_security.h" -#include "src/core/tsi/transport_security_adapter.h" typedef struct { grpc_channel_security_connector base; @@ -65,8 +64,7 @@ static void httpcli_ssl_add_handshakers(grpc_channel_security_connector* sc, } } grpc_handshake_manager_add( - handshake_mgr, grpc_security_handshaker_create( - tsi_create_adapter_handshaker(handshaker), &sc->base)); + handshake_mgr, grpc_security_handshaker_create(handshaker, &sc->base)); } static void httpcli_ssl_check_peer(grpc_security_connector* sc, tsi_peer peer, diff --git a/src/core/lib/security/security_connector/security_connector.cc b/src/core/lib/security/security_connector/security_connector.cc index 6eae30a6e5d..a30696703fc 100644 --- a/src/core/lib/security/security_connector/security_connector.cc +++ b/src/core/lib/security/security_connector/security_connector.cc @@ -44,7 +44,6 @@ #include "src/core/lib/security/transport/target_authority_table.h" #include "src/core/tsi/fake_transport_security.h" #include "src/core/tsi/ssl_transport_security.h" -#include "src/core/tsi/transport_security_adapter.h" grpc_core::DebugOnlyTraceFlag grpc_trace_security_connector_refcount( false, "security_connector_refcount"); @@ -673,8 +672,7 @@ static void ssl_channel_add_handshakers(grpc_channel_security_connector* sc, } // Create handshakers. grpc_handshake_manager_add( - handshake_mgr, grpc_security_handshaker_create( - tsi_create_adapter_handshaker(tsi_hs), &sc->base)); + handshake_mgr, grpc_security_handshaker_create(tsi_hs, &sc->base)); } static const char** fill_alpn_protocol_strings(size_t* num_alpn_protocols) { @@ -782,8 +780,7 @@ static void ssl_server_add_handshakers(grpc_server_security_connector* sc, } // Create handshakers. grpc_handshake_manager_add( - handshake_mgr, grpc_security_handshaker_create( - tsi_create_adapter_handshaker(tsi_hs), &sc->base)); + handshake_mgr, grpc_security_handshaker_create(tsi_hs, &sc->base)); } int grpc_ssl_host_matches_name(const tsi_peer* peer, const char* peer_name) { diff --git a/src/core/tsi/ssl_transport_security.cc b/src/core/tsi/ssl_transport_security.cc index 8d0729ba05d..8065a8b1856 100644 --- a/src/core/tsi/ssl_transport_security.cc +++ b/src/core/tsi/ssl_transport_security.cc @@ -57,6 +57,7 @@ extern "C" { #define TSI_SSL_MAX_PROTECTED_FRAME_SIZE_UPPER_BOUND 16384 #define TSI_SSL_MAX_PROTECTED_FRAME_SIZE_LOWER_BOUND 1024 +#define TSI_SSL_HANDSHAKER_OUTGOING_BUFFER_INITIAL_SIZE 1024 /* Putting a macro like this and littering the source file with #if is really bad practice. @@ -105,9 +106,19 @@ typedef struct { SSL* ssl; BIO* network_io; tsi_result result; + unsigned char* outgoing_bytes_buffer; + size_t outgoing_bytes_buffer_size; tsi_ssl_handshaker_factory* factory_ref; } tsi_ssl_handshaker; +typedef struct { + tsi_handshaker_result base; + SSL* ssl; + BIO* network_io; + unsigned char* unused_bytes; + size_t unused_bytes_size; +} tsi_ssl_handshaker_result; + typedef struct { tsi_frame_protector base; SSL* ssl; @@ -994,94 +1005,15 @@ static void tsi_ssl_handshaker_factory_init( gpr_ref_init(&factory->refcount, 1); } -/* --- tsi_handshaker methods implementation. ---*/ - -static tsi_result ssl_handshaker_get_bytes_to_send_to_peer(tsi_handshaker* self, - unsigned char* bytes, - size_t* bytes_size) { - tsi_ssl_handshaker* impl = reinterpret_cast(self); - int bytes_read_from_ssl = 0; - if (bytes == nullptr || bytes_size == nullptr || *bytes_size == 0 || - *bytes_size > INT_MAX) { - return TSI_INVALID_ARGUMENT; - } - GPR_ASSERT(*bytes_size <= INT_MAX); - bytes_read_from_ssl = - BIO_read(impl->network_io, bytes, static_cast(*bytes_size)); - if (bytes_read_from_ssl < 0) { - *bytes_size = 0; - if (!BIO_should_retry(impl->network_io)) { - impl->result = TSI_INTERNAL_ERROR; - return impl->result; - } else { - return TSI_OK; - } - } - *bytes_size = static_cast(bytes_read_from_ssl); - return BIO_pending(impl->network_io) == 0 ? TSI_OK : TSI_INCOMPLETE_DATA; -} - -static tsi_result ssl_handshaker_get_result(tsi_handshaker* self) { - tsi_ssl_handshaker* impl = reinterpret_cast(self); - if ((impl->result == TSI_HANDSHAKE_IN_PROGRESS) && - SSL_is_init_finished(impl->ssl)) { - impl->result = TSI_OK; - } - return impl->result; -} - -static tsi_result ssl_handshaker_process_bytes_from_peer( - tsi_handshaker* self, const unsigned char* bytes, size_t* bytes_size) { - tsi_ssl_handshaker* impl = reinterpret_cast(self); - int bytes_written_into_ssl_size = 0; - if (bytes == nullptr || bytes_size == nullptr || *bytes_size > INT_MAX) { - return TSI_INVALID_ARGUMENT; - } - GPR_ASSERT(*bytes_size <= INT_MAX); - bytes_written_into_ssl_size = - BIO_write(impl->network_io, bytes, static_cast(*bytes_size)); - if (bytes_written_into_ssl_size < 0) { - gpr_log(GPR_ERROR, "Could not write to memory BIO."); - impl->result = TSI_INTERNAL_ERROR; - return impl->result; - } - *bytes_size = static_cast(bytes_written_into_ssl_size); - - if (!tsi_handshaker_is_in_progress(self)) { - impl->result = TSI_OK; - return impl->result; - } else { - /* Get ready to get some bytes from SSL. */ - int ssl_result = SSL_do_handshake(impl->ssl); - ssl_result = SSL_get_error(impl->ssl, ssl_result); - switch (ssl_result) { - case SSL_ERROR_WANT_READ: - if (BIO_pending(impl->network_io) == 0) { - /* We need more data. */ - return TSI_INCOMPLETE_DATA; - } else { - return TSI_OK; - } - case SSL_ERROR_NONE: - return TSI_OK; - default: { - char err_str[256]; - ERR_error_string_n(ERR_get_error(), err_str, sizeof(err_str)); - gpr_log(GPR_ERROR, "Handshake failed with fatal error %s: %s.", - ssl_error_string(ssl_result), err_str); - impl->result = TSI_PROTOCOL_FAILURE; - return impl->result; - } - } - } -} +/* --- tsi_handshaker_result methods implementation. ---*/ -static tsi_result ssl_handshaker_extract_peer(tsi_handshaker* self, - tsi_peer* peer) { +static tsi_result ssl_handshaker_result_extract_peer( + const tsi_handshaker_result* self, tsi_peer* peer) { tsi_result result = TSI_OK; const unsigned char* alpn_selected = nullptr; unsigned int alpn_selected_len; - tsi_ssl_handshaker* impl = reinterpret_cast(self); + const tsi_ssl_handshaker_result* impl = + reinterpret_cast(self); X509* peer_cert = SSL_get_peer_certificate(impl->ssl); if (peer_cert != nullptr) { result = peer_from_x509(peer_cert, 1, peer); @@ -1127,12 +1059,14 @@ static tsi_result ssl_handshaker_extract_peer(tsi_handshaker* self, return result; } -static tsi_result ssl_handshaker_create_frame_protector( - tsi_handshaker* self, size_t* max_output_protected_frame_size, +static tsi_result ssl_handshaker_result_create_frame_protector( + const tsi_handshaker_result* self, size_t* max_output_protected_frame_size, tsi_frame_protector** protector) { size_t actual_max_output_protected_frame_size = TSI_SSL_MAX_PROTECTED_FRAME_SIZE_UPPER_BOUND; - tsi_ssl_handshaker* impl = reinterpret_cast(self); + tsi_ssl_handshaker_result* impl = + reinterpret_cast( + const_cast(self)); tsi_ssl_frame_protector* protector_impl = static_cast( gpr_zalloc(sizeof(*protector_impl))); @@ -1160,35 +1094,217 @@ static tsi_result ssl_handshaker_create_frame_protector( return TSI_INTERNAL_ERROR; } - /* Transfer ownership of ssl and network_io to the frame protector. It is OK - * as the caller cannot call anything else but destroy on the handshaker - * after this call. */ + /* Transfer ownership of ssl and network_io to the frame protector. */ protector_impl->ssl = impl->ssl; impl->ssl = nullptr; protector_impl->network_io = impl->network_io; impl->network_io = nullptr; - protector_impl->base.vtable = &frame_protector_vtable; *protector = &protector_impl->base; return TSI_OK; } +static tsi_result ssl_handshaker_result_get_unused_bytes( + const tsi_handshaker_result* self, const unsigned char** bytes, + size_t* bytes_size) { + const tsi_ssl_handshaker_result* impl = + reinterpret_cast(self); + *bytes_size = impl->unused_bytes_size; + *bytes = impl->unused_bytes; + return TSI_OK; +} + +static void ssl_handshaker_result_destroy(tsi_handshaker_result* self) { + tsi_ssl_handshaker_result* impl = + reinterpret_cast(self); + SSL_free(impl->ssl); + BIO_free(impl->network_io); + gpr_free(impl->unused_bytes); + gpr_free(impl); +} + +static const tsi_handshaker_result_vtable handshaker_result_vtable = { + ssl_handshaker_result_extract_peer, + nullptr, /* create_zero_copy_grpc_protector */ + ssl_handshaker_result_create_frame_protector, + ssl_handshaker_result_get_unused_bytes, + ssl_handshaker_result_destroy, +}; + +static tsi_result ssl_handshaker_result_create( + tsi_ssl_handshaker* handshaker, const unsigned char* unused_bytes, + size_t unused_bytes_size, tsi_handshaker_result** handshaker_result) { + if (handshaker == nullptr || handshaker_result == nullptr || + (unused_bytes_size > 0 && unused_bytes == nullptr)) { + return TSI_INVALID_ARGUMENT; + } + tsi_ssl_handshaker_result* result = + static_cast(gpr_zalloc(sizeof(*result))); + result->base.vtable = &handshaker_result_vtable; + /* Transfer ownership of ssl and network_io to the handshaker result. */ + result->ssl = handshaker->ssl; + handshaker->ssl = nullptr; + result->network_io = handshaker->network_io; + handshaker->network_io = nullptr; + if (unused_bytes_size > 0) { + result->unused_bytes = + static_cast(gpr_malloc(unused_bytes_size)); + memcpy(result->unused_bytes, unused_bytes, unused_bytes_size); + } + result->unused_bytes_size = unused_bytes_size; + *handshaker_result = &result->base; + return TSI_OK; +} + +/* --- tsi_handshaker methods implementation. ---*/ + +static tsi_result ssl_handshaker_get_bytes_to_send_to_peer( + tsi_ssl_handshaker* impl, unsigned char* bytes, size_t* bytes_size) { + int bytes_read_from_ssl = 0; + if (bytes == nullptr || bytes_size == nullptr || *bytes_size == 0 || + *bytes_size > INT_MAX) { + return TSI_INVALID_ARGUMENT; + } + GPR_ASSERT(*bytes_size <= INT_MAX); + bytes_read_from_ssl = + BIO_read(impl->network_io, bytes, static_cast(*bytes_size)); + if (bytes_read_from_ssl < 0) { + *bytes_size = 0; + if (!BIO_should_retry(impl->network_io)) { + impl->result = TSI_INTERNAL_ERROR; + return impl->result; + } else { + return TSI_OK; + } + } + *bytes_size = static_cast(bytes_read_from_ssl); + return BIO_pending(impl->network_io) == 0 ? TSI_OK : TSI_INCOMPLETE_DATA; +} + +static tsi_result ssl_handshaker_get_result(tsi_ssl_handshaker* impl) { + if ((impl->result == TSI_HANDSHAKE_IN_PROGRESS) && + SSL_is_init_finished(impl->ssl)) { + impl->result = TSI_OK; + } + return impl->result; +} + +static tsi_result ssl_handshaker_process_bytes_from_peer( + tsi_ssl_handshaker* impl, const unsigned char* bytes, size_t* bytes_size) { + int bytes_written_into_ssl_size = 0; + if (bytes == nullptr || bytes_size == nullptr || *bytes_size > INT_MAX) { + return TSI_INVALID_ARGUMENT; + } + GPR_ASSERT(*bytes_size <= INT_MAX); + bytes_written_into_ssl_size = + BIO_write(impl->network_io, bytes, static_cast(*bytes_size)); + if (bytes_written_into_ssl_size < 0) { + gpr_log(GPR_ERROR, "Could not write to memory BIO."); + impl->result = TSI_INTERNAL_ERROR; + return impl->result; + } + *bytes_size = static_cast(bytes_written_into_ssl_size); + + if (ssl_handshaker_get_result(impl) != TSI_HANDSHAKE_IN_PROGRESS) { + impl->result = TSI_OK; + return impl->result; + } else { + /* Get ready to get some bytes from SSL. */ + int ssl_result = SSL_do_handshake(impl->ssl); + ssl_result = SSL_get_error(impl->ssl, ssl_result); + switch (ssl_result) { + case SSL_ERROR_WANT_READ: + if (BIO_pending(impl->network_io) == 0) { + /* We need more data. */ + return TSI_INCOMPLETE_DATA; + } else { + return TSI_OK; + } + case SSL_ERROR_NONE: + return TSI_OK; + default: { + char err_str[256]; + ERR_error_string_n(ERR_get_error(), err_str, sizeof(err_str)); + gpr_log(GPR_ERROR, "Handshake failed with fatal error %s: %s.", + ssl_error_string(ssl_result), err_str); + impl->result = TSI_PROTOCOL_FAILURE; + return impl->result; + } + } + } +} + static void ssl_handshaker_destroy(tsi_handshaker* self) { tsi_ssl_handshaker* impl = reinterpret_cast(self); SSL_free(impl->ssl); BIO_free(impl->network_io); + gpr_free(impl->outgoing_bytes_buffer); tsi_ssl_handshaker_factory_unref(impl->factory_ref); gpr_free(impl); } +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, + size_t* bytes_to_send_size, tsi_handshaker_result** handshaker_result, + tsi_handshaker_on_next_done_cb cb, void* user_data) { + /* Input sanity check. */ + if ((received_bytes_size > 0 && received_bytes == nullptr) || + bytes_to_send == nullptr || bytes_to_send_size == nullptr || + handshaker_result == nullptr) { + return TSI_INVALID_ARGUMENT; + } + /* If there are received bytes, process them first. */ + tsi_ssl_handshaker* impl = reinterpret_cast(self); + tsi_result status = TSI_OK; + size_t bytes_consumed = received_bytes_size; + if (received_bytes_size > 0) { + status = ssl_handshaker_process_bytes_from_peer(impl, received_bytes, + &bytes_consumed); + if (status != TSI_OK) return status; + } + /* Get bytes to send to the peer, if available. */ + size_t offset = 0; + do { + size_t to_send_size = impl->outgoing_bytes_buffer_size - offset; + status = ssl_handshaker_get_bytes_to_send_to_peer( + impl, impl->outgoing_bytes_buffer + offset, &to_send_size); + offset += to_send_size; + if (status == TSI_INCOMPLETE_DATA) { + impl->outgoing_bytes_buffer_size *= 2; + impl->outgoing_bytes_buffer = static_cast(gpr_realloc( + impl->outgoing_bytes_buffer, impl->outgoing_bytes_buffer_size)); + } + } while (status == TSI_INCOMPLETE_DATA); + if (status != TSI_OK) return status; + *bytes_to_send = impl->outgoing_bytes_buffer; + *bytes_to_send_size = offset; + /* If handshake completes, create tsi_handshaker_result. */ + 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; + status = ssl_handshaker_result_create(impl, unused_bytes, unused_bytes_size, + handshaker_result); + if (status == TSI_OK) { + /* Indicates that the handshake has completed and that a handshaker_result + * has been created. */ + self->handshaker_result_created = true; + } + } + return status; +} + static const tsi_handshaker_vtable handshaker_vtable = { - ssl_handshaker_get_bytes_to_send_to_peer, - ssl_handshaker_process_bytes_from_peer, - ssl_handshaker_get_result, - ssl_handshaker_extract_peer, - ssl_handshaker_create_frame_protector, + nullptr, /* get_bytes_to_send_to_peer -- deprecated */ + nullptr, /* process_bytes_from_peer -- deprecated */ + nullptr, /* get_result -- deprecated */ + nullptr, /* extract_peer -- deprecated */ + nullptr, /* create_frame_protector -- deprecated */ ssl_handshaker_destroy, - nullptr, + ssl_handshaker_next, nullptr, /* shutdown */ }; @@ -1267,6 +1383,10 @@ static tsi_result create_tsi_ssl_handshaker(SSL_CTX* ctx, int is_client, impl->ssl = ssl; impl->network_io = network_io; impl->result = TSI_HANDSHAKE_IN_PROGRESS; + impl->outgoing_bytes_buffer_size = + TSI_SSL_HANDSHAKER_OUTGOING_BUFFER_INITIAL_SIZE; + impl->outgoing_bytes_buffer = + static_cast(gpr_zalloc(impl->outgoing_bytes_buffer_size)); impl->base.vtable = &handshaker_vtable; impl->factory_ref = tsi_ssl_handshaker_factory_ref(factory); diff --git a/src/core/tsi/transport_security_adapter.cc b/src/core/tsi/transport_security_adapter.cc deleted file mode 100644 index 642188e6197..00000000000 --- a/src/core/tsi/transport_security_adapter.cc +++ /dev/null @@ -1,242 +0,0 @@ -/* - * - * Copyright 2017 gRPC authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - * - */ - -#include - -#include "src/core/tsi/transport_security_adapter.h" - -#include - -#include -#include -#include "src/core/tsi/transport_security.h" - -#define TSI_ADAPTER_INITIAL_BUFFER_SIZE 256 - -/* --- tsi_adapter_handshaker_result implementation ---*/ - -typedef struct { - tsi_handshaker_result base; - tsi_handshaker* wrapped; - unsigned char* unused_bytes; - size_t unused_bytes_size; -} tsi_adapter_handshaker_result; - -static tsi_result adapter_result_extract_peer(const tsi_handshaker_result* self, - tsi_peer* peer) { - tsi_adapter_handshaker_result* impl = (tsi_adapter_handshaker_result*)self; - return tsi_handshaker_extract_peer(impl->wrapped, peer); -} - -static tsi_result adapter_result_create_frame_protector( - const tsi_handshaker_result* self, size_t* max_output_protected_frame_size, - tsi_frame_protector** protector) { - tsi_adapter_handshaker_result* impl = (tsi_adapter_handshaker_result*)self; - return tsi_handshaker_create_frame_protector( - impl->wrapped, max_output_protected_frame_size, protector); -} - -static tsi_result adapter_result_get_unused_bytes( - const tsi_handshaker_result* self, const unsigned char** bytes, - size_t* byte_size) { - tsi_adapter_handshaker_result* impl = (tsi_adapter_handshaker_result*)self; - *bytes = impl->unused_bytes; - *byte_size = impl->unused_bytes_size; - return TSI_OK; -} - -static void adapter_result_destroy(tsi_handshaker_result* self) { - tsi_adapter_handshaker_result* impl = - reinterpret_cast(self); - tsi_handshaker_destroy(impl->wrapped); - gpr_free(impl->unused_bytes); - gpr_free(self); -} - -static const tsi_handshaker_result_vtable result_vtable = { - adapter_result_extract_peer, - nullptr, /* create_zero_copy_grpc_protector */ - adapter_result_create_frame_protector, - adapter_result_get_unused_bytes, - adapter_result_destroy, -}; - -/* Ownership of wrapped tsi_handshaker is transferred to the result object. */ -static tsi_result tsi_adapter_create_handshaker_result( - tsi_handshaker* wrapped, const unsigned char* unused_bytes, - size_t unused_bytes_size, tsi_handshaker_result** handshaker_result) { - if (wrapped == nullptr || - (unused_bytes_size > 0 && unused_bytes == nullptr)) { - return TSI_INVALID_ARGUMENT; - } - tsi_adapter_handshaker_result* impl = - static_cast(gpr_zalloc(sizeof(*impl))); - impl->base.vtable = &result_vtable; - impl->wrapped = wrapped; - impl->unused_bytes_size = unused_bytes_size; - if (unused_bytes_size > 0) { - impl->unused_bytes = - static_cast(gpr_malloc(unused_bytes_size)); - memcpy(impl->unused_bytes, unused_bytes, unused_bytes_size); - } else { - impl->unused_bytes = nullptr; - } - *handshaker_result = &impl->base; - return TSI_OK; -} - -/* --- tsi_adapter_handshaker implementation ---*/ - -typedef struct { - tsi_handshaker base; - tsi_handshaker* wrapped; - unsigned char* adapter_buffer; - size_t adapter_buffer_size; -} tsi_adapter_handshaker; - -static tsi_result adapter_get_bytes_to_send_to_peer(tsi_handshaker* self, - unsigned char* bytes, - size_t* bytes_size) { - return tsi_handshaker_get_bytes_to_send_to_peer( - tsi_adapter_handshaker_get_wrapped(self), bytes, bytes_size); -} - -static tsi_result adapter_process_bytes_from_peer(tsi_handshaker* self, - const unsigned char* bytes, - size_t* bytes_size) { - return tsi_handshaker_process_bytes_from_peer( - tsi_adapter_handshaker_get_wrapped(self), bytes, bytes_size); -} - -static tsi_result adapter_get_result(tsi_handshaker* self) { - return tsi_handshaker_get_result(tsi_adapter_handshaker_get_wrapped(self)); -} - -static tsi_result adapter_extract_peer(tsi_handshaker* self, tsi_peer* peer) { - return tsi_handshaker_extract_peer(tsi_adapter_handshaker_get_wrapped(self), - peer); -} - -static tsi_result adapter_create_frame_protector( - tsi_handshaker* self, size_t* max_protected_frame_size, - tsi_frame_protector** protector) { - return tsi_handshaker_create_frame_protector( - tsi_adapter_handshaker_get_wrapped(self), max_protected_frame_size, - protector); -} - -static void adapter_destroy(tsi_handshaker* self) { - tsi_adapter_handshaker* impl = - reinterpret_cast(self); - tsi_handshaker_destroy(impl->wrapped); - gpr_free(impl->adapter_buffer); - gpr_free(self); -} - -static void adapter_shutdown(tsi_handshaker* self) { - tsi_adapter_handshaker* impl = - reinterpret_cast(self); - tsi_handshaker_shutdown(impl->wrapped); -} - -static tsi_result adapter_next( - tsi_handshaker* self, const unsigned char* received_bytes, - size_t received_bytes_size, const unsigned char** bytes_to_send, - size_t* bytes_to_send_size, tsi_handshaker_result** handshaker_result, - tsi_handshaker_on_next_done_cb cb, void* user_data) { - /* Input sanity check. */ - if ((received_bytes_size > 0 && received_bytes == nullptr) || - bytes_to_send == nullptr || bytes_to_send_size == nullptr || - handshaker_result == nullptr) { - return TSI_INVALID_ARGUMENT; - } - - /* If there are received bytes, process them first. */ - tsi_adapter_handshaker* impl = - reinterpret_cast(self); - tsi_result status = TSI_OK; - size_t bytes_consumed = received_bytes_size; - if (received_bytes_size > 0) { - status = tsi_handshaker_process_bytes_from_peer( - impl->wrapped, received_bytes, &bytes_consumed); - if (status != TSI_OK) return status; - } - - /* Get bytes to send to the peer, if available. */ - size_t offset = 0; - do { - size_t to_send_size = impl->adapter_buffer_size - offset; - status = tsi_handshaker_get_bytes_to_send_to_peer( - impl->wrapped, impl->adapter_buffer + offset, &to_send_size); - offset += to_send_size; - if (status == TSI_INCOMPLETE_DATA) { - impl->adapter_buffer_size *= 2; - impl->adapter_buffer = static_cast( - gpr_realloc(impl->adapter_buffer, impl->adapter_buffer_size)); - } - } while (status == TSI_INCOMPLETE_DATA); - if (status != TSI_OK) return status; - *bytes_to_send = impl->adapter_buffer; - *bytes_to_send_size = offset; - - /* If handshake completes, create tsi_handshaker_result. */ - if (tsi_handshaker_is_in_progress(impl->wrapped)) { - *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; - status = tsi_adapter_create_handshaker_result( - impl->wrapped, unused_bytes, unused_bytes_size, handshaker_result); - if (status == TSI_OK) { - impl->base.handshaker_result_created = true; - impl->wrapped = nullptr; - } - } - return status; -} - -static const tsi_handshaker_vtable handshaker_vtable = { - adapter_get_bytes_to_send_to_peer, - adapter_process_bytes_from_peer, - adapter_get_result, - adapter_extract_peer, - adapter_create_frame_protector, - adapter_destroy, - adapter_next, - adapter_shutdown, -}; - -tsi_handshaker* tsi_create_adapter_handshaker(tsi_handshaker* wrapped) { - GPR_ASSERT(wrapped != nullptr); - tsi_adapter_handshaker* impl = - static_cast(gpr_zalloc(sizeof(*impl))); - impl->base.vtable = &handshaker_vtable; - impl->wrapped = wrapped; - impl->adapter_buffer_size = TSI_ADAPTER_INITIAL_BUFFER_SIZE; - impl->adapter_buffer = - static_cast(gpr_malloc(impl->adapter_buffer_size)); - return &impl->base; -} - -tsi_handshaker* tsi_adapter_handshaker_get_wrapped(tsi_handshaker* adapter) { - if (adapter == nullptr) return nullptr; - tsi_adapter_handshaker* impl = - reinterpret_cast(adapter); - return impl->wrapped; -} diff --git a/src/core/tsi/transport_security_adapter.h b/src/core/tsi/transport_security_adapter.h deleted file mode 100644 index f83ecc53e5e..00000000000 --- a/src/core/tsi/transport_security_adapter.h +++ /dev/null @@ -1,41 +0,0 @@ -/* - * - * Copyright 2017 gRPC authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - * - */ - -#ifndef GRPC_CORE_TSI_TRANSPORT_SECURITY_ADAPTER_H -#define GRPC_CORE_TSI_TRANSPORT_SECURITY_ADAPTER_H - -#include - -#include "src/core/tsi/transport_security_interface.h" - -/* Create a tsi handshaker that takes an implementation of old interface and - converts into an implementation of new interface. In the old interface, - there are get_bytes_to_send_to_peer, process_bytes_from_peer, get_result, - extract_peer, and create_frame_protector. In the new interface, only next - method is needed. See transport_security_interface.h for details. Note that - this tsi adapter handshaker is temporary. It will be removed once TSI has - been fully migrated to the new interface. - Ownership of input tsi_handshaker is transferred to this new adapter. */ -tsi_handshaker* tsi_create_adapter_handshaker(tsi_handshaker* wrapped); - -/* Given a tsi adapter handshaker, return the original wrapped handshaker. The - adapter still owns the wrapped handshaker which should not be destroyed by - the caller. */ -tsi_handshaker* tsi_adapter_handshaker_get_wrapped(tsi_handshaker* adapter); - -#endif /* GRPC_CORE_TSI_TRANSPORT_SECURITY_ADAPTER_H */ diff --git a/src/core/tsi/transport_security_interface.h b/src/core/tsi/transport_security_interface.h index 07f2bdfd81a..7a0cdc3453a 100644 --- a/src/core/tsi/transport_security_interface.h +++ b/src/core/tsi/transport_security_interface.h @@ -333,6 +333,8 @@ void tsi_handshaker_result_destroy(tsi_handshaker_result* self); ------------------------------------------------------------------------ */ typedef struct tsi_handshaker tsi_handshaker; +/* TODO(jiangtaoli2016): Cleans up deprecated methods when we are ready. */ + /* TO BE DEPRECATED SOON. Use tsi_handshaker_next instead. Gets bytes that need to be sent to the peer. - bytes is the buffer that will be written with the data to be sent to the diff --git a/src/python/grpcio/grpc_core_dependencies.py b/src/python/grpcio/grpc_core_dependencies.py index 234f7634e2a..699d504c121 100644 --- a/src/python/grpcio/grpc_core_dependencies.py +++ b/src/python/grpcio/grpc_core_dependencies.py @@ -296,7 +296,6 @@ CORE_SOURCE_FILES = [ 'third_party/nanopb/pb_decode.c', 'third_party/nanopb/pb_encode.c', 'src/core/tsi/transport_security.cc', - 'src/core/tsi/transport_security_adapter.cc', 'src/core/ext/transport/chttp2/client/insecure/channel_create.cc', 'src/core/ext/transport/chttp2/client/insecure/channel_create_posix.cc', 'src/core/ext/transport/chttp2/client/authority.cc', diff --git a/test/core/tsi/ssl_transport_security_test.cc b/test/core/tsi/ssl_transport_security_test.cc index cf1ac824139..b477904d60d 100644 --- a/test/core/tsi/ssl_transport_security_test.cc +++ b/test/core/tsi/ssl_transport_security_test.cc @@ -24,7 +24,6 @@ #include "src/core/lib/security/security_connector/security_connector.h" #include "src/core/tsi/ssl_transport_security.h" #include "src/core/tsi/transport_security.h" -#include "src/core/tsi/transport_security_adapter.h" #include "src/core/tsi/transport_security_interface.h" #include "test/core/tsi/transport_security_test_lib.h" #include "test/core/util/test_config.h" @@ -164,19 +163,13 @@ static void ssl_test_setup_handshakers(tsi_test_fixture* fixture) { &server_options, &ssl_fixture->server_handshaker_factory) == TSI_OK); /* Create server and client handshakers. */ - tsi_handshaker* client_handshaker = nullptr; GPR_ASSERT(tsi_ssl_client_handshaker_factory_create_handshaker( ssl_fixture->client_handshaker_factory, ssl_fixture->server_name_indication, - &client_handshaker) == TSI_OK); - ssl_fixture->base.client_handshaker = - tsi_create_adapter_handshaker(client_handshaker); - tsi_handshaker* server_handshaker = nullptr; + &ssl_fixture->base.client_handshaker) == TSI_OK); GPR_ASSERT(tsi_ssl_server_handshaker_factory_create_handshaker( - ssl_fixture->server_handshaker_factory, &server_handshaker) == - TSI_OK); - ssl_fixture->base.server_handshaker = - tsi_create_adapter_handshaker(server_handshaker); + ssl_fixture->server_handshaker_factory, + &ssl_fixture->base.server_handshaker) == TSI_OK); } static void check_alpn(ssl_tsi_test_fixture* ssl_fixture, diff --git a/tools/doxygen/Doxyfile.core.internal b/tools/doxygen/Doxyfile.core.internal index 82b47695440..0969b9cfb27 100644 --- a/tools/doxygen/Doxyfile.core.internal +++ b/tools/doxygen/Doxyfile.core.internal @@ -1496,8 +1496,6 @@ src/core/tsi/ssl_transport_security.h \ src/core/tsi/ssl_types.h \ src/core/tsi/transport_security.cc \ src/core/tsi/transport_security.h \ -src/core/tsi/transport_security_adapter.cc \ -src/core/tsi/transport_security_adapter.h \ src/core/tsi/transport_security_grpc.cc \ src/core/tsi/transport_security_grpc.h \ src/core/tsi/transport_security_interface.h \ diff --git a/tools/run_tests/generated/sources_and_headers.json b/tools/run_tests/generated/sources_and_headers.json index 7fc32d973a7..4f0fc1d30dc 100644 --- a/tools/run_tests/generated/sources_and_headers.json +++ b/tools/run_tests/generated/sources_and_headers.json @@ -10702,7 +10702,6 @@ ], "headers": [ "src/core/tsi/transport_security.h", - "src/core/tsi/transport_security_adapter.h", "src/core/tsi/transport_security_interface.h" ], "is_filegroup": true, @@ -10711,8 +10710,6 @@ "src": [ "src/core/tsi/transport_security.cc", "src/core/tsi/transport_security.h", - "src/core/tsi/transport_security_adapter.cc", - "src/core/tsi/transport_security_adapter.h", "src/core/tsi/transport_security_interface.h" ], "third_party": false,