rpki-client: ensure there is no trailing garbage in signed objects

The d2i functions are designed in such a way that the caller is responsible
to check if the entire buffer was consumed. Add checks on deserializing a
signed object to ensure the entire file has been consumed. Reject the file
if it has trailing garbage.

found by & ok job, ok claudio

diff --git a/usr.sbin/rpki-client/cert.c b/usr.sbin/rpki-client/cert.c
index 1f1348c..1a74911 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.101 2022/11/30 09:12:34 job Exp $ */
+/*     $OpenBSD: cert.c,v 1.102 2023/02/21 10:18:47 tb Exp $ */
 /*
  * Copyright (c) 2022 Theo Buehler <tb@openbsd.org>
  * Copyright (c) 2021 Job Snijders <job@openbsd.org>
@@ -641,13 +641,14 @@ cert_parse_ee_cert(const char *fn, X509 *x)
 struct cert *
 cert_parse_pre(const char *fn, const unsigned char *der, size_t len)
 {
-       int              extsz;
-       int              sia_present = 0;
-       size_t           i;
-       X509            *x = NULL;
-       X509_EXTENSION  *ext = NULL;
-       ASN1_OBJECT     *obj;
-       struct parse     p;
+       const unsigned char     *oder;
+       int                      extsz;
+       int                      sia_present = 0;
+       size_t                   i;
+       X509                    *x = NULL;
+       X509_EXTENSION          *ext = NULL;
+       ASN1_OBJECT             *obj;
+       struct parse             p;
 
        /* just fail for empty buffers, the warning was printed elsewhere */
        if (der == NULL)
@@ -658,10 +659,15 @@ cert_parse_pre(const char *fn, const unsigned char *der, size_t len)
        if ((p.res = calloc(1, sizeof(struct cert))) == NULL)
                err(1, NULL);
 
+       oder = der;
        if ((x = d2i_X509(NULL, &der, len)) == NULL) {
                cryptowarnx("%s: d2i_X509", p.fn);
                goto out;
        }
+       if (der != oder + len) {
+               warnx("%s: %td bytes trailing garbage", fn, oder + len - der);
+               goto out;
+       }
 
        /* Cache X509v3 extensions, see X509_check_ca(3). */
        if (X509_check_purpose(x, -1, -1) <= 0) {

diff --git a/usr.sbin/rpki-client/cms.c b/usr.sbin/rpki-client/cms.c
index 1b2e699..79f6dbf 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.26 2022/12/28 21:30:18 jmc Exp $ */
+/*     $OpenBSD: cms.c,v 1.27 2023/02/21 10:18:47 tb Exp $ */
 /*
  * Copyright (c) 2019 Kristaps Dzonsons <kristaps@bsd.lv>
  *
@@ -64,9 +64,10 @@ cms_extract_econtent(const char *fn, CMS_ContentInfo *cms, unsigned char **res,
 
 static int
 cms_parse_validate_internal(X509 **xp, const char *fn, const unsigned char *der,
-    size_t derlen, const ASN1_OBJECT *oid, BIO *bio, unsigned char **res,
+    size_t len, const ASN1_OBJECT *oid, BIO *bio, unsigned char **res,
     size_t *rsz)
 {
+       const unsigned char             *oder;
        char                             buf[128], obuf[128];
        const ASN1_OBJECT               *obj, *octype;
        ASN1_OCTET_STRING               *kid = NULL;
@@ -89,10 +90,15 @@ cms_parse_validate_internal(X509 **xp, const char *fn, const unsigned char *der,
        if (der == NULL)
                return 0;
 
-       if ((cms = d2i_CMS_ContentInfo(NULL, &der, derlen)) == NULL) {
+       oder = der;
+       if ((cms = d2i_CMS_ContentInfo(NULL, &der, len)) == NULL) {
                cryptowarnx("%s: RFC 6488: failed CMS parse", fn);
                goto out;
        }
+       if (der != oder + len) {
+               warnx("%s: %td bytes trailing garbage", fn, oder + len - der);
+               goto out;
+       }
 
        /*
         * The CMS is self-signed with a signing certificate.

diff --git a/usr.sbin/rpki-client/crl.c b/usr.sbin/rpki-client/crl.c
index f5f7be6..da4eebf 100644 (file)
--- a/usr.sbin/rpki-client/crl.c
+++ b/usr.sbin/rpki-client/crl.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: crl.c,v 1.21 2022/11/30 09:03:44 job Exp $ */
+/*     $OpenBSD: crl.c,v 1.22 2023/02/21 10:18:47 tb Exp $ */
 /*
  * Copyright (c) 2019 Kristaps Dzonsons <kristaps@bsd.lv>
  *
@@ -25,9 +25,10 @@
 struct crl *
 crl_parse(const char *fn, const unsigned char *der, size_t len)
 {
-       struct crl      *crl;
-       const ASN1_TIME *at;
-       int              rc = 0;
+       const unsigned char     *oder;
+       struct crl              *crl;
+       const ASN1_TIME         *at;
+       int                      rc = 0;
 
        /* just fail for empty buffers, the warning was printed elsewhere */
        if (der == NULL)
@@ -36,10 +37,15 @@ crl_parse(const char *fn, const unsigned char *der, size_t len)
        if ((crl = calloc(1, sizeof(*crl))) == NULL)
                err(1, NULL);
 
+       oder = der;
        if ((crl->x509_crl = d2i_X509_CRL(NULL, &der, len)) == NULL) {
                cryptowarnx("%s: d2i_X509_CRL", fn);
                goto out;
        }
+       if (der != oder + len) {
+               warnx("%s: %td bytes trailing garbage", fn, oder + len - der);
+               goto out;
+       }
 
        if ((crl->aki = x509_crl_get_aki(crl->x509_crl, fn)) == NULL) {
                warnx("x509_crl_get_aki failed");