Check that SCT timestamps are not in the future

Reviewed-by: Viktor Dukhovni <viktor@openssl.org> Reviewed-by: Rich Salz <rsalz@openssl.org> (Merged from #1554) (cherry picked from commit 1fa9ffd)
openssl · Nov 15, 2016 · a99be00 · a99be00
1 parent 5be6f71
commit a99be00
Show file tree

Hide file tree

Showing 10 changed files with 80 additions and 7 deletions.
diff --git a/crypto/ct/ct_err.c b/crypto/ct/ct_err.c
@@ -36,6 +36,7 @@ static ERR_STRING_DATA CT_str_functs[] = {
     {ERR_FUNC(CT_F_O2I_SCT_LIST), "o2i_SCT_LIST"},
     {ERR_FUNC(CT_F_O2I_SCT_SIGNATURE), "o2i_SCT_signature"},
     {ERR_FUNC(CT_F_SCT_CTX_NEW), "SCT_CTX_new"},
+    {ERR_FUNC(CT_F_SCT_CTX_VERIFY), "SCT_CTX_verify"},
     {ERR_FUNC(CT_F_SCT_NEW), "SCT_new"},
     {ERR_FUNC(CT_F_SCT_NEW_FROM_BASE64), "SCT_new_from_base64"},
     {ERR_FUNC(CT_F_SCT_SET0_LOG_ID), "SCT_set0_log_id"},
@@ -45,7 +46,6 @@ static ERR_STRING_DATA CT_str_functs[] = {
     {ERR_FUNC(CT_F_SCT_SET_LOG_ENTRY_TYPE), "SCT_set_log_entry_type"},
     {ERR_FUNC(CT_F_SCT_SET_SIGNATURE_NID), "SCT_set_signature_nid"},
     {ERR_FUNC(CT_F_SCT_SET_VERSION), "SCT_set_version"},
-    {ERR_FUNC(CT_F_SCT_CTX_VERIFY), "SCT_CTX_verify"},
     {0, NULL}
 };
 
@@ -58,6 +58,7 @@ static ERR_STRING_DATA CT_str_reasons[] = {
      "log conf missing description"},
     {ERR_REASON(CT_R_LOG_CONF_MISSING_KEY), "log conf missing key"},
     {ERR_REASON(CT_R_LOG_KEY_INVALID), "log key invalid"},
+    {ERR_REASON(CT_R_SCT_FUTURE_TIMESTAMP), "sct future timestamp"},
     {ERR_REASON(CT_R_SCT_INVALID), "sct invalid"},
     {ERR_REASON(CT_R_SCT_INVALID_SIGNATURE), "sct invalid signature"},
     {ERR_REASON(CT_R_SCT_LIST_INVALID), "sct list invalid"},

diff --git a/crypto/ct/ct_locl.h b/crypto/ct/ct_locl.h
@@ -98,13 +98,17 @@ struct sct_ctx_st {
     /* pre-certificate encoding */
     unsigned char *preder;
     size_t prederlen;
+    /* milliseconds since epoch (to check that the SCT isn't from the future) */
+    uint64_t epoch_time_in_ms;
 };
 
 /* Context when evaluating whether a Certificate Transparency policy is met */
 struct ct_policy_eval_ctx_st {
     X509 *cert;
     X509 *issuer;
     CTLOG_STORE *log_store;
+    /* milliseconds since epoch (to check that SCTs aren't from the future) */
+    uint64_t epoch_time_in_ms;
 };
 
 /*
@@ -150,6 +154,14 @@ __owur int SCT_CTX_set1_issuer_pubkey(SCT_CTX *sctx, X509_PUBKEY *pubkey);
  */
 __owur int SCT_CTX_set1_pubkey(SCT_CTX *sctx, X509_PUBKEY *pubkey);
 
+/*
+ * Sets the current time, in milliseconds since the Unix epoch.
+ * The timestamp of the SCT will be compared to this, to check that it was not
+ * issued in the future. RFC6962 states that "TLS clients MUST reject SCTs whose
+ * timestamp is in the future", so SCT verification will fail in this case.
+ */
+void SCT_CTX_set_time(SCT_CTX *sctx, uint64_t time_in_ms);
+
 /*
  * Verifies an SCT with the given context.
  * Returns 1 if the SCT verifies successfully; any other value indicates

diff --git a/crypto/ct/ct_policy.c b/crypto/ct/ct_policy.c
@@ -59,6 +59,11 @@ void CT_POLICY_EVAL_CTX_set_shared_CTLOG_STORE(CT_POLICY_EVAL_CTX *ctx,
     ctx->log_store = log_store;
 }
 
+void CT_POLICY_EVAL_CTX_set_time(CT_POLICY_EVAL_CTX *ctx, uint64_t time_in_ms)
+{
+    ctx->epoch_time_in_ms = time_in_ms;
+}
+
 X509* CT_POLICY_EVAL_CTX_get0_cert(const CT_POLICY_EVAL_CTX *ctx)
 {
     return ctx->cert;
@@ -74,3 +79,7 @@ const CTLOG_STORE *CT_POLICY_EVAL_CTX_get0_log_store(const CT_POLICY_EVAL_CTX *c
     return ctx->log_store;
 }
 
+uint64_t CT_POLICY_EVAL_CTX_get_time(const CT_POLICY_EVAL_CTX *ctx)
+{
+    return ctx->epoch_time_in_ms;
+}
diff --git a/crypto/ct/ct_sct.c b/crypto/ct/ct_sct.c
@@ -329,6 +329,8 @@ int SCT_validate(SCT *sct, const CT_POLICY_EVAL_CTX *ctx)
             goto err;
     }
 
+    SCT_CTX_set_time(sctx, ctx->epoch_time_in_ms);
+
     /*
      * XXX: Potential for optimization.  This repeats some idempotent heavy
      * lifting on the certificate for each candidate SCT, and appears to not

diff --git a/crypto/ct/ct_sct_ctx.c b/crypto/ct/ct_sct_ctx.c
@@ -256,3 +256,8 @@ int SCT_CTX_set1_pubkey(SCT_CTX *sctx, X509_PUBKEY *pubkey)
     sctx->pkey = pkey;
     return 1;
 }
+
+void SCT_CTX_set_time(SCT_CTX *sctx, uint64_t time_in_ms)
+{
+    sctx->epoch_time_in_ms = time_in_ms;
+}
diff --git a/crypto/ct/ct_vfy.c b/crypto/ct/ct_vfy.c
@@ -113,6 +113,10 @@ int SCT_CTX_verify(const SCT_CTX *sctx, const SCT *sct)
         CTerr(CT_F_SCT_CTX_VERIFY, CT_R_SCT_LOG_ID_MISMATCH);
         return 0;
     }
+    if (sct->timestamp > sctx->epoch_time_in_ms) {
+        CTerr(CT_F_SCT_CTX_VERIFY, CT_R_SCT_FUTURE_TIMESTAMP);
+        return 0;
+    }
 
     ctx = EVP_MD_CTX_new();
     if (ctx == NULL)

diff --git a/include/openssl/ct.h b/include/openssl/ct.h
@@ -98,6 +98,21 @@ const CTLOG_STORE *CT_POLICY_EVAL_CTX_get0_log_store(const CT_POLICY_EVAL_CTX *c
 void CT_POLICY_EVAL_CTX_set_shared_CTLOG_STORE(CT_POLICY_EVAL_CTX *ctx,
                                                CTLOG_STORE *log_store);
 
+/*
+ * Gets the time, in milliseconds since the Unix epoch, that will be used as the
+ * current time when checking whether an SCT was issued in the future.
+ * Such SCTs will fail validation, as required by RFC6962.
+ */
+uint64_t CT_POLICY_EVAL_CTX_get_time(const CT_POLICY_EVAL_CTX *ctx);
+
+/*
+ * Sets the current time, in milliseconds since the Unix epoch.
+ * The timestamps of the SCTs will be compared to this, to check that they were
+ * not issued in the future. RFC6962 states that "TLS clients MUST reject SCTs
+ * whose timestamp is in the future", so an SCT will not validate in this case.
+ */
+void CT_POLICY_EVAL_CTX_set_time(CT_POLICY_EVAL_CTX *ctx, uint64_t time_in_ms);
+
 /*****************
  * SCT functions *
  *****************/
@@ -482,6 +497,7 @@ int ERR_load_CT_strings(void);
 # define CT_F_O2I_SCT_LIST                                111
 # define CT_F_O2I_SCT_SIGNATURE                           112
 # define CT_F_SCT_CTX_NEW                                 126
+# define CT_F_SCT_CTX_VERIFY                              128
 # define CT_F_SCT_NEW                                     100
 # define CT_F_SCT_NEW_FROM_BASE64                         127
 # define CT_F_SCT_SET0_LOG_ID                             101
@@ -491,7 +507,6 @@ int ERR_load_CT_strings(void);
 # define CT_F_SCT_SET_LOG_ENTRY_TYPE                      102
 # define CT_F_SCT_SET_SIGNATURE_NID                       103
 # define CT_F_SCT_SET_VERSION                             104
-# define CT_F_SCT_CTX_VERIFY                              128
 
 /* Reason codes. */
 # define CT_R_BASE64_DECODE_ERROR                         108
@@ -501,6 +516,7 @@ int ERR_load_CT_strings(void);
 # define CT_R_LOG_CONF_MISSING_DESCRIPTION                111
 # define CT_R_LOG_CONF_MISSING_KEY                        112
 # define CT_R_LOG_KEY_INVALID                             113
+# define CT_R_SCT_FUTURE_TIMESTAMP                        116
 # define CT_R_SCT_INVALID                                 104
 # define CT_R_SCT_INVALID_SIGNATURE                       107
 # define CT_R_SCT_LIST_INVALID                            105

diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
@@ -4175,6 +4175,7 @@ int ssl_validate_ct(SSL *s)
     CT_POLICY_EVAL_CTX_set1_cert(ctx, cert);
     CT_POLICY_EVAL_CTX_set1_issuer(ctx, issuer);
     CT_POLICY_EVAL_CTX_set_shared_CTLOG_STORE(ctx, s->ctx->ctlog_store);
+    CT_POLICY_EVAL_CTX_set_time(ctx, SSL_SESSION_get_time(SSL_get0_session(s)));
 
     scts = SSL_get0_peer_scts(s);
 

diff --git a/test/ct_test.c b/test/ct_test.c
@@ -29,13 +29,18 @@ static char *ct_dir = NULL;
 
 typedef struct ct_test_fixture {
     const char *test_case_name;
+    /* The current time in milliseconds */
+    uint64_t epoch_time_in_ms;
     /* The CT log store to use during tests */
     CTLOG_STORE* ctlog_store;
     /* Set the following to test handling of SCTs in X509 certificates */
     const char *certs_dir;
     char *certificate_file;
     char *issuer_file;
+    /* Expected number of SCTs */
     int expected_sct_count;
+    /* Expected number of valid SCTS */
+    int expected_valid_sct_count;
     /* Set the following to test handling of SCTs in TLS format */
     const unsigned char *tls_sct_list;
     size_t tls_sct_list_len;
@@ -49,7 +54,6 @@ typedef struct ct_test_fixture {
     const char *sct_text_file;
     /* Whether to test the validity of the SCT(s) */
     int test_validity;
-
 } CT_TEST_FIXTURE;
 
 static CT_TEST_FIXTURE set_up(const char *const test_case_name)
@@ -75,6 +79,7 @@ static CT_TEST_FIXTURE set_up(const char *const test_case_name)
     }
 
     fixture.test_case_name = test_case_name;
+    fixture.epoch_time_in_ms = 1473269626000; /* Sep 7 17:33:46 2016 GMT */
     fixture.ctlog_store = ctlog_store;
 
 end:
@@ -250,7 +255,7 @@ static int assert_validity(CT_TEST_FIXTURE fixture,
         }
     }
 
-    if (valid_sct_count != fixture.expected_sct_count) {
+    if (valid_sct_count != fixture.expected_valid_sct_count) {
         int unverified_sct_count = sk_SCT_num(scts) -
                 invalid_sct_count - valid_sct_count;
 
@@ -260,7 +265,7 @@ static int assert_validity(CT_TEST_FIXTURE fixture,
                 "%d SCTs were unverified\n",
                 invalid_sct_count,
                 valid_sct_count,
-                fixture.expected_sct_count,
+                fixture.expected_valid_sct_count,
                 unverified_sct_count);
         return 0;
     }
@@ -297,6 +302,8 @@ static int execute_cert_test(CT_TEST_FIXTURE fixture)
     CT_POLICY_EVAL_CTX_set_shared_CTLOG_STORE(
             ct_policy_ctx, fixture.ctlog_store);
 
+    CT_POLICY_EVAL_CTX_set_time(ct_policy_ctx, fixture.epoch_time_in_ms);
+
     if (fixture.certificate_file != NULL) {
         int sct_extension_index;
         X509_EXTENSION *sct_extension = NULL;
@@ -443,7 +450,7 @@ static int test_verify_one_sct()
     fixture.certs_dir = certs_dir;
     fixture.certificate_file = "embeddedSCTs1.pem";
     fixture.issuer_file = "embeddedSCTs1_issuer.pem";
-    fixture.expected_sct_count = 1;
+    fixture.expected_sct_count = fixture.expected_valid_sct_count = 1;
     fixture.test_validity = 1;
     EXECUTE_CT_TEST();
 }
@@ -454,7 +461,20 @@ static int test_verify_multiple_scts()
     fixture.certs_dir = certs_dir;
     fixture.certificate_file = "embeddedSCTs3.pem";
     fixture.issuer_file = "embeddedSCTs3_issuer.pem";
-    fixture.expected_sct_count = 3;
+    fixture.expected_sct_count = fixture.expected_valid_sct_count = 3;
+    fixture.test_validity = 1;
+    EXECUTE_CT_TEST();
+}
+
+static int test_verify_fails_for_future_sct()
+{
+    SETUP_CT_TEST_FIXTURE();
+    fixture.epoch_time_in_ms = 1365094800000; /* Apr 4 17:00:00 2013 GMT */
+    fixture.certs_dir = certs_dir;
+    fixture.certificate_file = "embeddedSCTs1.pem";
+    fixture.issuer_file = "embeddedSCTs1_issuer.pem";
+    fixture.expected_sct_count = 1;
+    fixture.expected_valid_sct_count = 0;
     fixture.test_validity = 1;
     EXECUTE_CT_TEST();
 }
@@ -548,6 +568,7 @@ int main(int argc, char *argv[])
     ADD_TEST(test_multiple_scts_in_certificate);
     ADD_TEST(test_verify_one_sct);
     ADD_TEST(test_verify_multiple_scts);
+    ADD_TEST(test_verify_fails_for_future_sct);
     ADD_TEST(test_decode_tls_sct);
     ADD_TEST(test_encode_tls_sct);
 

diff --git a/util/libcrypto.num b/util/libcrypto.num
@@ -4208,3 +4208,5 @@ OCSP_RESPID_set_by_key                  4158	1_1_0a	EXIST::FUNCTION:OCSP
 OCSP_RESPID_match                       4159	1_1_0a	EXIST::FUNCTION:OCSP
 DSO_pathbyaddr                          4170	1_1_0c	EXIST::FUNCTION:
 DSO_dsobyaddr                           4171	1_1_0c	EXIST::FUNCTION:
+CT_POLICY_EVAL_CTX_get_time             4172	1_1_1	EXIST::FUNCTION:CT
+CT_POLICY_EVAL_CTX_set_time             4173	1_1_1	EXIST::FUNCTION:CT