Add support for IP Addresses in Subject Alt Names.

2 files changed, 108 insertions, 81 deletions

diff --git a/test/core/tsi/transport_security_test.c b/test/core/tsi/transport_security_test.c
index 7ce343987b..f1e7f7f8a8 100644
--- a/test/core/tsi/transport_security_test.c
+++ b/test/core/tsi/transport_security_test.c
@@ -61,35 +61,37 @@ typedef struct {
      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 @@ const cert_name_test_entry cert_name_test_entries[] = {
 
     /* 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 @@ typedef struct {
   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 @@ name_list *name_list_add(const char *n) {
   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 @@ static parsed_dns_names parse_dns_names(const char *dns_names_str) {
   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 void destruct_parsed_dns_names(parsed_dns_names *pdn) {
   }
 }
 
-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 @@ static tsi_peer peer_from_cert_name_test_entry(
   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 8131a14ff7..4759818322 100644
--- a/test/cpp/end2end/end2end_test.cc
+++ b/test/cpp/end2end/end2end_test.cc
@@ -1371,11 +1371,12 @@ TEST_P(SecureEnd2endTest, ClientAuthContext) {
   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]));
   }
 }