From: Theo Buehler <tb@theobuehler.org>
Subject: rpki-client: disallow certs with zero serialNumber
To: tech@openbsd.org
Date: Thu, 29 Oct 2026 17:03:50 +0100

Consistency is hard.

RFC 5280 (eid3200 clarifies) and RFC 6487 disallow a zero serialNumber.

For extra fun, CRLNumber and manifestNumber are defined via ASN.1
explicitly allowing zero. So we need some extra dances.

Arguably eid3200 was resolved the wrong way.

Index: cert.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
diff -u -p -r1.248 cert.c
--- cert.c	14 Oct 2026 06:41:29 -0000	1.248
+++ cert.c	29 Oct 2026 15:49:14 -0000
@@ -1504,7 +1504,7 @@ cert_parse_internal(const char *fn, X509
 		warnx("%s: RFC 6487 4.2: missing serialNumber", fn);
 		goto out;
 	}
-	if (!x509_valid_seqnum(fn, "RFC 6487 4.2: serialNumber", serial))
+	if (!x509_valid_serial(fn, "RFC 6487 4.2: serialNumber", serial))
 		goto out;
 
 	if (!cert_check_sigalg(fn, cert))
Index: extern.h
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v
diff -u -p -r1.317 extern.h
--- extern.h	26 Oct 2026 08:19:00 -0000	1.317
+++ extern.h	29 Oct 2026 15:49:27 -0000
@@ -1011,6 +1011,8 @@ char		*x509_convert_seqnum(const char *,
 		    const ASN1_INTEGER *);
 int		 x509_valid_seqnum(const char *, const char *,
 		    const ASN1_INTEGER *);
+int		 x509_valid_serial(const char *, const char *,
+		    const ASN1_INTEGER *);
 int		 x509_check_tbs_sigalg(const char *, const X509_ALGOR *);
 int		 x509_location(const char *, const char *, GENERAL_NAME *,
 		    char **);
Index: x509.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/x509.c,v
diff -u -p -r1.136 x509.c
--- x509.c	9 Oct 2026 08:34:39 -0000	1.136
+++ x509.c	29 Oct 2026 15:58:22 -0000
@@ -479,11 +479,12 @@ x509_valid_issuer_name(const char *fn, c
 }
 
 /*
- * Check ASN1_INTEGER is non-negative and fits in 20 octets.
+ * Check ASN1_INTEGER is non-negative (or positive) and fits in 20 octets.
  * Returns allocated BIGNUM if true, NULL otherwise.
  */
 static BIGNUM *
-x509_seqnum_to_bn(const char *fn, const char *descr, const ASN1_INTEGER *i)
+x509_seqnum_to_bn(const char *fn, const char *descr, const ASN1_INTEGER *i,
+    int exclude_zero)
 {
 	BIGNUM *bn = NULL;
 
@@ -492,8 +493,9 @@ x509_seqnum_to_bn(const char *fn, const 
 		goto out;
 	}
 
-	if (BN_is_negative(bn)) {
-		warnx("%s: %s should be non-negative", fn, descr);
+	if (BN_is_negative(bn) || (exclude_zero && BN_is_zero(bn))) {
+		warnx("%s: %s should be %s", fn, descr,
+		    exclude_zero ? "positive" : "non-negative");
 		goto out;
 	}
 
@@ -524,7 +526,7 @@ x509_convert_seqnum(const char *fn, cons
 	if (i == NULL)
 		goto out;
 
-	if ((bn = x509_seqnum_to_bn(fn, descr, i)) == NULL)
+	if ((bn = x509_seqnum_to_bn(fn, descr, i, 0)) == NULL)
 		goto out;
 
 	if ((s = BN_bn2hex(bn)) == NULL)
@@ -535,12 +537,32 @@ x509_convert_seqnum(const char *fn, cons
 	return s;
 }
 
+/*
+ * Check seqnum is non-negative and fits into 20 octets:
+ *   RFC 5280: CRLNumber ::= INTEGER (0..MAX)
+ *   RFC 9286: manifestNumber  INTEGER (0..MAX)
+ */
 int
 x509_valid_seqnum(const char *fn, const char *descr, const ASN1_INTEGER *i)
 {
 	BIGNUM *bn;
 
-	if ((bn = x509_seqnum_to_bn(fn, descr, i)) == NULL)
+	if ((bn = x509_seqnum_to_bn(fn, descr, i, 0)) == NULL)
+		return 0;
+
+	BN_free(bn);
+	return 1;
+}
+
+/*
+ * RFC 5280, 4.1.2.2, RFC 6487, 4.2: serialNumber is positive (also eid3200).
+ */
+int
+x509_valid_serial(const char *fn, const char *descr, const ASN1_INTEGER *i)
+{
+	BIGNUM *bn;
+
+	if ((bn = x509_seqnum_to_bn(fn, descr, i, 1)) == NULL)
 		return 0;
 
 	BN_free(bn);