commit eb9eaf536340c87006bac93076ddee14f9d3f3ac
parent cc50eadaae277bc141f74c33dd0e0304517b5ae0
Author: Nick Mathewson <nickm@torproject.org>
Date: Thu, 6 Mar 2025 08:47:55 -0500
Stop using time(NULL) for certificate tests.
The canned testing certificates added in order to fix #41041
will start to expire in a couple of months;
to avoid a test failure then, we should only validate
them against a time when they are valid.
Previously, we got away with using time(NULL) because the old
canned certificate (taken from testing.torproject.org)
was not only signed using SHA-1: it was valid until 2043!
Diffstat:
6 files changed, 25 insertions(+), 17 deletions(-)
diff --git a/src/core/or/connection_or.c b/src/core/or/connection_or.c
@@ -1843,7 +1843,7 @@ connection_or_check_valid_tls_handshake(or_connection_t *conn,
if (has_cert) {
int v = tor_tls_verify(started_here?severity:LOG_INFO,
- conn->tls, &identity_rcvd);
+ conn->tls, time(NULL), &identity_rcvd);
if (started_here && v<0) {
log_fn(severity,LD_HANDSHAKE,"Tried connecting to router at %s: It"
" has a cert but it's invalid. Closing.",
diff --git a/src/lib/tls/tortls.c b/src/lib/tls/tortls.c
@@ -413,7 +413,8 @@ tor_tls_free_(tor_tls_t *tls)
* 0. Else, return -1 and log complaints with log-level <b>severity</b>.
*/
int
-tor_tls_verify(int severity, tor_tls_t *tls, crypto_pk_t **identity)
+tor_tls_verify(int severity, tor_tls_t *tls, time_t now,
+ crypto_pk_t **identity)
{
tor_x509_cert_impl_t *cert = NULL, *id_cert = NULL;
tor_x509_cert_t *peer_x509 = NULL, *id_x509 = NULL;
@@ -432,7 +433,7 @@ tor_tls_verify(int severity, tor_tls_t *tls, crypto_pk_t **identity)
id_x509 = tor_x509_cert_new(id_cert);
cert = id_cert = NULL; /* Prevent double-free */
- if (! tor_tls_cert_is_valid(severity, peer_x509, id_x509, time(NULL), 0)) {
+ if (! tor_tls_cert_is_valid(severity, peer_x509, id_x509, now, 0)) {
goto done;
}
diff --git a/src/lib/tls/tortls.h b/src/lib/tls/tortls.h
@@ -101,7 +101,8 @@ void tor_tls_free_(tor_tls_t *tls);
int tor_tls_peer_has_cert(tor_tls_t *tls);
MOCK_DECL(struct tor_x509_cert_t *,tor_tls_get_peer_cert,(tor_tls_t *tls));
MOCK_DECL(struct tor_x509_cert_t *,tor_tls_get_own_cert,(tor_tls_t *tls));
-int tor_tls_verify(int severity, tor_tls_t *tls, crypto_pk_t **identity);
+int tor_tls_verify(int severity, tor_tls_t *tls, time_t now,
+ crypto_pk_t **identity);
MOCK_DECL(int, tor_tls_read, (tor_tls_t *tls, char *cp, size_t len));
int tor_tls_write(tor_tls_t *tls, const char *cp, size_t n);
int tor_tls_handshake(tor_tls_t *tls);
diff --git a/src/test/test_tortls.c b/src/test/test_tortls.c
@@ -120,6 +120,9 @@ const char* caCertString =
"KPpdzvvtTnOPlC7SQZSYmdunr3Bf9b77AiC/ZidstK36dRILKz7OA54=\n"
"-----END CERTIFICATE-----\n";
+// A time at which the certs above are valid.
+const time_t cert_strings_valid_at = 1741267580;
+
static tor_x509_cert_t *fixed_x509_cert = NULL;
static tor_x509_cert_t *
get_peer_cert_mock_return_fixed(tor_tls_t *tls)
@@ -497,6 +500,7 @@ test_tortls_verify(void *ignored)
crypto_pk_t *k = NULL;
tor_x509_cert_impl_t *cert1 = NULL, *cert2 = NULL, *invalidCert = NULL,
*validCert = NULL, *caCert = NULL;
+ time_t now = cert_strings_valid_at;
validCert = read_cert_from(validCertString);
caCert = read_cert_from(caCertString);
@@ -507,23 +511,23 @@ test_tortls_verify(void *ignored)
MOCK(try_to_extract_certs_from_tls, fixed_try_to_extract_certs_from_tls);
fixed_try_to_extract_certs_from_tls_cert_out_result = cert1;
- ret = tor_tls_verify(LOG_WARN, tls, &k);
+ ret = tor_tls_verify(LOG_WARN, tls, now, &k);
tt_int_op(ret, OP_EQ, -1);
fixed_try_to_extract_certs_from_tls_id_cert_out_result = cert2;
- ret = tor_tls_verify(LOG_WARN, tls, &k);
+ ret = tor_tls_verify(LOG_WARN, tls, now, &k);
tt_int_op(ret, OP_EQ, -1);
fixed_try_to_extract_certs_from_tls_cert_out_result = invalidCert;
fixed_try_to_extract_certs_from_tls_id_cert_out_result = invalidCert;
- ret = tor_tls_verify(LOG_WARN, tls, &k);
+ ret = tor_tls_verify(LOG_WARN, tls, now, &k);
tt_int_op(ret, OP_EQ, -1);
fixed_try_to_extract_certs_from_tls_cert_out_result = validCert;
fixed_try_to_extract_certs_from_tls_id_cert_out_result = caCert;
- ret = tor_tls_verify(LOG_WARN, tls, &k);
+ ret = tor_tls_verify(LOG_WARN, tls, now, &k);
tt_int_op(ret, OP_EQ, 0);
tt_assert(k);
diff --git a/src/test/test_tortls.h b/src/test/test_tortls.h
@@ -9,5 +9,6 @@ tor_x509_cert_impl_t *read_cert_from(const char *str);
extern const char *notCompletelyValidCertString;
extern const char *validCertString;
extern const char *caCertString;
+extern const time_t cert_strings_valid_at;
#endif /* !defined(TEST_TORTLS_H) */
diff --git a/src/test/test_tortls_openssl.c b/src/test/test_tortls_openssl.c
@@ -2068,20 +2068,21 @@ test_tortls_cert_is_valid(void *ignored)
(void)ignored;
int ret;
tor_x509_cert_t *cert = NULL, *scert = NULL;
+ time_t now = cert_strings_valid_at;
scert = tor_malloc_zero(sizeof(tor_x509_cert_t));
- ret = tor_tls_cert_is_valid(LOG_WARN, cert, scert, time(NULL), 0);
+ ret = tor_tls_cert_is_valid(LOG_WARN, cert, scert, now, 0);
tt_int_op(ret, OP_EQ, 0);
cert = tor_malloc_zero(sizeof(tor_x509_cert_t));
- ret = tor_tls_cert_is_valid(LOG_WARN, cert, scert, time(NULL), 0);
+ ret = tor_tls_cert_is_valid(LOG_WARN, cert, scert, now, 0);
tt_int_op(ret, OP_EQ, 0);
tor_free(scert);
tor_free(cert);
cert = tor_x509_cert_new(read_cert_from(validCertString));
scert = tor_x509_cert_new(read_cert_from(caCertString));
- ret = tor_tls_cert_is_valid(LOG_WARN, cert, scert, time(NULL), 0);
+ ret = tor_tls_cert_is_valid(LOG_WARN, cert, scert, now, 0);
tt_int_op(ret, OP_EQ, 1);
#ifndef OPENSSL_OPAQUE
@@ -2092,7 +2093,7 @@ test_tortls_cert_is_valid(void *ignored)
ASN1_TIME_free(cert->cert->cert_info->validity->notAfter);
cert->cert->cert_info->validity->notAfter =
ASN1_TIME_set(NULL, time(NULL)-1000000);
- ret = tor_tls_cert_is_valid(LOG_WARN, cert, scert, time(NULL), 0);
+ ret = tor_tls_cert_is_valid(LOG_WARN, cert, scert, now, 0);
tt_int_op(ret, OP_EQ, 0);
tor_x509_cert_free(cert);
@@ -2101,7 +2102,7 @@ test_tortls_cert_is_valid(void *ignored)
scert = tor_x509_cert_new(read_cert_from(caCertString));
X509_PUBKEY_free(cert->cert->cert_info->key);
cert->cert->cert_info->key = NULL;
- ret = tor_tls_cert_is_valid(LOG_WARN, cert, scert, time(NULL), 1);
+ ret = tor_tls_cert_is_valid(LOG_WARN, cert, scert, now, 1);
tt_int_op(ret, OP_EQ, 0);
#endif /* !defined(OPENSSL_OPAQUE) */
@@ -2112,7 +2113,7 @@ test_tortls_cert_is_valid(void *ignored)
scert = tor_x509_cert_new(read_cert_from(caCertString));
/* This doesn't actually change the key in the cert. XXXXXX */
BN_one(EVP_PKEY_get1_RSA(X509_get_pubkey(cert->cert))->n);
- ret = tor_tls_cert_is_valid(LOG_WARN, cert, scert, time(NULL), 1);
+ ret = tor_tls_cert_is_valid(LOG_WARN, cert, scert, now, 1);
tt_int_op(ret, OP_EQ, 0);
tor_x509_cert_free(cert);
@@ -2121,7 +2122,7 @@ test_tortls_cert_is_valid(void *ignored)
scert = tor_x509_cert_new(read_cert_from(caCertString));
/* This doesn't actually change the key in the cert. XXXXXX */
X509_get_pubkey(cert->cert)->type = EVP_PKEY_EC;
- ret = tor_tls_cert_is_valid(LOG_WARN, cert, scert, time(NULL), 1);
+ ret = tor_tls_cert_is_valid(LOG_WARN, cert, scert, now, 1);
tt_int_op(ret, OP_EQ, 0);
tor_x509_cert_free(cert);
@@ -2130,7 +2131,7 @@ test_tortls_cert_is_valid(void *ignored)
scert = tor_x509_cert_new(read_cert_from(caCertString));
/* This doesn't actually change the key in the cert. XXXXXX */
X509_get_pubkey(cert->cert)->type = EVP_PKEY_EC;
- ret = tor_tls_cert_is_valid(LOG_WARN, cert, scert, time(NULL), 0);
+ ret = tor_tls_cert_is_valid(LOG_WARN, cert, scert, now, 0);
tt_int_op(ret, OP_EQ, 1);
tor_x509_cert_free(cert);
@@ -2140,7 +2141,7 @@ test_tortls_cert_is_valid(void *ignored)
/* This doesn't actually change the key in the cert. XXXXXX */
X509_get_pubkey(cert->cert)->type = EVP_PKEY_EC;
X509_get_pubkey(cert->cert)->ameth = NULL;
- ret = tor_tls_cert_is_valid(LOG_WARN, cert, scert, time(NULL), 0);
+ ret = tor_tls_cert_is_valid(LOG_WARN, cert, scert, now, 0);
tt_int_op(ret, OP_EQ, 0);
#endif /* 0 */
