Add support for IP Addresses in Subject Alt Names.
diff --git a/src/core/tsi/ssl_transport_security.c b/src/core/tsi/ssl_transport_security.c
index 6adcaac..7d0c74b 100644
--- a/src/core/tsi/ssl_transport_security.c
+++ b/src/core/tsi/ssl_transport_security.c
@@ -1,6 +1,6 @@
/*
*
- * Copyright 2015, Google Inc.
+ * Copyright 2015-2016, Google Inc.
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
@@ -33,8 +33,15 @@
#include "src/core/tsi/ssl_transport_security.h"
+#include <grpc/support/port_platform.h>
+
#include <limits.h>
#include <string.h>
+#ifdef GPR_WINSOCK_SOCKET
+#include <ws2tcpip.h>
+#else
+#include <arpa/inet.h>
+#endif
#include <grpc/support/log.h>
#include <grpc/support/sync.h>
@@ -296,21 +303,44 @@
sk_GENERAL_NAME_value(subject_alt_names, TSI_SIZE_AS_SIZE(i));
/* Filter out the non-dns entries names. */
if (subject_alt_name->type == GEN_DNS) {
- unsigned char *dns_name = NULL;
- int dns_name_size =
- ASN1_STRING_to_UTF8(&dns_name, subject_alt_name->d.dNSName);
- if (dns_name_size < 0) {
+ unsigned char *name = NULL;
+ int name_size;
+ name_size = ASN1_STRING_to_UTF8(&name, subject_alt_name->d.dNSName);
+ 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_SUBJECT_ALTERNATIVE_NAME_PEER_PROPERTY,
- (const char *)dns_name, (size_t)dns_name_size,
+ TSI_X509_SUBJECT_ALTERNATIVE_NAME_PEER_PROPERTY, (const char *)name,
+ (size_t)name_size, &peer->properties[peer->property_count++]);
+ OPENSSL_free(name);
+ } else if (subject_alt_name->type == GEN_IPADD) {
+ char ntop_buf[INET6_ADDRSTRLEN];
+ int af;
+
+ if (subject_alt_name->d.iPAddress->length == 4) {
+ af = AF_INET;
+ } else if (subject_alt_name->d.iPAddress->length == 16) {
+ af = AF_INET6;
+ } else {
+ gpr_log(GPR_ERROR, "SAN IP Address contained invalid IP");
+ result = TSI_INTERNAL_ERROR;
+ break;
+ }
+ const char *name = inet_ntop(af, subject_alt_name->d.iPAddress->data,
+ ntop_buf, INET6_ADDRSTRLEN);
+ if (name == NULL) {
+ gpr_log(GPR_ERROR, "Could not get IP string from asn1 octet.");
+ result = TSI_INTERNAL_ERROR;
+ break;
+ }
+
+ result = tsi_construct_string_peer_property_from_cstring(
+ TSI_X509_SUBJECT_ALTERNATIVE_NAME_PEER_PROPERTY, name,
&peer->properties[peer->property_count++]);
- OPENSSL_free(dns_name);
- if (result != TSI_OK) break;
}
+ if (result != TSI_OK) break;
}
return result;
}
@@ -1436,9 +1466,7 @@
size_t i = 0;
size_t san_count = 0;
const tsi_peer_property *cn_property = NULL;
-
- /* For now reject what looks like an IP address. */
- if (looks_like_ip_address(name)) return 0;
+ int like_ip = looks_like_ip_address(name);
/* Check the SAN first. */
for (i = 0; i < peer->property_count; i++) {
@@ -1447,8 +1475,15 @@
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)) {
+
+ if (!like_ip && does_entry_match_name(property->value.data,
+ property->value.length, name)) {
+ return 1;
+ } else if (like_ip &&
+ strncmp(name, property->value.data, property->value.length) ==
+ 0 &&
+ strlen(name) == property->value.length) {
+ /* IP Addresses are exact matches only. */
return 1;
}
} else if (strcmp(property->name,
@@ -1457,8 +1492,8 @@
}
}
- /* If there's no SAN, try the CN. */
- if (san_count == 0 && cn_property != NULL) {
+ /* If there's no SAN, try the CN, but only if its not like an IP Address */
+ if (san_count == 0 && cn_property != NULL && !like_ip) {
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 51c0003..b587d7c 100644
--- a/src/core/tsi/ssl_transport_security.h
+++ b/src/core/tsi/ssl_transport_security.h
@@ -162,8 +162,7 @@
Still TODO(jboeuf):
- handle mixed case.
- handle %encoded chars.
- - handle public suffix wildchar more strictly (e.g. *.co.uk)
- - handle IP addresses in SAN. */
+ - handle public suffix wildchar more strictly (e.g. *.co.uk) */
int tsi_ssl_peer_matches_name(const tsi_peer *peer, const char *name);
#ifdef __cplusplus
diff --git a/test/core/tsi/transport_security_test.c b/test/core/tsi/transport_security_test.c
index 7ce3439..f1e7f7f 100644
--- a/test/core/tsi/transport_security_test.c
+++ b/test/core/tsi/transport_security_test.c
@@ -61,35 +61,37 @@
of '#' will be replaced with a null character before processing. */
const char *dns_names;
+ /* Comma separated list of IP SANs to match aggainst */
+ const char *ip_names;
} cert_name_test_entry;
/* Largely inspired from:
chromium/src/net/cert/x509_certificate_unittest.cc.
TODO(jboeuf) uncomment test cases as we fix tsi_ssl_peer_matches_name. */
const cert_name_test_entry cert_name_test_entries[] = {
- {1, "foo.com", "foo.com", NULL},
- {1, "f", "f", NULL},
- {0, "h", "i", NULL},
- {1, "bar.foo.com", "*.foo.com", NULL},
+ {1, "foo.com", "foo.com", NULL, NULL},
+ {1, "f", "f", NULL, NULL},
+ {0, "h", "i", NULL, NULL},
+ {1, "bar.foo.com", "*.foo.com", NULL, NULL},
{1, "www.test.fr", "common.name",
- "*.test.com,*.test.co.uk,*.test.de,*.test.fr"},
+ "*.test.com,*.test.co.uk,*.test.de,*.test.fr", NULL},
/*
{1, "wwW.tESt.fr", "common.name", ",*.*,*.test.de,*.test.FR,www"},
*/
- {0, "f.uk", ".uk", NULL},
- {0, "w.bar.foo.com", "?.bar.foo.com", NULL},
- {0, "www.foo.com", "(www|ftp).foo.com", NULL},
- {0, "www.foo.com", "www.foo.com#", NULL}, /* # = null char. */
- {0, "www.foo.com", "", "www.foo.com#*.foo.com,#,#"},
- {0, "www.house.example", "ww.house.example", NULL},
- {0, "test.org", "", "www.test.org,*.test.org,*.org"},
- {0, "w.bar.foo.com", "w*.bar.foo.com", NULL},
- {0, "www.bar.foo.com", "ww*ww.bar.foo.com", NULL},
- {0, "wwww.bar.foo.com", "ww*ww.bar.foo.com", NULL},
- {0, "wwww.bar.foo.com", "w*w.bar.foo.com", NULL},
- {0, "wwww.bar.foo.com", "w*w.bar.foo.c0m", NULL},
- {0, "WALLY.bar.foo.com", "wa*.bar.foo.com", NULL},
- {0, "wally.bar.foo.com", "*Ly.bar.foo.com", NULL},
+ {0, "f.uk", ".uk", NULL, NULL},
+ {0, "w.bar.foo.com", "?.bar.foo.com", NULL, NULL},
+ {0, "www.foo.com", "(www|ftp).foo.com", NULL, NULL},
+ {0, "www.foo.com", "www.foo.com#", NULL, NULL}, /* # = null char. */
+ {0, "www.foo.com", "", "www.foo.com#*.foo.com,#,#", NULL},
+ {0, "www.house.example", "ww.house.example", NULL, NULL},
+ {0, "test.org", "", "www.test.org,*.test.org,*.org", NULL},
+ {0, "w.bar.foo.com", "w*.bar.foo.com", NULL, NULL},
+ {0, "www.bar.foo.com", "ww*ww.bar.foo.com", NULL, NULL},
+ {0, "wwww.bar.foo.com", "ww*ww.bar.foo.com", NULL, NULL},
+ {0, "wwww.bar.foo.com", "w*w.bar.foo.com", NULL, NULL},
+ {0, "wwww.bar.foo.com", "w*w.bar.foo.c0m", NULL, NULL},
+ {0, "WALLY.bar.foo.com", "wa*.bar.foo.com", NULL, NULL},
+ {0, "wally.bar.foo.com", "*Ly.bar.foo.com", NULL, NULL},
/*
{1, "ww%57.foo.com", "", "www.foo.com"},
{1, "www&.foo.com", "www%26.foo.com", NULL},
@@ -97,94 +99,101 @@
/* Common name must not be used if subject alternative name was provided. */
{0, "www.test.co.jp", "www.test.co.jp",
- "*.test.de,*.jp,www.test.co.uk,www.*.co.jp"},
+ "*.test.de,*.jp,www.test.co.uk,www.*.co.jp", NULL},
{0, "www.bar.foo.com", "www.bar.foo.com",
- "*.foo.com,*.*.foo.com,*.*.bar.foo.com,*..bar.foo.com,"},
+ "*.foo.com,*.*.foo.com,*.*.bar.foo.com,*..bar.foo.com,", NULL},
/* IDN tests */
- {1, "xn--poema-9qae5a.com.br", "xn--poema-9qae5a.com.br", NULL},
- {1, "www.xn--poema-9qae5a.com.br", "*.xn--poema-9qae5a.com.br", NULL},
+ {1, "xn--poema-9qae5a.com.br", "xn--poema-9qae5a.com.br", NULL, NULL},
+ {1, "www.xn--poema-9qae5a.com.br", "*.xn--poema-9qae5a.com.br", NULL, NULL},
{0, "xn--poema-9qae5a.com.br", "",
"*.xn--poema-9qae5a.com.br,"
"xn--poema-*.com.br,"
"xn--*-9qae5a.com.br,"
- "*--poema-9qae5a.com.br"},
+ "*--poema-9qae5a.com.br",
+ NULL},
/* The following are adapted from the examples quoted from
http://tools.ietf.org/html/rfc6125#section-6.4.3
(e.g., *.example.com would match foo.example.com but
not bar.foo.example.com or example.com). */
- {1, "foo.example.com", "*.example.com", NULL},
- {0, "bar.foo.example.com", "*.example.com", NULL},
- {0, "example.com", "*.example.com", NULL},
+ {1, "foo.example.com", "*.example.com", NULL, NULL},
+ {0, "bar.foo.example.com", "*.example.com", NULL, NULL},
+ {0, "example.com", "*.example.com", NULL, NULL},
/* Partial wildcards are disallowed, though RFC 2818 rules allow them.
That is, forms such as baz*.example.net, *baz.example.net, and
b*z.example.net should NOT match domains. Instead, the wildcard must
always be the left-most label, and only a single label. */
- {0, "baz1.example.net", "baz*.example.net", NULL},
- {0, "foobaz.example.net", "*baz.example.net", NULL},
- {0, "buzz.example.net", "b*z.example.net", NULL},
- {0, "www.test.example.net", "www.*.example.net", NULL},
+ {0, "baz1.example.net", "baz*.example.net", NULL, NULL},
+ {0, "foobaz.example.net", "*baz.example.net", NULL, NULL},
+ {0, "buzz.example.net", "b*z.example.net", NULL, NULL},
+ {0, "www.test.example.net", "www.*.example.net", NULL, NULL},
/* Wildcards should not be valid for public registry controlled domains,
and unknown/unrecognized domains, at least three domain components must
be present. */
- {1, "www.test.example", "*.test.example", NULL},
- {1, "test.example.co.uk", "*.example.co.uk", NULL},
- {0, "test.example", "*.example", NULL},
+ {1, "www.test.example", "*.test.example", NULL, NULL},
+ {1, "test.example.co.uk", "*.example.co.uk", NULL, NULL},
+ {0, "test.example", "*.example", NULL, NULL},
/*
{0, "example.co.uk", "*.co.uk", NULL},
*/
- {0, "foo.com", "*.com", NULL},
- {0, "foo.us", "*.us", NULL},
- {0, "foo", "*", NULL},
+ {0, "foo.com", "*.com", NULL, NULL},
+ {0, "foo.us", "*.us", NULL, NULL},
+ {0, "foo", "*", NULL, NULL},
/* IDN variants of wildcards and registry controlled domains. */
- {1, "www.xn--poema-9qae5a.com.br", "*.xn--poema-9qae5a.com.br", NULL},
- {1, "test.example.xn--mgbaam7a8h", "*.example.xn--mgbaam7a8h", NULL},
+ {1, "www.xn--poema-9qae5a.com.br", "*.xn--poema-9qae5a.com.br", NULL, NULL},
+ {1, "test.example.xn--mgbaam7a8h", "*.example.xn--mgbaam7a8h", NULL, NULL},
/*
{0, "xn--poema-9qae5a.com.br", "*.com.br", NULL},
*/
- {0, "example.xn--mgbaam7a8h", "*.xn--mgbaam7a8h", NULL},
+ {0, "example.xn--mgbaam7a8h", "*.xn--mgbaam7a8h", NULL, NULL},
/* Wildcards should be permissible for 'private' registry controlled
domains. */
- {1, "www.appspot.com", "*.appspot.com", NULL},
- {1, "foo.s3.amazonaws.com", "*.s3.amazonaws.com", NULL},
+ {1, "www.appspot.com", "*.appspot.com", NULL, NULL},
+ {1, "foo.s3.amazonaws.com", "*.s3.amazonaws.com", NULL, NULL},
/* Multiple wildcards are not valid. */
- {0, "foo.example.com", "*.*.com", NULL},
- {0, "foo.bar.example.com", "*.bar.*.com", NULL},
+ {0, "foo.example.com", "*.*.com", NULL, NULL},
+ {0, "foo.bar.example.com", "*.bar.*.com", NULL, NULL},
/* Absolute vs relative DNS name tests. Although not explicitly specified
in RFC 6125, absolute reference names (those ending in a .) should
match either absolute or relative presented names. */
- {1, "foo.com", "foo.com.", NULL},
- {1, "foo.com.", "foo.com", NULL},
- {1, "foo.com.", "foo.com.", NULL},
- {1, "f", "f.", NULL},
- {1, "f.", "f", NULL},
- {1, "f.", "f.", NULL},
- {1, "www-3.bar.foo.com", "*.bar.foo.com.", NULL},
- {1, "www-3.bar.foo.com.", "*.bar.foo.com", NULL},
- {1, "www-3.bar.foo.com.", "*.bar.foo.com.", NULL},
- {0, ".", ".", NULL},
- {0, "example.com", "*.com.", NULL},
- {0, "example.com.", "*.com", NULL},
- {0, "example.com.", "*.com.", NULL},
- {0, "foo.", "*.", NULL},
- {0, "foo", "*.", NULL},
+ {1, "foo.com", "foo.com.", NULL, NULL},
+ {1, "foo.com.", "foo.com", NULL, NULL},
+ {1, "foo.com.", "foo.com.", NULL, NULL},
+ {1, "f", "f.", NULL, NULL},
+ {1, "f.", "f", NULL, NULL},
+ {1, "f.", "f.", NULL, NULL},
+ {1, "www-3.bar.foo.com", "*.bar.foo.com.", NULL, NULL},
+ {1, "www-3.bar.foo.com.", "*.bar.foo.com", NULL, NULL},
+ {1, "www-3.bar.foo.com.", "*.bar.foo.com.", NULL, NULL},
+ {0, ".", ".", NULL, NULL},
+ {0, "example.com", "*.com.", NULL, NULL},
+ {0, "example.com.", "*.com", NULL, NULL},
+ {0, "example.com.", "*.com.", NULL, NULL},
+ {0, "foo.", "*.", NULL, NULL},
+ {0, "foo", "*.", NULL, NULL},
/*
{0, "foo.co.uk", "*.co.uk.", NULL},
{0, "foo.co.uk.", "*.co.uk.", NULL},
*/
/* An empty CN is OK. */
- {1, "test.foo.com", "", "test.foo.com"},
+ {1, "test.foo.com", "", "test.foo.com", NULL},
/* An IP should not be used for the CN. */
- {0, "173.194.195.139", "173.194.195.139", NULL},
+ {0, "173.194.195.139", "173.194.195.139", NULL, NULL},
+ /* An IP can be used if the SAN IP is present */
+ {1, "173.194.195.139", "foo.example.com", NULL, "173.194.195.139"},
+ {0, "173.194.195.139", "foo.example.com", NULL, "8.8.8.8"},
+ {0, "173.194.195.139", "foo.example.com", NULL, "8.8.8.8,8.8.4.4"},
+ {1, "173.194.195.139", "foo.example.com", NULL, "8.8.8.8,173.194.195.139"},
+ {0, "173.194.195.139", "foo.example.com", NULL, "173.194.195.13"},
};
typedef struct name_list {
@@ -196,7 +205,7 @@
size_t name_count;
char *buffer;
name_list *names;
-} parsed_dns_names;
+} parsed_names;
name_list *name_list_add(const char *n) {
name_list *result = gpr_malloc(sizeof(name_list));
@@ -205,18 +214,18 @@
return result;
}
-static parsed_dns_names parse_dns_names(const char *dns_names_str) {
- parsed_dns_names result;
+static parsed_names parse_names(const char *names_str) {
+ parsed_names result;
name_list *current_nl;
size_t i;
- memset(&result, 0, sizeof(parsed_dns_names));
- if (dns_names_str == 0) return result;
+ memset(&result, 0, sizeof(parsed_names));
+ if (names_str == 0) return result;
result.name_count = 1;
- result.buffer = gpr_strdup(dns_names_str);
+ result.buffer = gpr_strdup(names_str);
result.names = name_list_add(result.buffer);
current_nl = result.names;
- for (i = 0; i < strlen(dns_names_str); i++) {
- if (dns_names_str[i] == ',') {
+ for (i = 0; i < strlen(names_str); i++) {
+ if (names_str[i] == ',') {
result.buffer[i] = '\0';
result.name_count++;
i++;
@@ -227,7 +236,7 @@
return result;
}
-static void destruct_parsed_dns_names(parsed_dns_names *pdn) {
+static void destruct_parsed_names(parsed_names *pdn) {
name_list *nl = pdn->names;
if (pdn->buffer != NULL) gpr_free(pdn->buffer);
while (nl != NULL) {
@@ -237,8 +246,8 @@
}
}
-static char *processed_dns_name(const char *dns_name) {
- char *result = gpr_strdup(dns_name);
+static char *processed_name(const char *name) {
+ char *result = gpr_strdup(name);
size_t i;
for (i = 0; i < strlen(result); i++) {
if (result[i] == '#') {
@@ -253,31 +262,48 @@
size_t i;
tsi_peer peer;
name_list *nl;
- parsed_dns_names dns_entries = parse_dns_names(entry->dns_names);
+ parsed_names dns_entries = parse_names(entry->dns_names);
+ parsed_names ip_entries = parse_names(entry->ip_names);
nl = dns_entries.names;
- GPR_ASSERT(tsi_construct_peer(1 + dns_entries.name_count, &peer) == TSI_OK);
+ GPR_ASSERT(tsi_construct_peer(
+ 1 + dns_entries.name_count + ip_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);
i = 1;
while (nl != NULL) {
- char *processed = processed_dns_name(nl->name);
+ char *processed = processed_name(nl->name);
GPR_ASSERT(tsi_construct_string_peer_property(
TSI_X509_SUBJECT_ALTERNATIVE_NAME_PEER_PROPERTY, processed,
strlen(nl->name), &peer.properties[i++]) == TSI_OK);
nl = nl->next;
gpr_free(processed);
}
- destruct_parsed_dns_names(&dns_entries);
+
+ nl = ip_entries.names;
+ while (nl != NULL) {
+ char *processed = processed_name(nl->name);
+ GPR_ASSERT(tsi_construct_string_peer_property(
+ TSI_X509_SUBJECT_ALTERNATIVE_NAME_PEER_PROPERTY, processed,
+ strlen(nl->name), &peer.properties[i++]) == TSI_OK);
+ nl = nl->next;
+ gpr_free(processed);
+ }
+ destruct_parsed_names(&dns_entries);
+ destruct_parsed_names(&ip_entries);
return peer;
}
char *cert_name_test_entry_to_string(const cert_name_test_entry *entry) {
char *s;
- gpr_asprintf(
- &s, "{ success = %s, host_name = %s, common_name = %s, dns_names = %s}",
- entry->expected ? "true" : "false", entry->host_name, entry->common_name,
- entry->dns_names != NULL ? entry->dns_names : "");
+ gpr_asprintf(&s,
+ "{ success = %s, host_name = %s, common_name = %s, dns_names = "
+ "%s, ip_names = %s}",
+ entry->expected ? "true" : "false", entry->host_name,
+ entry->common_name,
+ entry->dns_names != NULL ? entry->dns_names : "",
+ entry->ip_names != NULL ? entry->ip_names : "");
return s;
}
diff --git a/test/cpp/end2end/end2end_test.cc b/test/cpp/end2end/end2end_test.cc
index 8131a14..4759818 100644
--- a/test/cpp/end2end/end2end_test.cc
+++ b/test/cpp/end2end/end2end_test.cc
@@ -1371,11 +1371,12 @@
if (GetParam().credentials_type == kTlsCredentialsType) {
EXPECT_EQ("x509_subject_alternative_name",
auth_ctx->GetPeerIdentityPropertyName());
- EXPECT_EQ(3u, auth_ctx->GetPeerIdentity().size());
+ EXPECT_EQ(4u, auth_ctx->GetPeerIdentity().size());
EXPECT_EQ("*.test.google.fr", ToString(auth_ctx->GetPeerIdentity()[0]));
EXPECT_EQ("waterzooi.test.google.be",
ToString(auth_ctx->GetPeerIdentity()[1]));
EXPECT_EQ("*.test.youtube.com", ToString(auth_ctx->GetPeerIdentity()[2]));
+ EXPECT_EQ("192.168.1.3", ToString(auth_ctx->GetPeerIdentity()[3]));
}
}