commit 0b4221f98dbb93c9322e7a778f04bcbcfcc79738
Author: Nick Mathewson <nickm(a)torproject.org>
Date: Wed Aug 10 20:02:03 2016 -0400
Make the current time an argument to x509 cert-checking functions
This makes the code a bit cleaner by having more of the functions be
pure functions that don't depend on the current time.
---
src/common/tortls.c | 25 ++++++++++++++-----------
src/common/tortls.h | 4 +++-
src/or/channeltls.c | 3 ++-
src/or/torcert.c | 15 +++++++++------
src/or/torcert.h | 4 ++--
src/test/test_tortls.c | 26 +++++++++++++-------------
6 files changed, 43 insertions(+), 34 deletions(-)
diff --git a/src/common/tortls.c b/src/common/tortls.c
index eaa5748..fd86981 100644
--- a/src/common/tortls.c
+++ b/src/common/tortls.c
@@ -136,6 +136,7 @@ static void tor_tls_context_decref(tor_tls_context_t *ctx);
static void tor_tls_context_incref(tor_tls_context_t *ctx);
static int check_cert_lifetime_internal(int severity, const X509 *cert,
+ time_t now,
int past_tolerance, int future_tolerance);
/** Global TLS contexts. We keep them here because nobody else needs
@@ -860,6 +861,7 @@ int
tor_tls_cert_is_valid(int severity,
const tor_x509_cert_t *cert,
const tor_x509_cert_t *signing_cert,
+ time_t now,
int check_rsa_1024)
{
check_no_tls_errors();
@@ -879,7 +881,7 @@ tor_tls_cert_is_valid(int severity,
/* okay, the signature checked out right. Now let's check the check the
* lifetime. */
- if (check_cert_lifetime_internal(severity, cert->cert,
+ if (check_cert_lifetime_internal(severity, cert->cert, now,
48*60*60, 30*24*60*60) < 0)
goto bad;
@@ -2028,13 +2030,13 @@ tor_tls_get_peer_cert,(tor_tls_t *tls))
/** Warn that a certificate lifetime extends through a certain range. */
static void
-log_cert_lifetime(int severity, const X509 *cert, const char *problem)
+log_cert_lifetime(int severity, const X509 *cert, const char *problem,
+ time_t now)
{
BIO *bio = NULL;
BUF_MEM *buf;
char *s1=NULL, *s2=NULL;
char mytime[33];
- time_t now = time(NULL);
struct tm tm;
size_t n;
@@ -2182,6 +2184,7 @@ tor_tls_verify(int severity, tor_tls_t *tls, crypto_pk_t **identity_key)
*/
int
tor_tls_check_lifetime(int severity, tor_tls_t *tls,
+ time_t now,
int past_tolerance, int future_tolerance)
{
X509 *cert;
@@ -2190,7 +2193,7 @@ tor_tls_check_lifetime(int severity, tor_tls_t *tls,
if (!(cert = SSL_get_peer_certificate(tls->ssl)))
goto done;
- if (check_cert_lifetime_internal(severity, cert,
+ if (check_cert_lifetime_internal(severity, cert, now,
past_tolerance, future_tolerance) < 0)
goto done;
@@ -2206,24 +2209,24 @@ tor_tls_check_lifetime(int severity, tor_tls_t *tls,
/** Helper: check whether <b>cert</b> is expired give or take
* <b>past_tolerance</b> seconds, or not-yet-valid give or take
- * <b>future_tolerance</b> seconds. If it is live, return 0. If it is not
- * live, log a message and return -1. */
+ * <b>future_tolerance</b> seconds. (Relative to the current time
+ * <b>now</b>.) If it is live, return 0. If it is not live, log a message
+ * and return -1. */
static int
check_cert_lifetime_internal(int severity, const X509 *cert,
+ time_t now,
int past_tolerance, int future_tolerance)
{
- time_t now, t;
-
- now = time(NULL);
+ time_t t;
t = now + future_tolerance;
if (X509_cmp_time(X509_get_notBefore_const(cert), &t) > 0) {
- log_cert_lifetime(severity, cert, "not yet valid");
+ log_cert_lifetime(severity, cert, "not yet valid", now);
return -1;
}
t = now - past_tolerance;
if (X509_cmp_time(X509_get_notAfter_const(cert), &t) < 0) {
- log_cert_lifetime(severity, cert, "already expired");
+ log_cert_lifetime(severity, cert, "already expired", now);
return -1;
}
diff --git a/src/common/tortls.h b/src/common/tortls.h
index fe5898e..3adb1b2 100644
--- a/src/common/tortls.h
+++ b/src/common/tortls.h
@@ -200,7 +200,8 @@ int tor_tls_peer_has_cert(tor_tls_t *tls);
MOCK_DECL(tor_x509_cert_t *,tor_tls_get_peer_cert,(tor_tls_t *tls));
int tor_tls_verify(int severity, tor_tls_t *tls, crypto_pk_t **identity);
int tor_tls_check_lifetime(int severity,
- tor_tls_t *tls, int past_tolerance,
+ tor_tls_t *tls, time_t now,
+ int past_tolerance,
int future_tolerance);
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);
@@ -259,6 +260,7 @@ MOCK_DECL(int,tor_tls_cert_matches_key,(const tor_tls_t *tls,
int tor_tls_cert_is_valid(int severity,
const tor_x509_cert_t *cert,
const tor_x509_cert_t *signing_cert,
+ time_t now,
int check_rsa_1024);
const char *tor_tls_get_ciphersuite_name(tor_tls_t *tls);
diff --git a/src/or/channeltls.c b/src/or/channeltls.c
index cf57c29..9315f80 100644
--- a/src/or/channeltls.c
+++ b/src/or/channeltls.c
@@ -1918,7 +1918,8 @@ channel_tls_process_certs_cell(var_cell_t *cell, channel_tls_t *chan)
if (! or_handshake_certs_rsa_ok(severity,
chan->conn->handshake_state->certs,
- chan->conn->tls))
+ chan->conn->tls,
+ time(NULL)))
ERR("Invalid RSA certificates!");
if (chan->conn->handshake_state->started_here) {
diff --git a/src/or/torcert.c b/src/or/torcert.c
index 94382f6..b7ed7f8 100644
--- a/src/or/torcert.c
+++ b/src/or/torcert.c
@@ -431,7 +431,8 @@ or_handshake_certs_free(or_handshake_certs_t *certs)
int
or_handshake_certs_rsa_ok(int severity,
or_handshake_certs_t *certs,
- tor_tls_t *tls)
+ tor_tls_t *tls,
+ time_t now)
{
tor_x509_cert_t *link_cert = certs->link_cert;
tor_x509_cert_t *auth_cert = certs->auth_cert;
@@ -442,17 +443,19 @@ or_handshake_certs_rsa_ok(int severity,
ERR("The certs we wanted were missing");
if (! tor_tls_cert_matches_key(tls, link_cert))
ERR("The link certificate didn't match the TLS public key");
- if (! tor_tls_cert_is_valid(severity, link_cert, id_cert, 0))
+ if (! tor_tls_cert_is_valid(severity, link_cert, id_cert, now, 0))
ERR("The link certificate was not valid");
- if (! tor_tls_cert_is_valid(severity, id_cert, id_cert, 1))
+ if (! tor_tls_cert_is_valid(severity, id_cert, id_cert, now, 1))
ERR("The ID certificate was not valid");
} else {
if (! (id_cert && auth_cert))
ERR("The certs we wanted were missing");
- /* Remember these certificates so we can check an AUTHENTICATE cell */
- if (! tor_tls_cert_is_valid(LOG_PROTOCOL_WARN, auth_cert, id_cert, 1))
+ /* Remember these certificates so we can check an AUTHENTICATE cell
+ * XXXX make sure we do that
+ */
+ if (! tor_tls_cert_is_valid(LOG_PROTOCOL_WARN, auth_cert, id_cert, now, 1))
ERR("The authentication certificate was not valid");
- if (! tor_tls_cert_is_valid(LOG_PROTOCOL_WARN, id_cert, id_cert, 1))
+ if (! tor_tls_cert_is_valid(LOG_PROTOCOL_WARN, id_cert, id_cert, now, 1))
ERR("The ID certificate was not valid");
}
diff --git a/src/or/torcert.h b/src/or/torcert.h
index 39439d9..f851b92 100644
--- a/src/or/torcert.h
+++ b/src/or/torcert.h
@@ -81,8 +81,8 @@ or_handshake_certs_t *or_handshake_certs_new(void);
void or_handshake_certs_free(or_handshake_certs_t *certs);
int or_handshake_certs_rsa_ok(int severity,
or_handshake_certs_t *certs,
- tor_tls_t *tls);
-int or_handshake_certs_ed25519_ok(or_handshake_certs_t *certs);
+ tor_tls_t *tls,
+ time_t now);
#endif
diff --git a/src/test/test_tortls.c b/src/test/test_tortls.c
index 8efcac2..44961c8 100644
--- a/src/test/test_tortls.c
+++ b/src/test/test_tortls.c
@@ -1086,13 +1086,13 @@ test_tortls_check_lifetime(void *ignored)
time_t now = time(NULL);
tls = tor_malloc_zero(sizeof(tor_tls_t));
- ret = tor_tls_check_lifetime(LOG_WARN, tls, 0, 0);
+ ret = tor_tls_check_lifetime(LOG_WARN, tls, time(NULL), 0, 0);
tt_int_op(ret, OP_EQ, -1);
tls->ssl = tor_malloc_zero(sizeof(SSL));
tls->ssl->session = tor_malloc_zero(sizeof(SSL_SESSION));
tls->ssl->session->peer = validCert;
- ret = tor_tls_check_lifetime(LOG_WARN, tls, 0, 0);
+ ret = tor_tls_check_lifetime(LOG_WARN, tls, time(NULL), 0, 0);
tt_int_op(ret, OP_EQ, 0);
ASN1_STRING_free(validCert->cert_info->validity->notBefore);
@@ -1100,10 +1100,10 @@ test_tortls_check_lifetime(void *ignored)
ASN1_STRING_free(validCert->cert_info->validity->notAfter);
validCert->cert_info->validity->notAfter = ASN1_TIME_set(NULL, now+60);
- ret = tor_tls_check_lifetime(LOG_WARN, tls, 0, -1000);
+ ret = tor_tls_check_lifetime(LOG_WARN, tls, time(NULL), 0, -1000);
tt_int_op(ret, OP_EQ, -1);
- ret = tor_tls_check_lifetime(LOG_WARN, tls, -1000, 0);
+ ret = tor_tls_check_lifetime(LOG_WARN, tls, time(NULL), -1000, 0);
tt_int_op(ret, OP_EQ, -1);
done:
@@ -2653,18 +2653,18 @@ test_tortls_cert_is_valid(void *ignored)
tor_x509_cert_t *cert = NULL, *scert = NULL;
scert = tor_malloc_zero(sizeof(tor_x509_cert_t));
- ret = tor_tls_cert_is_valid(LOG_WARN, cert, scert, 0);
+ ret = tor_tls_cert_is_valid(LOG_WARN, cert, scert, time(NULL), 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, 0);
+ ret = tor_tls_cert_is_valid(LOG_WARN, cert, scert, time(NULL), 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, 0);
+ ret = tor_tls_cert_is_valid(LOG_WARN, cert, scert, time(NULL), 0);
tt_int_op(ret, OP_EQ, 1);
#ifndef OPENSSL_OPAQUE
@@ -2675,7 +2675,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, 0);
+ ret = tor_tls_cert_is_valid(LOG_WARN, cert, scert, time(NULL), 0);
tt_int_op(ret, OP_EQ, 0);
tor_x509_cert_free(cert);
@@ -2684,7 +2684,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, 1);
+ ret = tor_tls_cert_is_valid(LOG_WARN, cert, scert, time(NULL), 1);
tt_int_op(ret, OP_EQ, 0);
#endif
@@ -2695,7 +2695,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, 1);
+ ret = tor_tls_cert_is_valid(LOG_WARN, cert, scert, time(NULL), 1);
tt_int_op(ret, OP_EQ, 0);
tor_x509_cert_free(cert);
@@ -2704,7 +2704,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, 1);
+ ret = tor_tls_cert_is_valid(LOG_WARN, cert, scert, time(NULL), 1);
tt_int_op(ret, OP_EQ, 0);
tor_x509_cert_free(cert);
@@ -2713,7 +2713,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, 0);
+ ret = tor_tls_cert_is_valid(LOG_WARN, cert, scert, time(NULL), 0);
tt_int_op(ret, OP_EQ, 1);
tor_x509_cert_free(cert);
@@ -2723,7 +2723,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, 0);
+ ret = tor_tls_cert_is_valid(LOG_WARN, cert, scert, time(NULL), 0);
tt_int_op(ret, OP_EQ, 0);
#endif