From: Nick Mathewson Date: Thu, 6 Mar 2025 13:47:55 +0000 (-0500) Subject: Stop using time(NULL) for certificate tests. X-Git-Tag: tor-0.4.9.2-alpha~30^2~11 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=eb9eaf536340c87006bac93076ddee14f9d3f3ac;p=thirdparty%2Ftor.git 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! --- diff --git a/src/core/or/connection_or.c b/src/core/or/connection_or.c index 30ce5e0c57..b1131d82b6 100644 --- 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 index 80f16e1c74..e532156511 100644 --- 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 severity. */ 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 index 96f93e2679..d65882d146 100644 --- 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 index dda3530d1a..34a1fe983e 100644 --- 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 index c14aba417b..41ec6ffc18 100644 --- 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 index 010e09c8eb..b8472c53d7 100644 --- 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 */