From 9025e29518419813880e10ef0f5a45d602e9f2f1 Mon Sep 17 00:00:00 2001 From: claudio Date: Fri, 25 Mar 2022 08:19:04 +0000 Subject: [PATCH] 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@ --- usr.sbin/rpki-client/cms.c | 142 +++++++++++++++++++++++++++++++++--- usr.sbin/rpki-client/x509.c | 16 +++- 2 files changed, 148 insertions(+), 10 deletions(-) diff --git a/usr.sbin/rpki-client/cms.c b/usr.sbin/rpki-client/cms.c index 950e7b6573f..ef3f8514151 100644 --- 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 * @@ -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 43abbdbfd47..2c2a1453a26 100644 --- 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 * Copyright (c) 2019 Kristaps Dzonsons @@ -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"); } /* -- 2.20.1