Download raw body.
iked: support for leaf cert files containing chains
On Wed, 10 Jul 2024 12:07:46 +0200 (CEST)
YASUOKA Masahiko <yasuoka@openbsd.org> wrote:
> On Wed, 10 Jul 2024 12:09:10 +1000
> Alex Wilson <alex@cooperi.net> 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.
I merged the "error depth" logging part to the diff. Also I got one
positive feedback on offline for the changing behaviour. If there is
no objection, i'd like to commit this.
comments?
ok?
Index: sbin/iked/ca.c
===================================================================
RCS file: /cvs/src/sbin/iked/ca.c,v
diff -u -p -r1.102 ca.c
--- sbin/iked/ca.c 18 Jun 2024 05:08:41 -0000 1.102
+++ sbin/iked/ca.c 12 Jul 2024 16:31:57 -0000
@@ -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 *);
@@ -1036,6 +1039,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
*/
ibuf_free(env->sc_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)
{
@@ -1843,6 +1881,7 @@ ca_validate_cert(struct iked *env, struc
const char *errstr = "failed";
X509_NAME *subj;
char *subj_name;
+ int depth;
if (issuerp)
*issuerp = NULL;
@@ -1906,6 +1945,7 @@ ca_validate_cert(struct iked *env, struc
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 +1976,8 @@ ca_validate_cert(struct iked *env, struc
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:
iked: support for leaf cert files containing chains