diff options
author | beck <> | 2024-04-08 23:46:21 +0000 |
---|---|---|
committer | beck <> | 2024-04-08 23:46:21 +0000 |
commit | 6a993ea0b1a8e5ba915e093a2eca5dc9fba06c74 (patch) | |
tree | a514bf75907bdd1906b32c4e2da374c0ea1ba065 | |
parent | 0164389f0b72283c34063736f590518874ffc3c8 (diff) | |
download | openbsd-6a993ea0b1a8e5ba915e093a2eca5dc9fba06c74.tar.gz openbsd-6a993ea0b1a8e5ba915e093a2eca5dc9fba06c74.tar.bz2 openbsd-6a993ea0b1a8e5ba915e093a2eca5dc9fba06c74.zip |
Remove notBefore and notAfter cacheing.
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@
-rw-r--r-- | src/lib/libcrypto/x509/x509_local.h | 4 | ||||
-rw-r--r-- | src/lib/libcrypto/x509/x509_purp.c | 5 | ||||
-rw-r--r-- | src/lib/libcrypto/x509/x509_verify.c | 46 | ||||
-rw-r--r-- | src/lib/libcrypto/x509/x509_vfy.c | 24 |
4 files changed, 16 insertions, 63 deletions
diff --git a/src/lib/libcrypto/x509/x509_local.h b/src/lib/libcrypto/x509/x509_local.h index 73cc582d7b..5b74b0d1bd 100644 --- a/src/lib/libcrypto/x509/x509_local.h +++ b/src/lib/libcrypto/x509/x509_local.h | |||
@@ -1,4 +1,4 @@ | |||
1 | /* $OpenBSD: x509_local.h,v 1.23 2024/03/26 05:39:47 tb Exp $ */ | 1 | /* $OpenBSD: x509_local.h,v 1.24 2024/04/08 23:46:21 beck Exp $ */ |
2 | /* Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL | 2 | /* Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL |
3 | * project 2013. | 3 | * project 2013. |
4 | */ | 4 | */ |
@@ -188,8 +188,6 @@ struct x509_st { | |||
188 | struct ASIdentifiers_st *rfc3779_asid; | 188 | struct ASIdentifiers_st *rfc3779_asid; |
189 | #endif | 189 | #endif |
190 | unsigned char hash[X509_CERT_HASH_LEN]; | 190 | unsigned char hash[X509_CERT_HASH_LEN]; |
191 | time_t not_before; | ||
192 | time_t not_after; | ||
193 | X509_CERT_AUX *aux; | 191 | X509_CERT_AUX *aux; |
194 | } /* X509 */; | 192 | } /* X509 */; |
195 | 193 | ||
diff --git a/src/lib/libcrypto/x509/x509_purp.c b/src/lib/libcrypto/x509/x509_purp.c index 53f4f2f967..8f4e5934e1 100644 --- a/src/lib/libcrypto/x509/x509_purp.c +++ b/src/lib/libcrypto/x509/x509_purp.c | |||
@@ -1,4 +1,4 @@ | |||
1 | /* $OpenBSD: x509_purp.c,v 1.39 2024/03/02 10:43:52 tb Exp $ */ | 1 | /* $OpenBSD: x509_purp.c,v 1.40 2024/04/08 23:46:21 beck Exp $ */ |
2 | /* Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL | 2 | /* Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL |
3 | * project 2001. | 3 | * project 2001. |
4 | */ | 4 | */ |
@@ -559,9 +559,6 @@ x509v3_cache_extensions_internal(X509 *x) | |||
559 | if (!x509_extension_oids_are_unique(x)) | 559 | if (!x509_extension_oids_are_unique(x)) |
560 | x->ex_flags |= EXFLAG_INVALID; | 560 | x->ex_flags |= EXFLAG_INVALID; |
561 | 561 | ||
562 | if (!x509_verify_cert_info_populate(x)) | ||
563 | x->ex_flags |= EXFLAG_INVALID; | ||
564 | |||
565 | x->ex_flags |= EXFLAG_SET; | 562 | x->ex_flags |= EXFLAG_SET; |
566 | } | 563 | } |
567 | 564 | ||
diff --git a/src/lib/libcrypto/x509/x509_verify.c b/src/lib/libcrypto/x509/x509_verify.c index 19bb925d9c..c7b2219fa9 100644 --- a/src/lib/libcrypto/x509/x509_verify.c +++ b/src/lib/libcrypto/x509/x509_verify.c | |||
@@ -1,4 +1,4 @@ | |||
1 | /* $OpenBSD: x509_verify.c,v 1.68 2024/02/01 23:16:38 beck Exp $ */ | 1 | /* $OpenBSD: x509_verify.c,v 1.69 2024/04/08 23:46:21 beck Exp $ */ |
2 | /* | 2 | /* |
3 | * Copyright (c) 2020-2021 Bob Beck <beck@openbsd.org> | 3 | * Copyright (c) 2020-2021 Bob Beck <beck@openbsd.org> |
4 | * | 4 | * |
@@ -52,6 +52,9 @@ x509_verify_asn1_time_to_time_t(const ASN1_TIME *atime, int notAfter, | |||
52 | struct tm tm = { 0 }; | 52 | struct tm tm = { 0 }; |
53 | int type; | 53 | int type; |
54 | 54 | ||
55 | if (atime == NULL) | ||
56 | return 0; | ||
57 | |||
55 | type = ASN1_time_parse(atime->data, atime->length, &tm, atime->type); | 58 | type = ASN1_time_parse(atime->data, atime->length, &tm, atime->type); |
56 | if (type == -1) | 59 | if (type == -1) |
57 | return 0; | 60 | return 0; |
@@ -80,35 +83,6 @@ x509_verify_asn1_time_to_time_t(const ASN1_TIME *atime, int notAfter, | |||
80 | return asn1_time_tm_to_time_t(&tm, out); | 83 | return asn1_time_tm_to_time_t(&tm, out); |
81 | } | 84 | } |
82 | 85 | ||
83 | /* | ||
84 | * Cache certificate hash, and values parsed out of an X509. | ||
85 | * called from cache_extensions() | ||
86 | */ | ||
87 | int | ||
88 | x509_verify_cert_info_populate(X509 *cert) | ||
89 | { | ||
90 | const ASN1_TIME *notBefore, *notAfter; | ||
91 | |||
92 | /* | ||
93 | * Parse and save the cert times, or remember that they | ||
94 | * are unacceptable/unparsable. | ||
95 | */ | ||
96 | |||
97 | cert->not_before = cert->not_after = -1; | ||
98 | |||
99 | if ((notBefore = X509_get_notBefore(cert)) == NULL) | ||
100 | return 0; | ||
101 | if ((notAfter = X509_get_notAfter(cert)) == NULL) | ||
102 | return 0; | ||
103 | |||
104 | if (!x509_verify_asn1_time_to_time_t(notBefore, 0, &cert->not_before)) | ||
105 | return 0; | ||
106 | if (!x509_verify_asn1_time_to_time_t(notAfter, 1, &cert->not_after)) | ||
107 | return 0; | ||
108 | |||
109 | return 1; | ||
110 | } | ||
111 | |||
112 | struct x509_verify_chain * | 86 | struct x509_verify_chain * |
113 | x509_verify_chain_new(void) | 87 | x509_verify_chain_new(void) |
114 | { | 88 | { |
@@ -840,26 +814,28 @@ x509_verify_set_check_time(struct x509_verify_ctx *ctx) | |||
840 | static int | 814 | static int |
841 | x509_verify_cert_times(X509 *cert, time_t *cmp_time, int *error) | 815 | x509_verify_cert_times(X509 *cert, time_t *cmp_time, int *error) |
842 | { | 816 | { |
843 | time_t when; | 817 | time_t when, not_before, not_after; |
844 | 818 | ||
845 | if (cmp_time == NULL) | 819 | if (cmp_time == NULL) |
846 | when = time(NULL); | 820 | when = time(NULL); |
847 | else | 821 | else |
848 | when = *cmp_time; | 822 | when = *cmp_time; |
849 | 823 | ||
850 | if (cert->not_before == -1) { | 824 | if (!x509_verify_asn1_time_to_time_t(X509_get_notBefore(cert), 0, |
825 | ¬_before)) { | ||
851 | *error = X509_V_ERR_ERROR_IN_CERT_NOT_BEFORE_FIELD; | 826 | *error = X509_V_ERR_ERROR_IN_CERT_NOT_BEFORE_FIELD; |
852 | return 0; | 827 | return 0; |
853 | } | 828 | } |
854 | if (when < cert->not_before) { | 829 | if (when < not_before) { |
855 | *error = X509_V_ERR_CERT_NOT_YET_VALID; | 830 | *error = X509_V_ERR_CERT_NOT_YET_VALID; |
856 | return 0; | 831 | return 0; |
857 | } | 832 | } |
858 | if (cert->not_after == -1) { | 833 | if (!x509_verify_asn1_time_to_time_t(X509_get_notAfter(cert), 1, |
834 | ¬_after)) { | ||
859 | *error = X509_V_ERR_ERROR_IN_CERT_NOT_AFTER_FIELD; | 835 | *error = X509_V_ERR_ERROR_IN_CERT_NOT_AFTER_FIELD; |
860 | return 0; | 836 | return 0; |
861 | } | 837 | } |
862 | if (when > cert->not_after) { | 838 | if (when > not_after) { |
863 | *error = X509_V_ERR_CERT_HAS_EXPIRED; | 839 | *error = X509_V_ERR_CERT_HAS_EXPIRED; |
864 | return 0; | 840 | return 0; |
865 | } | 841 | } |
diff --git a/src/lib/libcrypto/x509/x509_vfy.c b/src/lib/libcrypto/x509/x509_vfy.c index 5399658639..501f5e5710 100644 --- a/src/lib/libcrypto/x509/x509_vfy.c +++ b/src/lib/libcrypto/x509/x509_vfy.c | |||
@@ -1,4 +1,4 @@ | |||
1 | /* $OpenBSD: x509_vfy.c,v 1.142 2024/03/02 10:40:05 tb Exp $ */ | 1 | /* $OpenBSD: x509_vfy.c,v 1.143 2024/04/08 23:46:21 beck Exp $ */ |
2 | /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) | 2 | /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) |
3 | * All rights reserved. | 3 | * All rights reserved. |
4 | * | 4 | * |
@@ -1744,18 +1744,6 @@ verify_cb_cert(X509_STORE_CTX *ctx, X509 *x, int depth, int err) | |||
1744 | return ctx->verify_cb(0, ctx); | 1744 | return ctx->verify_cb(0, ctx); |
1745 | } | 1745 | } |
1746 | 1746 | ||
1747 | |||
1748 | /* Mimic OpenSSL '0 for failure' ick */ | ||
1749 | static int | ||
1750 | time_t_bogocmp(time_t a, time_t b) | ||
1751 | { | ||
1752 | if (a == -1 || b == -1) | ||
1753 | return 0; | ||
1754 | if (a <= b) | ||
1755 | return -1; | ||
1756 | return 1; | ||
1757 | } | ||
1758 | |||
1759 | /* | 1747 | /* |
1760 | * Check certificate validity times. | 1748 | * Check certificate validity times. |
1761 | * | 1749 | * |
@@ -1777,10 +1765,7 @@ x509_check_cert_time(X509_STORE_CTX *ctx, X509 *x, int depth) | |||
1777 | else | 1765 | else |
1778 | ptime = time(NULL); | 1766 | ptime = time(NULL); |
1779 | 1767 | ||
1780 | if (x->ex_flags & EXFLAG_SET) | 1768 | i = X509_cmp_time(X509_get_notBefore(x), &ptime); |
1781 | i = time_t_bogocmp(x->not_before, ptime); | ||
1782 | else | ||
1783 | i = X509_cmp_time(X509_get_notBefore(x), &ptime); | ||
1784 | 1769 | ||
1785 | if (i >= 0 && depth < 0) | 1770 | if (i >= 0 && depth < 0) |
1786 | return 0; | 1771 | return 0; |
@@ -1791,10 +1776,7 @@ x509_check_cert_time(X509_STORE_CTX *ctx, X509 *x, int depth) | |||
1791 | X509_V_ERR_CERT_NOT_YET_VALID)) | 1776 | X509_V_ERR_CERT_NOT_YET_VALID)) |
1792 | return 0; | 1777 | return 0; |
1793 | 1778 | ||
1794 | if (x->ex_flags & EXFLAG_SET) | 1779 | i = X509_cmp_time_internal(X509_get_notAfter(x), &ptime, 1); |
1795 | i = time_t_bogocmp(x->not_after, ptime); | ||
1796 | else | ||
1797 | i = X509_cmp_time_internal(X509_get_notAfter(x), &ptime, 1); | ||
1798 | 1780 | ||
1799 | if (i <= 0 && depth < 0) | 1781 | if (i <= 0 && depth < 0) |
1800 | return 0; | 1782 | return 0; |