Index | Thread | Search

From:
Alex Wilson <alex@cooperi.net>
Subject:
iked: support for leaf cert files containing chains
To:
tech@openbsd.org
Cc:
tobhe@openbsd.org, dlg@openbsd.org
Date:
Wed, 10 Jul 2024 12:09:10 +1000

Download raw body.

Thread
Hi,

At the moment, if you give iked(8) a leaf cert file that contains a 
"chain" (i.e. it's a PEM leaf cert concatenated with an intermediate CA 
cert), the code in ca_reload uses the leaf cert but just discards the chain.

acme-client(1) can output either a "full chain" file (leaf cert with 
chain as above) or a separate leaf cert + chain file, but it refuses to 
put a separated CA chain file in a different directory to the leaf cert 
unless it's a child of the leaf cert dir (since it only unveils the leaf 
cert dir). iked(8) on the other hand wants the leaf certs and CA certs 
in two different sibling directories, so it's kind of awkward using the 
two of them together at the moment.

I would like to use these two together with a private ACME server for 
issuing shortish-term IKE certs (lifetime in double-digit hours). Since 
the intermediate on the ACME server also has a relatively short lifetime 
(months), I would rather not have to separate it off by hand to put it 
in the IKE ca dir and keep that up to date: it would be much simpler if 
acme-client could just update the intermediate for iked directly.

I think the easiest way to do this would be to make iked able to grab 
intermediates out of leaf cert files and add them to its list of CA 
certs, so users who want to use these two together can just use a full 
chain file.

Patch attached for that solution, it also adds the "error depth" to the 
debug output of ca_validate_cert to make it a bit easier to figure out 
what's going wrong when things aren't validating.

We (dlg@ and I) have been running this at work for a while and it hasn't 
been a complete disaster there yet, if that counts for anything.

Thoughts? Opinions?

diff a/sbin/iked/ca.c b/sbin/iked/ca.c
--- a/sbin/iked/ca.c
+++ b/sbin/iked/ca.c
@@ -1005,6 +1005,56 @@ ca_reload(struct iked *env)
 	}
 	closedir(dir);
 
+	/*
+	 * Load certificates
+	 */
+	if ((dir = opendir(IKED_CERT_DIR)) == NULL)
+		return (-1);
+
+	while ((entry = readdir(dir)) != NULL) {
+		if ((entry->d_type != DT_REG) &&
+		    (entry->d_type != DT_LNK))
+			continue;
+
+		if (snprintf(file, sizeof(file), "%s%s",
+		    IKED_CERT_DIR, entry->d_name) < 0)
+			continue;
+
+		if (!X509_load_cert_file(store->ca_certlookup, file,
+		    X509_FILETYPE_PEM)) {
+			log_warn("%s: failed to load cert file %s", __func__,
+			    entry->d_name);
+			ca_sslerror(__func__);
+			continue;
+		}
+		log_debug("%s: loaded cert file %s", __func__, entry->d_name);
+	}
+	closedir(dir);
+
+	/*
+	 * Grab any CA certs (intermediates) which were included in the
+	 * leaf cert files and pull them across into the CA store. This handles
+	 * the case where we were given a cert file with a chain already
+	 * attached (such as e.g. output by acme-client(1))
+	 */
+	h = X509_STORE_get0_objects(store->ca_certs);
+	for (i = 0; i < sk_X509_OBJECT_num(h); i++) {
+		xo = sk_X509_OBJECT_value(h, i);
+		if (X509_OBJECT_get_type(xo) != X509_LU_X509)
+			continue;
+
+		x509 = X509_OBJECT_get0_X509(xo);
+
+		if (X509_check_ca(x509)) {
+			if (X509_STORE_add_cert(store->ca_cas, x509) != 1) {
+				log_warn("%s: failed to add CA cert to store",
+				    __func__);
+				ca_sslerror(__func__);
+				continue;
+			}
+		}
+	}
+
 	/*
 	 * Load CRLs for the CAs
 	 */
@@ -1035,6 +1085,24 @@ ca_reload(struct iked *env)
 	}
 	closedir(dir);
 
+	/*
+	 * Now go back through and validate all our leaf certs. This ensures
+	 * that we will print an error if our leaf certs depend on an
+	 * intermediate that we can't find.
+	 */
+	for (i = 0; i < sk_X509_OBJECT_num(h); i++) {
+		xo = sk_X509_OBJECT_value(h, i);
+		if (X509_OBJECT_get_type(xo) != X509_LU_X509)
+			continue;
+
+		x509 = X509_OBJECT_get0_X509(xo);
+
+		if (X509_check_ca(x509))
+			continue;
+
+		(void)ca_validate_cert(env, NULL, x509, 0, NULL, NULL);
+	}
+
 	/*
 	 * Save CAs signatures for the IKEv2 CERTREQ
 	 */
@@ -1085,43 +1153,6 @@ ca_reload(struct iked *env)
 		    iov, iovcnt);
 	}
 
-	/*
-	 * Load certificates
-	 */
-	if ((dir = opendir(IKED_CERT_DIR)) == NULL)
-		return (-1);
-
-	while ((entry = readdir(dir)) != NULL) {
-		if ((entry->d_type != DT_REG) &&
-		    (entry->d_type != DT_LNK))
-			continue;
-
-		if (snprintf(file, sizeof(file), "%s%s",
-		    IKED_CERT_DIR, entry->d_name) < 0)
-			continue;
-
-		if (!X509_load_cert_file(store->ca_certlookup, file,
-		    X509_FILETYPE_PEM)) {
-			log_warn("%s: failed to load cert file %s", __func__,
-			    entry->d_name);
-			ca_sslerror(__func__);
-			continue;
-		}
-		log_debug("%s: loaded cert file %s", __func__, entry->d_name);
-	}
-	closedir(dir);
-
-	h = X509_STORE_get0_objects(store->ca_certs);
-	for (i = 0; i < sk_X509_OBJECT_num(h); i++) {
-		xo = sk_X509_OBJECT_value(h, i);
-		if (X509_OBJECT_get_type(xo) != X509_LU_X509)
-			continue;
-
-		x509 = X509_OBJECT_get0_X509(xo);
-
-		(void)ca_validate_cert(env, NULL, x509, 0, NULL, NULL);
-	}
-
 	if (!env->sc_certreqtype)
 		env->sc_certreqtype = store->ca_pubkey.id_type;
 
@@ -1843,6 +1874,7 @@ ca_validate_cert(struct iked *env, struct iked_static_id *id,
 	const char		*errstr = "failed";
 	X509_NAME		*subj;
 	char			*subj_name;
+	int			 depth;
 
 	if (issuerp)
 		*issuerp = NULL;
@@ -1906,6 +1938,7 @@ ca_validate_cert(struct iked *env, struct iked_static_id *id,
 
 	result = X509_verify_cert(csc);
 	error = X509_STORE_CTX_get_error(csc);
+	depth = X509_STORE_CTX_get_error_depth(csc);
 	if (error == 0 && issuerp) {
 		if (X509_STORE_CTX_get1_issuer(issuerp, csc, cert) != 1) {
 			log_debug("%s: cannot get issuer", __func__);
@@ -1936,7 +1969,8 @@ ca_validate_cert(struct iked *env, struct iked_static_id *id,
 		subj_name = X509_NAME_oneline(subj, NULL, 0);
 		if (subj_name == NULL)
 			goto err;
-		log_debug("%s: %s %.100s", __func__, subj_name, errstr);
+		log_debug("%s: %s (depth = %d) %.100s", __func__, subj_name,
+		    depth, errstr);
 		OPENSSL_free(subj_name);
 	}
  err: