From 934fbb57cd6b0549754b91dafe57b3f63d754fad Mon Sep 17 00:00:00 2001 From: tb <> Date: Mon, 13 Nov 2023 10:33:00 +0000 Subject: Eliminate the timegm(3) dependency in libcrypto timegm(3) is not available on some operating systems we support in portable. We currently use musl's implementation, for which gcc-13 decided to emit warnings (which seem incorrect in general and are irrelevant in this case anyway). Instead of patching this up and diverge from upstream, we can avoid reports about compiler warnings by simply not depending on this function. Rework the caching of notBefore and notAfter by replacing timegm(3) with asn1_time_tm_to_time_t(3). Also make this API properly error checkable since at the time x509v3_cache_extensions(3) is called, nothing is known about the cert, in particular not whether it isn't malformed one way or the other. suggested by and ok beck --- src/lib/libcrypto/x509/x509_internal.h | 7 ++++--- src/lib/libcrypto/x509/x509_purp.c | 5 +++-- src/lib/libcrypto/x509/x509_verify.c | 38 ++++++++++++++++++++++++---------- src/lib/libcrypto/x509/x509_vfy.c | 5 ++--- 4 files changed, 36 insertions(+), 19 deletions(-) diff --git a/src/lib/libcrypto/x509/x509_internal.h b/src/lib/libcrypto/x509/x509_internal.h index 15efff6097..280d1ae46c 100644 --- a/src/lib/libcrypto/x509/x509_internal.h +++ b/src/lib/libcrypto/x509/x509_internal.h @@ -1,4 +1,4 @@ -/* $OpenBSD: x509_internal.h,v 1.26 2023/09/29 15:53:59 beck Exp $ */ +/* $OpenBSD: x509_internal.h,v 1.27 2023/11/13 10:33:00 tb Exp $ */ /* * Copyright (c) 2020 Bob Beck * @@ -96,7 +96,8 @@ int x509_vfy_callback_indicate_completion(X509_STORE_CTX *ctx); int x509v3_cache_extensions(X509 *x); X509 *x509_vfy_lookup_cert_match(X509_STORE_CTX *ctx, X509 *x); -time_t x509_verify_asn1_time_to_time_t(const ASN1_TIME *atime, int notafter); +int x509_verify_asn1_time_to_time_t(const ASN1_TIME *atime, int notafter, + time_t *out); struct x509_verify_ctx *x509_verify_ctx_new_from_xsc(X509_STORE_CTX *xsc); @@ -133,7 +134,7 @@ int x509_constraints_check(struct x509_constraints_names *names, struct x509_constraints_names *excluded, int *error); int x509_constraints_chain(STACK_OF(X509) *chain, int *error, int *depth); -void x509_verify_cert_info_populate(X509 *cert); +int x509_verify_cert_info_populate(X509 *cert); int x509_vfy_check_security_level(X509_STORE_CTX *ctx); __END_HIDDEN_DECLS diff --git a/src/lib/libcrypto/x509/x509_purp.c b/src/lib/libcrypto/x509/x509_purp.c index 0c92dfb19c..999ba639c5 100644 --- a/src/lib/libcrypto/x509/x509_purp.c +++ b/src/lib/libcrypto/x509/x509_purp.c @@ -1,4 +1,4 @@ -/* $OpenBSD: x509_purp.c,v 1.29 2023/08/18 08:42:41 tb Exp $ */ +/* $OpenBSD: x509_purp.c,v 1.30 2023/11/13 10:33:00 tb Exp $ */ /* Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL * project 2001. */ @@ -711,7 +711,8 @@ x509v3_cache_extensions_internal(X509 *x) if (!x509_extension_oids_are_unique(x)) x->ex_flags |= EXFLAG_INVALID; - x509_verify_cert_info_populate(x); + if (!x509_verify_cert_info_populate(x)) + x->ex_flags |= EXFLAG_INVALID; x->ex_flags |= EXFLAG_SET; } diff --git a/src/lib/libcrypto/x509/x509_verify.c b/src/lib/libcrypto/x509/x509_verify.c index ca4814d938..c4c89a23b9 100644 --- a/src/lib/libcrypto/x509/x509_verify.c +++ b/src/lib/libcrypto/x509/x509_verify.c @@ -1,4 +1,4 @@ -/* $OpenBSD: x509_verify.c,v 1.66 2023/05/07 07:11:50 tb Exp $ */ +/* $OpenBSD: x509_verify.c,v 1.67 2023/11/13 10:33:00 tb Exp $ */ /* * Copyright (c) 2020-2021 Bob Beck * @@ -27,6 +27,7 @@ #include #include +#include "asn1_local.h" #include "x509_internal.h" #include "x509_issuer_cache.h" @@ -44,21 +45,22 @@ static void x509_verify_chain_free(struct x509_verify_chain *chain); * Parse an asn1 to a representable time_t as per RFC 5280 rules. * Returns -1 if that can't be done for any reason. */ -time_t -x509_verify_asn1_time_to_time_t(const ASN1_TIME *atime, int notAfter) +int +x509_verify_asn1_time_to_time_t(const ASN1_TIME *atime, int notAfter, + time_t *out) { struct tm tm = { 0 }; int type; type = ASN1_time_parse(atime->data, atime->length, &tm, atime->type); if (type == -1) - return -1; + return 0; /* RFC 5280 section 4.1.2.5 */ if (tm.tm_year < 150 && type != V_ASN1_UTCTIME) - return -1; + return 0; if (tm.tm_year >= 150 && type != V_ASN1_GENERALIZEDTIME) - return -1; + return 0; if (notAfter) { /* @@ -67,7 +69,7 @@ x509_verify_asn1_time_to_time_t(const ASN1_TIME *atime, int notAfter) * date, limit the date to a 32 bit representable value. */ if (!ASN1_time_tm_clamp_notafter(&tm)) - return -1; + return 0; } /* @@ -75,22 +77,36 @@ x509_verify_asn1_time_to_time_t(const ASN1_TIME *atime, int notAfter) * a time_t. A time_t must be sane if you care about times after * Jan 19 2038. */ - return timegm(&tm); + return asn1_time_tm_to_time_t(&tm, out); } /* * Cache certificate hash, and values parsed out of an X509. * called from cache_extensions() */ -void +int x509_verify_cert_info_populate(X509 *cert) { + const ASN1_TIME *notBefore, *notAfter; + /* * Parse and save the cert times, or remember that they * are unacceptable/unparsable. */ - cert->not_before = x509_verify_asn1_time_to_time_t(X509_get_notBefore(cert), 0); - cert->not_after = x509_verify_asn1_time_to_time_t(X509_get_notAfter(cert), 1); + + cert->not_before = cert->not_after = -1; + + if ((notBefore = X509_get_notBefore(cert)) == NULL) + return 0; + if ((notAfter = X509_get_notAfter(cert)) == NULL) + return 0; + + if (!x509_verify_asn1_time_to_time_t(notBefore, 0, &cert->not_before)) + return 0; + if (!x509_verify_asn1_time_to_time_t(notAfter, 1, &cert->not_after)) + return 0; + + return 1; } struct x509_verify_chain * diff --git a/src/lib/libcrypto/x509/x509_vfy.c b/src/lib/libcrypto/x509/x509_vfy.c index c4ba3d5b14..6c0ad78ec8 100644 --- a/src/lib/libcrypto/x509/x509_vfy.c +++ b/src/lib/libcrypto/x509/x509_vfy.c @@ -1,4 +1,4 @@ -/* $OpenBSD: x509_vfy.c,v 1.125 2023/06/08 22:02:40 beck Exp $ */ +/* $OpenBSD: x509_vfy.c,v 1.126 2023/11/13 10:33:00 tb Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -1969,8 +1969,7 @@ X509_cmp_time_internal(const ASN1_TIME *ctm, time_t *cmp_time, int is_notafter) else compare = *cmp_time; - if ((cert_time = x509_verify_asn1_time_to_time_t(ctm, is_notafter)) == - -1) + if (!x509_verify_asn1_time_to_time_t(ctm, is_notafter, &cert_time)) return 0; /* invalid time */ if (cert_time <= compare) -- cgit v1.2.3-55-g6feb