commit: r1684 - in neon/branches/0.28.x: src test
joe at manyfish.co.uk
joe at manyfish.co.uk
Tue Aug 18 09:35:23 EDT 2009
Author: joe
Date: Tue Aug 18 06:35:23 2009
New Revision: 1684
Added:
neon/branches/0.28.x/test/nulca.pem
- copied unchanged from r1681, /neon/trunk/test/nulca.pem
neon/branches/0.28.x/test/nulcn.pem
- copied unchanged from r1681, /neon/trunk/test/nulcn.pem
neon/branches/0.28.x/test/nulsan.pem
- copied unchanged from r1681, /neon/trunk/test/nulsan.pem
neon/branches/0.28.x/test/nulsrv.key
- copied unchanged from r1681, /neon/trunk/test/nulsrv.key
Modified:
neon/branches/0.28.x/src/ne_gnutls.c
neon/branches/0.28.x/src/ne_openssl.c
neon/branches/0.28.x/src/ne_private.h
neon/branches/0.28.x/src/ne_session.c
neon/branches/0.28.x/src/ne_string.c
neon/branches/0.28.x/src/ne_string.h
neon/branches/0.28.x/test/ssl.c
Log:
Merge r1676, r1680 from trunk: modified to make new functions private.
* src/ne_string.c (ne__buffer_qappend): New function.
* src/ne_string.c (qappend_count, quoted_append): Factor out from
ne_buffer_qappend.
(ne__strnqdup): New function.
* src/ne_private.h (ne__strnqdup): New prototype.
* src/ne_string.h (ne__buffer_qappend): New prototype.
Merge r1681 from trunk:
Security fix for CVE-2009-2474, handling of "NUL" bytes in certificate
names:
* src/ne_private.h (ne__ssl_match_hostname): Take cn len, make cn
const.
* src/ne_session.c (ne__ssl_match_hostname): Drop handling of
unqualified hostnames; check CN length matches.
* src/ne_gnutls.c (check_identity): Adjust accordingly.
* src/ne_openssl.c (append_dirstring): Use a quoted append for ASCII
data. Check for embedded NUL bytes in UTF-8 data.
(dup_ia5string): Use quoted append.
* test/ssl.c (struct ssl_server_args): Add key field.
(ssl_server): Use key field from args.
(fail_ssl_request_with_error2): Rename from
fail_ssl_request_with_error, add host, fakehost
parameters.
(fail_ssl_request_with_error): Reimplement using
fail_ssl_request_with_error2.
(fail_nul_cn, fail_nul_san, nulcn_identity): New tests.
* test/nulca.pem, test/nulcn.pem, test/nulsan.pem, test/nulsrv.key:
Add test cases, thanks to Tomas Hoger <thoger redhat.com>.
Modified: neon/branches/0.28.x/src/ne_gnutls.c
==============================================================================
--- neon/branches/0.28.x/src/ne_gnutls.c (original)
+++ neon/branches/0.28.x/src/ne_gnutls.c Tue Aug 18 06:35:23 2009
@@ -350,7 +350,7 @@
case GNUTLS_SAN_DNSNAME:
name[len] = '\0';
if (identity && !found) *identity = ne_strdup(name);
- match = ne__ssl_match_hostname(name, hostname);
+ match = ne__ssl_match_hostname(name, len, hostname);
found = 1;
break;
case GNUTLS_SAN_IPADDRESS: {
@@ -419,7 +419,7 @@
seq, 0, name, &len);
if (ret == 0) {
if (identity) *identity = ne_strdup(name);
- match = ne__ssl_match_hostname(name, hostname);
+ match = ne__ssl_match_hostname(name, len, hostname);
}
} else {
return -1;
Modified: neon/branches/0.28.x/src/ne_openssl.c
==============================================================================
--- neon/branches/0.28.x/src/ne_openssl.c (original)
+++ neon/branches/0.28.x/src/ne_openssl.c Tue Aug 18 06:35:23 2009
@@ -1,6 +1,6 @@
/*
neon SSL/TLS support using OpenSSL
- Copyright (C) 2002-2007, Joe Orton <joe at manyfish.co.uk>
+ Copyright (C) 2002-2009, Joe Orton <joe at manyfish.co.uk>
Portions are:
Copyright (C) 1999-2000 Tommi Komulainen <Tommi.Komulainen at iki.fi>
@@ -92,10 +92,16 @@
int len;
switch (str->type) {
- case V_ASN1_UTF8STRING:
case V_ASN1_IA5STRING: /* definitely ASCII */
case V_ASN1_VISIBLESTRING: /* probably ASCII */
case V_ASN1_PRINTABLESTRING: /* subset of ASCII */
+ ne__buffer_qappend(buf, str->data, str->length);
+ break;
+ case V_ASN1_UTF8STRING:
+ /* Fail for embedded NUL bytes. */
+ if (strlen((char *)str->data) != (size_t)str->length) {
+ return -1;
+ }
ne_buffer_append(buf, (char *)str->data, str->length);
break;
case V_ASN1_UNIVERSALSTRING:
@@ -103,8 +109,15 @@
case V_ASN1_BMPSTRING:
len = ASN1_STRING_to_UTF8(&tmp, str);
if (len > 0) {
- ne_buffer_append(buf, (char *)tmp, len);
- OPENSSL_free(tmp);
+ /* Fail if there were embedded NUL bytes. */
+ if (strlen((char *)tmp) != (size_t)len) {
+ OPENSSL_free(tmp);
+ return -1;
+ }
+ else {
+ ne_buffer_append(buf, (char *)tmp, len);
+ OPENSSL_free(tmp);
+ }
break;
} else {
ERR_clear_error();
@@ -119,13 +132,11 @@
return 0;
}
-/* Returns a malloc-allocate version of IA5 string AS. Really only
- * here to prevent char * vs unsigned char * type mismatches without
- * losing all hope at type-safety. */
+/* Returns a malloc-allocated version of IA5 string AS, escaped for
+ * safety. */
static char *dup_ia5string(const ASN1_IA5STRING *as)
{
- unsigned char *data = as->data;
- return ne_strndup((char *)data, as->length);
+ return ne__strnqdup(as->data, as->length);
}
char *ne_ssl_readable_dname(const ne_ssl_dname *name)
@@ -236,7 +247,7 @@
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, hostname);
+ match = ne__ssl_match_hostname(name, strlen(name), hostname);
ne_free(name);
found = 1;
}
@@ -320,7 +331,7 @@
return -1;
}
if (identity) *identity = ne_strdup(cname->data);
- match = ne__ssl_match_hostname(cname->data, hostname);
+ match = ne__ssl_match_hostname(cname->data, cname->used - 1, hostname);
ne_buffer_destroy(cname);
}
Modified: neon/branches/0.28.x/src/ne_private.h
==============================================================================
--- neon/branches/0.28.x/src/ne_private.h (original)
+++ neon/branches/0.28.x/src/ne_private.h Tue Aug 18 06:35:23 2009
@@ -1,6 +1,6 @@
/*
HTTP Request Handling
- Copyright (C) 1999-2008, Joe Orton <joe at manyfish.co.uk>
+ Copyright (C) 1999-2009, Joe Orton <joe at manyfish.co.uk>
This library is free software; you can redistribute it and/or
modify it under the terms of the GNU Library General Public
@@ -128,8 +128,17 @@
void ne__ssl_set_verify_err(ne_session *sess, int failures);
/* Return non-zero if hostname from certificate (cn) matches hostname
- * used for session (hostname); follows RFC2818 logic. cn is modified
- * in-place. */
-int ne__ssl_match_hostname(char *cn, const char *hostname);
+ * used for session (hostname); follows RFC2818 logic. */
+int ne__ssl_match_hostname(const char *cn, size_t cnlen, const char *hostname);
+
+/* Return a malloc-allocated copy of 'data', of length 'len', with all
+ * non-ASCII bytes, and ASCII control characters escaped. (Note that
+ * the escaping includes the NUL byte). */
+char *ne__strnqdup(const unsigned char *data, size_t len);
+
+/* Append 'len' bytes of 'data' to buf. All non-ASCII bytes, and
+ * ASCII control characters, are escaped. (Note that this includes
+ * the NUL byte). */
+void ne__buffer_qappend(ne_buffer *buf, const unsigned char *data, size_t len);
#endif /* HTTP_PRIVATE_H */
Modified: neon/branches/0.28.x/src/ne_session.c
==============================================================================
--- neon/branches/0.28.x/src/ne_session.c (original)
+++ neon/branches/0.28.x/src/ne_session.c Tue Aug 18 06:35:23 2009
@@ -403,24 +403,21 @@
/* This doesn't actually implement complete RFC 2818 logic; omits
* "f*.example.com" support for simplicity. */
-int ne__ssl_match_hostname(char *cn, const char *hostname)
+int ne__ssl_match_hostname(const char *cn, size_t cnlen, const char *hostname)
{
const char *dot;
- dot = strchr(hostname, '.');
- if (dot == NULL) {
- char *pnt = strchr(cn, '.');
- /* hostname is not fully-qualified; unqualify the cn. */
- if (pnt != NULL) {
- *pnt = '\0';
- }
- }
- else if (strncmp(cn, "*.", 2) == 0) {
+ NE_DEBUG(NE_DBG_SSL, "ssl: Match common name '%s' against '%s'\n",
+ cn, hostname);
+
+ if (strncmp(cn, "*.", 2) == 0 && cnlen > 2
+ && (dot = strchr(hostname, '.')) != NULL) {
hostname = dot + 1;
cn += 2;
+ cnlen -= 2;
}
- return !ne_strcasecmp(cn, hostname);
+ return cnlen == strlen(hostname) && !ne_strcasecmp(cn, hostname);
}
#endif /* NE_HAVE_SSL */
Modified: neon/branches/0.28.x/src/ne_string.c
==============================================================================
--- neon/branches/0.28.x/src/ne_string.c (original)
+++ neon/branches/0.28.x/src/ne_string.c Tue Aug 18 06:35:23 2009
@@ -1,6 +1,6 @@
/*
String utility functions
- Copyright (C) 1999-2007, Joe Orton <joe at manyfish.co.uk>
+ Copyright (C) 1999-2007, 2009, Joe Orton <joe at manyfish.co.uk>
strcasecmp/strncasecmp implementations are:
Copyright (C) 1991, 1992, 1995, 1996, 1997 Free Software Foundation, Inc.
This file is part of the GNU C Library.
@@ -38,6 +38,8 @@
#include "ne_alloc.h"
#include "ne_string.h"
+/* hack for 0.28.x backport of ne_strnqdup, ne_buffer_qappend */
+#include "ne_private.h"
char *ne_token(char **str, char separator)
{
@@ -252,6 +254,98 @@
buf->used = strlen(buf->data) + 1;
}
+
+/* ascii_quote[n] gives the number of bytes needed by
+ * ne_buffer_qappend() to append character 'n'. */
+static const unsigned char ascii_quote[256] = {
+ 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4,
+ 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4,
+ 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1,
+ 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1,
+ 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1,
+ 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1,
+ 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1,
+ 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 4,
+ 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4,
+ 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4,
+ 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4,
+ 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4,
+ 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4,
+ 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4,
+ 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4,
+ 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4
+};
+
+static const char hex_chars[16] = "0123456789ABCDEF";
+
+/* Return the expected number of bytes needed to append the string
+ * beginning at byte 's', where 'send' points to the last byte after
+ * 's'. */
+static size_t qappend_count(const unsigned char *s, const unsigned char *send)
+{
+ const unsigned char *p;
+ size_t ret;
+
+ for (p = s, ret = 0; p < send; p++) {
+ ret += ascii_quote[*p];
+ }
+
+ return ret;
+}
+
+/* Append the string 's', up to but not including 'send', to string
+ * 'dest', quoting along the way. Returns pointer to NUL. */
+static char *quoted_append(char *dest, const unsigned char *s,
+ const unsigned char *send)
+{
+ const unsigned char *p;
+ char *q = dest;
+
+ for (p = s; p < send; p++) {
+ if (ascii_quote[*p] == 1) {
+ *q++ = *p;
+ }
+ else {
+ *q++ = '\\';
+ *q++ = 'x';
+ *q++ = hex_chars[(*p >> 4) & 0x0f];
+ *q++ = hex_chars[*p & 0x0f];
+ }
+ }
+
+ /* NUL terminate after the last character */
+ *q = '\0';
+
+ return q;
+}
+
+void ne__buffer_qappend(ne_buffer *buf, const unsigned char *data, size_t len)
+{
+ const unsigned char *dend = data + len;
+ char *q, *qs;
+
+ ne_buffer_grow(buf, buf->used + qappend_count(data, dend));
+
+ /* buf->used >= 1, so this is safe. */
+ qs = buf->data + buf->used - 1;
+
+ q = quoted_append(qs, data, dend);
+
+ /* used already accounts for a NUL, so increment by number of
+ * characters appended, *before* the NUL. */
+ buf->used += q - qs;
+}
+
+char *ne__strnqdup(const unsigned char *data, size_t len)
+{
+ const unsigned char *dend = data + len;
+ char *dest = malloc(qappend_count(data, dend) + 1);
+
+ quoted_append(dest, data, dend);
+
+ return dest;
+}
+
static const char b64_alphabet[] =
"ABCDEFGHIJKLMNOPQRSTUVWXYZ"
"abcdefghijklmnopqrstuvwxyz"
@@ -345,9 +439,9 @@
return outp - *out;
}
-/* Character map array; array[n] = isprint(n) ? 0x20 : n. Used by
- * ne_strclean as a locale-independent isprint(). */
-static const unsigned char ascii_printable[256] = {
+/* Character map array; ascii_clean[n] = isprint(n) ? n : 0x20. Used
+ * by ne_strclean as a locale-independent isprint(). */
+static const unsigned char ascii_clean[256] = {
0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20,
0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20,
0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20,
@@ -387,7 +481,7 @@
unsigned char *pnt;
for (pnt = (unsigned char *)str; *pnt; pnt++)
- *pnt = (char)ascii_printable[*pnt];
+ *pnt = (char)ascii_clean[*pnt];
return str;
}
Modified: neon/branches/0.28.x/src/ne_string.h
==============================================================================
--- neon/branches/0.28.x/src/ne_string.h (original)
+++ neon/branches/0.28.x/src/ne_string.h Tue Aug 18 06:35:23 2009
@@ -1,6 +1,6 @@
/*
String utility functions
- Copyright (C) 1999-2008, Joe Orton <joe at manyfish.co.uk>
+ Copyright (C) 1999-2009, Joe Orton <joe at manyfish.co.uk>
This library is free software; you can redistribute it and/or
modify it under the terms of the GNU Library General Public
Modified: neon/branches/0.28.x/test/ssl.c
==============================================================================
--- neon/branches/0.28.x/test/ssl.c (original)
+++ neon/branches/0.28.x/test/ssl.c Tue Aug 18 06:35:23 2009
@@ -1,6 +1,6 @@
/*
neon test suite
- Copyright (C) 2002-2008, Joe Orton <joe at manyfish.co.uk>
+ Copyright (C) 2002-2009, Joe Orton <joe at manyfish.co.uk>
This program is free software; you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
@@ -86,6 +86,8 @@
int count; /* internal use. */
int use_ssl2; /* force use of SSLv2 only */
+
+ const char *key;
};
/* default response string if args->response is NULL */
@@ -97,6 +99,7 @@
struct ssl_server_args *args = userdata;
int ret;
char buf[BUFSIZ];
+ const char *key;
static ne_ssl_context *ctx = NULL;
if (ctx == NULL) {
@@ -106,11 +109,13 @@
ONV(ctx == NULL, ("could not create SSL context"));
- ONV(ne_ssl_context_keypair(ctx, args->cert, server_key),
+ key = args->key ? args->key : server_key;
+ NE_DEBUG(NE_DBG_HTTP, "SSL server init with keypair (%s, %s).\n",
+ args->cert, key);
+
+ ONV(ne_ssl_context_keypair(ctx, args->cert, key),
("failed to load server keypair: ..."));
- NE_DEBUG(NE_DBG_HTTP, "using server cert %s\n", args->cert);
-
if (args->require_cc && !args->ca_list) {
args->ca_list = CA_CERT;
}
@@ -720,17 +725,37 @@
/* Helper function: run a request using the given self-signed server
* certificate, and expect the request to fail with the given
* verification failure flags. */
-static int fail_ssl_request(char *cert, char *cacert, const char *host,
- const char *msg, int failures)
+static int fail_ssl_request_with_error2(char *cert, char *key, char *cacert,
+ const char *host, const char *fakehost,
+ const char *msg, int failures,
+ const char *errstr)
{
ne_session *sess = ne_session_create("https", host, 7777);
int gotf = 0, ret;
+ struct ssl_server_args args = {0};
+ ne_sock_addr *addr;
+ const ne_inet_addr *list[1];
- ret = any_ssl_request(sess, fail_serve, cert, cacert,
+ if (fakehost) {
+ addr = ne_addr_resolve(fakehost, 0);
+
+ ONV(ne_addr_result(addr),
+ ("fake hostname lookup failed for %s", fakehost));
+
+ list[0] = ne_addr_first(addr);
+
+ ne_set_addrlist(sess, list, 1);
+ }
+
+ args.cert = cert;
+ args.key = key;
+ args.fail_silently = 1;
+
+ ret = any_ssl_request(sess, ssl_server, &args, cacert,
get_failures, &gotf);
ONV(gotf == 0,
- ("no error in verification callback; request failed: %s",
+ ("no error in verification callback; error string: %s",
ne_get_error(sess)));
ONV(gotf & ~NE_SSL_FAILMASK,
@@ -748,6 +773,16 @@
return OK;
}
+/* Helper function: run a request using the given self-signed server
+ * certificate, and expect the request to fail with the given
+ * verification failure flags. */
+static int fail_ssl_request(char *cert, char *cacert, const char *host,
+ const char *msg, int failures)
+{
+ return fail_ssl_request_with_error2(cert, cacert, NULL, host, NULL,
+ msg, failures, NULL);
+}
+
/* Note that the certs used for fail_* are mostly self-signed, so the
* cert is passed as CA cert and server cert to fail_ssl_request. */
@@ -760,6 +795,24 @@
NE_SSL_IDMISMATCH);
}
+static int fail_nul_cn(void)
+{
+ return fail_ssl_request_with_error2("nulcn.pem", "nulsrv.key", "nulca.pem",
+ "www.bank.com", "localhost",
+ "certificate with incorrect CN was accepted",
+ NE_SSL_IDMISMATCH,
+ "certificate issued for a different hostname");
+}
+
+static int fail_nul_san(void)
+{
+ return fail_ssl_request_with_error2("nulsan.pem", "nulsrv.key", "nulca.pem",
+ "www.bank.com", "localhost",
+ "certificate with incorrect CN was accepted",
+ NE_SSL_IDMISMATCH,
+ "certificate issued for a different hostname");
+}
+
/* Check that an expired certificate is flagged as such. */
static int fail_expired(void)
{
@@ -1292,6 +1345,24 @@
return OK;
}
+static int nulcn_identity(void)
+{
+ ne_ssl_certificate *cert = ne_ssl_cert_read("nulcn.pem");
+ const char *id, *expected = "www.bank.com\\x00.badguy.com";
+
+ ONN("could not read nulcn.pem", cert == NULL);
+
+ id = ne_ssl_cert_identity(cert);
+
+ ONV(id != NULL
+ && strcmp(id, expected) != 0,
+ ("certificate `nulcn.pem' had identity `%s' not `%s'",
+ id, expected));
+
+ ne_ssl_cert_free(cert);
+ return OK;
+}
+
static int check_validity(const char *fname,
const char *from, const char *until)
{
@@ -1722,6 +1793,10 @@
T(fail_bad_ipaltname),
T(fail_bad_urialtname),
+ T(nulcn_identity),
+ T(fail_nul_cn),
+ T(fail_nul_san),
+
T(session_cache),
T(fail_tunnel),
More information about the neon-commits
mailing list