Rework X509_verify_cert to support alt chains on certificate verification,

via boringssl.
ok jsing@ miod@

1 files changed, 265 insertions, 117 deletions

diff --git a/lib/libcrypto/x509/x509_vfy.c b/lib/libcrypto/x509/x509_vfy.c
index 7a6d2720230..abd5c65e31b 100644
--- a/lib/libcrypto/x509/x509_vfy.c
+++ b/lib/libcrypto/x509/x509_vfy.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: x509_vfy.c,v 1.51 2016/11/04 18:07:23 beck Exp $ */
+/* $OpenBSD: x509_vfy.c,v 1.52 2016/11/06 10:37:38 beck Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -74,6 +74,7 @@
 #include <openssl/x509.h>
 #include <openssl/x509v3.h>
 #include "x509_lcl.h"
+#include "vpm_int.h"
 
 /* CRL score values */
 
@@ -153,41 +154,115 @@ x509_subject_cmp(X509 **a, X509 **b)
 }
 #endif
 
+/* Return 1 is a certificate is self signed */
+static int
+cert_self_signed(X509 *x)
+{
+	X509_check_purpose(x, -1, 0);
+	if (x->ex_flags & EXFLAG_SS)
+		return 1;
+	else
+		return 0;
+}
+
+static int
+check_id_error(X509_STORE_CTX *ctx, int errcode)
+{
+	ctx->error = errcode;
+	ctx->current_cert = ctx->cert;
+	ctx->error_depth = 0;
+	return ctx->verify_cb(0, ctx);
+}
+
+static int
+check_hosts(X509 *x, X509_VERIFY_PARAM_ID *id)
+{
+	size_t i;
+	size_t n = sk_OPENSSL_STRING_num(id->hosts);
+	char *name;
+
+	free(id->peername);
+	id->peername = NULL;
+
+	for (i = 0; i < n; ++i) {
+		name = sk_OPENSSL_STRING_value(id->hosts, i);
+		if (X509_check_host(x, name, strlen(name), id->hostflags,
+		    &id->peername) > 0)
+			return 1;
+	}
+	return n == 0;
+}
+
+static int
+check_id(X509_STORE_CTX *ctx)
+{
+	X509_VERIFY_PARAM *vpm = ctx->param;
+	X509_VERIFY_PARAM_ID *id = vpm->id;
+	X509 *x = ctx->cert;
+
+	if (id->hosts && check_hosts(x, id) <= 0) {
+		if (!check_id_error(ctx, X509_V_ERR_HOSTNAME_MISMATCH))
+			return 0;
+	}
+	if (id->email != NULL && X509_check_email(x, id->email, id->emaillen, 0)
+	    <= 0) {
+		if (!check_id_error(ctx, X509_V_ERR_EMAIL_MISMATCH))
+			return 0;
+	}
+	if (id->ip != NULL && X509_check_ip(x, id->ip, id->iplen, 0) <= 0) {
+		if (!check_id_error(ctx, X509_V_ERR_IP_ADDRESS_MISMATCH))
+			return 0;
+	}
+	return 1;
+}
+
 int
 X509_verify_cert(X509_STORE_CTX *ctx)
 {
-	X509 *x, *xtmp, *chain_ss = NULL;
+	X509 *x, *xtmp, *xtmp2, *chain_ss = NULL;
 	int bad_chain = 0;
 	X509_VERIFY_PARAM *param = ctx->param;
 	int depth, i, ok = 0;
-	int num;
-	int (*cb)(int xok, X509_STORE_CTX *xctx);
+	int num, j, retry, trust;
+	int (*cb) (int xok, X509_STORE_CTX *xctx);
 	STACK_OF(X509) *sktmp = NULL;
-
 	if (ctx->cert == NULL) {
 		X509err(X509_F_X509_VERIFY_CERT,
 		    X509_R_NO_CERT_SET_FOR_US_TO_VERIFY);
+		ctx->error = X509_V_ERR_INVALID_CALL;
+		return -1;
+	}
+	if (ctx->chain != NULL) {
+		/*
+		 * This X509_STORE_CTX has already been used to verify
+		 * a cert. We cannot do another one.
+		 */
+		X509err(X509_F_X509_VERIFY_CERT,
+		    ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED);
+		ctx->error = X509_V_ERR_INVALID_CALL;
 		return -1;
 	}
 
 	cb = ctx->verify_cb;
 
-	/* first we make sure the chain we are going to build is
-	 * present and that the first entry is in place */
-	if (ctx->chain == NULL) {
-		if (((ctx->chain = sk_X509_new_null()) == NULL) ||
-		    (!sk_X509_push(ctx->chain, ctx->cert))) {
-			X509err(X509_F_X509_VERIFY_CERT, ERR_R_MALLOC_FAILURE);
-			goto end;
-		}
-		CRYPTO_add(&ctx->cert->references, 1, CRYPTO_LOCK_X509);
-		ctx->last_untrusted = 1;
+	/*
+	 * First we make sure the chain we are going to build is
+	 * present and that the first entry is in place.
+	 */
+	ctx->chain = sk_X509_new_null();
+	if (ctx->chain == NULL || !sk_X509_push(ctx->chain, ctx->cert)) {
+		X509err(X509_F_X509_VERIFY_CERT, ERR_R_MALLOC_FAILURE);
+		ctx->error = X509_V_ERR_OUT_OF_MEM;
+		goto end;
 	}
+	X509_up_ref(ctx->cert);
+	ctx->last_untrusted = 1;
 
 	/* We use a temporary STACK so we can chop and hack at it */
 	if (ctx->untrusted != NULL &&
 	    (sktmp = sk_X509_dup(ctx->untrusted)) == NULL) {
 		X509err(X509_F_X509_VERIFY_CERT, ERR_R_MALLOC_FAILURE);
+		ctx->error = X509_V_ERR_OUT_OF_MEM;
 		goto end;
 	}
 
@@ -197,17 +272,34 @@ X509_verify_cert(X509_STORE_CTX *ctx)
 
 	for (;;) {
 		/* If we have enough, we break */
+		/* FIXME: If this happens, we should take
+		 * note of it and, if appropriate, use the
+		 * X509_V_ERR_CERT_CHAIN_TOO_LONG error code
+		 * later.
+		 */
 		if (depth < num)
-			break;	/* FIXME: If this happens, we should take
-				 * note of it and, if appropriate, use the
-				 * X509_V_ERR_CERT_CHAIN_TOO_LONG error
-				 * code later.
-				 */
-
+			break;
 		/* If we are self signed, we break */
-		if (ctx->check_issued(ctx, x, x))
+		if (cert_self_signed(x))
 			break;
-
+		/*
+		 * If asked see if we can find issuer in trusted store first
+		 */
+		if (ctx->param->flags & X509_V_FLAG_TRUSTED_FIRST) {
+			ok = ctx->get_issuer(&xtmp, ctx, x);
+			if (ok < 0) {
+				ctx->error = X509_V_ERR_STORE_LOOKUP;
+				goto end;
+			}
+			/*
+			 * If successful for now free up cert so it
+			 * will be picked up again later.
+			 */
+			if (ok > 0) {
+				X509_free(xtmp);
+				break;
+			}
+		}
 		/* If we were passed a cert chain, use it first */
 		if (ctx->untrusted != NULL) {
 			xtmp = find_issuer(ctx, sktmp, x);
@@ -215,109 +307,174 @@ X509_verify_cert(X509_STORE_CTX *ctx)
 				if (!sk_X509_push(ctx->chain, xtmp)) {
 					X509err(X509_F_X509_VERIFY_CERT,
 					    ERR_R_MALLOC_FAILURE);
+					ctx->error = X509_V_ERR_OUT_OF_MEM;
+					ok = 0;
 					goto end;
 				}
-				CRYPTO_add(&xtmp->references, 1,
-				    CRYPTO_LOCK_X509);
+				X509_up_ref(xtmp);
 				(void)sk_X509_delete_ptr(sktmp, xtmp);
 				ctx->last_untrusted++;
 				x = xtmp;
 				num++;
-				/* reparse the full chain for
-				 * the next one */
+				/*
+				 * reparse the full chain for the next one
+				 */
 				continue;
 			}
 		}
 		break;
 	}
-	sk_X509_free(sktmp);
-	sktmp = NULL;
-	
-	/* at this point, chain should contain a list of untrusted
+	/* Remember how many untrusted certs we have */
+	j = num;
+
+	/*
+	 * At this point, chain should contain a list of untrusted
 	 * certificates.  We now need to add at least one trusted one,
-	 * if possible, otherwise we complain. */
-
-	/* Examine last certificate in chain and see if it
- 	 * is self signed.
- 	 */
-
-	i = sk_X509_num(ctx->chain);
-	x = sk_X509_value(ctx->chain, i - 1);
-	if (ctx->check_issued(ctx, x, x)) {
-		/* we have a self signed certificate */
-		if (sk_X509_num(ctx->chain) == 1) {
-			/* We have a single self signed certificate: see if
-			 * we can find it in the store. We must have an exact
-			 * match to avoid possible impersonation.
-			 */
-			ok = ctx->get_issuer(&xtmp, ctx, x);
-			if ((ok <= 0) || X509_cmp(x, xtmp)) {
-				ctx->error =
-				    X509_V_ERR_DEPTH_ZERO_SELF_SIGNED_CERT;
-				ctx->current_cert = x;
-				ctx->error_depth = i - 1;
-				if (ok == 1)
-					X509_free(xtmp);
-				bad_chain = 1;
-				ok = cb(0, ctx);
-				if (!ok)
-					goto end;
+	 * if possible, otherwise we complain.
+	 */
+
+	do {
+		/*
+		 * Examine last certificate in chain and see if it is
+		 * self signed.
+		 */
+		i = sk_X509_num(ctx->chain);
+		x = sk_X509_value(ctx->chain, i - 1);
+		if (cert_self_signed(x)) {
+			/* we have a self signed certificate */
+			if (i == 1) {
+				/*
+				 * We have a single self signed
+				 * certificate: see if we can find it
+				 * in the store. We must have an exact
+				 * match to avoid possible
+				 * impersonation.
+				 */
+				ok = ctx->get_issuer(&xtmp, ctx, x);
+				if ((ok <= 0) || X509_cmp(x, xtmp)) {
+					ctx->error = X509_V_ERR_DEPTH_ZERO_SELF_SIGNED_CERT;
+					ctx->current_cert = x;
+					ctx->error_depth = i - 1;
+					if (ok == 1)
+						X509_free(xtmp);
+					bad_chain = 1;
+					ok = cb(0, ctx);
+					if (!ok)
+						goto end;
+				} else {
+					/*
+					 * We have a match: replace
+					 * certificate with store
+					 * version so we get any trust
+					 * settings.
+					 */
+					X509_free(x);
+					x = xtmp;
+					(void)sk_X509_set(ctx->chain, i - 1, x);
+					ctx->last_untrusted = 0;
+				}
 			} else {
-				/* We have a match: replace certificate with store version
-				 * so we get any trust settings.
+				/*
+				 * extract and save self signed
+				 * certificate for later use
 				 */
-				X509_free(x);
-				x = xtmp;
-				(void)sk_X509_set(ctx->chain, i - 1, x);
-				ctx->last_untrusted = 0;
+				chain_ss = sk_X509_pop(ctx->chain);
+				ctx->last_untrusted--;
+				num--;
+				j--;
+				x = sk_X509_value(ctx->chain, num - 1);
 			}
-		} else {
-			/* extract and save self signed certificate for later use */
-			chain_ss = sk_X509_pop(ctx->chain);
-			ctx->last_untrusted--;
-			num--;
-			x = sk_X509_value(ctx->chain, num - 1);
 		}
-	}
-
-	/* We now lookup certs from the certificate store */
-	for (;;) {
-		/* If we have enough, we break */
-		if (depth < num)
-			break;
+		/* We now lookup certs from the certificate store */
+		for (;;) {
+			/* If we have enough, we break */
+			if (depth < num)
+				break;
+			/* If we are self signed, we break */
+			if (cert_self_signed(x))
+				break;
+			ok = ctx->get_issuer(&xtmp, ctx, x);
 
-		/* If we are self signed, we break */
-		if (ctx->check_issued(ctx, x, x))
-			break;
+			if (ok < 0) {
+				ctx->error = X509_V_ERR_STORE_LOOKUP;
+				goto end;
+			}
+			if (ok == 0)
+				break;
+			x = xtmp;
+			if (!sk_X509_push(ctx->chain, x)) {
+				X509_free(xtmp);
+				X509err(X509_F_X509_VERIFY_CERT,
+				    ERR_R_MALLOC_FAILURE);
+				ctx->error = X509_V_ERR_OUT_OF_MEM;
+				ok = 0;
+				goto end;
+			}
+			num++;
+		}
 
-		ok = ctx->get_issuer(&xtmp, ctx, x);
-		if (ok < 0)
-			return ok;
-		if (ok == 0)
-			break;
+		/* we now have our chain, lets check it... */
+		trust = check_trust(ctx);
 
-		x = xtmp;
-		if (!sk_X509_push(ctx->chain, x)) {
-			X509_free(xtmp);
-			X509err(X509_F_X509_VERIFY_CERT, ERR_R_MALLOC_FAILURE);
-			return 0;
+		/* If explicitly rejected error */
+		if (trust == X509_TRUST_REJECTED) {
+			ok = 0;
+			goto end;
 		}
-		num++;
-	}
-
-	/* we now have our chain, lets check it... */
+		/*
+		 * If it's not explicitly trusted then check if there
+		 * is an alternative chain that could be used. We only
+		 * do this if we haven't already checked via
+		 * TRUSTED_FIRST and the user hasn't switched off
+		 * alternate chain checking
+		 */
+		retry = 0;
+		if (trust != X509_TRUST_TRUSTED &&
+		    !(ctx->param->flags & X509_V_FLAG_TRUSTED_FIRST) &&
+		    !(ctx->param->flags & X509_V_FLAG_NO_ALT_CHAINS)) {
+			while (j-- > 1) {
+				xtmp2 = sk_X509_value(ctx->chain, j - 1);
+				ok = ctx->get_issuer(&xtmp, ctx, xtmp2);
+				if (ok < 0)
+					goto end;
+				/* Check if we found an alternate chain */
+				if (ok > 0) {
+					/*
+					 * Free up the found cert
+					 * we'll add it again later
+					 */
+					X509_free(xtmp);
+					/*
+					 * Dump all the certs above
+					 * this point - we've found an
+					 * alternate chain
+					 */
+					while (num > j) {
+						xtmp = sk_X509_pop(ctx->chain);
+						X509_free(xtmp);
+						num--;
+					}
+					ctx->last_untrusted = sk_X509_num(ctx->chain);
+					retry = 1;
+					break;
+				}
+			}
+		}
+	} while (retry);
 
-	/* Is last certificate looked up self signed? */
-	if (!ctx->check_issued(ctx, x, x)) {
-		if ((chain_ss == NULL) ||
-		    !ctx->check_issued(ctx, x, chain_ss)) {
+	/*
+	 * If not explicitly trusted then indicate error unless it's a single
+	 * self signed certificate in which case we've indicated an error already
+	 * and set bad_chain == 1
+	 */
+	if (trust != X509_TRUST_TRUSTED && !bad_chain) {
+		if ((chain_ss == NULL) || !ctx->check_issued(ctx, x, chain_ss)) {
 			if (ctx->last_untrusted >= num)
 				ctx->error = X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT_LOCALLY;
 			else
 				ctx->error = X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT;
 			ctx->current_cert = x;
 		} else {
-
 			if (!sk_X509_push(ctx->chain, chain_ss)) {
 				X509_free(chain_ss);
 				X509err(X509_F_X509_VERIFY_CERT, ERR_R_MALLOC_FAILURE);
@@ -350,19 +507,13 @@ X509_verify_cert(X509_STORE_CTX *ctx)
 	if (!ok)
 		goto end;
 
-	/* The chain extensions are OK: check trust */
-
-	if (param->trust > 0)
-		ok = check_trust(ctx);
+	ok = check_id(ctx);
 
 	if (!ok)
 		goto end;
-
-	/* We may as well copy down any DSA parameters that are required */
-	X509_get_pubkey_parameters(NULL, ctx->chain);
-
-	/* Check revocation status: we do this after copying parameters
-	 * because they may be needed for CRL signature verification.
+	/*
+	 * Check revocation status: we do this after copying parameters because
+	 * they may be needed for CRL signature verification.
 	 */
 
 	ok = ctx->check_revocation(ctx);
@@ -376,23 +527,20 @@ X509_verify_cert(X509_STORE_CTX *ctx)
 		ok = internal_verify(ctx);
 	if (!ok)
 		goto end;
-
 	/* If we get this far evaluate policies */
 	if (!bad_chain && (ctx->param->flags & X509_V_FLAG_POLICY_CHECK))
 		ok = ctx->check_policy(ctx);
-	if (!ok)
-		goto end;
-	if (0) {
-end:
-		X509_get_pubkey_parameters(NULL, ctx->chain);
-	}
+ end:
 	if (sktmp != NULL)
 		sk_X509_free(sktmp);
 	X509_free(chain_ss);
+
+	/* Safety net, error returns must set ctx->error */
+	if (ok <= 0 && ctx->error == X509_V_OK)
+		ctx->error = X509_V_ERR_UNSPECIFIED;
 	return ok;
 }
 
-
 /* Given a STACK_OF(X509) find the issuer of cert (if any)
  */