diff --git a/src/ne_gnutls.c b/src/ne_gnutls.c index 7d318ac8..cc7be228 100644 --- a/src/ne_gnutls.c +++ b/src/ne_gnutls.c @@ -364,7 +364,7 @@ void ne_ssl_cert_validity_time(const ne_ssl_certificate *cert, * If 'identity' is non-NULL, store the malloc-allocated identity in * *identity. If 'server' is non-NULL, it must be the network address * of the server in use, and identity must be NULL. */ -static int check_identity(const ne_uri *server, gnutls_x509_crt_t cert, +static int check_identity(const struct host_info *server, gnutls_x509_crt_t cert, char **identity) { char name[255]; @@ -374,7 +374,7 @@ static int check_identity(const ne_uri *server, gnutls_x509_crt_t cert, size_t len; const char *hostname; - hostname = server ? server->host : ""; + hostname = server ? server->hostname : ""; do { len = sizeof name - 1; @@ -384,7 +384,10 @@ static int check_identity(const ne_uri *server, gnutls_x509_crt_t cert, case GNUTLS_SAN_DNSNAME: name[len] = '\0'; if (identity && !found) *identity = ne_strdup(name); - match = ne__ssl_match_hostname(name, len, hostname); + /* Only match if the server was not identified by a + * literal IP address; avoiding wildcard matches. */ + if (server && !server->literal) + match = ne__ssl_match_hostname(name, len, hostname); found = 1; break; case GNUTLS_SAN_IPADDRESS: { @@ -396,44 +399,17 @@ static int check_identity(const ne_uri *server, gnutls_x509_crt_t cert, else ia = NULL; if (ia) { - char buf[128]; - - match = strcmp(hostname, - ne_iaddr_print(ia, buf, sizeof buf)) == 0; - if (identity) *identity = ne_strdup(buf); + if (server && server->literal) + match = ne_iaddr_cmp(ia, server->literal) == 0; found = 1; ne_iaddr_free(ia); - } else { + } + else { NE_DEBUG(NE_DBG_SSL, "iPAddress name with unsupported " "address type (length %" NE_FMT_SIZE_T "), skipped.\n", len); } } break; - case GNUTLS_SAN_URI: { - ne_uri uri; - - name[len] = '\0'; - - if (ne_uri_parse(name, &uri) == 0 && uri.host && uri.scheme) { - ne_uri tmp; - - if (identity && !found) *identity = ne_strdup(name); - found = 1; - - if (server) { - /* For comparison purposes, all that matters is - * host, scheme and port; ignore the rest. */ - memset(&tmp, 0, sizeof tmp); - tmp.host = uri.host; - tmp.scheme = uri.scheme; - tmp.port = uri.port; - - match = ne_uri_cmp(server, &tmp) == 0; - } - } - - ne_uri_free(&uri); - } break; default: break; @@ -453,7 +429,8 @@ static int check_identity(const ne_uri *server, gnutls_x509_crt_t cert, seq, 0, name, &len); if (ret == 0) { if (identity) *identity = ne_strdup(name); - match = ne__ssl_match_hostname(name, len, hostname); + if (server && !server->literal) + match = ne__ssl_match_hostname(name, len, hostname); } } else { return -1; @@ -940,13 +917,9 @@ static int check_certificate(ne_session *sess, gnutls_session_t sock, ne_ssl_certificate *chain) { int ret, failures = 0; - ne_uri server; unsigned int status; - memset(&server, 0, sizeof server); - ne_fill_server_uri(sess, &server); - ret = check_identity(&server, chain->subject, NULL); - ne_uri_free(&server); + ret = check_identity(&sess->server, chain->subject, NULL); if (ret < 0) { ne_set_error(sess, _("Server certificate was missing commonName " diff --git a/src/ne_openssl.c b/src/ne_openssl.c index ac89d609..6769c4ad 100644 --- a/src/ne_openssl.c +++ b/src/ne_openssl.c @@ -244,13 +244,14 @@ void ne_ssl_cert_validity_time(const ne_ssl_certificate *cert, * identity does not match, or <0 if the certificate had no identity. * If 'identity' is non-NULL, store the malloc-allocated identity in * *identity. Logic specified by RFC 2818 and RFC 3280. */ -static int check_identity(const ne_uri *server, X509 *cert, char **identity) +static int check_identity(const struct host_info *server, X509 *cert, + char **identity) { STACK_OF(GENERAL_NAME) *names; int match = 0, found = 0; const char *hostname; - hostname = server ? server->host : ""; + hostname = server ? server->hostname : ""; names = X509_get_ext_d2i(cert, NID_subject_alt_name, NULL, NULL); if (names) { @@ -264,12 +265,16 @@ static int check_identity(const ne_uri *server, X509 *cert, char **identity) if (nm->type == GEN_DNS) { char *name = dup_ia5string(nm->d.ia5); if (identity && !found) *identity = ne_strdup(name); - match = ne__ssl_match_hostname(name, strlen(name), hostname); + + /* Only match if the server was not identified by a + * literal IP address; avoiding wildcard matches. */ + if (server && !server->literal) + match = ne__ssl_match_hostname(name, strlen(name), hostname); ne_free(name); found = 1; } - else if (nm->type == GEN_IPADD) { - /* compare IP address with server IP address. */ + else if (nm->type == GEN_IPADD && server && server->literal) { + /* compare IP addfress with server literal IP address. */ ne_inet_addr *ia; if (nm->d.ip->length == 4) ia = ne_iaddr_make(ne_iaddr_ipv4, nm->d.ip->data); @@ -279,10 +284,7 @@ static int check_identity(const ne_uri *server, X509 *cert, char **identity) ia = NULL; /* ne_iaddr_make returns NULL if address type is unsupported */ if (ia != NULL) { /* address type was supported. */ - char buf[128]; - - match = strcmp(hostname, - ne_iaddr_print(ia, buf, sizeof buf)) == 0; + match = ne_iaddr_cmp(ia, server->literal) == 0; found = 1; ne_iaddr_free(ia); } else { @@ -290,33 +292,8 @@ static int check_identity(const ne_uri *server, X509 *cert, char **identity) "address type (length %d), skipped.\n", nm->d.ip->length); } - } - else if (nm->type == GEN_URI) { - char *name = dup_ia5string(nm->d.ia5); - ne_uri uri; - - if (ne_uri_parse(name, &uri) == 0 && uri.host && uri.scheme) { - ne_uri tmp; - - if (identity && !found) *identity = ne_strdup(name); - found = 1; - - if (server) { - /* For comparison purposes, all that matters is - * host, scheme and port; ignore the rest. */ - memset(&tmp, 0, sizeof tmp); - tmp.host = uri.host; - tmp.scheme = uri.scheme; - tmp.port = uri.port; - - match = ne_uri_cmp(server, &tmp) == 0; - } - } - - ne_uri_free(&uri); - ne_free(name); } - } + } /* free the whole stack. */ sk_GENERAL_NAME_pop_free(names, GENERAL_NAME_free); } @@ -348,7 +325,8 @@ static int check_identity(const ne_uri *server, X509 *cert, char **identity) return -1; } if (identity) *identity = ne_strdup(cname->data); - match = ne__ssl_match_hostname(cname->data, cname->used - 1, hostname); + if (server && !server->literal) + match = ne__ssl_match_hostname(cname->data, cname->used-1, hostname); ne_buffer_destroy(cname); } @@ -459,7 +437,6 @@ static int check_certificate(ne_session *sess, SSL *ssl, ne_ssl_certificate *cha { X509 *cert = chain->subject; int ret, failures = sess->ssl_context->failures; - ne_uri server; /* If the verification callback hit a case which can't be mapped * to one of the exported error bits, it's treated as a hard @@ -476,10 +453,7 @@ static int check_certificate(ne_session *sess, SSL *ssl, ne_ssl_certificate *cha /* Check certificate was issued to this server; pass URI of * server. */ - memset(&server, 0, sizeof server); - ne_fill_server_uri(sess, &server); - ret = check_identity(&server, cert, NULL); - ne_uri_free(&server); + ret = check_identity(&sess->server, cert, NULL); if (ret < 0) { ne_set_error(sess, _("Server certificate was missing commonName " "attribute in subject name")); diff --git a/src/ne_session.c b/src/ne_session.c index 3fc67d6d..0f7b2d5e 100644 --- a/src/ne_session.c +++ b/src/ne_session.c @@ -610,25 +610,6 @@ int ne__ssl_match_hostname(const char *cn, size_t cnlen, const char *hostname) if (strncmp(cn, "*.", 2) == 0 && cnlen > 2 && (dot = strchr(hostname, '.')) != NULL) { - ne_inet_addr *ia; - - /* Prevent wildcard CN matches against anything which can be - * parsed as an IP address (i.e. a CN of "*.1.1.1" should not - * be match 8.1.1.1). draft-saintandre-tls-server-id-check - * will require some more significant changes to cert ID - * verification which will probably obviate this check, but - * this is a desirable policy tightening in the mean time. */ - ia = ne_iaddr_parse(hostname, ne_iaddr_ipv4); - if (ia == NULL) - ia = ne_iaddr_parse(hostname, ne_iaddr_ipv6); - - if (ia) { - NE_DEBUG(NE_DBG_SSL, "ssl: Denying wildcard match for numeric " - "IP address.\n"); - ne_iaddr_free(ia); - return 0; - } - hostname = dot + 1; cn += 2; cnlen -= 2; diff --git a/test/makekeys.sh b/test/makekeys.sh index 8ee90ae0..8c03bbbb 100755 --- a/test/makekeys.sh +++ b/test/makekeys.sh @@ -196,7 +196,7 @@ ${CA} -startdate `asn1date "2 days ago"` -enddate `asn1date "yesterday"` -in exp ${CA} -startdate `asn1date "tomorrow"` -enddate `asn1date "2 days"` -in notyet.csr -out notyet.cert -for n in 1 2 3 4 5 6 7 8 9; do +for n in 1 2 3 4 5 6 9; do ${CA} -extensions altExt${n} -days 900 \ -in altname${n}.csr -out altname${n}.cert done diff --git a/test/openssl.conf b/test/openssl.conf index 39e95ec6..c96ed759 100644 --- a/test/openssl.conf +++ b/test/openssl.conf @@ -74,14 +74,6 @@ subjectAltName = IP:127.0.0.1 [altExt6] subjectAltName = IP:1.2.3.4 -# an AltName with a good URI -[altExt7] -subjectAltName = URI:https://localhost:7777/ - -# an AltName with a bad URI -[altExt8] -subjectAltName = URI:http://nohost.example.com/ - # AltName with wildcard [altExt9] subjectAltName = DNS:*.example.com diff --git a/test/ssl.c b/test/ssl.c index 1d7c7524..2131d756 100644 --- a/test/ssl.c +++ b/test/ssl.c @@ -592,11 +592,6 @@ static int ipaddr_altname(void) return accept_signed_cert_for_hostname("altname5.cert", "127.0.0.1"); } -static int uri_altname(void) -{ - return accept_signed_cert_for_hostname("altname7.cert", "localhost"); -} - /* test that the *most specific* commonName attribute is used. */ static int multi_commonName(void) { @@ -788,42 +783,25 @@ static int fail_ssl_request_with_error2(char *cert, char *key, char *cacert, const char *msg, int failures, const char *errstr) { - ne_session *sess = ne_session_create("https", host, 7777); + ne_session *sess; int gotf = 0, ret; struct ssl_server_args args = {0}; ne_sock_addr *addr = NULL; const ne_inet_addr **list = NULL; - if (realhost) { - size_t n; - const ne_inet_addr *ia; - - addr = ne_addr_resolve(realhost, 0); - - ONV(ne_addr_result(addr), - ("fake hostname lookup failed for %s", realhost)); - - NE_DEBUG(NE_DBG_SSL, "ssl: Using fake hostname '%s'\n", realhost); - - for (n = 0, ia = ne_addr_first(addr); ia; ia = ne_addr_next(addr)) - n++; - - NE_DEBUG(NE_DBG_SSL, "ssl: Address count '%lu'\n", n); - - list = ne_calloc(n * sizeof(*list)); - - for (n = 0, ia = ne_addr_first(addr); ia; ia = ne_addr_next(addr)) - list[n++] = ia; - - ne_set_addrlist(sess, list, n); - } - args.cert = cert; args.key = key; args.fail_silently = 1; - ret = any_ssl_request(sess, ssl_server, &args, cacert, - get_failures, &gotf); + CALL(fakeproxied_session_server(&sess, "https", host, 7777, ssl_server, &args)); + + if (cacert) { + CALL(load_and_trust_cert(sess, cacert)); + } + + ne_ssl_set_verify(sess, get_failures, &gotf); + + ret = any_request(sess, "/expect-to-fail"); ONV(gotf == 0, ("no error in verification callback; request rv %d error string: %s", @@ -970,7 +948,7 @@ static int fail_missing_CN(void) /* test for a bad ipAddress altname */ static int fail_bad_ipaltname(void) { - return fail_ssl_request("altname6.cert", CA_CERT, get_lh_addr(), + return fail_ssl_request("altname6.cert", CA_CERT, "127.0.0.1", "bad IP altname cert", NE_SSL_IDMISMATCH); } @@ -982,12 +960,6 @@ static int fail_host_ipaltname(void) "bad IP altname cert", NE_SSL_IDMISMATCH); } -static int fail_bad_urialtname(void) -{ - return fail_ssl_request("altname8.cert", CA_CERT, "localhost", - "bad URI altname cert", NE_SSL_IDMISMATCH); -} - static int fail_wildcard(void) { return fail_ssl_request("altname9.cert", CA_CERT, "localhost", @@ -996,7 +968,7 @@ static int fail_wildcard(void) static int fail_wildcard_ip(void) { - return fail_ssl_request("wildip.cert", CA_CERT, get_lh_addr(), + return fail_ssl_request("wildip.cert", CA_CERT, "127.0.0.1", "wildcard IP", NE_SSL_IDMISMATCH); } @@ -1501,7 +1473,6 @@ static int cert_identities(void) { "altname2.cert", "nohost.example.com" }, { "altname4.cert", "localhost" }, { "ca4.pem", "fourth.example.com" }, - { "altname8.cert", "http://nohost.example.com/" }, { NULL, NULL } }; int n; @@ -1958,7 +1929,6 @@ ne_test tests[] = { T(two_subject_altname2), T(notdns_altname), T(ipaddr_altname), - T(uri_altname), T(multi_commonName), T(commonName_first), @@ -1971,7 +1941,6 @@ ne_test tests[] = { T(fail_missing_CN), T(fail_host_ipaltname), T(fail_bad_ipaltname), - T(fail_bad_urialtname), T(fail_wildcard), T(fail_wildcard_ip), T(fail_ca_notyetvalid),