From: job <job@openbsd.org>
Date: Mon, 14 Jun 2021 12:08:50 +0000 (+0000)
Subject: Fix ROA & MFT version check handling
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=f6a8e99484a9a5e649a4b7056ebc97e09759f97b;p=openbsd

Fix ROA & MFT version check handling

This check was incomplete: to correctly verify the ASN1 INTEGER holding
the version component, first the context specific explicit tag would need
to be checked.

However, the X.690 spec (section 11.5) states that if the one should not
encode a component if it is the default value. There are no RFCs specifying
new versions of ROA or MFT, so checking the content of the version component
should be skipped entirely.

Thus if the version component is present, something is wrong.
---

diff --git a/usr.sbin/rpki-client/mft.c b/usr.sbin/rpki-client/mft.c
index e1c68ac1b11..a866ab2c09a 100644
--- a/usr.sbin/rpki-client/mft.c
+++ b/usr.sbin/rpki-client/mft.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: mft.c,v 1.34 2021/05/11 11:32:51 claudio Exp $ */
+/*	$OpenBSD: mft.c,v 1.35 2021/06/14 12:08:50 job Exp $ */
 /*
  * Copyright (c) 2019 Kristaps Dzonsons <kristaps@bsd.lv>
  *
@@ -260,8 +260,7 @@ mft_parse_econtent(const unsigned char *d, size_t dsz, struct parse *p)
 	const ASN1_TYPE		*t;
 	const ASN1_GENERALIZEDTIME *from, *until;
 	BIGNUM			*mft_seqnum = NULL;
-	long			 mft_version;
-	int			 i, rc = -1;
+	int			 i = 0, rc = -1;
 
 	if ((seq = d2i_ASN1_SEQUENCE_ANY(NULL, &d, dsz)) == NULL) {
 		cryptowarnx("%s: RFC 6486 section 4.2: Manifest: "
@@ -269,7 +268,7 @@ mft_parse_econtent(const unsigned char *d, size_t dsz, struct parse *p)
 		goto out;
 	}
 
-	/* The profile version is optional. */
+	/* The profile version field is optional. */
 
 	if (sk_ASN1_TYPE_num(seq) != 5 &&
 	    sk_ASN1_TYPE_num(seq) != 6) {
@@ -279,27 +278,11 @@ mft_parse_econtent(const unsigned char *d, size_t dsz, struct parse *p)
 		goto out;
 	}
 
-	/* Start with optional profile version. */
-
-	i = 0;
 	if (sk_ASN1_TYPE_num(seq) == 6) {
-		t = sk_ASN1_TYPE_value(seq, i++);
-		if (t->type != V_ASN1_INTEGER) {
-			warnx("%s: RFC 6486 section 4.2.1: version: "
-			    "want ASN.1 integer, have %s (NID %d)",
-			    p->fn, ASN1_tag2str(t->type), t->type);
-			goto out;
-		}
-
-		if (t->value.integer == NULL)
-			goto out;
-
-		mft_version = ASN1_INTEGER_get(t->value.integer);
-		if (mft_version != 0) {
-			warnx("%s: RFC 6486 section 4.2.1: version: "
-			    "want 0, have %ld", p->fn, mft_version);
-			goto out;
-		}
+		warnx("%s: RFC 6486 section 4.2.1 and X.690, 11.5: not "
+		    "expecting version field, as only version 0 is supported",
+		    p->fn);
+		goto out;
 	}
 
 	/* Now the manifest sequence number. */
diff --git a/usr.sbin/rpki-client/roa.c b/usr.sbin/rpki-client/roa.c
index 4bcf60ea7d7..5d78d3156e9 100644
--- a/usr.sbin/rpki-client/roa.c
+++ b/usr.sbin/rpki-client/roa.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: roa.c,v 1.19 2021/05/11 11:32:51 claudio Exp $ */
+/*	$OpenBSD: roa.c,v 1.20 2021/06/14 12:08:50 job Exp $ */
 /*
  * Copyright (c) 2019 Kristaps Dzonsons <kristaps@bsd.lv>
  *
@@ -268,25 +268,9 @@ roa_parse_econtent(const unsigned char *d, size_t dsz, struct parse *p)
 	/* RFC 6482, section 3.1. */
 
 	if (sz == 3) {
-		t = sk_ASN1_TYPE_value(seq, i++);
-
-		/*
-		 * This check with ASN1_INTEGER_get() is fine since
-		 * we're looking for a value of zero anyway, so any
-		 * overflowing number will be definition be wrong.
-		 */
-
-		if (t->type != V_ASN1_INTEGER) {
-			warnx("%s: RFC 6482 section 3.1: version: "
-			    "want ASN.1 integer, have %s (NID %d)",
-			    p->fn, ASN1_tag2str(t->type), t->type);
-			goto out;
-		} else if (ASN1_INTEGER_get(t->value.integer) != 0) {
-			warnx("%s: RFC 6482 section 3.1: version: "
-			    "want version 0, have %ld",
-			    p->fn, ASN1_INTEGER_get(t->value.integer));
-			goto out;
-		}
+		warnx("%s: RFC 6482 section 3 and X.690, 11.5: not expecting "
+		    "version field, as only version 0 is supported", p->fn);
+		goto out;
 	}
 
 	/*