Remove notBefore and notAfter cacheing.
authorbeck <beck@openbsd.org>
Mon, 8 Apr 2024 23:46:21 +0000 (23:46 +0000)
committerbeck <beck@openbsd.org>
Mon, 8 Apr 2024 23:46:21 +0000 (23:46 +0000)
This cache was added because our time conversion used timegm()
and gmtime() which aren't very cheap. These calls were noticably
expensive when profiling things like rpki-client which do many
X.509 validations.

Now that we convert times using julien seconds from the unix
epoch, BoringSSL style, instead of a julien days from a
Byzantine date, we no longer use timegm() and gmtime().

Since the julien seconds calculaitons are cheap for conversion,
we don't need to bother caching this, it doesn't have a noticable
performance impact.

While we are at this correct a bug where
x509_verify_asn1_time_to_time_t was not NULL safe.

Tested for performance regressions by tb@ and job@

ok tb@ job@

lib/libcrypto/x509/x509_local.h
lib/libcrypto/x509/x509_purp.c
lib/libcrypto/x509/x509_verify.c
lib/libcrypto/x509/x509_vfy.c

index 73cc582..5b74b0d 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: x509_local.h,v 1.23 2024/03/26 05:39:47 tb Exp $ */
+/*     $OpenBSD: x509_local.h,v 1.24 2024/04/08 23:46:21 beck Exp $ */
 /* Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL
  * project 2013.
  */
@@ -188,8 +188,6 @@ struct x509_st {
        struct ASIdentifiers_st *rfc3779_asid;
 #endif
        unsigned char hash[X509_CERT_HASH_LEN];
-       time_t not_before;
-       time_t not_after;
        X509_CERT_AUX *aux;
 } /* X509 */;
 
index 53f4f2f..8f4e593 100644 (file)
@@ -1,4 +1,4 @@
-/* $OpenBSD: x509_purp.c,v 1.39 2024/03/02 10:43:52 tb Exp $ */
+/* $OpenBSD: x509_purp.c,v 1.40 2024/04/08 23:46:21 beck Exp $ */
 /* Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL
  * project 2001.
  */
@@ -559,9 +559,6 @@ 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;
-
        x->ex_flags |= EXFLAG_SET;
 }
 
index 19bb925..c7b2219 100644 (file)
@@ -1,4 +1,4 @@
-/* $OpenBSD: x509_verify.c,v 1.68 2024/02/01 23:16:38 beck Exp $ */
+/* $OpenBSD: x509_verify.c,v 1.69 2024/04/08 23:46:21 beck Exp $ */
 /*
  * Copyright (c) 2020-2021 Bob Beck <beck@openbsd.org>
  *
@@ -52,6 +52,9 @@ x509_verify_asn1_time_to_time_t(const ASN1_TIME *atime, int notAfter,
        struct tm tm = { 0 };
        int type;
 
+       if (atime == NULL)
+               return 0;
+
        type = ASN1_time_parse(atime->data, atime->length, &tm, atime->type);
        if (type == -1)
                return 0;
@@ -80,35 +83,6 @@ x509_verify_asn1_time_to_time_t(const ASN1_TIME *atime, int notAfter,
        return asn1_time_tm_to_time_t(&tm, out);
 }
 
-/*
- * Cache certificate hash, and values parsed out of an X509.
- * called from cache_extensions()
- */
-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 = 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 *
 x509_verify_chain_new(void)
 {
@@ -840,26 +814,28 @@ x509_verify_set_check_time(struct x509_verify_ctx *ctx)
 static int
 x509_verify_cert_times(X509 *cert, time_t *cmp_time, int *error)
 {
-       time_t when;
+       time_t when, not_before, not_after;
 
        if (cmp_time == NULL)
                when = time(NULL);
        else
                when = *cmp_time;
 
-       if (cert->not_before == -1) {
+       if (!x509_verify_asn1_time_to_time_t(X509_get_notBefore(cert), 0,
+           &not_before)) {
                *error = X509_V_ERR_ERROR_IN_CERT_NOT_BEFORE_FIELD;
                return 0;
        }
-       if (when < cert->not_before) {
+       if (when < not_before) {
                *error = X509_V_ERR_CERT_NOT_YET_VALID;
                return 0;
        }
-       if (cert->not_after == -1) {
+       if (!x509_verify_asn1_time_to_time_t(X509_get_notAfter(cert), 1,
+           &not_after)) {
                *error = X509_V_ERR_ERROR_IN_CERT_NOT_AFTER_FIELD;
                return 0;
        }
-       if (when > cert->not_after) {
+       if (when > not_after) {
                *error = X509_V_ERR_CERT_HAS_EXPIRED;
                return 0;
        }
index 5399658..501f5e5 100644 (file)
@@ -1,4 +1,4 @@
-/* $OpenBSD: x509_vfy.c,v 1.142 2024/03/02 10:40:05 tb Exp $ */
+/* $OpenBSD: x509_vfy.c,v 1.143 2024/04/08 23:46:21 beck Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -1744,18 +1744,6 @@ verify_cb_cert(X509_STORE_CTX *ctx, X509 *x, int depth, int err)
        return ctx->verify_cb(0, ctx);
 }
 
-
-/* Mimic OpenSSL '0 for failure' ick */
-static int
-time_t_bogocmp(time_t a, time_t b)
-{
-       if (a == -1 || b == -1)
-               return 0;
-       if (a <= b)
-               return -1;
-       return 1;
-}
-
 /*
  * Check certificate validity times.
  *
@@ -1777,10 +1765,7 @@ x509_check_cert_time(X509_STORE_CTX *ctx, X509 *x, int depth)
        else
                ptime = time(NULL);
 
-       if (x->ex_flags & EXFLAG_SET)
-               i = time_t_bogocmp(x->not_before, ptime);
-       else
-               i = X509_cmp_time(X509_get_notBefore(x), &ptime);
+       i = X509_cmp_time(X509_get_notBefore(x), &ptime);
 
        if (i >= 0 && depth < 0)
                return 0;
@@ -1791,10 +1776,7 @@ x509_check_cert_time(X509_STORE_CTX *ctx, X509 *x, int depth)
            X509_V_ERR_CERT_NOT_YET_VALID))
                return 0;
 
-       if (x->ex_flags & EXFLAG_SET)
-               i = time_t_bogocmp(x->not_after, ptime);
-       else
-               i = X509_cmp_time_internal(X509_get_notAfter(x), &ptime, 1);
+       i = X509_cmp_time_internal(X509_get_notAfter(x), &ptime, 1);
 
        if (i <= 0 && depth < 0)
                return 0;