Chiebot-Mirror
/
grpc
mirror of https://github.com/grpc/grpc.git
8
0
Fork 0
Code Issues Projects Releases Wiki Activity

Merge pull request #5666 from pquerna/pq/ip_addr_san

Browse Source 
Add support for IP Addresses in Subject Alt Names.
pull/5756/head
jboeuf 9 years ago
parent f9b26d9463 4a9e7c4cb1
commit aa84ef363f
4 changed files with 176 additions and 103 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Split View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 76
      src/core/tsi/ssl_transport_security.c
  2. 5
      src/core/tsi/ssl_transport_security.h
  3. 195
      test/core/tsi/transport_security_test.c
  4. 3
      test/cpp/end2end/end2end_test.cc

76
src/core/tsi/ssl_transport_security.c
Unescape View File

@ -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,9 +33,18 @@
#include "src/core/tsi/ssl_transport_security.h"
#include <grpc/support/port_platform.h>
#include <limits.h>
#include <string.h>
/* TODO(jboeuf): refactor inet_ntop into a portability header. */
#ifdef GPR_WINSOCK_SOCKET
#include <ws2tcpip.h>
#else
#include <arpa/inet.h>
#endif
#include <grpc/support/log.h>
#include <grpc/support/sync.h>
#include <grpc/support/thd.h>
@ -197,13 +206,16 @@ static void ssl_info_callback(const SSL *ssl, int where, int ret) {
}
/* Returns 1 if name looks like an IP address, 0 otherwise.
This is a very rough heuristic as it does not handle IPV6 or things like:
0300.0250.00.01, 0xC0.0Xa8.0x0.0x1, 000030052000001, 0xc0.052000001 */
This is a very rough heuristic, and only handles IPv6 in hexadecimal form. */
static int looks_like_ip_address(const char *name) {
size_t i;
size_t dot_count = 0;
size_t num_size = 0;
for (i = 0; i < strlen(name); i++) {
if (name[i] == ':') {
/* IPv6 Address in hexadecimal form, : is not allowed in DNS names. */
return 1;
}
if (name[i] >= '0' && name[i] <= '9') {
if (num_size > 3) return 0;
num_size++;
@ -296,21 +308,44 @@ static tsi_result add_subject_alt_names_properties_to_peer(
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 +1471,7 @@ 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 *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 +1480,15 @@ int tsi_ssl_peer_matches_name(const tsi_peer *peer, const char *name) {
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 +1497,8 @@ int tsi_ssl_peer_matches_name(const tsi_peer *peer, const char *name) {
}
}
/* 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;

5
src/core/tsi/ssl_transport_security.h
Unescape View File

@ -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
@ -162,8 +162,7 @@ void tsi_ssl_handshaker_factory_destroy(tsi_ssl_handshaker_factory *self);
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

195
test/core/tsi/transport_security_test.c
Unescape View File

@ -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
@ -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,108 @@ 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"},
{0, "2001:db8:a0b:12f0::1", "foo.example.com", NULL, "173.194.195.13"},
{1, "2001:db8:a0b:12f0::1", "foo.example.com", NULL,
"2001:db8:a0b:12f0::1"},
{0, "2001:db8:a0b:12f0::1", "foo.example.com", NULL,
"2001:db8:a0b:12f0::2"},
{1, "2001:db8:a0b:12f0::1", "foo.example.com", NULL,
"2001:db8:a0b:12f0::2,2001:db8:a0b:12f0::1,8.8.8.8"},
};
typedef struct name_list {
@ -196,7 +212,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 +221,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 +243,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 +253,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 +269,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;
}

3
test/cpp/end2end/end2end_test.cc
Unescape View File

@ -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]));
}
}

Write Preview
Loading…
Cancel
Save