*alternatively* the cache here was added mainly because timegm()
gmtime() are kind of horribly inefficient and so repeatly beating on
them mattered as demonstrated by claudio profiling things in
rpki-client. At the time I added this cache in the glorious spirit of
OpenSSL that no performance problem can't be solved by adding another
thing to a sea of global state (and it worked, we avoided the time
calls) ;)

What has changed now is we brought in the BoringSSL style time
conversion using a julien date in seconds based on the unix epoch in
zulu time, rather than the OpenSSL julien date conversion of days from
some random year important only to time nerds for Byzantine reasons
(<- dad joke), so we no longer need to use timegm() and gmtime().
These BoringSSL style conversions aren't slow. So I don't believe
caching these values will have significant performance impact anymore,
and perhaps we could just remove the caching of these values entirely
if people prefer.

On Sat, Apr 06, 2024 at 11:42:55AM -0600, Bob Beck wrote:
> Populating the notbefore/notafter cache should not be fatal.
> 
> The addition of the cache of the parsed notbefore/notafter was not
> meant to mark a certificate invalid at an earlier time, merely to
> avoid re-parsing times repeatedly (in things like rpki-client)
> 
> This reverts to the earlier behaviour of saving the values as marked
> invalid, so that they may be considered in certificate validation
> later (which may then fail)
> 
> This should correct Mischa's strange ftp behavior.
> 
> We still have a long-standing bug here that has been exposed as to why
> extensions parsing failues fail the handshake even when server
> certificate validation is not requested - we really should not 
> have to care if we are not validating the server certificate. but
> this appears to be a long standing OpenSSL inconsistency - we 
> should just not pile failures on here to make it worse.
> 
> ok?
> 
> ---
>  lib/libcrypto/x509/x509_internal.h |  2 +-
>  lib/libcrypto/x509/x509_purp.c     | 11 +++++++++--
>  lib/libcrypto/x509/x509_verify.c   | 21 +++++----------------
>  3 files changed, 15 insertions(+), 19 deletions(-)
> 
> diff --git a/lib/libcrypto/x509/x509_internal.h b/lib/libcrypto/x509/x509_internal.h
> index 280d1ae46cf..d9da539a588 100644
> --- a/lib/libcrypto/x509/x509_internal.h
> +++ b/lib/libcrypto/x509/x509_internal.h
> @@ -134,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);
> -int x509_verify_cert_info_populate(X509 *cert);
> +void x509_verify_cert_info_populate(X509 *cert);
>  int x509_vfy_check_security_level(X509_STORE_CTX *ctx);
>  
>  __END_HIDDEN_DECLS
> diff --git a/lib/libcrypto/x509/x509_purp.c b/lib/libcrypto/x509/x509_purp.c
> index 53f4f2f9675..49d54f33516 100644
> --- a/lib/libcrypto/x509/x509_purp.c
> +++ b/lib/libcrypto/x509/x509_purp.c
> @@ -559,8 +559,15 @@ x509v3_cache_extensions_internal(X509 *x)
>  	if (!x509_extension_oids_are_unique(x))
>  		x->ex_flags |= EXFLAG_INVALID;
>  
> -	if (!x509_verify_cert_info_populate(x))
> -		x->ex_flags |= EXFLAG_INVALID;
> +	/*
> +	 * Populate a cache of notbefore/notafter values
> +	 * to avoid re-parsing time later.
> +	 *
> +	 * These could be cached as invalid, and may fail
> +	 * later in certificate validation, but should
> +	 * not fail here.
> +	 */
> +	x509_verify_cert_info_populate(x);
>  
>  	x->ex_flags |= EXFLAG_SET;
>  }
> diff --git a/lib/libcrypto/x509/x509_verify.c b/lib/libcrypto/x509/x509_verify.c
> index 19bb925d9c6..83841d2a4c0 100644
> --- a/lib/libcrypto/x509/x509_verify.c
> +++ b/lib/libcrypto/x509/x509_verify.c
> @@ -84,29 +84,18 @@ x509_verify_asn1_time_to_time_t(const ASN1_TIME *atime, int notAfter,
>   * Cache certificate hash, and values parsed out of an X509.
>   * called from cache_extensions()
>   */
> -int
> +void
>  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 = 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;
> +	(void) x509_verify_asn1_time_to_time_t(X509_get_notBefore(cert), 0,
> +	    &cert->not_before);
> +	(void) x509_verify_asn1_time_to_time_t(X509_get_notAfter(cert), 1,
> +	    &cert->not_after);
>  }
>  
>  struct x509_verify_chain *
> -- 
> 2.44.0
>