From 1d8c64435016c723a0f6b5b79669a39bcb132a60 Mon Sep 17 00:00:00 2001 From: tb Date: Wed, 12 Jun 2024 10:03:09 +0000 Subject: [PATCH] rpki-client: avoid hard error when hitting the maximum cert id Instead, continue processing what we can but avoid lots of warning noise. Error out at the end of the parser process to avoid loading a bad config into bgpd. This isn't great as it is and can be refined in tree. ok claudio --- usr.sbin/rpki-client/cert.c | 12 ++++++---- usr.sbin/rpki-client/crl.c | 5 +--- usr.sbin/rpki-client/filemode.c | 6 ++++- usr.sbin/rpki-client/parser.c | 42 ++++++++++++++++++++++++++++++--- usr.sbin/rpki-client/validate.c | 5 +--- 5 files changed, 54 insertions(+), 16 deletions(-) diff --git a/usr.sbin/rpki-client/cert.c b/usr.sbin/rpki-client/cert.c index c0c32a92599..c6a762fa3d3 100644 --- a/usr.sbin/rpki-client/cert.c +++ b/usr.sbin/rpki-client/cert.c @@ -1,4 +1,4 @@ -/* $OpenBSD: cert.c,v 1.147 2024/06/12 04:01:20 tb Exp $ */ +/* $OpenBSD: cert.c,v 1.148 2024/06/12 10:03:09 tb Exp $ */ /* * Copyright (c) 2022 Theo Buehler * Copyright (c) 2021 Job Snijders @@ -34,7 +34,7 @@ extern ASN1_OBJECT *carepo_oid; /* 1.3.6.1.5.5.7.48.5 (caRepository) */ extern ASN1_OBJECT *manifest_oid; /* 1.3.6.1.5.5.7.48.10 (rpkiManifest) */ extern ASN1_OBJECT *notify_oid; /* 1.3.6.1.5.5.7.48.13 (rpkiNotify) */ -static int certid = TALSZ_MAX; +int certid = TALSZ_MAX; /* * Append an IP address structure to our list of results. @@ -1271,8 +1271,12 @@ auth_insert(const char *fn, struct auth_tree *auths, struct cert *cert, cert->certid = cert->talid; } else { cert->certid = ++certid; - if (certid > CERTID_MAX) - errx(1, "%s: too many certificates in store", fn); + if (certid > CERTID_MAX) { + if (certid == CERTID_MAX + 1) + warnx("%s: too many certificates in store", fn); + free(na); + return NULL; + } na->depth = issuer->depth + 1; } diff --git a/usr.sbin/rpki-client/crl.c b/usr.sbin/rpki-client/crl.c index 5ec57c9b3cf..0cf97caff99 100644 --- a/usr.sbin/rpki-client/crl.c +++ b/usr.sbin/rpki-client/crl.c @@ -1,4 +1,4 @@ -/* $OpenBSD: crl.c,v 1.40 2024/06/11 15:33:46 tb Exp $ */ +/* $OpenBSD: crl.c,v 1.41 2024/06/12 10:03:09 tb Exp $ */ /* * Copyright (c) 2024 Theo Buehler * Copyright (c) 2019 Kristaps Dzonsons @@ -296,9 +296,6 @@ crl_get(struct crl_tree *crlt, const struct auth *a) { struct crl find; - if (a == NULL) - return NULL; - find.aki = a->cert->ski; find.mftpath = a->cert->mft; diff --git a/usr.sbin/rpki-client/filemode.c b/usr.sbin/rpki-client/filemode.c index 689817c7c1b..70c6cf11259 100644 --- a/usr.sbin/rpki-client/filemode.c +++ b/usr.sbin/rpki-client/filemode.c @@ -1,4 +1,4 @@ -/* $OpenBSD: filemode.c,v 1.45 2024/06/08 13:34:59 tb Exp $ */ +/* $OpenBSD: filemode.c,v 1.46 2024/06/12 10:03:09 tb Exp $ */ /* * Copyright (c) 2019 Claudio Jeker * Copyright (c) 2019 Kristaps Dzonsons @@ -191,6 +191,10 @@ parse_load_certchain(char *uri) for (i = 0; i < MAX_CERT_DEPTH; i++) { if ((cert = uripath_lookup(uri)) != NULL) { a = auth_find(&auths, cert->certid); + if (a == NULL) { + warnx("failed to find issuer for %s", uri); + goto fail; + } break; } filestack[i] = uri; diff --git a/usr.sbin/rpki-client/parser.c b/usr.sbin/rpki-client/parser.c index b79ac3f4d73..c11ec98fd25 100644 --- a/usr.sbin/rpki-client/parser.c +++ b/usr.sbin/rpki-client/parser.c @@ -1,4 +1,4 @@ -/* $OpenBSD: parser.c,v 1.140 2024/06/10 11:49:29 tb Exp $ */ +/* $OpenBSD: parser.c,v 1.141 2024/06/12 10:03:09 tb Exp $ */ /* * Copyright (c) 2019 Claudio Jeker * Copyright (c) 2019 Kristaps Dzonsons @@ -38,6 +38,8 @@ #include "extern.h" +extern int certid; + static X509_STORE_CTX *ctx; static struct auth_tree auths = RB_INITIALIZER(&auths); static struct crl_tree crlt = RB_INITIALIZER(&crlt); @@ -98,7 +100,9 @@ find_issuer(const char *fn, int id, const char *aki, const char *mftaki) a = auth_find(&auths, id); if (a == NULL) { - warnx("%s: RFC 6487: unknown cert with SKI %s", fn, aki); + if (certid <= CERTID_MAX) + warnx("%s: RFC 6487: unknown cert with SKI %s", fn, + aki); return NULL; } @@ -171,6 +175,11 @@ proc_parser_roa(char *file, const unsigned char *der, size_t len, return NULL; a = find_issuer(file, entp->certid, roa->aki, entp->mftaki); + if (a == NULL) { + X509_free(x509); + roa_free(roa); + return NULL; + } crl = crl_get(&crlt, a); if (!valid_x509(file, ctx, x509, a, crl, &errstr)) { @@ -206,6 +215,11 @@ proc_parser_spl(char *file, const unsigned char *der, size_t len, return NULL; a = find_issuer(file, entp->certid, spl->aki, entp->mftaki); + if (a == NULL) { + X509_free(x509); + spl_free(spl); + return NULL; + } crl = crl_get(&crlt, a); if (!valid_x509(file, ctx, x509, a, crl, &errstr)) { @@ -370,6 +384,8 @@ proc_parser_mft_pre(struct entity *entp, char *file, struct crl **crl, *crl = parse_load_crl_from_mft(entp, mft, DIR_VALID, crlfile); a = find_issuer(file, entp->certid, mft->aki, NULL); + if (a == NULL) + goto err; if (!valid_x509(file, ctx, x509, a, *crl, errstr)) goto err; X509_free(x509); @@ -494,7 +510,8 @@ proc_parser_mft(struct entity *entp, struct mft **mp, char **crlfile, err2 = err1; if (err2 == NULL) err2 = "no valid manifest available"; - warnx("%s: %s", file2, err2); + if (certid <= CERTID_MAX) + warnx("%s: %s", file2, err2); } mft_free(mft1); @@ -542,6 +559,10 @@ proc_parser_cert(char *file, const unsigned char *der, size_t len, return NULL; a = find_issuer(file, entp->certid, cert->aki, entp->mftaki); + if (a == NULL) { + cert_free(cert); + return NULL; + } crl = crl_get(&crlt, a); if (!valid_x509(file, ctx, cert->x509, a, crl, &errstr) || @@ -684,6 +705,11 @@ proc_parser_gbr(char *file, const unsigned char *der, size_t len, return NULL; a = find_issuer(file, entp->certid, gbr->aki, entp->mftaki); + if (a == NULL) { + X509_free(x509); + gbr_free(gbr); + return NULL; + } crl = crl_get(&crlt, a); if (!valid_x509(file, ctx, x509, a, crl, &errstr)) { @@ -716,6 +742,11 @@ proc_parser_aspa(char *file, const unsigned char *der, size_t len, return NULL; a = find_issuer(file, entp->certid, aspa->aki, entp->mftaki); + if (a == NULL) { + X509_free(x509); + aspa_free(aspa); + return NULL; + } crl = crl_get(&crlt, a); if (!valid_x509(file, ctx, x509, a, crl, &errstr)) { @@ -751,6 +782,8 @@ proc_parser_tak(char *file, const unsigned char *der, size_t len, return NULL; a = find_issuer(file, entp->certid, tak->aki, entp->mftaki); + if (a == NULL) + goto out; crl = crl_get(&crlt, a); if (!valid_x509(file, ctx, x509, a, crl, &errstr)) { @@ -1076,5 +1109,8 @@ proc_parser(int fd) ibuf_free(inbuf); + if (certid > CERTID_MAX) + errx(1, "processing incomplete: too many certificates"); + exit(0); } diff --git a/usr.sbin/rpki-client/validate.c b/usr.sbin/rpki-client/validate.c index f66518a475b..f4c6e7c260f 100644 --- a/usr.sbin/rpki-client/validate.c +++ b/usr.sbin/rpki-client/validate.c @@ -1,4 +1,4 @@ -/* $OpenBSD: validate.c,v 1.74 2024/05/20 15:51:43 claudio Exp $ */ +/* $OpenBSD: validate.c,v 1.75 2024/06/12 10:03:09 tb Exp $ */ /* * Copyright (c) 2019 Kristaps Dzonsons * @@ -304,9 +304,6 @@ build_chain(const struct auth *a, STACK_OF(X509) **intermediates, *intermediates = NULL; *root = NULL; - if (a == NULL) - return; - if ((*intermediates = sk_X509_new_null()) == NULL) err(1, "sk_X509_new_null"); if ((*root = sk_X509_new_null()) == NULL) -- 2.20.1