Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify handling of IP literal addresses #142

Merged
merged 3 commits into from
Jan 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 13 additions & 40 deletions src/ne_gnutls.c
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand All @@ -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;
Expand All @@ -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: {
Expand All @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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 "
Expand Down
56 changes: 15 additions & 41 deletions src/ne_openssl.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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);
Expand All @@ -279,44 +284,16 @@ 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 {
NE_DEBUG(NE_DBG_SSL, "iPAddress name with unsupported "
"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);
}
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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
Expand All @@ -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"));
Expand Down
2 changes: 2 additions & 0 deletions src/ne_private.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ struct host_info {
/* If override is non-NULL, the host is identified by this network
* address. */
const ne_inet_addr *network;
/* Result of a literal string IP address lookup. */
ne_inet_addr *literal;
struct host_info *next;
};

Expand Down
2 changes: 2 additions & 0 deletions src/ne_request.c
Original file line number Diff line number Diff line change
Expand Up @@ -1606,6 +1606,8 @@ static int do_connect(ne_session *sess, struct host_info *host)
if (sess->local_addr)
ne_sock_prebind(sess->socket, sess->local_addr, 0);

/* Pick the first address, or if the address was pre-determined
(e.g. an IP-literal passed to ne_session_create) fetch that. */
if (host->current == NULL)
host->current = resolve_first(host);

Expand Down
56 changes: 25 additions & 31 deletions src/ne_session.c
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ static void free_hostinfo(struct host_info *hi)
if (hi->hostname) ne_free(hi->hostname);
if (hi->hostport) ne_free(hi->hostport);
if (hi->address) ne_addr_destroy(hi->address);
if (hi->literal) ne_iaddr_free(hi->literal);
}

/* Destroy the sess->proxies array. */
Expand Down Expand Up @@ -148,9 +149,32 @@ static void set_hostport(struct host_info *host, unsigned int defaultport)
static void set_hostinfo(struct host_info *hi, enum proxy_type type,
const char *hostname, unsigned int port)
{
ne_inet_addr *ia;
size_t hlen;

hi->hostname = ne_strdup(hostname);
hi->port = port;
hi->proxy = type;

hlen = strlen(hi->hostname);

/* IP literal parsing */
ia = ne_iaddr_parse(hi->hostname, ne_iaddr_ipv4);
if (!ia && hlen > 4
&& hi->hostname[0] == '[' && hi->hostname[hlen-1] == ']') {
char *v6lit = ne_strdup(hi->hostname + 1);

v6lit[hlen-2] = '\0';

ia = ne_iaddr_parse(v6lit, ne_iaddr_ipv6);
ne_free(v6lit);
}

if (ia) {
NE_DEBUG(NE_DBG_HTTP, "sess: Using IP literal address for %s\n",
hostname);
hi->network = hi->literal = ia;
}
}

ne_session *ne_session_create(const char *scheme,
Expand All @@ -172,21 +196,10 @@ ne_session *ne_session_create(const char *scheme,

#ifdef NE_HAVE_SSL
if (sess->use_ssl) {
ne_inet_addr *ia;

sess->ssl_context = ne_ssl_context_create(0);
sess->flags[NE_SESSFLAG_SSLv2] = 1;

/* If the hostname parses as an IP address, don't
* enable SNI by default. */
ia = ne_iaddr_parse(hostname, ne_iaddr_ipv4);
if (ia == NULL)
ia = ne_iaddr_parse(hostname, ne_iaddr_ipv6);

if (ia) {
ne_iaddr_free(ia);
}
else {
if (!sess->server.literal) {
sess->flags[NE_SESSFLAG_TLS_SNI] = 1;
}
NE_DEBUG(NE_DBG_SSL, "ssl: SNI %s by default.\n",
Expand Down Expand Up @@ -597,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;
Expand Down
17 changes: 10 additions & 7 deletions src/ne_session.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,16 @@ NE_BEGIN_DECLS

typedef struct ne_session_s ne_session;

/* Create a session to the given server, using the given scheme. If
* "https" is passed as the scheme, SSL will be used to connect to the
* server. */
ne_session *ne_session_create(const char *scheme,
const char *hostname, unsigned int port);

/* Finish an HTTP session */
/* Create a session to the server 'host', using the given scheme. If
* "https" is passed as the scheme, TLS will be used to connect to the
* server. The host string must follow the definition of 'host' in RFC
* 3986, which can be an IP-literal or registered (DNS) hostname. An
* IPv6 literal address must be enclosed in square brackets (for
* example "[::1]"). */
ne_session *ne_session_create(const char *scheme, const char *host,
unsigned int port);

/* Finish an HTTP session, freeing associated memory. */
void ne_session_destroy(ne_session *sess);

/* Prematurely force the connection to be closed for the given
Expand Down
2 changes: 1 addition & 1 deletion test/makekeys.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 0 additions & 8 deletions test/openssl.conf
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading
Loading