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.
(Cherry-picked from
https://android.googlesource.com/platform/external/tlsdate/+/c339766a51d2db711171cb704e30b7ae916a987f%5E%21/)
BUG=chromium:736453
TEST=Deploy, reboot, tlsdate correctly sets date.
Change-Id: I8512e083a50be55fffaa5d2bbcb66f3badb4a4ee
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/tlsdate/+/2203402
Tested-by: Jorge Lucangeli Obes <jorgelo@chromium.org>
Commit-Queue: Jorge Lucangeli Obes <jorgelo@chromium.org>
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Pavol Marko <pmarko@chromium.org>
diff --git a/src/tlsdate-helper.c b/src/tlsdate-helper.c
index fd0b3fe..476f301 100644
--- a/src/tlsdate-helper.c
+++ b/src/tlsdate-helper.c
@@ -322,45 +322,42 @@
}
-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 &&
-#if OPENSSL_VERSION_NUMBER < 0x10100000L
- (ssl->state == SSL3_ST_CR_SRVR_HELLO_A || ssl->state == SSL3_ST_CR_SRVR_HELLO_B))
-#else
- (SSL_get_state(ssl) == TLS_ST_CR_SRVR_HELLO))
-#endif
+ 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\n");
+ 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\n");
- 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\n",
- ntohl (server_time), compiled_time);
- verb ("V: freezing time with X509_VERIFY_PARAM_set_time\n");
- 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\n",
- ntohl (server_time), compiled_time);
- }
+ verb ("V: remote peer provided: %d, preferred over compile time: %d\n",
+ ntohl (server_time), compiled_time);
+ verb ("V: freezing time with X509_VERIFY_PARAM_set_time\n");
+ 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\n",
+ ntohl (server_time), compiled_time);
+ }
+
+ return X509_verify_cert (store_ctx);
}
uint32_t
@@ -878,16 +875,18 @@
}
}
}
+
+ if (time_is_an_illusion)
+ {
+ SSL_CTX_set_cert_verify_callback (ctx, verify_with_server_time, NULL);
+ }
+
verb ("V: setting up connection to %s:%s\n", g_host, g_port);
if (NULL == (s_bio = make_ssl_bio(ctx, g_host, g_port, g_proxy)))
die ("SSL BIO setup failed\n");
BIO_get_ssl (s_bio, &ssl);
if (NULL == ssl)
die ("SSL setup failed\n");
- if (time_is_an_illusion)
- {
- SSL_set_info_callback (ssl, openssl_time_callback);
- }
SSL_set_mode (ssl, SSL_MODE_AUTO_RETRY);
SSL_set_tlsext_host_name (ssl, g_host);
if (NULL == BIO_new_fp (stdout, BIO_NOCLOSE))
diff --git a/src/tlsdate-helper.h b/src/tlsdate-helper.h
index 0e89b9a..d68c3be 100644
--- a/src/tlsdate-helper.h
+++ b/src/tlsdate-helper.h
@@ -79,7 +79,6 @@
// To support our RFC 2595 wildcard verification
#define RFC2595_MIN_LABEL_COUNT 3
-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);
