From f999fe57db76cb816829fc573cadc0d6921a960a Mon Sep 17 00:00:00 2001 From: claudio Date: Fri, 1 Apr 2022 17:22:07 +0000 Subject: [PATCH] Change x509_get_aki(), x509_get_ski(), x509_get_aia(), and x509_get_crl() to work more like x509_get_expire(). They will return an error if the extension extraction failed but not if it was not present. The callers must now do that check but most did already. With this cert_parse_inner() no longer cares about TA vs non-TA certs. Feedback and OK tb@ --- usr.sbin/rpki-client/cert.c | 35 +++++++------- usr.sbin/rpki-client/extern.h | 10 ++-- usr.sbin/rpki-client/gbr.c | 23 +++++---- usr.sbin/rpki-client/mft.c | 11 +++-- usr.sbin/rpki-client/parser.c | 13 ++++-- usr.sbin/rpki-client/roa.c | 11 +++-- usr.sbin/rpki-client/x509.c | 88 ++++++++++++++++------------------- 7 files changed, 103 insertions(+), 88 deletions(-) diff --git a/usr.sbin/rpki-client/cert.c b/usr.sbin/rpki-client/cert.c index c2f2f814fae..9ad6b4e2e1e 100644 --- a/usr.sbin/rpki-client/cert.c +++ b/usr.sbin/rpki-client/cert.c @@ -1,4 +1,4 @@ -/* $OpenBSD: cert.c,v 1.58 2022/04/01 13:27:38 claudio Exp $ */ +/* $OpenBSD: cert.c,v 1.59 2022/04/01 17:22:07 claudio Exp $ */ /* * Copyright (c) 2021 Job Snijders * Copyright (c) 2019 Kristaps Dzonsons @@ -1052,13 +1052,10 @@ certificate_policies(struct parse *p, X509_EXTENSION *ext) /* * Parse and partially validate an RPKI X509 certificate (either a trust * anchor or a certificate) as defined in RFC 6487. - * If "ta" is set, this is a trust anchor and must be self-signed. - * Returns the parse results or NULL on failure ("xp" will be NULL too). - * On success, free the pointer with cert_free() and make sure that "xp" - * is also dereferenced. + * Returns the parse results or NULL on failure. */ static struct cert * -cert_parse_inner(const char *fn, const unsigned char *der, size_t len, int ta) +cert_parse_inner(const char *fn, const unsigned char *der, size_t len) { int rc = 0, extsz, c; int sia_present = 0; @@ -1132,12 +1129,14 @@ cert_parse_inner(const char *fn, const unsigned char *der, size_t len, int ta) goto out; } - p.res->aki = x509_get_aki(x, ta, p.fn); - p.res->ski = x509_get_ski(x, p.fn); - if (!ta) { - p.res->aia = x509_get_aia(x, p.fn); - p.res->crl = x509_get_crl(x, p.fn); - } + if (!x509_get_aki(x, p.fn, &p.res->aki)) + goto out; + if (!x509_get_ski(x, p.fn, &p.res->ski)) + goto out; + if (!x509_get_aia(x, p.fn, &p.res->aia)) + goto out; + if (!x509_get_crl(x, p.fn, &p.res->crl)) + goto out; if (!x509_get_expire(x, p.fn, &p.res->expires)) goto out; p.res->purpose = x509_get_purpose(x, p.fn); @@ -1198,7 +1197,7 @@ cert_parse(const char *fn, const unsigned char *der, size_t len) { struct cert *p; - if ((p = cert_parse_inner(fn, der, len, 0)) == NULL) + if ((p = cert_parse_inner(fn, der, len)) == NULL) return NULL; if (p->aki == NULL) { @@ -1212,8 +1211,12 @@ cert_parse(const char *fn, const unsigned char *der, size_t len) goto badcert; } if (p->aia == NULL) { - warnx("%s: RFC 6487 section 8.4.7: " - "non-trust anchor missing AIA", fn); + warnx("%s: RFC 6487 section 8.4.7: AIA: extension missing", fn); + goto badcert; + } + if (p->crl == NULL) { + warnx("%s: RFC 6487 section 4.8.6: CRL: " + "no CRL distribution point extension", fn); goto badcert; } return p; @@ -1231,7 +1234,7 @@ ta_parse(const char *fn, const unsigned char *der, size_t len, EVP_PKEY *pk = NULL, *opk = NULL; struct cert *p; - if ((p = cert_parse_inner(fn, der, len, 1)) == NULL) + if ((p = cert_parse_inner(fn, der, len)) == NULL) return NULL; /* first check pubkey against the one from the TAL */ diff --git a/usr.sbin/rpki-client/extern.h b/usr.sbin/rpki-client/extern.h index da707a8003a..3e18b455907 100644 --- a/usr.sbin/rpki-client/extern.h +++ b/usr.sbin/rpki-client/extern.h @@ -1,4 +1,4 @@ -/* $OpenBSD: extern.h,v 1.122 2022/03/31 12:00:00 job Exp $ */ +/* $OpenBSD: extern.h,v 1.123 2022/04/01 17:22:07 claudio Exp $ */ /* * Copyright (c) 2019 Kristaps Dzonsons * @@ -578,11 +578,11 @@ struct ibuf *io_buf_recvfd(int, struct ibuf **); /* X509 helpers. */ void x509_init_oid(void); -char *x509_get_aia(X509 *, const char *); -char *x509_get_aki(X509 *, int, const char *); -char *x509_get_ski(X509 *, const char *); +int x509_get_aia(X509 *, const char *, char **); +int x509_get_aki(X509 *, const char *, char **); +int x509_get_ski(X509 *, const char *, char **); int x509_get_expire(X509 *, const char *, time_t *); -char *x509_get_crl(X509 *, const char *); +int x509_get_crl(X509 *, const char *, char **); char *x509_crl_get_aki(X509_CRL *, const char *); char *x509_get_pubkey(X509 *, const char *); enum cert_purpose x509_get_purpose(X509 *, const char *); diff --git a/usr.sbin/rpki-client/gbr.c b/usr.sbin/rpki-client/gbr.c index ecdb1f57551..483a168645c 100644 --- a/usr.sbin/rpki-client/gbr.c +++ b/usr.sbin/rpki-client/gbr.c @@ -1,4 +1,4 @@ -/* $OpenBSD: gbr.c,v 1.14 2022/01/18 16:24:55 claudio Exp $ */ +/* $OpenBSD: gbr.c,v 1.15 2022/04/01 17:22:07 claudio Exp $ */ /* * Copyright (c) 2020 Claudio Jeker * @@ -63,19 +63,24 @@ gbr_parse(X509 **x509, const char *fn, const unsigned char *der, size_t len) err(1, NULL); free(cms); - p.res->aia = x509_get_aia(*x509, fn); - p.res->aki = x509_get_aki(*x509, 0, fn); - p.res->ski = x509_get_ski(*x509, fn); + if (!x509_get_aia(*x509, fn, &p.res->aia)) + goto out; + if (!x509_get_aki(*x509, fn, &p.res->aki)) + goto out; + if (!x509_get_ski(*x509, fn, &p.res->ski)) + goto out; if (p.res->aia == NULL || p.res->aki == NULL || p.res->ski == NULL) { warnx("%s: RFC 6487 section 4.8: " "missing AIA, AKI or SKI X509 extension", fn); - gbr_free(p.res); - X509_free(*x509); - *x509 = NULL; - return NULL; + goto out; } - return p.res; + +out: + gbr_free(p.res); + X509_free(*x509); + *x509 = NULL; + return NULL; } /* diff --git a/usr.sbin/rpki-client/mft.c b/usr.sbin/rpki-client/mft.c index 9b83cb14b47..53707ca7d51 100644 --- a/usr.sbin/rpki-client/mft.c +++ b/usr.sbin/rpki-client/mft.c @@ -1,4 +1,4 @@ -/* $OpenBSD: mft.c,v 1.54 2022/03/31 12:00:00 job Exp $ */ +/* $OpenBSD: mft.c,v 1.55 2022/04/01 17:22:07 claudio Exp $ */ /* * Copyright (c) 2019 Kristaps Dzonsons * @@ -444,9 +444,12 @@ mft_parse(X509 **x509, const char *fn, const unsigned char *der, size_t len) if ((p.res = calloc(1, sizeof(struct mft))) == NULL) err(1, NULL); - p.res->aia = x509_get_aia(*x509, fn); - p.res->aki = x509_get_aki(*x509, 0, fn); - p.res->ski = x509_get_ski(*x509, fn); + if (!x509_get_aia(*x509, fn, &p.res->aia)) + goto out; + if (!x509_get_aki(*x509, fn, &p.res->aki)) + goto out; + if (!x509_get_ski(*x509, fn, &p.res->ski)) + goto out; if (p.res->aia == NULL || p.res->aki == NULL || p.res->ski == NULL) { warnx("%s: RFC 6487 section 4.8: " "missing AIA, AKI or SKI X509 extension", fn); diff --git a/usr.sbin/rpki-client/parser.c b/usr.sbin/rpki-client/parser.c index 1b737edd8fd..c57c226e007 100644 --- a/usr.sbin/rpki-client/parser.c +++ b/usr.sbin/rpki-client/parser.c @@ -1,4 +1,4 @@ -/* $OpenBSD: parser.c,v 1.64 2022/02/10 15:33:47 claudio Exp $ */ +/* $OpenBSD: parser.c,v 1.65 2022/04/01 17:22:07 claudio Exp $ */ /* * Copyright (c) 2019 Claudio Jeker * Copyright (c) 2019 Kristaps Dzonsons @@ -360,7 +360,14 @@ proc_parser_mft_pre(char *file, const unsigned char *der, size_t len, a = valid_ski_aki(file, &auths, mft->ski, mft->aki); /* load CRL by hand, since it is referenced by the MFT itself */ - c = x509_get_crl(x509, file); + if (!x509_get_crl(x509, file, &c) || c == NULL) { + if (c == NULL) + warnx("%s: RFC 6487 section 4.8.6: CRL: " + "no CRL distribution point extension", file); + mft_free(mft); + X509_free(x509); + return NULL; + } crlfile = strrchr(c, '/'); if (crlfile != NULL) crlfile++; @@ -1078,7 +1085,7 @@ proc_parser_file(char *file, unsigned char *buf, size_t len) struct crl *c; char *crl_uri; - crl_uri = x509_get_crl(x509, file); + x509_get_crl(x509, file, &crl_uri); parse_load_crl(crl_uri); free(crl_uri); if (auth_find(&auths, aki) == NULL) diff --git a/usr.sbin/rpki-client/roa.c b/usr.sbin/rpki-client/roa.c index de3534421d5..aa12ec3d206 100644 --- a/usr.sbin/rpki-client/roa.c +++ b/usr.sbin/rpki-client/roa.c @@ -1,4 +1,4 @@ -/* $OpenBSD: roa.c,v 1.38 2022/02/10 15:33:47 claudio Exp $ */ +/* $OpenBSD: roa.c,v 1.39 2022/04/01 17:22:07 claudio Exp $ */ /* * Copyright (c) 2019 Kristaps Dzonsons * @@ -351,9 +351,12 @@ roa_parse(X509 **x509, const char *fn, const unsigned char *der, size_t len) if ((p.res = calloc(1, sizeof(struct roa))) == NULL) err(1, NULL); - p.res->aia = x509_get_aia(*x509, fn); - p.res->aki = x509_get_aki(*x509, 0, fn); - p.res->ski = x509_get_ski(*x509, fn); + if (!x509_get_aia(*x509, fn, &p.res->aia)) + goto out; + if (!x509_get_aki(*x509, fn, &p.res->aki)) + goto out; + if (!x509_get_ski(*x509, fn, &p.res->ski)) + goto out; if (p.res->aia == NULL || p.res->aki == NULL || p.res->ski == NULL) { warnx("%s: RFC 6487 section 4.8: " "missing AIA, AKI or SKI X509 extension", fn); diff --git a/usr.sbin/rpki-client/x509.c b/usr.sbin/rpki-client/x509.c index 2c2a1453a26..56959defc8e 100644 --- a/usr.sbin/rpki-client/x509.c +++ b/usr.sbin/rpki-client/x509.c @@ -1,4 +1,4 @@ -/* $OpenBSD: x509.c,v 1.37 2022/03/25 08:19:04 claudio Exp $ */ +/* $OpenBSD: x509.c,v 1.38 2022/04/01 17:22:07 claudio Exp $ */ /* * Copyright (c) 2021 Claudio Jeker * Copyright (c) 2019 Kristaps Dzonsons @@ -83,22 +83,18 @@ x509_init_oid(void) * Returns the AKI or NULL if it could not be parsed. * The AKI is formatted as a hex string. */ -char * -x509_get_aki(X509 *x, int ta, const char *fn) +int +x509_get_aki(X509 *x, const char *fn, char **aki) { const unsigned char *d; AUTHORITY_KEYID *akid; ASN1_OCTET_STRING *os; - int dsz, crit; - char *res = NULL; + int dsz, crit, rc = 0; + *aki = NULL; akid = X509_get_ext_d2i(x, NID_authority_key_identifier, &crit, NULL); - if (akid == NULL) { - if (!ta) - warnx("%s: RFC 6487 section 4.8.3: AKI: " - "extension missing", fn); - return NULL; - } + if (akid == NULL) + return 1; if (crit != 0) { warnx("%s: RFC 6487 section 4.8.3: " "AKI: extension not non-critical", fn); @@ -128,11 +124,11 @@ x509_get_aki(X509 *x, int ta, const char *fn) goto out; } - res = hex_encode(d, dsz); - + *aki = hex_encode(d, dsz); + rc = 1; out: AUTHORITY_KEYID_free(akid); - return res; + return rc; } /* @@ -140,19 +136,17 @@ out: * Returns the SKI or NULL if it could not be parsed. * The SKI is formatted as a hex string. */ -char * -x509_get_ski(X509 *x, const char *fn) +int +x509_get_ski(X509 *x, const char *fn, char **ski) { const unsigned char *d; ASN1_OCTET_STRING *os; - int dsz, crit; - char *res = NULL; + int dsz, crit, rc = 0; + *ski = NULL; os = X509_get_ext_d2i(x, NID_subject_key_identifier, &crit, NULL); - if (os == NULL) { - warnx("%s: RFC 6487 section 4.8.2: SKI: extension missing", fn); - return NULL; - } + if (os == NULL) + return 1; if (crit != 0) { warnx("%s: RFC 6487 section 4.8.2: " "SKI: extension not non-critical", fn); @@ -169,10 +163,11 @@ x509_get_ski(X509 *x, const char *fn) goto out; } - res = hex_encode(d, dsz); + *ski = hex_encode(d, dsz); + rc = 1; out: ASN1_OCTET_STRING_free(os); - return res; + return rc; } /* @@ -281,19 +276,18 @@ x509_get_pubkey(X509 *x, const char *fn) * Returns NULL on failure, on success returns the AIA URI * (which has to be freed after use). */ -char * -x509_get_aia(X509 *x, const char *fn) +int +x509_get_aia(X509 *x, const char *fn, char **aia) { ACCESS_DESCRIPTION *ad; AUTHORITY_INFO_ACCESS *info; - char *aia = NULL; - int crit; + int crit, rc = 0; + *aia = NULL; info = X509_get_ext_d2i(x, NID_info_access, &crit, NULL); - if (info == NULL) { - warnx("%s: RFC 6487 section 4.8.7: AIA: extension missing", fn); - return NULL; - } + if (info == NULL) + return 1; + if (crit != 0) { warnx("%s: RFC 6487 section 4.8.7: " "AIA: extension not non-critical", fn); @@ -325,15 +319,16 @@ x509_get_aia(X509 *x, const char *fn) goto out; } - aia = strndup( + *aia = strndup( ASN1_STRING_get0_data(ad->location->d.uniformResourceIdentifier), ASN1_STRING_length(ad->location->d.uniformResourceIdentifier)); - if (aia == NULL) + if (*aia == NULL) err(1, NULL); + rc = 1; out: AUTHORITY_INFO_ACCESS_free(info); - return aia; + return rc; } /* @@ -364,21 +359,19 @@ x509_get_expire(X509 *x, const char *fn, time_t *tt) * Returns NULL on failure, the crl URI on success which has to be freed * after use. */ -char * -x509_get_crl(X509 *x, const char *fn) +int +x509_get_crl(X509 *x, const char *fn, char **crl) { CRL_DIST_POINTS *crldp; DIST_POINT *dp; GENERAL_NAME *name; - char *crl = NULL; - int crit; + int crit, rc = 0; + *crl = NULL; crldp = X509_get_ext_d2i(x, NID_crl_distribution_points, &crit, NULL); - if (crldp == NULL) { - warnx("%s: RFC 6487 section 4.8.6: CRL: " - "no CRL distribution point extension", fn); - return NULL; - } + if (crldp == NULL) + return 1; + if (crit != 0) { warnx("%s: RFC 6487 section 4.8.6: " "CRL distribution point: extension not non-critical", fn); @@ -425,14 +418,15 @@ x509_get_crl(X509 *x, const char *fn) goto out; } - crl = strndup(ASN1_STRING_get0_data(name->d.uniformResourceIdentifier), + *crl = strndup(ASN1_STRING_get0_data(name->d.uniformResourceIdentifier), ASN1_STRING_length(name->d.uniformResourceIdentifier)); - if (crl == NULL) + if (*crl == NULL) err(1, NULL); + rc = 1; out: CRL_DIST_POINTS_free(crldp); - return crl; + return rc; } /* -- 2.20.1