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.

6 files changed, 43 insertions, 34 deletions

diff --git a/src/common/tortls.c b/src/common/tortls.c
index eaa5748f33..fd8698128b 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 fe5898ef5c..3adb1b2f6e 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 cf57c29afb..9315f80fb8 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 94382f6e64..b7ed7f8083 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 39439d9d13..f851b92036 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 8efcac242f..44961c88e7 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