diff --git a/changes/bug4371 b/changes/bug4371 new file mode 100644 index 0000000000..37e6faa840 --- /dev/null +++ b/changes/bug4371 @@ -0,0 +1,3 @@ + o Minor bugfixes: + - Tolerate servers with more clock skew than previously. Fixes bug 4371; + bugfix on 0.2.3.6-alpha. diff --git a/src/common/tortls.c b/src/common/tortls.c index 8cf396cdac..a41a10d136 100644 --- a/src/common/tortls.c +++ b/src/common/tortls.c @@ -212,7 +212,8 @@ static int tor_tls_context_init_one(tor_tls_context_t **ppcontext, static tor_tls_context_t *tor_tls_context_new(crypto_pk_env_t *identity, unsigned int key_lifetime, int is_client); -static int check_cert_lifetime_internal(const X509 *cert, int tolerance); +static int check_cert_lifetime_internal(int severity, const X509 *cert, + int past_tolerance, int future_tolerance); /** Global TLS contexts. We keep them here because nobody else needs * to touch them. */ @@ -944,7 +945,8 @@ tor_tls_cert_matches_key(const tor_tls_t *tls, const tor_cert_t *cert) * the key is long enough. Return 1 if the cert is good, and 0 if it's bad or * we couldn't check it. */ int -tor_tls_cert_is_valid(const tor_cert_t *cert, +tor_tls_cert_is_valid(int severity, + const tor_cert_t *cert, const tor_cert_t *signing_cert, int check_rsa_1024) { @@ -960,8 +962,8 @@ tor_tls_cert_is_valid(const tor_cert_t *cert, /* okay, the signature checked out right. Now let's check the check the * lifetime. */ - /*XXXX tolerance might be iffy here */ - if (check_cert_lifetime_internal(cert->cert, 60*60) < 0) + if (check_cert_lifetime_internal(severity, cert->cert, + 48*60*60, 30*24*60*60) < 0) return 0; cert_key = X509_get_pubkey(cert->cert); @@ -1924,7 +1926,7 @@ tor_tls_get_peer_cert(tor_tls_t *tls) /** Warn that a certificate lifetime extends through a certain range. */ static void -log_cert_lifetime(const X509 *cert, const char *problem) +log_cert_lifetime(int severity, const X509 *cert, const char *problem) { BIO *bio = NULL; BUF_MEM *buf; @@ -1934,9 +1936,10 @@ log_cert_lifetime(const X509 *cert, const char *problem) struct tm tm; if (problem) - log_warn(LD_GENERAL, - "Certificate %s: is your system clock set incorrectly?", - problem); + log(severity, LD_GENERAL, + "Certificate %s. Either their clock is set wrong, or your clock " + "is wrong.", + problem); if (!(bio = BIO_new(BIO_s_mem()))) { log_warn(LD_GENERAL, "Couldn't allocate BIO!"); goto end; @@ -1958,9 +1961,9 @@ log_cert_lifetime(const X509 *cert, const char *problem) strftime(mytime, 32, "%b %d %H:%M:%S %Y GMT", tor_gmtime_r(&now, &tm)); - log_warn(LD_GENERAL, - "(certificate lifetime runs from %s through %s. Your time is %s.)", - s1,s2,mytime); + log(severity, LD_GENERAL, + "(certificate lifetime runs from %s through %s. Your time is %s.)", + s1,s2,mytime); end: /* Not expected to get invoked */ @@ -2062,14 +2065,15 @@ tor_tls_verify(int severity, tor_tls_t *tls, crypto_pk_env_t **identity_key) return r; } -/** Check whether the certificate set on the connection tls is - * expired or not-yet-valid, give or take tolerance - * seconds. Return 0 for valid, -1 for failure. +/** Check whether the certificate set on the connection tls is expired + * give or take past_tolerance seconds, or not-yet-valid give or take + * future_tolerance seconds. Return 0 for valid, -1 for failure. * * NOTE: you should call tor_tls_verify before tor_tls_check_lifetime. */ int -tor_tls_check_lifetime(tor_tls_t *tls, int tolerance) +tor_tls_check_lifetime(int severity, tor_tls_t *tls, + int past_tolerance, int future_tolerance) { X509 *cert; int r = -1; @@ -2077,7 +2081,8 @@ tor_tls_check_lifetime(tor_tls_t *tls, int tolerance) if (!(cert = SSL_get_peer_certificate(tls->ssl))) goto done; - if (check_cert_lifetime_internal(cert, tolerance) < 0) + if (check_cert_lifetime_internal(severity, cert, + past_tolerance, future_tolerance) < 0) goto done; r = 0; @@ -2090,24 +2095,26 @@ tor_tls_check_lifetime(tor_tls_t *tls, int tolerance) return r; } -/** Helper: check whether cert is currently live, give or take - * tolerance seconds. If it is live, return 0. If it is not live, - * log a message and return -1. */ +/** Helper: check whether cert is expired give or take + * past_tolerance seconds, or not-yet-valid give or take + * future_tolerance seconds. If it is live, return 0. If it is not + * live, log a message and return -1. */ static int -check_cert_lifetime_internal(const X509 *cert, int tolerance) +check_cert_lifetime_internal(int severity, const X509 *cert, int past_tolerance, + int future_tolerance) { time_t now, t; now = time(NULL); - t = now + tolerance; + t = now + future_tolerance; if (X509_cmp_time(X509_get_notBefore(cert), &t) > 0) { - log_cert_lifetime(cert, "not yet valid"); + log_cert_lifetime(severity, cert, "not yet valid"); return -1; } - t = now - tolerance; + t = now - past_tolerance; if (X509_cmp_time(X509_get_notAfter(cert), &t) < 0) { - log_cert_lifetime(cert, "already expired"); + log_cert_lifetime(severity, cert, "already expired"); return -1; } diff --git a/src/common/tortls.h b/src/common/tortls.h index d0a13cd804..673f18dfe8 100644 --- a/src/common/tortls.h +++ b/src/common/tortls.h @@ -68,7 +68,9 @@ void tor_tls_free(tor_tls_t *tls); int tor_tls_peer_has_cert(tor_tls_t *tls); tor_cert_t *tor_tls_get_peer_cert(tor_tls_t *tls); int tor_tls_verify(int severity, tor_tls_t *tls, crypto_pk_env_t **identity); -int tor_tls_check_lifetime(tor_tls_t *tls, int tolerance); +int tor_tls_check_lifetime(int severity, + tor_tls_t *tls, int past_tolerance, + int future_tolerance); 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); @@ -123,7 +125,8 @@ int tor_tls_get_my_certs(int server, crypto_pk_env_t *tor_tls_get_my_client_auth_key(void); crypto_pk_env_t *tor_tls_cert_get_key(tor_cert_t *cert); int tor_tls_cert_matches_key(const tor_tls_t *tls, const tor_cert_t *cert); -int tor_tls_cert_is_valid(const tor_cert_t *cert, +int tor_tls_cert_is_valid(int severity, + const tor_cert_t *cert, const tor_cert_t *signing_cert, int check_rsa_1024); diff --git a/src/or/command.c b/src/or/command.c index 5d0ebaa68a..a963d4210b 100644 --- a/src/or/command.c +++ b/src/or/command.c @@ -991,9 +991,9 @@ command_process_cert_cell(var_cell_t *cell, or_connection_t *conn) if (! tor_tls_cert_matches_key(conn->tls, link_cert)) { ERR("The link certificate didn't match the TLS public key"); } - if (! tor_tls_cert_is_valid(link_cert, id_cert, 0)) + if (! tor_tls_cert_is_valid(LOG_PROTOCOL_WARN, link_cert, id_cert, 0)) ERR("The link certificate was not valid"); - if (! tor_tls_cert_is_valid(id_cert, id_cert, 1)) + if (! tor_tls_cert_is_valid(LOG_PROTOCOL_WARN, id_cert, id_cert, 1)) ERR("The ID certificate was not valid"); conn->handshake_state->authenticated = 1; @@ -1026,9 +1026,9 @@ command_process_cert_cell(var_cell_t *cell, or_connection_t *conn) ERR("The certs we wanted were missing"); /* Remember these certificates so we can check an AUTHENTICATE cell */ - if (! tor_tls_cert_is_valid(auth_cert, id_cert, 1)) + if (! tor_tls_cert_is_valid(LOG_PROTOCOL_WARN, auth_cert, id_cert, 1)) ERR("The authentication certificate was not valid"); - if (! tor_tls_cert_is_valid(id_cert, id_cert, 1)) + if (! tor_tls_cert_is_valid(LOG_PROTOCOL_WARN, id_cert, id_cert, 1)) ERR("The ID certificate was not valid"); log_info(LD_OR, "Got some good certificates from %s:%d: "