On Mon, May 22, 2023 at 05:48:01PM +0200, Theo Buehler wrote:
> LibreSSL 3.6 added ASN1_INTEGER_get_uint64() from OpenSSL. While this
> still isn't great, at least it allows for unambiguous error checking.
>
> In as_id_parse() we can replace some hand-rolled parsing which
> simplifies things a bit.
>
> The ASN1_INTEGER_get() API should not be used since it doesn't allow you
> to distinguish errors from the (sometimes) legitimate return value -1.
> Either you ignore that or you get to do some extra dances.
>
> The conversion of the ASN1_INTEGER_get() uses is straightforward. The
> price to pay is a cast for the printing, since that is the lesser evil
> than the ugly PRIu64 macro.
Looks good to me but did not manage to test it yet.
This is a big improvement over the previous mess.
OK claudio@
> Index: as.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/as.c,v
> retrieving revision 1.11
> diff -u -p -r1.11 as.c
> --- as.c 30 Nov 2022 08:17:21 -0000 1.11
> +++ as.c 9 May 2023 11:16:58 -0000
> @@ -23,38 +23,16 @@
>
> #include "extern.h"
>
> -/*
> - * Parse a uint32_t AS identifier from an ASN1_INTEGER.
> - * This relies on the specification for ASN1_INTEGER itself, which is
> - * essentially a series of big-endian bytes in the unsigned case.
> - * All we do here is check if the number is negative then start copying
> - * over bytes.
> - * This is necessary because ASN1_INTEGER_get() on a 32-bit machine
> - * (e.g., i386) will fail for AS numbers of UINT32_MAX.
> - */
> +/* Parse a uint32_t AS identifier from an ASN1_INTEGER. */
> int
> as_id_parse(const ASN1_INTEGER *v, uint32_t *out)
> {
> - int i;
> - uint32_t res = 0;
> + uint64_t res = 0;
>
> - /* If the negative bit is set, this is wrong. */
> -
> - if (v->type & V_ASN1_NEG)
> + if (!ASN1_INTEGER_get_uint64(&res, v))
> return 0;
> -
> - /* Too many bytes for us to consider. */
> -
> - if ((size_t)v->length > sizeof(uint32_t))
> + if (res > UINT32_MAX)
> return 0;
> -
> - /* Stored as big-endian bytes. */
> -
> - for (i = 0; i < v->length; i++) {
> - res <<= 8;
> - res |= v->data[i];
> - }
> -
> *out = res;
> return 1;
> }
> Index: roa.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/roa.c,v
> retrieving revision 1.66
> diff -u -p -r1.66 roa.c
> --- roa.c 26 Apr 2023 16:32:41 -0000 1.66
> +++ roa.c 9 May 2023 11:59:44 -0000
> @@ -107,7 +107,7 @@ roa_parse_econtent(const unsigned char *
> int addrsz;
> enum afi afi;
> const ROAIPAddress *addr;
> - long maxlen;
> + uint64_t maxlen;
> struct ip_addr ipaddr;
> struct roa_ip *res;
> int ipaddrblocksz;
> @@ -168,21 +168,23 @@ roa_parse_econtent(const unsigned char *
> maxlen = ipaddr.prefixlen;
>
> if (addr->maxLength != NULL) {
> - maxlen = ASN1_INTEGER_get(addr->maxLength);
> - if (maxlen < 0) {
> + if (!ASN1_INTEGER_get_uint64(&maxlen,
> + addr->maxLength)) {
> warnx("%s: RFC 6482 section 3.2: "
> - "ASN1_INTEGER_get failed", p->fn);
> + "ASN1_INTEGER_get_uint64 failed",
> + p->fn);
> goto out;
> }
> if (ipaddr.prefixlen > maxlen) {
> warnx("%s: prefixlen (%d) larger than "
> - "maxLength (%ld)", p->fn,
> - ipaddr.prefixlen, maxlen);
> + "maxLength (%llu)", p->fn,
> + ipaddr.prefixlen,
> + (unsigned long long)maxlen);
> goto out;
> }
> if (maxlen > ((afi == AFI_IPV4) ? 32 : 128)) {
> - warnx("%s: maxLength (%ld) too large",
> - p->fn, maxlen);
> + warnx("%s: maxLength (%llu) too large",
> + p->fn, (unsigned long long)maxlen);
> goto out;
> }
> }
> Index: validate.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/validate.c,v
> retrieving revision 1.61
> diff -u -p -r1.61 validate.c
> --- validate.c 11 May 2023 14:05:31 -0000 1.61
> +++ validate.c 12 May 2023 05:37:43 -0000
> @@ -516,13 +516,13 @@ valid_rsc(const char *fn, struct cert *c
> int
> valid_econtent_version(const char *fn, const ASN1_INTEGER *aint)
> {
> - long version;
> + uint64_t version;
>
> if (aint == NULL)
> return 1;
>
> - if ((version = ASN1_INTEGER_get(aint)) < 0) {
> - warnx("%s: ASN1_INTEGER_get failed", fn);
> + if (!ASN1_INTEGER_get_uint64(&version, aint)) {
> + warnx("%s: ASN1_INTEGER_get_uint64 failed", fn);
> return 0;
> }
>
> @@ -531,7 +531,8 @@ valid_econtent_version(const char *fn, c
> warnx("%s: incorrect encoding for version 0", fn);
> return 0;
> default:
> - warnx("%s: version %ld not supported (yet)", fn, version);
> + warnx("%s: version %llu not supported (yet)", fn,
> + (unsigned long long)version);
> return 0;
> }
> }
>
--
:wq Claudio
