diff --git a/src/core/lib/security/security_connector/alts/alts_security_connector.cc b/src/core/lib/security/security_connector/alts/alts_security_connector.cc index 5c78b7635a3..d9edd565085 100644 --- a/src/core/lib/security/security_connector/alts/alts_security_connector.cc +++ b/src/core/lib/security/security_connector/alts/alts_security_connector.cc @@ -235,9 +235,9 @@ grpc_alts_auth_context_from_tsi_peer(const tsi_peer* peer) { } /* Add alts context to auth context. */ if (strcmp(tsi_prop->name, TSI_ALTS_CONTEXT) == 0) { - grpc_auth_context_add_property( - ctx.get(), TSI_ALTS_CONTEXT, - tsi_prop->value.data, tsi_prop->value.length); + grpc_auth_context_add_property(ctx.get(), TSI_ALTS_CONTEXT, + tsi_prop->value.data, + tsi_prop->value.length); } } if (!grpc_auth_context_peer_is_authenticated(ctx.get())) { diff --git a/src/core/tsi/alts/handshaker/alts_tsi_handshaker.cc b/src/core/tsi/alts/handshaker/alts_tsi_handshaker.cc index 77758a45acd..afba2cda89f 100644 --- a/src/core/tsi/alts/handshaker/alts_tsi_handshaker.cc +++ b/src/core/tsi/alts/handshaker/alts_tsi_handshaker.cc @@ -29,7 +29,6 @@ #include #include #include -#include "src/core/ext/upb-generated/src/proto/grpc/gcp/altscontext.upb.h" #include "src/core/lib/gprpp/thd.h" #include "src/core/lib/iomgr/closure.h" @@ -114,8 +113,9 @@ static tsi_result handshaker_result_extract_peer( index++; GPR_ASSERT(&peer->properties[index] != nullptr); ok = tsi_construct_string_peer_property( - TSI_ALTS_CONTEXT, reinterpret_cast(GRPC_SLICE_START_PTR(result->serialized_context)), GRPC_SLICE_LENGTH(result->serialized_context), - &peer->properties[index]); + TSI_ALTS_CONTEXT, + reinterpret_cast(GRPC_SLICE_START_PTR(result->serialized_context)), + GRPC_SLICE_LENGTH(result->serialized_context), &peer->properties[index]); if (ok != TSI_OK) { tsi_peer_destruct(peer); gpr_log(GPR_ERROR, "Failed to set tsi peer property"); @@ -194,6 +194,7 @@ static void handshaker_result_destroy(tsi_handshaker_result* self) { gpr_free(result->key_data); gpr_free(result->unused_bytes); grpc_slice_unref_internal(result->rpc_versions); + grpc_slice_unref_internal(result->serialized_context); gpr_free(result); } @@ -218,9 +219,10 @@ tsi_result alts_tsi_handshaker_result_create(grpc_gcp_HandshakerResp* resp, gpr_log(GPR_ERROR, "Invalid identity"); return TSI_FAILED_PRECONDITION; } - upb_strview service_account = grpc_gcp_Identity_service_account(identity); - if (service_account.size == 0) { - gpr_log(GPR_ERROR, "Invalid service account"); + upb_strview peer_service_account = + grpc_gcp_Identity_service_account(identity); + if (peer_service_account.size == 0) { + gpr_log(GPR_ERROR, "Invalid peer service account"); return TSI_FAILED_PRECONDITION; } upb_strview key_data = grpc_gcp_HandshakerResult_key_data(hresult); @@ -234,12 +236,14 @@ tsi_result alts_tsi_handshaker_result_create(grpc_gcp_HandshakerResp* resp, gpr_log(GPR_ERROR, "Peer does not set RPC protocol versions."); return TSI_FAILED_PRECONDITION; } - upb_strview application_protocol = grpc_gcp_HandshakerResult_application_protocol(hresult); + upb_strview application_protocol = + grpc_gcp_HandshakerResult_application_protocol(hresult); if (application_protocol.size == 0) { gpr_log(GPR_ERROR, "Invalid application protocol"); return TSI_FAILED_PRECONDITION; } - upb_strview record_protocol = grpc_gcp_HandshakerResult_record_protocol(hresult); + upb_strview record_protocol = + grpc_gcp_HandshakerResult_record_protocol(hresult); if (record_protocol.size == 0) { gpr_log(GPR_ERROR, "Invalid record protocol"); return TSI_FAILED_PRECONDITION; @@ -250,7 +254,8 @@ tsi_result alts_tsi_handshaker_result_create(grpc_gcp_HandshakerResp* resp, gpr_log(GPR_ERROR, "Invalid local identity"); return TSI_FAILED_PRECONDITION; } - upb_strview local_service_account = grpc_gcp_Identity_service_account(local_identity); + upb_strview local_service_account = + grpc_gcp_Identity_service_account(local_identity); if (local_service_account.size == 0) { gpr_log(GPR_ERROR, "Invalid local service account"); return TSI_FAILED_PRECONDITION; @@ -261,11 +266,12 @@ tsi_result alts_tsi_handshaker_result_create(grpc_gcp_HandshakerResp* resp, static_cast(gpr_zalloc(kAltsAes128GcmRekeyKeyLength)); memcpy(result->key_data, key_data.data, kAltsAes128GcmRekeyKeyLength); result->peer_identity = - static_cast(gpr_zalloc(service_account.size + 1)); - memcpy(result->peer_identity, service_account.data, service_account.size); - upb::Arena rpc_protocol_arena; + static_cast(gpr_zalloc(peer_service_account.size + 1)); + memcpy(result->peer_identity, peer_service_account.data, + peer_service_account.size); + upb::Arena rpc_versions_arena; bool serialized = grpc_gcp_rpc_protocol_versions_encode( - peer_rpc_version, rpc_protocol_arena.ptr(), &result->rpc_versions); + peer_rpc_version, rpc_versions_arena.ptr(), &result->rpc_versions); if (!serialized) { gpr_log(GPR_ERROR, "Failed to serialize peer's RPC protocol versions."); return TSI_FAILED_PRECONDITION; @@ -274,18 +280,23 @@ tsi_result alts_tsi_handshaker_result_create(grpc_gcp_HandshakerResp* resp, grpc_gcp_AltsContext* context = grpc_gcp_AltsContext_new(context_arena.ptr()); grpc_gcp_AltsContext_set_application_protocol(context, application_protocol); grpc_gcp_AltsContext_set_record_protocol(context, record_protocol); + // ALTS currently only supports the security level of 2, + // which is "grpc_gcp_INTEGRITY_AND_PRIVACY" grpc_gcp_AltsContext_set_security_level(context, 2); - grpc_gcp_AltsContext_set_peer_service_account(context, service_account); - grpc_gcp_AltsContext_set_local_service_account(context, local_service_account); - grpc_gcp_AltsContext_set_peer_rpc_versions(context, const_cast(peer_rpc_version)); + grpc_gcp_AltsContext_set_peer_service_account(context, peer_service_account); + grpc_gcp_AltsContext_set_local_service_account(context, + local_service_account); + grpc_gcp_AltsContext_set_peer_rpc_versions( + context, const_cast(peer_rpc_version)); size_t serialized_ctx_length; - char* serialized_ctx = - grpc_gcp_AltsContext_serialize(context, context_arena.ptr(), &serialized_ctx_length); + char* serialized_ctx = grpc_gcp_AltsContext_serialize( + context, context_arena.ptr(), &serialized_ctx_length); if (serialized_ctx == nullptr) { gpr_log(GPR_ERROR, "Failed to serialize peer's ALTS context."); return TSI_FAILED_PRECONDITION; } - result->serialized_context = grpc_slice_from_copied_buffer(serialized_ctx, serialized_ctx_length); + result->serialized_context = + grpc_slice_from_copied_buffer(serialized_ctx, serialized_ctx_length); result->is_client = is_client; result->base.vtable = &result_vtable; *self = &result->base; @@ -316,8 +327,8 @@ static void on_handshaker_service_resp_recv_dedicated(void* arg, alts_shared_resource_dedicated* resource = grpc_alts_get_shared_resource_dedicated(); grpc_cq_end_op(resource->cq, arg, GRPC_ERROR_NONE, - [](void* done_arg, grpc_cq_completion* storage) {}, nullptr, - &resource->storage); + [](void* /*done_arg*/, grpc_cq_completion* /*storage*/) {}, + nullptr, &resource->storage); } static tsi_result handshaker_next( diff --git a/src/core/tsi/alts/handshaker/alts_tsi_handshaker.h b/src/core/tsi/alts/handshaker/alts_tsi_handshaker.h index 5bea01a6854..3508aa021e0 100644 --- a/src/core/tsi/alts/handshaker/alts_tsi_handshaker.h +++ b/src/core/tsi/alts/handshaker/alts_tsi_handshaker.h @@ -28,6 +28,7 @@ #include "src/core/tsi/alts/handshaker/alts_handshaker_client.h" #include "src/core/tsi/transport_security.h" #include "src/core/tsi/transport_security_interface.h" +#include "src/proto/grpc/gcp/altscontext.upb.h" #include "src/proto/grpc/gcp/handshaker.upb.h" #define TSI_ALTS_SERVICE_ACCOUNT_PEER_PROPERTY "service_account" diff --git a/test/core/security/alts_security_connector_test.cc b/test/core/security/alts_security_connector_test.cc index e32ef970ef9..255fb325cef 100644 --- a/test/core/security/alts_security_connector_test.cc +++ b/test/core/security/alts_security_connector_test.cc @@ -135,13 +135,14 @@ static void test_alts_peer_to_auth_context_success() { GRPC_SLICE_START_PTR(serialized_peer_versions)), GRPC_SLICE_LENGTH(serialized_peer_versions), &peer.properties[2]) == TSI_OK); - grpc_slice serialized_alts_ctx; - GPR_ASSERT(tsi_construct_string_peer_property( - TSI_ALTS_CONTEXT, - reinterpret_cast( - GRPC_SLICE_START_PTR(serialized_alts_ctx)), - GRPC_SLICE_LENGTH(serialized_alts_ctx), - &peer.properties[3]) == TSI_OK); + char test_ctx[] = "test serialized context"; + grpc_slice serialized_alts_ctx = grpc_slice_from_copied_string(test_ctx); + GPR_ASSERT( + tsi_construct_string_peer_property( + TSI_ALTS_CONTEXT, + reinterpret_cast(GRPC_SLICE_START_PTR(serialized_alts_ctx)), + GRPC_SLICE_LENGTH(serialized_alts_ctx), + &peer.properties[3]) == TSI_OK); grpc_core::RefCountedPtr ctx = grpc_alts_auth_context_from_tsi_peer(&peer); GPR_ASSERT(ctx != nullptr); diff --git a/test/core/tsi/alts/handshaker/alts_tsi_handshaker_test.cc b/test/core/tsi/alts/handshaker/alts_tsi_handshaker_test.cc index 22e4100c7f6..0d378a1cd54 100644 --- a/test/core/tsi/alts/handshaker/alts_tsi_handshaker_test.cc +++ b/test/core/tsi/alts/handshaker/alts_tsi_handshaker_test.cc @@ -27,8 +27,8 @@ #include "src/core/tsi/alts/handshaker/alts_shared_resource.h" #include "src/core/tsi/alts/handshaker/alts_tsi_handshaker.h" #include "src/core/tsi/alts/handshaker/alts_tsi_handshaker_private.h" +#include "src/proto/grpc/gcp/altscontext.upb.h" #include "test/core/tsi/alts/handshaker/alts_handshaker_service_api_test_lib.h" -#include "src/core/ext/upb-generated/src/proto/grpc/gcp/altscontext.upb.h" #define ALTS_TSI_HANDSHAKER_TEST_RECV_BYTES "Hello World" #define ALTS_TSI_HANDSHAKER_TEST_OUT_FRAME "Hello Google" @@ -44,7 +44,8 @@ #define ALTS_TSI_HANDSHAKER_TEST_MIN_RPC_VERSION_MAJOR 2 #define ALTS_TSI_HANDSHAKER_TEST_MIN_RPC_VERSION_MINOR 1 #define ALTS_TSI_HANDSHAKER_TEST_LOCAL_IDENTITY "chapilocal@service.google.com" -#define ALTS_TSI_HANDSHAKER_TEST_APPLICATION_PROTOCOL "test application protocol" +#define ALTS_TSI_HANDSHAKER_TEST_APPLICATION_PROTOCOL \ + "test application protocol" #define ALTS_TSI_HANDSHAKER_TEST_RECORD_PROTOCOL "test record protocol" using grpc_core::internal::alts_handshaker_client_check_fields_for_testing; @@ -154,7 +155,8 @@ static grpc_byte_buffer* generate_handshaker_response( local_identity, upb_strview_makez(ALTS_TSI_HANDSHAKER_TEST_LOCAL_IDENTITY)); grpc_gcp_HandshakerResult_set_application_protocol( - result, upb_strview_makez(ALTS_TSI_HANDSHAKER_TEST_APPLICATION_PROTOCOL)); + result, + upb_strview_makez(ALTS_TSI_HANDSHAKER_TEST_APPLICATION_PROTOCOL)); grpc_gcp_HandshakerResult_set_record_protocol( result, upb_strview_makez(ALTS_TSI_HANDSHAKER_TEST_RECORD_PROTOCOL)); break; @@ -180,7 +182,8 @@ static grpc_byte_buffer* generate_handshaker_response( local_identity, upb_strview_makez(ALTS_TSI_HANDSHAKER_TEST_LOCAL_IDENTITY)); grpc_gcp_HandshakerResult_set_application_protocol( - result, upb_strview_makez(ALTS_TSI_HANDSHAKER_TEST_APPLICATION_PROTOCOL)); + result, + upb_strview_makez(ALTS_TSI_HANDSHAKER_TEST_APPLICATION_PROTOCOL)); grpc_gcp_HandshakerResult_set_record_protocol( result, upb_strview_makez(ALTS_TSI_HANDSHAKER_TEST_RECORD_PROTOCOL)); break; @@ -286,24 +289,22 @@ static void on_client_next_success_cb(tsi_result status, void* user_data, peer.properties[1].value.length) == 0); /* Validate alts context. */ upb::Arena context_arena; - grpc_gcp_AltsContext* ctx = - grpc_gcp_AltsContext_parse(peer.properties[3].value.data, peer.properties[3].value.length, context_arena.ptr()); + grpc_gcp_AltsContext* ctx = grpc_gcp_AltsContext_parse( + peer.properties[3].value.data, peer.properties[3].value.length, + context_arena.ptr()); GPR_ASSERT(ctx != nullptr); - upb_strview application_protocol = grpc_gcp_AltsContext_application_protocol(ctx); + upb_strview application_protocol = + grpc_gcp_AltsContext_application_protocol(ctx); upb_strview record_protocol = grpc_gcp_AltsContext_record_protocol(ctx); upb_strview peer_account = grpc_gcp_AltsContext_peer_service_account(ctx); upb_strview local_account = grpc_gcp_AltsContext_local_service_account(ctx); GPR_ASSERT(memcmp(ALTS_TSI_HANDSHAKER_TEST_APPLICATION_PROTOCOL, - application_protocol.data, - application_protocol.size) == 0); + application_protocol.data, application_protocol.size) == 0); GPR_ASSERT(memcmp(ALTS_TSI_HANDSHAKER_TEST_RECORD_PROTOCOL, - record_protocol.data, - record_protocol.size) == 0); - GPR_ASSERT(memcmp(ALTS_TSI_HANDSHAKER_TEST_PEER_IDENTITY, - peer_account.data, + record_protocol.data, record_protocol.size) == 0); + GPR_ASSERT(memcmp(ALTS_TSI_HANDSHAKER_TEST_PEER_IDENTITY, peer_account.data, peer_account.size) == 0); - GPR_ASSERT(memcmp(ALTS_TSI_HANDSHAKER_TEST_LOCAL_IDENTITY, - local_account.data, + GPR_ASSERT(memcmp(ALTS_TSI_HANDSHAKER_TEST_LOCAL_IDENTITY, local_account.data, local_account.size) == 0); tsi_peer_destruct(&peer); /* Validate unused bytes. */ @@ -342,6 +343,25 @@ static void on_server_next_success_cb(tsi_result status, void* user_data, GPR_ASSERT(memcmp(ALTS_TSI_HANDSHAKER_TEST_PEER_IDENTITY, peer.properties[1].value.data, peer.properties[1].value.length) == 0); + /* Validate alts context. */ + upb::Arena context_arena; + grpc_gcp_AltsContext* ctx = grpc_gcp_AltsContext_parse( + peer.properties[3].value.data, peer.properties[3].value.length, + context_arena.ptr()); + GPR_ASSERT(ctx != nullptr); + upb_strview application_protocol = + grpc_gcp_AltsContext_application_protocol(ctx); + upb_strview record_protocol = grpc_gcp_AltsContext_record_protocol(ctx); + upb_strview peer_account = grpc_gcp_AltsContext_peer_service_account(ctx); + upb_strview local_account = grpc_gcp_AltsContext_local_service_account(ctx); + GPR_ASSERT(memcmp(ALTS_TSI_HANDSHAKER_TEST_APPLICATION_PROTOCOL, + application_protocol.data, application_protocol.size) == 0); + GPR_ASSERT(memcmp(ALTS_TSI_HANDSHAKER_TEST_RECORD_PROTOCOL, + record_protocol.data, record_protocol.size) == 0); + GPR_ASSERT(memcmp(ALTS_TSI_HANDSHAKER_TEST_PEER_IDENTITY, peer_account.data, + peer_account.size) == 0); + GPR_ASSERT(memcmp(ALTS_TSI_HANDSHAKER_TEST_LOCAL_IDENTITY, local_account.data, + local_account.size) == 0); tsi_peer_destruct(&peer); /* Validate unused bytes. */ const unsigned char* bytes = nullptr;