Index | Thread | Search

From:
Theo Buehler <tb@theobuehler.org>
Subject:
rpki-client: fix some incorrect checks
To:
tech@openbsd.org
Date:
Mon, 7 Oct 2024 11:29:41 +0200

Download raw body.

Thread
The validity interval is defined to be

   the date on which the certificate validity period begins (notBefore)
   and the date on which the certificate validity period ends
   (notAfter).

https://www.rfc-editor.org/rfc/rfc5280#section-4.1.2.5

This should therefore include the end points.

Instead of trying to be smart, let's check the BN itself. This way we
can be sure the top bit isn't set and we will only accept numbers in
the interval [0..2^159 - 1].

Index: cert.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
diff -u -p -r1.150 cert.c
--- cert.c	8 Jul 2024 15:31:58 -0000	1.150
+++ cert.c	7 Oct 2024 09:17:53 -0000
@@ -1085,11 +1085,11 @@ ta_parse(const char *fn, struct cert *p,
 		    "pubkey does not match TAL pubkey", fn);
 		goto badcert;
 	}
-	if (p->notbefore >= now) {
+	if (p->notbefore > now) {
 		warnx("%s: certificate not yet valid", fn);
 		goto badcert;
 	}
-	if (p->notafter <= now) {
+	if (p->notafter < now) {
 		warnx("%s: certificate has expired", fn);
 		goto badcert;
 	}
Index: x509.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/x509.c,v
diff -u -p -r1.101 x509.c
--- x509.c	12 Sep 2024 10:33:25 -0000	1.101
+++ x509.c	7 Oct 2024 09:18:53 -0000
@@ -1023,11 +1023,6 @@ x509_seqnum_to_bn(const char *fn, const 
 {
 	BIGNUM *bn = NULL;
 
-	if (ASN1_STRING_length(i) > 20) {
-		warnx("%s: %s should fit in 20 octets", fn, descr);
-		goto out;
-	}
-
 	if ((bn = ASN1_INTEGER_to_BN(i, NULL)) == NULL) {
 		warnx("%s: %s: ASN1_INTEGER_to_BN error", fn, descr);
 		goto out;
@@ -1035,6 +1030,12 @@ x509_seqnum_to_bn(const char *fn, const 
 
 	if (BN_is_negative(bn)) {
 		warnx("%s: %s should be non-negative", fn, descr);
+		goto out;
+	}
+
+	/* Reject values larger than or equal to 2^159. */
+	if (BN_num_bytes(bn) > 20 || BN_is_bit_set(bn, 159)) {
+		warnx("%s: %s should fit in 20 octets", fn, descr);
 		goto out;
 	}