Skip to content

Commit

Permalink
Use existing read buffer in security handshaker if present
Browse files Browse the repository at this point in the history
  • Loading branch information
apolcyn committed Nov 14, 2017
1 parent a2465b0 commit 3f6b10a
Show file tree
Hide file tree
Showing 10 changed files with 525 additions and 220 deletions.
38 changes: 38 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -466,6 +466,9 @@ endif()
if(_gRPC_PLATFORM_LINUX)
add_dependencies(buildtests_c handshake_server)
endif()
if(_gRPC_PLATFORM_LINUX)
add_dependencies(buildtests_c handshake_server_with_readahead_handshaker)
endif()
add_dependencies(buildtests_c hpack_parser_test)
add_dependencies(buildtests_c hpack_table_test)
add_dependencies(buildtests_c http_parser_test)
Expand Down Expand Up @@ -7395,6 +7398,7 @@ if(_gRPC_PLATFORM_LINUX)

add_executable(handshake_server
test/core/handshake/server_ssl.c
test/core/handshake/server_ssl_common.c
)


Expand All @@ -7421,6 +7425,40 @@ target_link_libraries(handshake_server
gpr
)

endif()
endif (gRPC_BUILD_TESTS)
if (gRPC_BUILD_TESTS)
if(_gRPC_PLATFORM_LINUX)

add_executable(handshake_server_with_readahead_handshaker
test/core/handshake/readahead_handshaker_server_ssl.c
test/core/handshake/server_ssl_common.c
)


target_include_directories(handshake_server_with_readahead_handshaker
PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}
PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/include
PRIVATE ${BORINGSSL_ROOT_DIR}/include
PRIVATE ${PROTOBUF_ROOT_DIR}/src
PRIVATE ${BENCHMARK_ROOT_DIR}/include
PRIVATE ${ZLIB_ROOT_DIR}
PRIVATE ${CMAKE_CURRENT_BINARY_DIR}/third_party/zlib
PRIVATE ${CARES_INCLUDE_DIR}
PRIVATE ${CMAKE_CURRENT_BINARY_DIR}/third_party/cares/cares
PRIVATE ${CMAKE_CURRENT_BINARY_DIR}/third_party/gflags/include
PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/third_party/abseil-cpp
)

target_link_libraries(handshake_server_with_readahead_handshaker
${_gRPC_SSL_LIBRARIES}
${_gRPC_ALLTARGETS_LIBRARIES}
grpc_test_util
grpc
gpr_test_util
gpr
)

endif()
endif (gRPC_BUILD_TESTS)
if (gRPC_BUILD_TESTS)
Expand Down
42 changes: 42 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -1023,6 +1023,7 @@ grpc_ssl_credentials_test: $(BINDIR)/$(CONFIG)/grpc_ssl_credentials_test
grpc_verify_jwt: $(BINDIR)/$(CONFIG)/grpc_verify_jwt
handshake_client: $(BINDIR)/$(CONFIG)/handshake_client
handshake_server: $(BINDIR)/$(CONFIG)/handshake_server
handshake_server_with_readahead_handshaker: $(BINDIR)/$(CONFIG)/handshake_server_with_readahead_handshaker
hpack_parser_fuzzer_test: $(BINDIR)/$(CONFIG)/hpack_parser_fuzzer_test
hpack_parser_test: $(BINDIR)/$(CONFIG)/hpack_parser_test
hpack_table_test: $(BINDIR)/$(CONFIG)/hpack_table_test
Expand Down Expand Up @@ -1417,6 +1418,7 @@ buildtests_c: privatelibs_c \
$(BINDIR)/$(CONFIG)/grpc_ssl_credentials_test \
$(BINDIR)/$(CONFIG)/handshake_client \
$(BINDIR)/$(CONFIG)/handshake_server \
$(BINDIR)/$(CONFIG)/handshake_server_with_readahead_handshaker \
$(BINDIR)/$(CONFIG)/hpack_parser_test \
$(BINDIR)/$(CONFIG)/hpack_table_test \
$(BINDIR)/$(CONFIG)/http_parser_test \
Expand Down Expand Up @@ -1894,6 +1896,8 @@ test_c: buildtests_c
$(Q) $(BINDIR)/$(CONFIG)/handshake_client || ( echo test handshake_client failed ; exit 1 )
$(E) "[RUN] Testing handshake_server"
$(Q) $(BINDIR)/$(CONFIG)/handshake_server || ( echo test handshake_server failed ; exit 1 )
$(E) "[RUN] Testing handshake_server_with_readahead_handshaker"
$(Q) $(BINDIR)/$(CONFIG)/handshake_server_with_readahead_handshaker || ( echo test handshake_server_with_readahead_handshaker failed ; exit 1 )
$(E) "[RUN] Testing hpack_parser_test"
$(Q) $(BINDIR)/$(CONFIG)/hpack_parser_test || ( echo test hpack_parser_test failed ; exit 1 )
$(E) "[RUN] Testing hpack_table_test"
Expand Down Expand Up @@ -11221,6 +11225,7 @@ endif

HANDSHAKE_SERVER_SRC = \
test/core/handshake/server_ssl.c \
test/core/handshake/server_ssl_common.c \

HANDSHAKE_SERVER_OBJS = $(addprefix $(OBJDIR)/$(CONFIG)/, $(addsuffix .o, $(basename $(HANDSHAKE_SERVER_SRC))))
ifeq ($(NO_SECURE),true)
Expand All @@ -11242,6 +11247,8 @@ endif

$(OBJDIR)/$(CONFIG)/test/core/handshake/server_ssl.o: $(LIBDIR)/$(CONFIG)/libgrpc_test_util.a $(LIBDIR)/$(CONFIG)/libgrpc.a $(LIBDIR)/$(CONFIG)/libgpr_test_util.a $(LIBDIR)/$(CONFIG)/libgpr.a

$(OBJDIR)/$(CONFIG)/test/core/handshake/server_ssl_common.o: $(LIBDIR)/$(CONFIG)/libgrpc_test_util.a $(LIBDIR)/$(CONFIG)/libgrpc.a $(LIBDIR)/$(CONFIG)/libgpr_test_util.a $(LIBDIR)/$(CONFIG)/libgpr.a

deps_handshake_server: $(HANDSHAKE_SERVER_OBJS:.o=.dep)

ifneq ($(NO_SECURE),true)
Expand All @@ -11251,6 +11258,41 @@ endif
endif


HANDSHAKE_SERVER_WITH_READAHEAD_HANDSHAKER_SRC = \
test/core/handshake/readahead_handshaker_server_ssl.c \
test/core/handshake/server_ssl_common.c \

HANDSHAKE_SERVER_WITH_READAHEAD_HANDSHAKER_OBJS = $(addprefix $(OBJDIR)/$(CONFIG)/, $(addsuffix .o, $(basename $(HANDSHAKE_SERVER_WITH_READAHEAD_HANDSHAKER_SRC))))
ifeq ($(NO_SECURE),true)

# You can't build secure targets if you don't have OpenSSL.

$(BINDIR)/$(CONFIG)/handshake_server_with_readahead_handshaker: openssl_dep_error

else



$(BINDIR)/$(CONFIG)/handshake_server_with_readahead_handshaker: $(HANDSHAKE_SERVER_WITH_READAHEAD_HANDSHAKER_OBJS) $(LIBDIR)/$(CONFIG)/libgrpc_test_util.a $(LIBDIR)/$(CONFIG)/libgrpc.a $(LIBDIR)/$(CONFIG)/libgpr_test_util.a $(LIBDIR)/$(CONFIG)/libgpr.a
$(E) "[LD] Linking $@"
$(Q) mkdir -p `dirname $@`
$(Q) $(LD) $(LDFLAGS) $(HANDSHAKE_SERVER_WITH_READAHEAD_HANDSHAKER_OBJS) $(LIBDIR)/$(CONFIG)/libgrpc_test_util.a $(LIBDIR)/$(CONFIG)/libgrpc.a $(LIBDIR)/$(CONFIG)/libgpr_test_util.a $(LIBDIR)/$(CONFIG)/libgpr.a $(LDLIBS) $(LDLIBS_SECURE) -o $(BINDIR)/$(CONFIG)/handshake_server_with_readahead_handshaker

endif

$(OBJDIR)/$(CONFIG)/test/core/handshake/readahead_handshaker_server_ssl.o: $(LIBDIR)/$(CONFIG)/libgrpc_test_util.a $(LIBDIR)/$(CONFIG)/libgrpc.a $(LIBDIR)/$(CONFIG)/libgpr_test_util.a $(LIBDIR)/$(CONFIG)/libgpr.a

$(OBJDIR)/$(CONFIG)/test/core/handshake/server_ssl_common.o: $(LIBDIR)/$(CONFIG)/libgrpc_test_util.a $(LIBDIR)/$(CONFIG)/libgrpc.a $(LIBDIR)/$(CONFIG)/libgpr_test_util.a $(LIBDIR)/$(CONFIG)/libgpr.a

deps_handshake_server_with_readahead_handshaker: $(HANDSHAKE_SERVER_WITH_READAHEAD_HANDSHAKER_OBJS:.o=.dep)

ifneq ($(NO_SECURE),true)
ifneq ($(NO_DEPS),true)
-include $(HANDSHAKE_SERVER_WITH_READAHEAD_HANDSHAKER_OBJS:.o=.dep)
endif
endif


HPACK_PARSER_FUZZER_TEST_SRC = \
test/core/transport/chttp2/hpack_parser_fuzzer_test.c \

Expand Down
17 changes: 17 additions & 0 deletions build.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2579,6 +2579,23 @@ targets:
language: c
src:
- test/core/handshake/server_ssl.c
- test/core/handshake/server_ssl_common.c
deps:
- grpc_test_util
- grpc
- gpr_test_util
- gpr
exclude_iomgrs:
- uv
platforms:
- linux
secure: true
- name: handshake_server_with_readahead_handshaker
build: test
language: c
src:
- test/core/handshake/readahead_handshaker_server_ssl.c
- test/core/handshake/server_ssl_common.c
deps:
- grpc_test_util
- grpc
Expand Down
45 changes: 25 additions & 20 deletions src/core/lib/security/transport/security_handshaker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,25 @@ typedef struct {
tsi_handshaker_result *handshaker_result;
} security_handshaker;

static size_t move_read_buffer_into_handshake_buffer(grpc_exec_ctx *exec_ctx,
security_handshaker *h) {
size_t bytes_in_read_buffer = h->args->read_buffer->length;
if (h->handshake_buffer_size < bytes_in_read_buffer) {
h->handshake_buffer =
(uint8_t *)gpr_realloc(h->handshake_buffer, bytes_in_read_buffer);
h->handshake_buffer_size = bytes_in_read_buffer;
}
size_t offset = 0;
while (h->args->read_buffer->count > 0) {
grpc_slice next_slice = grpc_slice_buffer_take_first(h->args->read_buffer);
memcpy(h->handshake_buffer + offset, GRPC_SLICE_START_PTR(next_slice),
GRPC_SLICE_LENGTH(next_slice));
offset += GRPC_SLICE_LENGTH(next_slice);
grpc_slice_unref_internal(exec_ctx, next_slice);
}
return bytes_in_read_buffer;
}

static void security_handshaker_unref(grpc_exec_ctx *exec_ctx,
security_handshaker *h) {
if (gpr_unref(&h->refs)) {
Expand Down Expand Up @@ -177,8 +196,6 @@ static void on_peer_checked_inner(grpc_exec_ctx *exec_ctx,
}
tsi_handshaker_result_destroy(h->handshaker_result);
h->handshaker_result = NULL;
// Clear out the read buffer before it gets passed to the transport.
grpc_slice_buffer_reset_and_unref_internal(exec_ctx, h->args->read_buffer);
// Add auth context to channel args.
grpc_arg auth_context_arg = grpc_auth_context_to_arg(h->auth_context);
grpc_channel_args *tmp_args = h->args->args;
Expand Down Expand Up @@ -311,23 +328,8 @@ static void on_handshake_data_received_from_peer(grpc_exec_ctx *exec_ctx,
return;
}
// Copy all slices received.
size_t i;
size_t bytes_received_size = 0;
for (i = 0; i < h->args->read_buffer->count; i++) {
bytes_received_size += GRPC_SLICE_LENGTH(h->args->read_buffer->slices[i]);
}
if (bytes_received_size > h->handshake_buffer_size) {
h->handshake_buffer =
(uint8_t *)gpr_realloc(h->handshake_buffer, bytes_received_size);
h->handshake_buffer_size = bytes_received_size;
}
size_t offset = 0;
for (i = 0; i < h->args->read_buffer->count; i++) {
size_t slice_size = GPR_SLICE_LENGTH(h->args->read_buffer->slices[i]);
memcpy(h->handshake_buffer + offset,
GRPC_SLICE_START_PTR(h->args->read_buffer->slices[i]), slice_size);
offset += slice_size;
}
size_t bytes_received_size =
move_read_buffer_into_handshake_buffer(exec_ctx, h);
// Call TSI handshaker.
error = do_handshaker_next_locked(exec_ctx, h, h->handshake_buffer,
bytes_received_size);
Expand Down Expand Up @@ -403,7 +405,10 @@ static void security_handshaker_do_handshake(grpc_exec_ctx *exec_ctx,
h->args = args;
h->on_handshake_done = on_handshake_done;
gpr_ref(&h->refs);
grpc_error *error = do_handshaker_next_locked(exec_ctx, h, NULL, 0);
size_t bytes_received_size =
move_read_buffer_into_handshake_buffer(exec_ctx, h);
grpc_error *error = do_handshaker_next_locked(
exec_ctx, h, h->handshake_buffer, bytes_received_size);
if (error != GRPC_ERROR_NONE) {
security_handshake_failed_locked(exec_ctx, h, error);
gpr_mu_unlock(&h->mu);
Expand Down
103 changes: 103 additions & 0 deletions test/core/handshake/readahead_handshaker_server_ssl.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
/*
*
* Copyright 2016 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 <arpa/inet.h>
#include <openssl/err.h>
#include <openssl/ssl.h>
#include <string.h>
#include <sys/socket.h>
#include <unistd.h>

#include <grpc/grpc.h>
#include <grpc/grpc_security.h>
#include <grpc/support/alloc.h>
#include <grpc/support/log.h>
#include <grpc/support/string_util.h>
#include <grpc/support/sync.h>
#include <grpc/support/thd.h>
#include "src/core/lib/iomgr/load_file.h"
#include "test/core/util/port.h"
#include "test/core/util/test_config.h"

#include "src/core/lib/channel/handshaker_factory.h"
#include "src/core/lib/channel/handshaker_registry.h"
#include "src/core/lib/security/transport/security_handshaker.h"

#include "test/core/handshake/server_ssl_common.h"

/* The purpose of this test is to exercise the case when a
* grpc *security_handshaker* begins its handshake with data already
* in the read buffer of the handshaker arg. This scenario is created by
* adding a fake "readahead" handshaker at the beginning of the server's
* handshaker list, which just reads from the connection and then places
* read bytes into the read buffer of the handshake arg (to be passed down
* to the security_handshaker). This test is meant to protect code relying on
* this functionality that lives outside of this repo. */

static void readahead_handshaker_destroy(grpc_exec_ctx *ctx,
grpc_handshaker *handshaker) {
gpr_free(handshaker);
}

static void readahead_handshaker_shutdown(grpc_exec_ctx *ctx,
grpc_handshaker *handshaker,
grpc_error *error) {}

static void readahead_handshaker_do_handshake(
grpc_exec_ctx *ctx, grpc_handshaker *handshaker,
grpc_tcp_server_acceptor *acceptor, grpc_closure *on_handshake_done,
grpc_handshaker_args *args) {
grpc_endpoint_read(ctx, args->endpoint, args->read_buffer, on_handshake_done);
}

const grpc_handshaker_vtable readahead_handshaker_vtable = {
readahead_handshaker_destroy, readahead_handshaker_shutdown,
readahead_handshaker_do_handshake};

static grpc_handshaker *readahead_handshaker_create(grpc_exec_ctx *ctx) {
grpc_handshaker *h = (grpc_handshaker *)gpr_zalloc(sizeof(grpc_handshaker));
grpc_handshaker_init(&readahead_handshaker_vtable, h);
return h;
}

static void readahead_handshaker_factory_add_handshakers(
grpc_exec_ctx *exec_ctx, grpc_handshaker_factory *hf,
const grpc_channel_args *args, grpc_handshake_manager *handshake_mgr) {
grpc_handshake_manager_add(handshake_mgr,
readahead_handshaker_create(exec_ctx));
}

static void readahead_handshaker_factory_destroy(
grpc_exec_ctx *exec_ctx, grpc_handshaker_factory *handshaker_factory) {}

static const grpc_handshaker_factory_vtable
readahead_handshaker_factory_vtable = {
readahead_handshaker_factory_add_handshakers,
readahead_handshaker_factory_destroy};

int main(int argc, char *argv[]) {
grpc_handshaker_factory readahead_handshaker_factory = {
&readahead_handshaker_factory_vtable};
grpc_init();
grpc_handshaker_factory_register(true /* at_start */, HANDSHAKER_SERVER,
&readahead_handshaker_factory);
const char *full_alpn_list[] = {"grpc-exp", "h2"};
GPR_ASSERT(server_ssl_test(full_alpn_list, 2, "grpc-exp"));
grpc_shutdown();
return 0;
}
Loading

0 comments on commit 3f6b10a

Please sign in to comment.