From ee19ac2aba599d2c4d351109dcc0e903f3db34b4 Mon Sep 17 00:00:00 2001 From: ZhenLian Date: Tue, 5 May 2020 13:14:43 -0700 Subject: [PATCH] fix test failures --- .../security/security_connector/ssl_utils.cc | 5 +- src/core/tsi/ssl_transport_security.cc | 53 +++++++++++++------ test/core/tsi/ssl_transport_security_test.cc | 20 +++++-- 3 files changed, 57 insertions(+), 21 deletions(-) diff --git a/src/core/lib/security/security_connector/ssl_utils.cc b/src/core/lib/security/security_connector/ssl_utils.cc index b876d734fbf..74a7fa98768 100644 --- a/src/core/lib/security/security_connector/ssl_utils.cc +++ b/src/core/lib/security/security_connector/ssl_utils.cc @@ -256,7 +256,7 @@ grpc_core::RefCountedPtr grpc_ssl_peer_to_auth_context( ctx.get(), GRPC_TRANSPORT_SECURITY_TYPE_PROPERTY_NAME, transport_security_type); const char* spiffe_data = nullptr; - size_t spiffe_length; + size_t spiffe_length = 0; int spiffe_id_count = 0; for (i = 0; i < peer->property_count; i++) { const tsi_peer_property* prop = &peer->properties[i]; @@ -352,6 +352,9 @@ tsi_peer grpc_shallow_peer_from_ssl_auth_context( 0) { add_shallow_auth_property_to_peer(&peer, prop, TSI_X509_PEM_CERT_CHAIN_PROPERTY); + } else if (strcmp(prop->name, GRPC_PEER_SPIFFE_ID_PROPERTY_NAME) == 0) { + add_shallow_auth_property_to_peer(&peer, prop, + TSI_X509_URI_PEER_PROPERTY); } } } diff --git a/src/core/tsi/ssl_transport_security.cc b/src/core/tsi/ssl_transport_security.cc index 820a6537971..069b9f9f90a 100644 --- a/src/core/tsi/ssl_transport_security.cc +++ b/src/core/tsi/ssl_transport_security.cc @@ -346,13 +346,10 @@ static tsi_result add_pem_certificate(X509* cert, tsi_peer_property* property) { /* Gets the subject SANs from an X509 cert as a tsi_peer_property. */ static tsi_result add_subject_alt_names_properties_to_peer( tsi_peer* peer, GENERAL_NAMES* subject_alt_names, - size_t subject_alt_name_count) { + size_t subject_alt_name_count, int* current_insert_index) { size_t i; tsi_result result = TSI_OK; - /* Reset for DNS entries filtering. */ - peer->property_count -= subject_alt_name_count; - for (i = 0; i < subject_alt_name_count; i++) { GENERAL_NAME* subject_alt_name = sk_GENERAL_NAME_value(subject_alt_names, TSI_SIZE_AS_SIZE(i)); @@ -377,13 +374,7 @@ static tsi_result add_subject_alt_names_properties_to_peer( result = tsi_construct_string_peer_property( TSI_X509_SUBJECT_ALTERNATIVE_NAME_PEER_PROPERTY, reinterpret_cast(name), static_cast(name_size), - &peer->properties[peer->property_count++]); - if (subject_alt_name->type == GEN_URI) { - result = tsi_construct_string_peer_property( - TSI_X509_URI_PEER_PROPERTY, reinterpret_cast(name), - static_cast(name_size), - &peer->properties[peer->property_count++]); - } + &peer->properties[(*current_insert_index)++]); OPENSSL_free(name); } else if (subject_alt_name->type == GEN_IPADD) { char ntop_buf[INET6_ADDRSTRLEN]; @@ -408,7 +399,24 @@ static tsi_result add_subject_alt_names_properties_to_peer( result = tsi_construct_string_peer_property_from_cstring( TSI_X509_SUBJECT_ALTERNATIVE_NAME_PEER_PROPERTY, name, - &peer->properties[peer->property_count++]); + &peer->properties[(*current_insert_index)++]); + } + if (result != TSI_OK) break; + if (subject_alt_name->type == GEN_URI) { + unsigned char* name = nullptr; + int name_size; + name_size = ASN1_STRING_to_UTF8( + &name, subject_alt_name->d.uniformResourceIdentifier); + if (name_size < 0) { + gpr_log(GPR_ERROR, "Could not get utf8 from asn1 string."); + result = TSI_INTERNAL_ERROR; + break; + } + result = tsi_construct_string_peer_property( + TSI_X509_URI_PEER_PROPERTY, reinterpret_cast(name), + static_cast(name_size), + &peer->properties[(*current_insert_index)++]); + OPENSSL_free(name); } if (result != TSI_OK) break; } @@ -431,26 +439,35 @@ static tsi_result peer_from_x509(X509* cert, int include_certificate_type, property_count = (include_certificate_type ? static_cast(1) : 0) + 2 /* common name, certificate */ + static_cast(subject_alt_name_count); + for (int i = 0; i < subject_alt_name_count; i++) { + GENERAL_NAME* subject_alt_name = + sk_GENERAL_NAME_value(subject_alt_names, TSI_SIZE_AS_SIZE(i)); + if (subject_alt_name->type == GEN_URI) { + property_count += 1; + } + } result = tsi_construct_peer(property_count, peer); if (result != TSI_OK) return result; + int current_insert_index = 0; do { if (include_certificate_type) { result = tsi_construct_string_peer_property_from_cstring( TSI_CERTIFICATE_TYPE_PEER_PROPERTY, TSI_X509_CERTIFICATE_TYPE, - &peer->properties[0]); + &peer->properties[current_insert_index++]); if (result != TSI_OK) break; } result = peer_property_from_x509_common_name( - cert, &peer->properties[include_certificate_type ? 1 : 0]); + cert, &peer->properties[current_insert_index++]); if (result != TSI_OK) break; - result = add_pem_certificate( - cert, &peer->properties[include_certificate_type ? 2 : 1]); + result = + add_pem_certificate(cert, &peer->properties[current_insert_index++]); if (result != TSI_OK) break; if (subject_alt_name_count != 0) { result = add_subject_alt_names_properties_to_peer( - peer, subject_alt_names, static_cast(subject_alt_name_count)); + peer, subject_alt_names, static_cast(subject_alt_name_count), + ¤t_insert_index); if (result != TSI_OK) break; } } while (0); @@ -459,6 +476,8 @@ static tsi_result peer_from_x509(X509* cert, int include_certificate_type, sk_GENERAL_NAME_pop_free(subject_alt_names, GENERAL_NAME_free); } if (result != TSI_OK) tsi_peer_destruct(peer); + + GPR_ASSERT((int)peer->property_count == current_insert_index); return result; } diff --git a/test/core/tsi/ssl_transport_security_test.cc b/test/core/tsi/ssl_transport_security_test.cc index 70b8355bc79..1319004a9ca 100644 --- a/test/core/tsi/ssl_transport_security_test.cc +++ b/test/core/tsi/ssl_transport_security_test.cc @@ -259,6 +259,18 @@ static bool check_subject_alt_name(tsi_peer* peer, const char* name) { return false; } +static bool check_uri(tsi_peer* peer, const char* name) { + for (size_t i = 0; i < peer->property_count; i++) { + const tsi_peer_property* prop = &peer->properties[i]; + if (strcmp(prop->name, TSI_X509_URI_PEER_PROPERTY) == 0) { + if (memcmp(prop->value.data, name, prop->value.length) == 0) { + return true; + } + } + } + return false; +} + void check_server1_peer(tsi_peer* peer) { const tsi_peer_property* property = check_basic_authenticated_peer_and_get_common_name(peer); @@ -862,9 +874,9 @@ void ssl_tsi_test_extract_x509_subject_names() { tsi_peer peer; GPR_ASSERT(tsi_ssl_extract_x509_subject_names_from_pem_cert(cert, &peer) == TSI_OK); - // One for common name, one for certificate, one for security level, and six - // for SAN fields. - size_t expected_property_count = 8; + // tsi_peer should include one common name, one certificate, one security + // level, six SAN fields, and two URI fields. + size_t expected_property_count = 10; GPR_ASSERT(peer.property_count == expected_property_count); // Check common name const char* expected_cn = "xpigors"; @@ -885,6 +897,8 @@ void ssl_tsi_test_extract_x509_subject_names() { check_subject_alt_name(&peer, "https://foo.test.domain.com/test") == 1); GPR_ASSERT( check_subject_alt_name(&peer, "https://bar.test.domain.com/test") == 1); + GPR_ASSERT(check_uri(&peer, "https://foo.test.domain.com/test") == 1); + GPR_ASSERT(check_uri(&peer, "https://bar.test.domain.com/test") == 1); // Check email address GPR_ASSERT(check_subject_alt_name(&peer, "foo@test.domain.com") == 1); GPR_ASSERT(check_subject_alt_name(&peer, "bar@test.domain.com") == 1);