From 77e8c1cfb99bc93469e06e75897bbece7d40b26c Mon Sep 17 00:00:00 2001
From: Julien Boeuf <jboeuf@google.com>
Date: Wed, 13 May 2015 13:50:59 -0700
Subject: [PATCH] Clean up tsi properties.
---
src/core/security/security_connector.c | 34 ++++---
src/core/tsi/ssl_transport_security.c | 106 +++++++++-----------
src/core/tsi/ssl_transport_security.h | 8 +-
src/core/tsi/transport_security.c | 97 ++----------------
src/core/tsi/transport_security.h | 9 --
src/core/tsi/transport_security_interface.h | 37 +------
test/core/tsi/transport_security_test.c | 11 +-
7 files changed, 83 insertions(+), 219 deletions(-)
diff --git a/src/core/security/security_connector.c b/src/core/security/security_connector.c
index dbd79c5b22..61cb20f6b9 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 b7c2859a1c..a475a8dd54 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 192f7acf0e..b2aa2f393e 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 f4ab9d2bc6..ec02a478ba 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 59e5dcff9a..4cd0ec2cfb 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 0edff54235..936b0c25b0 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 d591e43faa..e45602bab7 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);
}
--
GitLab