commit 0b4221f98dbb93c9322e7a778f04bcbcfcc79738 Author: Nick Mathewson <nickm@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