rpki-client: check for duplicate certificate extensions

RFC 5280 disallows multiple extensions with the same OID. Since libcrypto
does not check that currently, do this by hand. This only deals with CA
certs for now, EE certs could do that similarly.

Found with BBN test corpora

ok job

diff --git a/usr.sbin/rpki-client/cert.c b/usr.sbin/rpki-client/cert.c
index 387ee9d..ecef202 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.110 2023/06/23 07:26:21 tb Exp $ */
+/*     $OpenBSD: cert.c,v 1.111 2023/06/23 07:40:28 tb Exp $ */
 /*
  * Copyright (c) 2022 Theo Buehler <tb@openbsd.org>
  * Copyright (c) 2021 Job Snijders <job@openbsd.org>
@@ -662,7 +662,6 @@ cert_parse_pre(const char *fn, const unsigned char *der, size_t len)
 {
        const unsigned char     *oder;
        int                      extsz;
-       int                      sia_present = 0;
        size_t                   i;
        X509                    *x = NULL;
        X509_EXTENSION          *ext = NULL;
@@ -672,7 +671,10 @@ cert_parse_pre(const char *fn, const unsigned char *der, size_t len)
        ASN1_OBJECT             *obj;
        EVP_PKEY                *pkey;
        struct parse             p;
-       int                      nid;
+       int                      nid, ip, as, sia, cp, crldp, aia, aki, ski,
+                                eku, bc, ku;
+
+       nid = ip = as = sia = cp = crldp = aia = aki = ski = eku = bc = ku = 0;
 
        /* just fail for empty buffers, the warning was printed elsewhere */
        if (der == NULL)
@@ -735,38 +737,58 @@ cert_parse_pre(const char *fn, const unsigned char *der, size_t len)
                obj = X509_EXTENSION_get_object(ext);
                assert(obj != NULL);
 
-               switch (OBJ_obj2nid(obj)) {
+               switch ((nid = OBJ_obj2nid(obj))) {
                case NID_sbgp_ipAddrBlock:
+                       if (ip++ >= 1)
+                               goto dup;
                        if (!sbgp_ipaddrblk(&p, ext))
                                goto out;
                        break;
                case NID_sbgp_autonomousSysNum:
+                       if (as++ > 0)
+                               goto dup;
                        if (!sbgp_assysnum(&p, ext))
                                goto out;
                        break;
                case NID_sinfo_access:
-                       sia_present = 1;
+                       if (sia++ > 0)
+                               goto dup;
                        if (!sbgp_sia(&p, ext))
                                goto out;
                        break;
                case NID_certificate_policies:
+                       if (cp++ > 0)
+                               goto dup;
                        if (!certificate_policies(&p, ext))
                                goto out;
                        break;
                case NID_crl_distribution_points:
-                       /* ignored here, handled later */
+                       if (crldp++ > 0)
+                               goto dup;
                        break;
                case NID_info_access:
+                       if (aia++ > 0)
+                               goto dup;
                        break;
                case NID_authority_key_identifier:
+                       if (aki++ > 0)
+                               goto dup;
                        break;
                case NID_subject_key_identifier:
+                       if (ski++ > 0)
+                               goto dup;
                        break;
                case NID_ext_key_usage:
+                       if (eku++ > 0)
+                               goto dup;
                        break;
                case NID_basic_constraints:
+                       if (bc++ > 0)
+                               goto dup;
                        break;
                case NID_key_usage:
+                       if (ku++ > 0)
+                               goto dup;
                        break;
                default:
                        /* unexpected extensions warrant investigation */
@@ -845,7 +867,7 @@ cert_parse_pre(const char *fn, const unsigned char *der, size_t len)
                                goto out;
                        }
                }
-               if (sia_present) {
+               if (sia) {
                        warnx("%s: unexpected SIA extension in BGPsec cert",
                            p.fn);
                        goto out;
@@ -864,7 +886,10 @@ cert_parse_pre(const char *fn, const unsigned char *der, size_t len)
        p.res->x509 = x;
        return p.res;
 
-out:
+ dup:
+       warnx("%s: RFC 5280 section 4.2: duplicate %s extension", fn,
+           OBJ_nid2sn(nid));
+ out:
        cert_free(p.res);
        X509_free(x);
        return NULL;