Clamp the manifestNumber to 20 octets value
authortb <tb@openbsd.org>
Sun, 24 Mar 2024 00:38:58 +0000 (00:38 +0000)
committertb <tb@openbsd.org>
Sun, 24 Mar 2024 00:38:58 +0000 (00:38 +0000)
The standards contain somewhat ambiguous language as to what the largest
acceptable value for a crlNumber or manifestNumber could be, due to a
limitation to 20 octets. The question is what 20 octets specifically are
meant...

Consensus seems to have emerged that the largest value is 2^159-1 since
2^160-1 would encode to 21 octets due to a padding octet to disambiguate
ff .. ff from -7f ff .. ff (iow the top bit of the first octet is a sign
bit).

Thus, switch from 2^160 - 1 to 2^159 - 1 as an upper bound by checking
the length of the value portion of the DER encoded ASN.1 integer to be
at most 20 octets.

Thanks to Martin Hoffmann, Tom Harrison, and Ben Maddison for raising and
discussing the issue. Thanks also to the spec authors for making me waste
a few hours of my life on a single bit.

ok job

usr.sbin/rpki-client/x509.c

index a2257cc..45aa100 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: x509.c,v 1.84 2024/03/22 03:38:12 job Exp $ */
+/*     $OpenBSD: x509.c,v 1.85 2024/03/24 00:38:58 tb Exp $ */
 /*
  * Copyright (c) 2022 Theo Buehler <tb@openbsd.org>
  * Copyright (c) 2021 Claudio Jeker <claudio@openbsd.org>
@@ -1024,6 +1024,12 @@ x509_convert_seqnum(const char *fn, const ASN1_INTEGER *i)
        if (i == NULL)
                goto out;
 
+       if (ASN1_STRING_length(i) > 20) {
+               warnx("%s: %s: want 20 octets or fewer, have more.",
+                   __func__, fn);
+               goto out;
+       }
+
        seqnum = ASN1_INTEGER_to_BN(i, NULL);
        if (seqnum == NULL) {
                warnx("%s: ASN1_INTEGER_to_BN error", fn);
@@ -1036,12 +1042,6 @@ x509_convert_seqnum(const char *fn, const ASN1_INTEGER *i)
                goto out;
        }
 
-       if (BN_num_bytes(seqnum) > 20) {
-               warnx("%s: %s: want 20 octets or fewer, have more.",
-                   __func__, fn);
-               goto out;
-       }
-
        s = BN_bn2hex(seqnum);
        if (s == NULL)
                warnx("%s: BN_bn2hex error", fn);