[PATCH] catch expired certificates in the chain
Joe Orton
joe at manyfish.co.uk
Thu Feb 12 10:20:24 EST 2009
On Wed, Feb 11, 2009 at 12:47:32PM +0000, Joe Orton wrote:
> This is what I have in my current wc: it is over-complicated since it
> requires doing a re-verification of the cert. It should be possible to
> hook into the OpenSSL verify callback (SSL_CTX_set_verify) to do this
> properly but my naive attempts to do so caused test failures.
I got it working via that callback, and it's much simpler. I'll commit
the below soon.
Index: src/ne_session.h
===================================================================
--- src/ne_session.h (revision 1617)
+++ src/ne_session.h (working copy)
@@ -1,6 +1,6 @@
/*
HTTP session 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
@@ -218,11 +218,19 @@
* not trusted: there is no indicatation the server is who they claim
* to be: */
#define NE_SSL_UNTRUSTED (0x08)
+/* The certificate chain contained a certificate other than the server
+ * cert which failed verification for a reason other than lack of
+ * trust; for example, due to a CA cert being outside its validity
+ * period: */
+#define NE_SSL_BADCHAIN (0x10)
+/* N.B.: 0x20 is reserved. */
-/* The bitmask of known failure bits: if (failures & ~NE_SSL_FAILMASK)
- * is non-zero, an unrecognized failure is given, and the verification
- * should be failed. */
-#define NE_SSL_FAILMASK (0x0f)
+/* For purposes of forwards-compatibility, the bitmask of all
+ * currently defined failure bits is given as NE_SSL_FAILMASK. If the
+ * expression (failures & ~NE_SSL_FAILMASK) is non-zero a failure type
+ * is present which the application does not recognize but must treat
+ * as a verification failure nonetheless. */
+#define NE_SSL_FAILMASK (0x1f)
/* A callback which is used when server certificate verification is
* needed. The reasons for verification failure are given in the
Index: src/ne_openssl.c
===================================================================
--- src/ne_openssl.c (revision 1617)
+++ src/ne_openssl.c (working copy)
@@ -1,6 +1,6 @@
/*
neon SSL/TLS support using OpenSSL
- Copyright (C) 2002-2008, 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>
@@ -84,6 +84,8 @@
char *friendly_name;
};
+#define NE_SSL_UNHANDLED (0x20) /* failure bit for unhandled case. */
+
/* Append an ASN.1 DirectoryString STR to buffer BUF as UTF-8.
* Returns zero on success or non-zero on error. */
static int append_dirstring(ne_buffer *buf, ASN1_STRING *str)
@@ -342,6 +344,61 @@
return cert;
}
+/* OpenSSL cert verification callback. This is invoked for *each*
+ * error which is encoutered whilst verifying the cert chain; multiple
+ * invocations for any particular cert in the chain are possible. */
+static int verify_callback(int ok, X509_STORE_CTX *ctx)
+{
+ /* OpenSSL, living in its own little happy world of global state,
+ * where userdata was just a twinkle in the eye of an API designer
+ * yet to be born. Or... "Seriously, wtf?" */
+ SSL *ssl = X509_STORE_CTX_get_ex_data(ctx,
+ SSL_get_ex_data_X509_STORE_CTX_idx());
+ ne_session *sess = SSL_get_app_data(ssl);
+ int depth = X509_STORE_CTX_get_error_depth(ctx);
+ int err = X509_STORE_CTX_get_error(ctx);
+ int failures = 0;
+
+ /* If there's no error, nothing to do here. */
+ if (ok) return ok;
+
+ NE_DEBUG(NE_DBG_SSL, "ssl: Verify callback @ %d => %d\n", depth, err);
+
+ /* Map the error code onto any of the exported cert validation
+ * errors, if possible. */
+ switch (err) {
+ case X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT_LOCALLY:
+ case X509_V_ERR_SELF_SIGNED_CERT_IN_CHAIN:
+ case X509_V_ERR_DEPTH_ZERO_SELF_SIGNED_CERT:
+ case X509_V_ERR_CERT_UNTRUSTED:
+ case X509_V_ERR_UNABLE_TO_VERIFY_LEAF_SIGNATURE:
+ failures |= NE_SSL_UNTRUSTED;
+ break;
+ case X509_V_ERR_CERT_NOT_YET_VALID:
+ failures |= depth > 0 ? NE_SSL_BADCHAIN : NE_SSL_NOTYETVALID;
+ break;
+ case X509_V_ERR_CERT_HAS_EXPIRED:
+ failures |= depth > 0 ? NE_SSL_BADCHAIN : NE_SSL_EXPIRED;
+ break;
+ case X509_V_OK:
+ break;
+ default:
+ /* Clear the failures bitmask so check_certificate knows this
+ * is a bailout. */
+ sess->ssl_context->failures |= NE_SSL_UNHANDLED;
+ NE_DEBUG(NE_DBG_SSL, "ssl: Unhandled verification error %d -> %s\n",
+ err, X509_verify_cert_error_string(err));
+ return 0;
+ }
+
+ sess->ssl_context->failures |= failures;
+
+ NE_DEBUG(NE_DBG_SSL, "ssl: Verify failures |= %d => %d\n", failures,
+ sess->ssl_context->failures);
+
+ return 1;
+}
+
/* Return a linked list of certificate objects from an OpenSSL chain. */
static ne_ssl_certificate *make_chain(STACK_OF(X509) *chain)
{
@@ -374,18 +431,22 @@
static int check_certificate(ne_session *sess, SSL *ssl, ne_ssl_certificate *chain)
{
X509 *cert = chain->subject;
- ASN1_TIME *notBefore = X509_get_notBefore(cert);
- ASN1_TIME *notAfter = X509_get_notAfter(cert);
- int ret, failures = 0;
- long result;
+ int ret, failures = sess->ssl_context->failures;
ne_uri server;
- /* check expiry dates */
- if (X509_cmp_current_time(notBefore) >= 0)
- failures |= NE_SSL_NOTYETVALID;
- else if (X509_cmp_current_time(notAfter) <= 0)
- failures |= NE_SSL_EXPIRED;
+ /* 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
+ * failure rather than invoking the callback, which can't present
+ * a useful error to the user. "Um, something is wrong. OK?" */
+ if (failures & NE_SSL_UNHANDLED) {
+ long result = SSL_get_verify_result(ssl);
+ ne_set_error(sess, _("Certificate verification error: %s"),
+ X509_verify_cert_error_string(result));
+
+ return NE_ERROR;
+ }
+
/* Check certificate was issued to this server; pass URI of
* server. */
memset(&server, 0, sizeof server);
@@ -398,38 +459,6 @@
return NE_ERROR;
} else if (ret > 0) failures |= NE_SSL_IDMISMATCH;
- /* get the result of the cert verification out of OpenSSL */
- result = SSL_get_verify_result(ssl);
-
- NE_DEBUG(NE_DBG_SSL, "Verify result: %ld = %s\n", result,
- X509_verify_cert_error_string(result));
-
- switch (result) {
- case X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT_LOCALLY:
- case X509_V_ERR_SELF_SIGNED_CERT_IN_CHAIN:
- case X509_V_ERR_DEPTH_ZERO_SELF_SIGNED_CERT:
- /* TODO: and probably more result codes here... */
- failures |= NE_SSL_UNTRUSTED;
- break;
- /* ignore these, since we've already noticed them: */
- case X509_V_ERR_CERT_NOT_YET_VALID:
- case X509_V_ERR_CERT_HAS_EXPIRED:
- /* cert was trusted: */
- case X509_V_OK:
- break;
- default:
- /* TODO: tricky to handle the 30-odd failure cases OpenSSL
- * presents here (see x509_vfy.h), and present a useful API to
- * the application so it in turn can then present a meaningful
- * UI to the user. The only thing to do really would be to
- * pass back the error string, but that's not localisable. So
- * just fail the verification here - better safe than
- * sorry. */
- ne_set_error(sess, _("Certificate verification error: %s"),
- X509_verify_cert_error_string(result));
- return NE_ERROR;
- }
-
if (failures == 0) {
/* verified OK! */
ret = NE_OK;
@@ -524,6 +553,7 @@
SSL_CTX_set_client_cert_cb(ctx->ctx, provide_client_cert);
/* enable workarounds for buggy SSL server implementations */
SSL_CTX_set_options(ctx->ctx, SSL_OP_ALL);
+ SSL_CTX_set_verify(ctx->ctx, SSL_VERIFY_PEER, verify_callback);
} else if (mode == NE_SSL_CTX_SERVER) {
ctx->ctx = SSL_CTX_new(SSLv23_server_method());
SSL_CTX_set_session_cache_mode(ctx->ctx, SSL_SESS_CACHE_CLIENT);
@@ -608,6 +638,7 @@
sess->flags[NE_SESSFLAG_TLS_SNI] ? sess->server.hostname : NULL;
sess->ssl_cc_requested = 0;
+ ctx->failures = 0;
if (ne_sock_connect_ssl(sess->socket, ctx, sess)) {
if (ctx->sess) {
Index: src/ne_privssl.h
===================================================================
--- src/ne_privssl.h (revision 1617)
+++ src/ne_privssl.h (working copy)
@@ -1,6 +1,6 @@
/*
SSL interface definitions internal to neon.
- Copyright (C) 2003-2005, 2008, Joe Orton <joe at manyfish.co.uk>
+ Copyright (C) 2003-2005, 2008, 2009, Joe Orton <joe at manyfish.co.uk>
Copyright (C) 2004, Aleix Conchillo Flaque <aleix at member.fsf.org>
This library is free software; you can redistribute it and/or
@@ -40,6 +40,7 @@
SSL_CTX *ctx;
SSL_SESSION *sess;
const char *hostname; /* for SNI */
+ int failures; /* bitmask of exposed failure bits. */
};
typedef SSL *ne_ssl_socket;
Index: src/ne_gnutls.c
===================================================================
--- src/ne_gnutls.c (revision 1617)
+++ src/ne_gnutls.c (working copy)
@@ -456,7 +456,9 @@
return match ? 0 : 1;
}
-/* Populate an ne_ssl_certificate structure from an X509 object. */
+/* Populate an ne_ssl_certificate structure from an X509 object. Note
+ * that x5 is owned by returned cert object and must not be otherwise
+ * freed by the caller. */
static ne_ssl_certificate *populate_cert(ne_ssl_certificate *cert,
gnutls_x509_crt x5)
{
@@ -778,15 +780,22 @@
time_t before, after, now = time(NULL);
int ret, failures = 0;
ne_uri server;
+ ne_ssl_certificate *cert;
- before = gnutls_x509_crt_get_activation_time(chain->subject);
- after = gnutls_x509_crt_get_expiration_time(chain->subject);
+ /* Check that all certs within the chain are inside their defined
+ * validity period. Note that the errors flagged for the server
+ * cert are different from the generic error for issues higher up
+ * the chain. */
+ for (cert = chain; cert; cert = cert->issuer) {
+ before = gnutls_x509_crt_get_activation_time(chain->subject);
+ after = gnutls_x509_crt_get_expiration_time(chain->subject);
+
+ if (now < before)
+ failures |= (cert == chain) ? NE_SSL_NOTYETVALID : NE_SSL_BADCHAIN;
+ else if (now > after)
+ failures |= (cert == chain) ? NE_SSL_EXPIRED : NE_SSL_BADCHAIN;
+ }
- if (now < before)
- failures |= NE_SSL_NOTYETVALID;
- else if (now > after)
- failures |= NE_SSL_EXPIRED;
-
memset(&server, 0, sizeof server);
ne_fill_server_uri(sess, &server);
ret = check_identity(&server, chain->subject, NULL);
Index: test/makekeys.sh
===================================================================
--- test/makekeys.sh (revision 1617)
+++ test/makekeys.sh (working copy)
@@ -12,7 +12,8 @@
REQDN=reqDN
STRMASK=default
-export REQDN STRMASK
+CADIR=./ca
+export REQDN STRMASK CADIR
asn1date() {
date -d "$1" "+%y%m%d%H%M%SZ"
@@ -22,17 +23,14 @@
set -ex
-rm -rf ca ca2
-mkdir ca
-touch ca/index.txt
-echo 01 > ca/serial
+for i in ca ca1 ca2 ca3; do
+ rm -rf $i
+ mkdir $i
+ touch $i/index.txt
+ echo 01 > $i/serial
+ ${OPENSSL} genrsa -rand ${srcdir}/../configure > $i/key.pem
+done
-mkdir ca2
-touch ca2/index.txt
-echo 01 > ca2/serial
-
-${OPENSSL} genrsa -rand ${srcdir}/../configure > ca/key.pem
-${OPENSSL} genrsa -rand ${srcdir}/../configure > ca2/key.pem
${OPENSSL} genrsa -rand ${srcdir}/../configure > client.key
${OPENSSL} dsaparam -genkey -rand ${srcdir}/../configure 1024 > client.dsap
@@ -72,6 +70,16 @@
csr_fields IntermediaryCA | ${REQ} -new -key ca2/key.pem -out ca2.csr
${CA} -extensions caExt -days 3560 -in ca2.csr -out ca2/cert.pem
+csr_fields ExpiredCA | ${REQ} -new -key ca1/key.pem -out ca1/cert.csr
+
+csr_fields NotYetValidCA | ${REQ} -new -key ca3/key.pem -out ca3/cert.csr
+
+CADIR=./ca1 ${CA} -name neoncainit -startdate `asn1date "2 days ago"` -enddate `asn1date "yesterday"` \
+ -in ca1/cert.csr -keyfile ca1/key.pem -out ca1/cert.pem -selfsign
+
+CADIR=./ca3 ${CA} -name neoncainit -startdate `asn1date "1 year"` -enddate `asn1date "2 years"` \
+ -in ca3/cert.csr -keyfile ca3/key.pem -out ca3/cert.pem -selfsign
+
csr_fields | ${REQ} -new -key ${srcdir}/server.key -out server.csr
csr_fields | ${REQ} -new -key ${srcdir}/server.key -out expired.csr
@@ -190,11 +198,17 @@
done
# Sign this CSR using the intermediary CA
-${CA} -name neonca2 -days 900 -in server.csr -out ca2server.cert
+CADIR=./ca2 ${CA} -days 900 -in server.csr -out ca2server.cert
# And create a file with the concatenation of both EE and intermediary
# cert.
cat ca2server.cert ca2/cert.pem > ca2server.pem
+
+# sign with expired CA
+CADIR=./ca1 ${CA} -days 3 -in server.csr -out ca1server.cert
+# sign with not yet valid CA
+CADIR=./ca3 ${CA} -days 3 -in server.csr -out ca3server.cert
+
MKPKCS12="${OPENSSL} pkcs12 -export -passout stdin -in client.cert -inkey client.key"
# generate a PKCS12 cert from the client cert: -passOUT because it's the
Index: test/ssl.c
===================================================================
--- test/ssl.c (revision 1617)
+++ test/ssl.c (working copy)
@@ -864,6 +864,18 @@
"subjaltname not honored", NE_SSL_IDMISMATCH);
}
+static int fail_ca_expired(void)
+{
+ return fail_ssl_request("ca1server.cert", "ca1/cert.pem", "localhost",
+ "isser ca expired", NE_SSL_BADCHAIN);
+}
+
+static int fail_ca_notyetvalid(void)
+{
+ return fail_ssl_request("ca3server.cert", "ca3/cert.pem", "localhost",
+ "isser ca not yet valid", NE_SSL_BADCHAIN);
+}
+
/* Test that the SSL session is cached across connections. */
static int session_cache(void)
{
@@ -1763,6 +1775,8 @@
T(fail_bad_ipaltname),
T(fail_bad_urialtname),
T(fail_wildcard),
+ T(fail_ca_notyetvalid),
+ T(fail_ca_expired),
T(session_cache),
Index: test/openssl.conf
===================================================================
--- test/openssl.conf (revision 1617)
+++ test/openssl.conf (working copy)
@@ -2,7 +2,7 @@
default_ca = neonca
[neonca]
-dir = ./ca
+dir = ${ENV::CADIR}
database = $dir/index.txt
new_certs_dir = $dir
certificate = $dir/cert.pem
@@ -13,17 +13,19 @@
x509_extensions = issuedExt
unique_subject = no
-[neonca2]
-dir = ./ca2
+# same as neonca1 just +basicConstraints and without certificate to
+# allow creation of the initial self signed certificate
+[neoncainit]
+dir = ${ENV::CADIR}
database = $dir/index.txt
new_certs_dir = $dir
-certificate = $dir/cert.pem
serial = $dir/serial
private_key = $dir/key.pem
policy = policy_any
default_md = md5
x509_extensions = issuedExt
unique_subject = no
+basicConstraints = CA:TRUE
[policy_any]
countryName = optional
More information about the neon
mailing list