From: YASUOKA Masahiko Subject: Re: iked: support for leaf cert files containing chains To: alex@cooperi.net Cc: tech@openbsd.org, tobhe@openbsd.org, dlg@openbsd.org Date: Wed, 10 Jul 2024 12:07:46 +0200 On Wed, 10 Jul 2024 12:09:10 +1000 Alex Wilson wrote: > 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? Great. I think this functionality is needed. I also planed to suggest a diff which does almost same thing. Attached at last. The main difference is my diff assumes the first cert of a file is a server's cert and the second and after certificates are intermediate ca's cert. Previously if a file has multiple certificates, they are treated as server certificates. But I suppose that was not intentional. So the diff proposes to change the behavior. diff --git a/sbin/iked/ca.c b/sbin/iked/ca.c index 042bce995f2..5832c862c68 100644 --- a/sbin/iked/ca.c +++ b/sbin/iked/ca.c @@ -44,6 +44,8 @@ #include "iked.h" #include "ikev2.h" +struct ca_store; + void ca_run(struct privsep *, struct privsep_proc *, void *); void ca_shutdown(void); void ca_reset(struct iked *); @@ -53,6 +55,7 @@ int ca_cert_local(struct iked *, X509 *); int ca_getreq(struct iked *, struct imsg *); int ca_getcert(struct iked *, struct imsg *); int ca_getauth(struct iked *, struct imsg *); +int ca_load_cert_file(struct ca_store *, char *); X509 *ca_by_subjectpubkey(X509_STORE *, uint8_t *, size_t); X509 *ca_by_issuer(X509_STORE *, X509_NAME *, struct iked_static_id *); X509 *ca_by_subjectaltname(X509_STORE *, struct iked_static_id *); @@ -1035,6 +1038,31 @@ 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 (!ca_load_cert_file(store, file)) { + 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); + /* * Save CAs signatures for the IKEv2 CERTREQ */ @@ -1085,32 +1113,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); @@ -1137,6 +1139,42 @@ ca_reload(struct iked *env) return (0); } +int +ca_load_cert_file(struct ca_store *store, char *file) +{ + int ret = 0, i, count = 0; + BIO *in = NULL; + X509 *x = NULL; + + in = BIO_new(BIO_s_file()); + + if ((in == NULL) || (BIO_read_filename(in, file) <= 0)) + goto done; + + if ((x = PEM_read_bio_X509_AUX(in, NULL, NULL, "")) == NULL) + goto done; + if ((i = X509_STORE_add_cert(store->ca_certs, x)) == 0) + goto done; + count++; + X509_free(x); + x = NULL; + + while ((x = PEM_read_bio_X509_AUX(in, NULL, NULL, "")) != NULL) { + i = X509_STORE_add_cert(store->ca_cas, x); + if (i == 0) + goto done; + count++; + X509_free(x); + x = NULL; + } + ret = count; +done: + X509_free(x); + BIO_free(in); + + return (ret); +} + X509 * ca_by_subjectpubkey(X509_STORE *ctx, uint8_t *sig, size_t siglen) {