When looking through the list of things to check in RFC 6488 section 3,
I wondered why we don't check for the SignedData content-type (1a).
I'm sure we discussed back when reworked this code that this is implicit
in CMS_get0_SignerInfos() and the assert following it...
However, I think this assert() should be an error check. As far as I can
see, up to this point nothing guarantees that we're not fed a CMS object
whose content-type isn't SignedData in which case rpki-client dies
instead of dropping that object on the floor.
We could also do an explicit check using CMS_get0_type() and converting
the resulting obj to a NID. The below incurs no additional cost and
should make it clear enough that we actually do ensure 1a.
Index: cms.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/cms.c,v
retrieving revision 1.33
diff -u -p -r1.33 cms.c
--- cms.c 13 Mar 2023 19:46:55 -0000 1.33
+++ cms.c 23 May 2023 09:14:45 -0000
@@ -144,8 +144,17 @@ cms_parse_validate_internal(X509 **xp, c
/* 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);
+ /* Only returns NULL if cms content-type is not SignedData. */
+ if ((sinfos = CMS_get0_SignerInfos(cms)) == NULL) {
+ if ((obj = CMS_get0_type(cms)) == NULL) {
+ warnx("%s: RFC 6488: missing content-type", fn);
+ goto out;
+ }
+ OBJ_obj2txt(buf, sizeof(buf), obj, 1);
+ warnx("%s: RFC 6488: no signerInfo in CMS object of type %s",
+ fn, buf);
+ goto out;
+ }
if (sk_CMS_SignerInfo_num(sinfos) != 1) {
cryptowarnx("%s: RFC 6488: CMS has multiple signerInfos", fn);
goto out;
