Implement most of the CMS related checks required by RFC 6488 section 3

Verify that SignerInfo and Signed Attributes are set according to the RFC.
Especially enforce that the right attributes are signed. Check that there
are no unsigned attributes, no CRL and that the correct content-type,
digest and signature algorithm are used.

The OpenSSL API makes it impossible to verify the versions and some other
more suttle differences like detecting signle attributes vs a SET OF one.
Similarly OpenSSL accepts both DER and BER encoding in the payload.
These smaller differences to the RFC are not optimal but not a risk.

Lots of feedback and OK tb@

diff --git a/usr.sbin/rpki-client/cms.c b/usr.sbin/rpki-client/cms.c
index 950e7b6..ef3f851 100644 (file)
--- a/usr.sbin/rpki-client/cms.c
+++ b/usr.sbin/rpki-client/cms.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: cms.c,v 1.13 2022/01/18 16:24:55 claudio Exp $ */
+/*     $OpenBSD: cms.c,v 1.14 2022/03/25 08:19:04 claudio Exp $ */
 /*
  * Copyright (c) 2019 Kristaps Dzonsons <kristaps@bsd.lv>
  *
@@ -27,6 +27,11 @@
 
 #include "extern.h"
 
+extern ASN1_OBJECT     *cnt_type_oid;
+extern ASN1_OBJECT     *msg_dgst_oid;
+extern ASN1_OBJECT     *sign_time_oid;
+extern ASN1_OBJECT     *bin_sign_time_oid;
+
 /*
  * Parse and validate a self-signed CMS message, where the signing X509
  * certificate has been hashed to dgst (optional).
@@ -38,12 +43,20 @@ unsigned char *
 cms_parse_validate(X509 **xp, const char *fn, const unsigned char *der,
     size_t derlen, const ASN1_OBJECT *oid, size_t *rsz)
 {
-       const ASN1_OBJECT       *obj;
-       ASN1_OCTET_STRING       **os = NULL;
-       CMS_ContentInfo         *cms;
-       int                      rc = 0;
-       STACK_OF(X509)          *certs = NULL;
-       unsigned char           *res = NULL;
+       char                             buf[128], obuf[128];
+       const ASN1_OBJECT               *obj, *octype;
+       ASN1_OCTET_STRING               **os = NULL;
+       CMS_ContentInfo                 *cms;
+       int                              rc = 0;
+       STACK_OF(X509)                  *certs = NULL;
+       STACK_OF(X509_CRL)              *crls;
+       STACK_OF(CMS_SignerInfo)        *sinfos;
+       CMS_SignerInfo                  *si;
+       X509_ALGOR                      *pdig, *psig;
+       unsigned char                   *res = NULL;
+       int                              i, nattrs, nid;
+       int                              has_ct = 0, has_md = 0, has_st = 0,
+                                        has_bst = 0;
 
        *rsz = 0;
        *xp = NULL;
@@ -68,6 +81,97 @@ cms_parse_validate(X509 **xp, const char *fn, const unsigned char *der,
                goto out;
        }
 
+       /* RFC 6488 section 3 verify the CMS */
+       /* the version of SignedData and SignerInfos can't be verified */
+
+       sinfos = CMS_get0_SignerInfos(cms);
+       assert(sinfos != NULL);
+       if (sk_CMS_SignerInfo_num(sinfos) != 1) {
+               cryptowarnx("%s: RFC 6488: CMS has multiple signerInfos", fn);
+               goto out;
+       }
+       si = sk_CMS_SignerInfo_value(sinfos, 0);
+
+       nattrs = CMS_signed_get_attr_count(si);
+       if (nattrs <= 0) {
+               cryptowarnx("%s: RFC 6488: error extracting signedAttrs", fn);
+               goto out;
+       }
+       for (i = 0; i < nattrs; i++) {
+               X509_ATTRIBUTE *attr;
+
+               attr = CMS_signed_get_attr(si, i);
+               if (attr == NULL || X509_ATTRIBUTE_count(attr) != 1) {
+                       cryptowarnx("%s: RFC 6488: "
+                           "bad signed attribute encoding", fn);
+                       goto out;
+               }
+
+               obj = X509_ATTRIBUTE_get0_object(attr);
+               if (obj == NULL) {
+                       cryptowarnx("%s: RFC 6488: bad signed attribute", fn);
+                       goto out;
+               }
+               if (OBJ_cmp(obj, cnt_type_oid) == 0) {
+                       if (has_ct++ != 0) {
+                               cryptowarnx("%s: RFC 6488: duplicate "
+                                   "signed attribute", fn);
+                               goto out;
+                       }
+               } else if (OBJ_cmp(obj, msg_dgst_oid) == 0) {
+                       if (has_md++ != 0) {
+                               cryptowarnx("%s: RFC 6488: duplicate "
+                                   "signed attribute", fn);
+                               goto out;
+                       }
+               } else if (OBJ_cmp(obj, sign_time_oid) == 0) {
+                       if (has_st++ != 0) {
+                               cryptowarnx("%s: RFC 6488: duplicate "
+                                   "signed attribute", fn);
+                               goto out;
+                       }
+               } else if (OBJ_cmp(obj, bin_sign_time_oid) == 0) {
+                       if (has_bst++ != 0) {
+                               cryptowarnx("%s: RFC 6488: duplicate "
+                                   "signed attribute", fn);
+                               goto out;
+                       }
+               } else {
+                       OBJ_obj2txt(buf, sizeof(buf), obj, 1);
+                       cryptowarnx("%s: RFC 6488: "
+                           "CMS has unexpected signed attribute %s",
+                           fn, buf);
+                       goto out;
+               }
+       }
+       if (!has_ct || !has_md) {
+               cryptowarnx("%s: RFC 6488: CMS missing required "
+                   "signed attribute", fn);
+               goto out;
+       }
+       if (CMS_unsigned_get_attr_count(si) > 0) {
+               cryptowarnx("%s: RFC 6488: CMS has unsignedAttrs", fn);
+               goto out;
+       }
+
+       /* Check digest and signature algorithms */
+       CMS_SignerInfo_get0_algs(si, NULL, NULL, &pdig, &psig);
+       X509_ALGOR_get0(&obj, NULL, NULL, pdig);
+       nid = OBJ_obj2nid(obj);
+       if (nid != NID_sha256) {
+               warnx("%s: RFC 6488: wrong digest %s, want %s", fn,
+                   OBJ_nid2ln(nid), OBJ_nid2ln(NID_sha256));
+               goto out;
+       }
+       X509_ALGOR_get0(&obj, NULL, NULL, psig);
+       nid = OBJ_obj2nid(obj);
+       /* RFC7395 last paragraph of section 2 specifies the allowed psig */
+       if (nid != NID_rsaEncryption && nid != NID_sha256WithRSAEncryption) {
+               warnx("%s: RFC 6488: wrong signature algorithm %s, want %s",
+                   fn, OBJ_nid2ln(nid), OBJ_nid2ln(NID_rsaEncryption));
+               goto out;
+       }
+
        /* RFC 6488 section 2.1.3.1: check the object's eContentType. */
 
        obj = CMS_get0_eContentType(cms);
@@ -77,8 +181,6 @@ cms_parse_validate(X509 **xp, const char *fn, const unsigned char *der,
                goto out;
        }
        if (OBJ_cmp(obj, oid) != 0) {
-               char buf[128], obuf[128];
-
                OBJ_obj2txt(buf, sizeof(buf), obj, 1);
                OBJ_obj2txt(obuf, sizeof(obuf), oid, 1);
                warnx("%s: RFC 6488 section 2.1.3.1: eContentType: "
@@ -86,6 +188,28 @@ cms_parse_validate(X509 **xp, const char *fn, const unsigned char *der,
                goto out;
        }
 
+       /* Compare content-type with eContentType */
+       octype = CMS_signed_get0_data_by_OBJ(si, cnt_type_oid,
+           -3, V_ASN1_OBJECT);
+       assert(octype != NULL);
+       if (OBJ_cmp(obj, octype) != 0) {
+               OBJ_obj2txt(buf, sizeof(buf), obj, 1);
+               OBJ_obj2txt(obuf, sizeof(obuf), oid, 1);
+               warnx("%s: RFC 6488: eContentType does not match Content-Type "
+                   "OID: %s, want %s", fn, buf, obuf);
+               goto out;
+       }
+
+       /*
+        * Check that there are no CRLS in this CMS message.
+        */
+       crls = CMS_get1_crls(cms);
+       if (crls != NULL) {
+               sk_X509_CRL_pop_free(crls, X509_CRL_free);
+               cryptowarnx("%s: RFC 6488: CMS has CRLs", fn);
+               goto out;
+       }
+
        /*
         * The self-signing certificate is further signed by the input
         * signing authority according to RFC 6488, 2.1.4.

diff --git a/usr.sbin/rpki-client/x509.c b/usr.sbin/rpki-client/x509.c
index 43abbdb..2c2a145 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.36 2022/02/10 17:33:28 claudio Exp $ */
+/*     $OpenBSD: x509.c,v 1.37 2022/03/25 08:19:04 claudio Exp $ */
 /*
  * Copyright (c) 2021 Claudio Jeker <claudio@openbsd.org>
  * Copyright (c) 2019 Kristaps Dzonsons <kristaps@bsd.lv>
@@ -38,6 +38,10 @@ ASN1_OBJECT  *roa_oid;       /* id-ct-routeOriginAuthz CMS content type */
 ASN1_OBJECT    *mft_oid;       /* id-ct-rpkiManifest CMS content type */
 ASN1_OBJECT    *gbr_oid;       /* id-ct-rpkiGhostbusters CMS content type */
 ASN1_OBJECT    *bgpsec_oid;    /* id-kp-bgpsec-router Key Purpose */
+ASN1_OBJECT    *cnt_type_oid;  /* pkcs-9 id-contentType */
+ASN1_OBJECT    *msg_dgst_oid;  /* pkcs-9 id-messageDigest */
+ASN1_OBJECT    *sign_time_oid; /* pkcs-9 id-signingTime */
+ASN1_OBJECT    *bin_sign_time_oid;     /* pkcs-9 id-aa-binarySigningTime */
 
 void
 x509_init_oid(void)
@@ -62,6 +66,16 @@ x509_init_oid(void)
                    "1.2.840.113549.1.9.16.1.35");
        if ((bgpsec_oid = OBJ_txt2obj("1.3.6.1.5.5.7.3.30", 1)) == NULL)
                errx(1, "OBJ_txt2obj for %s failed", "1.3.6.1.5.5.7.3.30");
+       if ((cnt_type_oid = OBJ_txt2obj("1.2.840.113549.1.9.3", 1)) == NULL)
+               errx(1, "OBJ_txt2obj for %s failed", "1.2.840.113549.1.9.3");
+       if ((msg_dgst_oid = OBJ_txt2obj("1.2.840.113549.1.9.4", 1)) == NULL)
+               errx(1, "OBJ_txt2obj for %s failed", "1.2.840.113549.1.9.4");
+       if ((sign_time_oid = OBJ_txt2obj("1.2.840.113549.1.9.5", 1)) == NULL)
+               errx(1, "OBJ_txt2obj for %s failed", "1.2.840.113549.1.9.5");
+       if ((bin_sign_time_oid =
+           OBJ_txt2obj("1.2.840.113549.1.9.16.2.46", 1)) == NULL)
+               errx(1, "OBJ_txt2obj for %s failed",
+                   "1.2.840.113549.1.9.16.2.46");
 }
 
 /*