Do not depend on internals of the SSL state machine.
tlsdate has a "time_is_an_illusion" parameter which uses the server's
reported time (within some bounds) to check the certificate against. It
does this by configuring the time on the SSL's X509_VERIFY_PARAM when
one of the SSL3_ST_CR_SRVR_HELLO_A and SSL3_ST_CR_SRVR_HELLO_B states
passes.
In addition to depending on quirks of the OpenSSL state machine which
BoringSSL would otherwise need to emulate, this code is wrong. It needs
to run at a point after the server_random is filled in. In the original
OpenSSL code, SSL3_ST_CR_SRVR_HELLO_A is when the message header is
read, so this is too early. The _B also wouldn't work in a non-blocking
socket because state mcahine might pause halfway through reading the
body. This probably only worked because it only uses blocking BIOs.
This also depends on OpenSSL's info_callback hacking the state
transitions so SSL_state returned the previous state during the
callback.
Rather than ossify all these bugs, use SSL_CTX_set_cert_verify_callback.
This overrides OpenSSL's call to X509_verify_cert. By looking up the
server random immediately before verification, we are guaranteed
server_random is filled in. At this point we also have an X509_STORE_CTX
available, so we may set the time on it directly.
Change-Id: I0a830984539d7e9e53c78891dea07f27f71edcbf
Test: mma
diff --git a/src/tlsdate-helper.c b/src/tlsdate-helper.c
index 319497f..3960458 100644
--- a/src/tlsdate-helper.c
+++ b/src/tlsdate-helper.c
@@ -354,39 +354,38 @@
free(ptr);
}
-void
-openssl_time_callback (const SSL* ssl, int where, int ret)
+static int
+verify_with_server_time (X509_STORE_CTX *store_ctx, void *arg)
{
- if (where == SSL_CB_CONNECT_LOOP &&
- (SSL_state(ssl) == SSL3_ST_CR_SRVR_HELLO_A ||
- SSL_state(ssl) == SSL3_ST_CR_SRVR_HELLO_B))
+ SSL *ssl = X509_STORE_CTX_get_ex_data(
+ store_ctx, SSL_get_ex_data_X509_STORE_CTX_idx());
+
+ // XXX TODO: If we want to trust the remote system for time,
+ // can we just read that time out of the remote system and if the
+ // cert verifies, decide that the time is reasonable?
+ // Such a process seems to indicate that a once valid cert would be
+ // forever valid - we stopgap that by ensuring it isn't less than
+ // the latest compiled_time and isn't above max_reasonable_time...
+ // XXX TODO: Solve eternal question about the Chicken and the Egg...
+ uint32_t compiled_time = RECENT_COMPILE_DATE;
+ uint32_t max_reasonable_time = MAX_REASONABLE_TIME;
+ uint32_t server_time;
+ verb("V: freezing time for x509 verification");
+ SSL_get_server_random(ssl, (unsigned char*)&server_time, sizeof(uint32_t));
+ if (compiled_time < ntohl(server_time)
+ &&
+ ntohl(server_time) < max_reasonable_time)
{
- // XXX TODO: If we want to trust the remote system for time,
- // can we just read that time out of the remote system and if the
- // cert verifies, decide that the time is reasonable?
- // Such a process seems to indicate that a once valid cert would be
- // forever valid - we stopgap that by ensuring it isn't less than
- // the latest compiled_time and isn't above max_reasonable_time...
- // XXX TODO: Solve eternal question about the Chicken and the Egg...
- uint32_t compiled_time = RECENT_COMPILE_DATE;
- uint32_t max_reasonable_time = MAX_REASONABLE_TIME;
- uint32_t server_time;
- verb("V: freezing time for x509 verification");
- SSL_get_server_random(ssl, (unsigned char*)&server_time, sizeof(uint32_t));
- if (compiled_time < ntohl(server_time)
- &&
- ntohl(server_time) < max_reasonable_time)
- {
- verb("V: remote peer provided: %d, preferred over compile time: %d",
- ntohl(server_time), compiled_time);
- verb("V: freezing time with X509_VERIFY_PARAM_set_time");
- X509_VERIFY_PARAM_set_time(SSL_get0_param((SSL*)ssl),
- (time_t) ntohl(server_time) + 86400);
- } else {
- die("V: the remote server is a false ticker! server: %d compile: %d",
- ntohl(server_time), compiled_time);
- }
+ verb("V: remote peer provided: %d, preferred over compile time: %d",
+ ntohl(server_time), compiled_time);
+ verb("V: freezing time with X509_VERIFY_PARAM_set_time");
+ X509_STORE_CTX_set_time(store_ctx, 0, (time_t) ntohl(server_time) + 86400);
+ } else {
+ die("V: the remote server is a false ticker! server: %d compile: %d",
+ ntohl(server_time), compiled_time);
}
+
+ return X509_verify_cert(store_ctx);
}
static const char *
@@ -1165,6 +1164,11 @@
}
}
+ if (time_is_an_illusion)
+ {
+ SSL_CTX_set_cert_verify_callback(ctx, verify_with_server_time, NULL);
+ }
+
if (NULL == (s_bio = BIO_new(BIO_s_connect())))
die ("connect BIO setup failed");
setup_proxy(s_bio);
@@ -1172,11 +1176,6 @@
die ("SSL setup failed");
SSL_set_bio(ssl, s_bio, s_bio);
- if (time_is_an_illusion)
- {
- SSL_set_info_callback(ssl, openssl_time_callback);
- }
-
SSL_set_mode(ssl, SSL_MODE_AUTO_RETRY);
verb("V: opening socket to %s:%s", host, port);
if ( (1 != BIO_set_conn_hostname(s_bio, host)) ||
diff --git a/src/tlsdate-helper.h b/src/tlsdate-helper.h
index 64e4092..fa9861a 100644
--- a/src/tlsdate-helper.h
+++ b/src/tlsdate-helper.h
@@ -124,7 +124,6 @@
static const char *ca_cert_container;
#ifndef USE_POLARSSL
-void openssl_time_callback (const SSL* ssl, int where, int ret);
uint32_t get_certificate_keybits (EVP_PKEY *public_key);
uint32_t check_cn (SSL *ssl, const char *hostname);
uint32_t check_san (SSL *ssl, const char *hostname);
