diff --git a/src/core/security/security_connector.c b/src/core/security/security_connector.c index dbd79c5b22a..61cb20f6b92 100644 --- a/src/core/security/security_connector.c +++ b/src/core/security/security_connector.c @@ -81,6 +81,24 @@ static const char *ssl_cipher_suites(void) { /* -- Common methods. -- */ +/* Returns the first property with that name. */ +static const tsi_peer_property *tsi_peer_get_property_by_name( + const tsi_peer *peer, const char *name) { + size_t i; + if (peer == NULL) return NULL; + for (i = 0; i < peer->property_count; i++) { + const tsi_peer_property* property = &peer->properties[i]; + if (name == NULL && property->name == NULL) { + return property; + } + if (name != NULL && property->name != NULL && + strcmp(property->name, name) == 0) { + return property; + } + } + return NULL; +} + grpc_security_status grpc_security_connector_create_handshaker( grpc_security_connector *sc, tsi_handshaker **handshaker) { if (sc == NULL || handshaker == NULL) return GRPC_SECURITY_ERROR; @@ -212,13 +230,8 @@ static grpc_security_status fake_check_peer(grpc_security_connector *sc, status = GRPC_SECURITY_ERROR; goto end; } - if (peer.properties[0].type != TSI_PEER_PROPERTY_TYPE_STRING) { - gpr_log(GPR_ERROR, "Invalid type of cert type property."); - status = GRPC_SECURITY_ERROR; - goto end; - } - if (strncmp(peer.properties[0].value.string.data, TSI_FAKE_CERTIFICATE_TYPE, - peer.properties[0].value.string.length)) { + if (strncmp(peer.properties[0].value.data, TSI_FAKE_CERTIFICATE_TYPE, + peer.properties[0].value.length)) { gpr_log(GPR_ERROR, "Invalid value for cert type property."); status = GRPC_SECURITY_ERROR; goto end; @@ -365,12 +378,7 @@ static grpc_security_status ssl_check_peer(const char *peer_name, gpr_log(GPR_ERROR, "Missing selected ALPN property."); return GRPC_SECURITY_ERROR; } - if (p->type != TSI_PEER_PROPERTY_TYPE_STRING) { - gpr_log(GPR_ERROR, "Invalid selected ALPN property."); - return GRPC_SECURITY_ERROR; - } - if (!grpc_chttp2_is_alpn_version_supported(p->value.string.data, - p->value.string.length)) { + if (!grpc_chttp2_is_alpn_version_supported(p->value.data, p->value.length)) { gpr_log(GPR_ERROR, "Invalid ALPN value."); return GRPC_SECURITY_ERROR; } diff --git a/src/core/tsi/ssl_transport_security.c b/src/core/tsi/ssl_transport_security.c index b7c2859a1c9..a475a8dd543 100644 --- a/src/core/tsi/ssl_transport_security.c +++ b/src/core/tsi/ssl_transport_security.c @@ -268,31 +268,16 @@ static tsi_result peer_property_from_x509_common_name( } /* Gets the subject SANs from an X509 cert as a tsi_peer_property. */ -static tsi_result peer_property_from_x509_subject_alt_names( - X509* cert, tsi_peer_property* property) { - int i = 0; - int subject_alt_name_count = 0; +static tsi_result add_subject_alt_names_properties_to_peer( + tsi_peer* peer, GENERAL_NAMES* subject_alt_names, + int subject_alt_name_count) { + int i; tsi_result result = TSI_OK; - GENERAL_NAMES* subject_alt_names = - X509_get_ext_d2i(cert, NID_subject_alt_name, 0, 0); - if (subject_alt_names == NULL) { - /* Empty list. */ - return tsi_construct_list_peer_property( - TSI_X509_SUBJECT_ALTERNATIVE_NAMES_PEER_PROPERTY, 0, property); - } - - subject_alt_name_count = sk_GENERAL_NAME_num(subject_alt_names); - result = tsi_construct_list_peer_property( - TSI_X509_SUBJECT_ALTERNATIVE_NAMES_PEER_PROPERTY, subject_alt_name_count, - property); - if (result != TSI_OK) return result; /* Reset for DNS entries filtering. */ - subject_alt_name_count = property->value.list.child_count; - property->value.list.child_count = 0; + peer->property_count -= subject_alt_name_count; for (i = 0; i < subject_alt_name_count; i++) { - tsi_peer_property* child_property = NULL; GENERAL_NAME* subject_alt_name = sk_GENERAL_NAME_value(subject_alt_names, i); /* Filter out the non-dns entries names. */ @@ -305,40 +290,50 @@ static tsi_result peer_property_from_x509_subject_alt_names( result = TSI_INTERNAL_ERROR; break; } - child_property = - &property->value.list.children[property->value.list.child_count++]; result = tsi_construct_string_peer_property( - NULL, (const char*)dns_name, dns_name_size, child_property); + TSI_X509_SUBJECT_ALTERNATIVE_NAME_PEER_PROPERTY, + (const char*)dns_name, dns_name_size, + &peer->properties[peer->property_count++]); OPENSSL_free(dns_name); if (result != TSI_OK) break; } } - if (result != TSI_OK) tsi_peer_property_destruct(property); - sk_GENERAL_NAME_pop_free(subject_alt_names, GENERAL_NAME_free); - return TSI_OK; + return result; } /* Gets information about the peer's X509 cert as a tsi_peer object. */ static tsi_result peer_from_x509(X509* cert, int include_certificate_type, tsi_peer* peer) { /* TODO(jboeuf): Maybe add more properties. */ - size_t property_count = include_certificate_type ? 3 : 2; + GENERAL_NAMES* subject_alt_names = + X509_get_ext_d2i(cert, NID_subject_alt_name, 0, 0); + int subject_alt_name_count = + (subject_alt_names != NULL) ? sk_GENERAL_NAME_num(subject_alt_names) : 0; + size_t property_count = (include_certificate_type ? 1 : 0) + + 1 /* common name */ + subject_alt_name_count; tsi_result result = tsi_construct_peer(property_count, peer); if (result != TSI_OK) return result; do { - result = peer_property_from_x509_common_name(cert, &peer->properties[0]); - if (result != TSI_OK) break; - result = - peer_property_from_x509_subject_alt_names(cert, &peer->properties[1]); - if (result != TSI_OK) break; if (include_certificate_type) { result = tsi_construct_string_peer_property_from_cstring( TSI_CERTIFICATE_TYPE_PEER_PROPERTY, TSI_X509_CERTIFICATE_TYPE, - &peer->properties[2]); + &peer->properties[0]); + if (result != TSI_OK) break; + } + result = peer_property_from_x509_common_name( + cert, &peer->properties[include_certificate_type ? 1 : 0]); + if (result != TSI_OK) break; + + if (subject_alt_name_count != 0) { + result = add_subject_alt_names_properties_to_peer(peer, subject_alt_names, + subject_alt_name_count); if (result != TSI_OK) break; } } while (0); + if (subject_alt_names != NULL) { + sk_GENERAL_NAME_pop_free(subject_alt_names, GENERAL_NAME_free); + } if (result != TSI_OK) tsi_peer_destruct(peer); return result; } @@ -1344,43 +1339,32 @@ tsi_result tsi_create_ssl_server_handshaker_factory( int tsi_ssl_peer_matches_name(const tsi_peer* peer, const char* name) { size_t i = 0; size_t san_count = 0; - const tsi_peer_property* property = NULL; + const tsi_peer_property* cn_property = NULL; /* For now reject what looks like an IP address. */ if (looks_like_ip_address(name)) return 0; /* Check the SAN first. */ - property = tsi_peer_get_property_by_name( - peer, TSI_X509_SUBJECT_ALTERNATIVE_NAMES_PEER_PROPERTY); - if (property == NULL || property->type != TSI_PEER_PROPERTY_TYPE_LIST) { - gpr_log(GPR_ERROR, "Invalid x509 subject alternative names property."); - return 0; - } - - san_count = property->value.list.child_count; - for (i = 0; i < san_count; i++) { - const tsi_peer_property* alt_name_property = - &property->value.list.children[i]; - if (alt_name_property->type != TSI_PEER_PROPERTY_TYPE_STRING) { - gpr_log(GPR_ERROR, "Invalid x509 subject alternative name property."); - return 0; - } - if (does_entry_match_name(alt_name_property->value.string.data, - alt_name_property->value.string.length, name)) { - return 1; + for (i = 0; i < peer->property_count; i++) { + const tsi_peer_property* property = &peer->properties[i]; + if (property->name == NULL) continue; + if (strcmp(property->name, + TSI_X509_SUBJECT_ALTERNATIVE_NAME_PEER_PROPERTY) == 0) { + san_count++; + if (does_entry_match_name(property->value.data, property->value.length, + name)) { + return 1; + } + } else if (strcmp(property->name, + TSI_X509_SUBJECT_COMMON_NAME_PEER_PROPERTY) == 0) { + cn_property = property; } } /* If there's no SAN, try the CN. */ - if (san_count == 0) { - property = tsi_peer_get_property_by_name( - peer, TSI_X509_SUBJECT_COMMON_NAME_PEER_PROPERTY); - if (property == NULL || property->type != TSI_PEER_PROPERTY_TYPE_STRING) { - gpr_log(GPR_ERROR, "Invalid x509 subject common name property."); - return 0; - } - if (does_entry_match_name(property->value.string.data, - property->value.string.length, name)) { + if (san_count == 0 && cn_property != NULL) { + if (does_entry_match_name(cn_property->value.data, + cn_property->value.length, name)) { return 1; } } diff --git a/src/core/tsi/ssl_transport_security.h b/src/core/tsi/ssl_transport_security.h index 192f7acf0e0..b2aa2f393e5 100644 --- a/src/core/tsi/ssl_transport_security.h +++ b/src/core/tsi/ssl_transport_security.h @@ -45,13 +45,9 @@ extern "C" { /* This property is of type TSI_PEER_PROPERTY_STRING. */ #define TSI_X509_SUBJECT_COMMON_NAME_PEER_PROPERTY "x509_subject_common_name" +#define TSI_X509_SUBJECT_ALTERNATIVE_NAME_PEER_PROPERTY \ + "x509_subject_alternative_name" -/* This property is of type TSI_PEER_PROPERTY_LIST and the children contain - unnamed (name == NULL) properties of type TSI_PEER_PROPERTY_STRING. */ -#define TSI_X509_SUBJECT_ALTERNATIVE_NAMES_PEER_PROPERTY \ - "x509_subject_alternative_names" - -/* This property is of type TSI_PEER_PROPERTY_STRING. */ #define TSI_SSL_ALPN_SELECTED_PROTOCOL "ssl_alpn_selected_protocol" /* --- tsi_ssl_handshaker_factory object --- diff --git a/src/core/tsi/transport_security.c b/src/core/tsi/transport_security.c index f4ab9d2bc6d..ec02a478ba4 100644 --- a/src/core/tsi/transport_security.c +++ b/src/core/tsi/transport_security.c @@ -198,23 +198,6 @@ void tsi_handshaker_destroy(tsi_handshaker* self) { /* --- tsi_peer implementation. --- */ -const tsi_peer_property* tsi_peer_get_property_by_name(const tsi_peer* self, - const char* name) { - size_t i; - if (self == NULL) return NULL; - for (i = 0; i < self->property_count; i++) { - const tsi_peer_property* property = &self->properties[i]; - if (name == NULL && property->name == NULL) { - return property; - } - if (name != NULL && property->name != NULL && - strcmp(property->name, name) == 0) { - return property; - } - } - return NULL; -} - tsi_peer_property tsi_init_peer_property(void) { tsi_peer_property property; memset(&property, 0, sizeof(tsi_peer_property)); @@ -234,18 +217,8 @@ void tsi_peer_property_destruct(tsi_peer_property* property) { if (property->name != NULL) { free(property->name); } - switch (property->type) { - case TSI_PEER_PROPERTY_TYPE_STRING: - if (property->value.string.data != NULL) { - free(property->value.string.data); - } - break; - case TSI_PEER_PROPERTY_TYPE_LIST: - tsi_peer_destroy_list_property(property->value.list.children, - property->value.list.child_count); - default: - /* Nothing to free. */ - break; + if (property->value.data != NULL) { + free(property->value.data); } *property = tsi_init_peer_property(); /* Reset everything to 0. */ } @@ -259,57 +232,20 @@ void tsi_peer_destruct(tsi_peer* self) { self->property_count = 0; } -tsi_result tsi_construct_signed_integer_peer_property( - const char* name, int64_t value, tsi_peer_property* property) { - *property = tsi_init_peer_property(); - property->type = TSI_PEER_PROPERTY_TYPE_SIGNED_INTEGER; - if (name != NULL) { - property->name = tsi_strdup(name); - if (property->name == NULL) return TSI_OUT_OF_RESOURCES; - } - property->value.signed_int = value; - return TSI_OK; -} - -tsi_result tsi_construct_unsigned_integer_peer_property( - const char* name, uint64_t value, tsi_peer_property* property) { - *property = tsi_init_peer_property(); - property->type = TSI_PEER_PROPERTY_TYPE_UNSIGNED_INTEGER; - if (name != NULL) { - property->name = tsi_strdup(name); - if (property->name == NULL) return TSI_OUT_OF_RESOURCES; - } - property->value.unsigned_int = value; - return TSI_OK; -} - -tsi_result tsi_construct_real_peer_property(const char* name, double value, - tsi_peer_property* property) { - *property = tsi_init_peer_property(); - property->type = TSI_PEER_PROPERTY_TYPE_REAL; - if (name != NULL) { - property->name = tsi_strdup(name); - if (property->name == NULL) return TSI_OUT_OF_RESOURCES; - } - property->value.real = value; - return TSI_OK; -} - tsi_result tsi_construct_allocated_string_peer_property( const char* name, size_t value_length, tsi_peer_property* property) { *property = tsi_init_peer_property(); - property->type = TSI_PEER_PROPERTY_TYPE_STRING; if (name != NULL) { property->name = tsi_strdup(name); if (property->name == NULL) return TSI_OUT_OF_RESOURCES; } if (value_length > 0) { - property->value.string.data = calloc(1, value_length); - if (property->value.string.data == NULL) { + property->value.data = calloc(1, value_length); + if (property->value.data == NULL) { tsi_peer_property_destruct(property); return TSI_OUT_OF_RESOURCES; } - property->value.string.length = value_length; + property->value.length = value_length; } return TSI_OK; } @@ -328,28 +264,7 @@ tsi_result tsi_construct_string_peer_property(const char* name, name, value_length, property); if (result != TSI_OK) return result; if (value_length > 0) { - memcpy(property->value.string.data, value, value_length); - } - return TSI_OK; -} - -tsi_result tsi_construct_list_peer_property(const char* name, - size_t child_count, - tsi_peer_property* property) { - *property = tsi_init_peer_property(); - property->type = TSI_PEER_PROPERTY_TYPE_LIST; - if (name != NULL) { - property->name = tsi_strdup(name); - if (property->name == NULL) return TSI_OUT_OF_RESOURCES; - } - if (child_count > 0) { - property->value.list.children = - calloc(child_count, sizeof(tsi_peer_property)); - if (property->value.list.children == NULL) { - tsi_peer_property_destruct(property); - return TSI_OUT_OF_RESOURCES; - } - property->value.list.child_count = child_count; + memcpy(property->value.data, value, value_length); } return TSI_OK; } diff --git a/src/core/tsi/transport_security.h b/src/core/tsi/transport_security.h index 59e5dcff9a2..4cd0ec2cfb1 100644 --- a/src/core/tsi/transport_security.h +++ b/src/core/tsi/transport_security.h @@ -92,12 +92,6 @@ struct tsi_handshaker { tsi_result tsi_construct_peer(size_t property_count, tsi_peer* peer); tsi_peer_property tsi_init_peer_property(void); void tsi_peer_property_destruct(tsi_peer_property* property); -tsi_result tsi_construct_signed_integer_peer_property( - const char* name, int64_t value, tsi_peer_property* property); -tsi_result tsi_construct_unsigned_integer_peer_property( - const char* name, uint64_t value, tsi_peer_property* property); -tsi_result tsi_construct_real_peer_property(const char* name, double value, - tsi_peer_property* property); tsi_result tsi_construct_string_peer_property(const char* name, const char* value, size_t value_length, @@ -106,9 +100,6 @@ tsi_result tsi_construct_allocated_string_peer_property( const char* name, size_t value_length, tsi_peer_property* property); tsi_result tsi_construct_string_peer_property_from_cstring( const char* name, const char* value, tsi_peer_property* property); -tsi_result tsi_construct_list_peer_property(const char* name, - size_t child_count, - tsi_peer_property* property); /* Utils. */ char* tsi_strdup(const char* src); /* Sadly, no strdup in C89. */ diff --git a/src/core/tsi/transport_security_interface.h b/src/core/tsi/transport_security_interface.h index 0edff542350..936b0c25b0a 100644 --- a/src/core/tsi/transport_security_interface.h +++ b/src/core/tsi/transport_security_interface.h @@ -179,33 +179,13 @@ void tsi_frame_protector_destroy(tsi_frame_protector* self); /* This property is of type TSI_PEER_PROPERTY_STRING. */ #define TSI_CERTIFICATE_TYPE_PEER_PROPERTY "certificate_type" -/* Properties of type TSI_PEER_PROPERTY_TYPE_STRING may contain NULL characters - just like C++ strings. The length field gives the length of the string. */ -typedef enum { - TSI_PEER_PROPERTY_TYPE_SIGNED_INTEGER, - TSI_PEER_PROPERTY_TYPE_UNSIGNED_INTEGER, - TSI_PEER_PROPERTY_TYPE_REAL, - TSI_PEER_PROPERTY_TYPE_STRING, - TSI_PEER_PROPERTY_TYPE_LIST -} tsi_peer_property_type; - -/* The relevant field in the union value is dictated by the type field. - name may be NULL in case of an unnamed property. */ +/* Property values may contain NULL characters just like C++ strings. + The length field gives the length of the string. */ typedef struct tsi_peer_property { char* name; - tsi_peer_property_type type; - union { - int64_t signed_int; - uint64_t unsigned_int; - double real; - struct { - char* data; - size_t length; - } string; - struct { - struct tsi_peer_property* children; - size_t child_count; - } list; + struct { + char* data; + size_t length; } value; } tsi_peer_property; @@ -214,13 +194,6 @@ typedef struct { size_t property_count; } tsi_peer; -/* Gets the first property with the specified name. Iteration over the - properties of the peer should be used if the client of the API is expecting - several properties with the same name. - Returns NULL if there is no corresponding property. */ -const tsi_peer_property* tsi_peer_get_property_by_name(const tsi_peer* self, - const char* name); - /* Destructs the tsi_peer object. */ void tsi_peer_destruct(tsi_peer* self); diff --git a/test/core/tsi/transport_security_test.c b/test/core/tsi/transport_security_test.c index d591e43faae..e45602bab7f 100644 --- a/test/core/tsi/transport_security_test.c +++ b/test/core/tsi/transport_security_test.c @@ -256,19 +256,16 @@ static tsi_peer peer_from_cert_name_test_entry( name_list *nl; parsed_dns_names dns_entries = parse_dns_names(entry->dns_names); nl = dns_entries.names; - GPR_ASSERT(tsi_construct_peer(2, &peer) == TSI_OK); + GPR_ASSERT(tsi_construct_peer(1 + dns_entries.name_count, &peer) == TSI_OK); GPR_ASSERT(tsi_construct_string_peer_property_from_cstring( TSI_X509_SUBJECT_COMMON_NAME_PEER_PROPERTY, entry->common_name, &peer.properties[0]) == TSI_OK); - GPR_ASSERT(tsi_construct_list_peer_property( - TSI_X509_SUBJECT_ALTERNATIVE_NAMES_PEER_PROPERTY, - dns_entries.name_count, &peer.properties[1]) == TSI_OK); - i = 0; + i = 1; while (nl != NULL) { char *processed = processed_dns_name(nl->name); GPR_ASSERT(tsi_construct_string_peer_property( - NULL, processed, strlen(nl->name), - &peer.properties[1].value.list.children[i++]) == TSI_OK); + TSI_X509_SUBJECT_ALTERNATIVE_NAME_PEER_PROPERTY, processed, + strlen(nl->name), &peer.properties[i++]) == TSI_OK); nl = nl->next; gpr_free(processed); }