From: tb <tb@openbsd.org>
Date: Tue, 21 Feb 2023 10:18:47 +0000 (+0000)
Subject: rpki-client: ensure there is no trailing garbage in signed objects
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=797cceee728f53256e7696af4456493737a92932;p=openbsd

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 1f1348c16c3..1a749116abe 100644
--- 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 1b2e6998885..79f6dbf91de 100644
--- 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 f5f7be669e3..da4eebfe7f7 100644
--- 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");