On Tue, May 09, 2023 at 05:30:26PM +0200, Theo Buehler wrote:
> beck ported the OpenSSL ASN1_TIME API to use the ASN1_time* API under
> the hood, so for LibreSSL the diff here is a noop.
>
> This allows us to eliminate most of the gross openssl hacks in regress.
> The unistd.h thing is still needed because of STACK_OF discrepancies,
> but the reacharound into libcrypto disappears, which is already a big
> step forward.
>
> This will simplify portable as well at the cost of requiring LibreSSL 3.6
> at a minimum. I think we have enough reason to require this for the next
> release and you should not be using LibreSSL 3.5 anymore anyway.
>
> If you are using rpki-client with OpenSSL, you're already using these
> functions under the hood, so the yolo factor isn't really increased.
>
> As usual, there are some weird API quirks such as passing a NULL time
> resulting in ASN1_TIME_to_tm() silently returning the current time, so
> check for this, because who would want that and why...
>
> If you really only want to accept an ASN1_GENERALIZEDTIME with Zulu
> time, you're supposed to set a flag on the time. LibreSSL doesn't accept
> anything but Zulu time and doesn't have this flag. Since nobody appears
> to have discovered this yet, we certainly don't want it, so I decided to
> do a type and length check by hand and to punt on checking for the Z at
> the end because that's just gross.
>
> ASN1_time_tm_cmp() could have been replaced by a straightforward
> comparison of time_t a long time ago.
I like this diff. As usual OpenSSL make sure you can never ever have
something nice.
Diff is OK claudio@
> Index: usr.sbin/rpki-client/crl.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/crl.c,v
> retrieving revision 1.24
> diff -u -p -r1.24 crl.c
> --- usr.sbin/rpki-client/crl.c 10 Mar 2023 12:44:56 -0000 1.24
> +++ usr.sbin/rpki-client/crl.c 9 May 2023 14:14:24 -0000
> @@ -75,7 +75,7 @@ crl_parse(const char *fn, const unsigned
> goto out;
> }
> if (!x509_get_time(at, &crl->lastupdate)) {
> - warnx("%s: ASN1_time_parse failed", fn);
> + warnx("%s: ASN1_TIME_to_tm failed", fn);
> goto out;
> }
>
> @@ -85,7 +85,7 @@ crl_parse(const char *fn, const unsigned
> goto out;
> }
> if (!x509_get_time(at, &crl->nextupdate)) {
> - warnx("%s: ASN1_time_parse failed", fn);
> + warnx("%s: ASN1_TIME_to_tm failed", fn);
> goto out;
> }
>
> Index: usr.sbin/rpki-client/mft.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/mft.c,v
> retrieving revision 1.91
> diff -u -p -r1.91 mft.c
> --- usr.sbin/rpki-client/mft.c 26 Apr 2023 16:32:41 -0000 1.91
> +++ usr.sbin/rpki-client/mft.c 9 May 2023 14:47:40 -0000
> @@ -87,6 +87,8 @@ ASN1_SEQUENCE(Manifest) = {
> DECLARE_ASN1_FUNCTIONS(Manifest);
> IMPLEMENT_ASN1_FUNCTIONS(Manifest);
>
> +#define GENTIME_LENGTH 15
> +
> /*
> * Convert an ASN1_GENERALIZEDTIME to a struct tm.
> * Returns 1 on success, 0 on failure.
> @@ -94,15 +96,18 @@ IMPLEMENT_ASN1_FUNCTIONS(Manifest);
> static int
> generalizedtime_to_tm(const ASN1_GENERALIZEDTIME *gtime, struct tm *tm)
> {
> - const char *data;
> - size_t len;
> -
> - data = ASN1_STRING_get0_data(gtime);
> - len = ASN1_STRING_length(gtime);
> + /*
> + * ASN1_GENERALIZEDTIME is just another name for ASN1_STRING. Check
> + * its type and length, so we don't accidentally accept a UTCTime.
> + * We don't want to mess about with silly flags on gtime.
> + */
> + if (ASN1_STRING_type(gtime) != V_ASN1_GENERALIZEDTIME)
> + return 0;
> + if (ASN1_STRING_length(gtime) != GENTIME_LENGTH)
> + return 0;
>
> memset(tm, 0, sizeof(*tm));
> - return ASN1_time_parse(data, len, tm, V_ASN1_GENERALIZEDTIME) ==
> - V_ASN1_GENERALIZEDTIME;
> + return ASN1_TIME_to_tm(gtime, tm);
> }
>
> /*
> @@ -124,15 +129,14 @@ mft_parse_time(const ASN1_GENERALIZEDTIM
> return 0;
> }
>
> - /* check that until is not before from */
> - if (ASN1_time_tm_cmp(&tm_until, &tm_from) < 0) {
> - warnx("%s: bad update interval", p->fn);
> - return 0;
> - }
> -
> if ((p->res->thisupdate = timegm(&tm_from)) == -1 ||
> (p->res->nextupdate = timegm(&tm_until)) == -1)
> errx(1, "%s: timegm failed", p->fn);
> +
> + if (p->res->thisupdate > p->res->nextupdate) {
> + warnx("%s: bad update interval", p->fn);
> + return 0;
> + }
>
> return 1;
> }
> Index: usr.sbin/rpki-client/x509.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/x509.c,v
> retrieving revision 1.70
> diff -u -p -r1.70 x509.c
> --- usr.sbin/rpki-client/x509.c 14 Mar 2023 07:09:11 -0000 1.70
> +++ usr.sbin/rpki-client/x509.c 9 May 2023 14:14:24 -0000
> @@ -506,7 +506,7 @@ x509_get_notbefore(X509 *x, const char *
> return 0;
> }
> if (!x509_get_time(at, tt)) {
> - warnx("%s: ASN1_time_parse failed", fn);
> + warnx("%s: ASN1_TIME_to_tm failed", fn);
> return 0;
> }
> return 1;
> @@ -526,7 +526,7 @@ x509_get_notafter(X509 *x, const char *f
> return 0;
> }
> if (!x509_get_time(at, tt)) {
> - warnx("%s: ASN1_time_parse failed", fn);
> + warnx("%s: ASN1_TIME_to_tm failed", fn);
> return 0;
> }
> return 1;
> @@ -757,7 +757,10 @@ x509_get_time(const ASN1_TIME *at, time_
>
> *t = 0;
> memset(&tm, 0, sizeof(tm));
> - if (ASN1_time_parse(at->data, at->length, &tm, 0) == -1)
> + /* Fail instead of silently falling back to the current time. */
> + if (at == NULL)
> + return 0;
> + if (!ASN1_TIME_to_tm(at, &tm))
> return 0;
> if ((*t = timegm(&tm)) == -1)
> errx(1, "timegm failed");
> Index: regress/usr.sbin/rpki-client/openssl11/Makefile
> ===================================================================
> RCS file: /cvs/src/regress/usr.sbin/rpki-client/openssl11/Makefile,v
> retrieving revision 1.17
> diff -u -p -r1.17 Makefile
> --- regress/usr.sbin/rpki-client/openssl11/Makefile 14 Apr 2023 15:35:12
> -0000 1.17
> +++ regress/usr.sbin/rpki-client/openssl11/Makefile 9 May 2023 15:07:54
> -0000
> @@ -2,43 +2,7 @@
>
> LDADD += -Wl,-rpath,/usr/local/lib/eopenssl11 -L/usr/local/lib/eopenssl11
> CFLAGS += -I${.CURDIR}/ -I/usr/local/include/eopenssl11/
> -CFLAGS += -DLIBRESSL_INTERNAL
> -
> -# For mft.c we need ASN1_time_parse() and ASN1_time_tm_cmp() from LibreSSL
> -
> -# Provide a missing prototype
> -a_time_tm_gen.c: a_time_tm.c
> - echo '#include <openssl/asn1.h>\n' > $@.tmp
> - echo '#include "bytestring.h"\n' >> $@.tmp
> - echo '#define ASN1error(err) ASN1err(0, (err));' >> $@.tmp
> - cat $> >> $@.tmp
> - mv -f $@.tmp $@
> -
> -CLEANFILES += a_time_tm_gen.c a_time_tm_gen.c.tmp
> -
> -LIBCRYPTO_COMPAT += a_time_tm_gen.c a_time_posix.c
> -LIBCRYPTO_COMPAT += bs_ber.c bs_cbb.c bs_cbs.c
> -
> -SRCS_test-ip = ${LIBCRYPTO_COMPAT}
> -SRCS_test-mft = ${LIBCRYPTO_COMPAT}
> -SRCS_test-roa = ${LIBCRYPTO_COMPAT}
> -SRCS_test-cert = ${LIBCRYPTO_COMPAT}
> -SRCS_test-gbr = ${LIBCRYPTO_COMPAT}
> -SRCS_test-geofeed = ${LIBCRYPTO_COMPAT}
> -SRCS_test-tal = ${LIBCRYPTO_COMPAT}
> -SRCS_test-bgpsec = ${LIBCRYPTO_COMPAT}
> -SRCS_test-rrdp = ${LIBCRYPTO_COMPAT}
> -SRCS_test-rsc = ${LIBCRYPTO_COMPAT}
> -SRCS_test-aspa = ${LIBCRYPTO_COMPAT}
> -SRCS_test-tak = ${LIBCRYPTO_COMPAT}
> -
> -CFLAGS += -I${.CURDIR}/../../../../lib/libcrypto/
> -CFLAGS += -I${.CURDIR}/../../../../lib/libcrypto/asn1
> -CFLAGS += -I${.CURDIR}/../../../../lib/libcrypto/bytestring
>
> .PATH: ${.CURDIR}/..
> -.PATH: ${.CURDIR}/../../../../lib/libcrypto
> -.PATH: ${.CURDIR}/../../../../lib/libcrypto/asn1
> -.PATH: ${.CURDIR}/../../../../lib/libcrypto/bytestring
>
> .include <bsd.regress.mk>
> Index: regress/usr.sbin/rpki-client/openssl11/unistd.h
> ===================================================================
> RCS file: /cvs/src/regress/usr.sbin/rpki-client/openssl11/unistd.h,v
> retrieving revision 1.3
> diff -u -p -r1.3 unistd.h
> --- regress/usr.sbin/rpki-client/openssl11/unistd.h 30 Jun 2022 07:28:38
> -0000 1.3
> +++ regress/usr.sbin/rpki-client/openssl11/unistd.h 9 May 2023 15:12:02
> -0000
> @@ -6,11 +6,7 @@
>
> #include_next <unistd.h>
>
> -#include <openssl/asn1.h>
> #include <openssl/stack.h>
> -
> -int ASN1_time_parse(const char *, size_t, struct tm *, int);
> -int ASN1_time_tm_cmp(struct tm *, struct tm *);
>
> #ifndef DECLARE_STACK_OF
> #define DECLARE_STACK_OF DEFINE_STACK_OF
>
--
:wq Claudio
