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@

diff --git a/usr.sbin/rpki-client/cert.c b/usr.sbin/rpki-client/cert.c
index c2f2f81..9ad6b4e 100644 (file)
--- 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 <job@openbsd.org>
  * Copyright (c) 2019 Kristaps Dzonsons <kristaps@bsd.lv>
@@ -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 da707a8..3e18b45 100644 (file)
--- 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 <kristaps@bsd.lv>
  *
@@ -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 ecdb1f5..483a168 100644 (file)
--- 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 <claudio@openbsd.org>
  *
@@ -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 9b83cb1..53707ca 100644 (file)
--- 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 <kristaps@bsd.lv>
  *
@@ -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 1b737ed..c57c226 100644 (file)
--- 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 <claudio@openbsd.org>
  * Copyright (c) 2019 Kristaps Dzonsons <kristaps@bsd.lv>
@@ -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 de35344..aa12ec3 100644 (file)
--- 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 <kristaps@bsd.lv>
  *
@@ -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 2c2a145..56959de 100644 (file)
--- 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 <claudio@openbsd.org>
  * Copyright (c) 2019 Kristaps Dzonsons <kristaps@bsd.lv>
@@ -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;
 }
 
 /*