From 1fa9ffd934429f140edcfbaf76d2f32cc21e449b Mon Sep 17 00:00:00 2001 From: Rob Percival Date: Thu, 8 Sep 2016 16:02:46 +0100 Subject: [PATCH] Check that SCT timestamps are not in the future Reviewed-by: Viktor Dukhovni Reviewed-by: Rich Salz (Merged from https://github.com/openssl/openssl/pull/1554) --- crypto/ct/ct_err.c | 3 +- crypto/ct/ct_locl.h | 12 +++++++ crypto/ct/ct_policy.c | 9 ++++++ crypto/ct/ct_sct.c | 2 ++ crypto/ct/ct_sct_ctx.c | 5 +++ crypto/ct/ct_vfy.c | 4 +++ doc/man3/CT_POLICY_EVAL_CTX_new.pod | 16 ++++++++-- doc/man3/SCT_validate.pod | 8 +++-- .../SSL_CTX_set_ct_validation_callback.pod | 6 +++- include/openssl/ct.h | 18 ++++++++++- ssl/ssl_lib.c | 1 + test/ct_test.c | 31 ++++++++++++++++--- util/libcrypto.num | 2 ++ 13 files changed, 104 insertions(+), 13 deletions(-) diff --git a/crypto/ct/ct_err.c b/crypto/ct/ct_err.c index df232dc488..fe0778b278 100644 --- 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 index 7adc4961b3..4b5e344191 100644 --- a/crypto/ct/ct_locl.h +++ b/crypto/ct/ct_locl.h @@ -98,6 +98,8 @@ 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 */ @@ -105,6 +107,8 @@ 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 index 33738de103..074589db93 100644 --- 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 index 2f0fef7833..1b13d9e6cb 100644 --- a/crypto/ct/ct_sct.c +++ b/crypto/ct/ct_sct.c @@ -332,6 +332,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 index 28fd04485f..75a5027df0 100644 --- 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 index 724f65579b..cabcf5782a 100644 --- 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/doc/man3/CT_POLICY_EVAL_CTX_new.pod b/doc/man3/CT_POLICY_EVAL_CTX_new.pod index 37f3ed598a..0f50078b51 100644 --- a/doc/man3/CT_POLICY_EVAL_CTX_new.pod +++ b/doc/man3/CT_POLICY_EVAL_CTX_new.pod @@ -5,7 +5,8 @@ CT_POLICY_EVAL_CTX_new, CT_POLICY_EVAL_CTX_free, CT_POLICY_EVAL_CTX_get0_cert, CT_POLICY_EVAL_CTX_set1_cert, CT_POLICY_EVAL_CTX_get0_issuer, CT_POLICY_EVAL_CTX_set1_issuer, -CT_POLICY_EVAL_CTX_get0_log_store, CT_POLICY_EVAL_CTX_set_shared_CTLOG_STORE - +CT_POLICY_EVAL_CTX_get0_log_store, CT_POLICY_EVAL_CTX_set_shared_CTLOG_STORE, +CT_POLICY_EVAL_CTX_get_time, CT_POLICY_EVAL_CTX_set_time - Encapsulates the data required to evaluate whether SCTs meet a Certificate Transparency policy =head1 SYNOPSIS @@ -20,13 +21,16 @@ Encapsulates the data required to evaluate whether SCTs meet a Certificate Trans int CT_POLICY_EVAL_CTX_set1_issuer(CT_POLICY_EVAL_CTX *ctx, X509 *issuer); const CTLOG_STORE *CT_POLICY_EVAL_CTX_get0_log_store(const CT_POLICY_EVAL_CTX *ctx); void CT_POLICY_EVAL_CTX_set_shared_CTLOG_STORE(CT_POLICY_EVAL_CTX *ctx, CTLOG_STORE *log_store); + uint64_t CT_POLICY_EVAL_CTX_get_time(const CT_POLICY_EVAL_CTX *ctx); + void CT_POLICY_EVAL_CTX_set_time(CT_POLICY_EVAL_CTX *ctx, uint64_t time_in_ms); =head1 DESCRIPTION A B is used by functions that evaluate whether Signed Certificate Timestamps (SCTs) fulfil a Certificate Transparency (CT) policy. This policy may be, for example, that at least one valid SCT is available. To -determine this, an SCT's signature must be verified. This requires: +determine this, an SCT's timestamp and signature must be verified. +This requires: =over @@ -36,6 +40,8 @@ determine this, an SCT's signature must be verified. This requires: =item * the issuer certificate (if the SCT was issued for a pre-certificate) +=item * the current time + =back The above requirements are met using the setters described below. @@ -58,6 +64,12 @@ Increments the reference count of the certificate. Holds a pointer to the CTLOG_STORE, so the CTLOG_STORE must outlive the CT_POLICY_EVAL_CTX. +=item * CT_POLICY_EVAL_CTX_set_time() to provide the current time + +The SCT timestamp will be compared to this time to check whether the SCT was +supposedly issued in the future. RFC6962 states that "TLS clients MUST reject +SCTs whose timestamp is in the future". + =back Each setter has a matching getter for accessing the current value. diff --git a/doc/man3/SCT_validate.pod b/doc/man3/SCT_validate.pod index 98ae61822e..9868a282b5 100644 --- a/doc/man3/SCT_validate.pod +++ b/doc/man3/SCT_validate.pod @@ -54,9 +54,11 @@ status will be SCT_VALIDATION_STATUS_UNKNOWN_LOG. If the SCT is of an unsupported version (only v1 is currently supported), the validation status will be SCT_VALIDATION_STATUS_UNKNOWN_VERSION. -If the SCT's signature is incorrect, the validation status will be -SCT_VALIDATION_STATUS_INVALID. Otherwise, if all checks have passed, the -validation status will be SCT_VALIDATION_STATUS_VALID. +If the SCT's signature is incorrect, its timestamp is in the future (relative to +the time in CT_POLICY_EVAL_CTX), or if it is otherwise invalid, the validation +status will be SCT_VALIDATION_STATUS_INVALID. + +If all checks pass, the validation status will be SCT_VALIDATION_STATUS_VALID. =head1 NOTES diff --git a/doc/man3/SSL_CTX_set_ct_validation_callback.pod b/doc/man3/SSL_CTX_set_ct_validation_callback.pod index a6cbe8f527..d818e00fc5 100644 --- a/doc/man3/SSL_CTX_set_ct_validation_callback.pod +++ b/doc/man3/SSL_CTX_set_ct_validation_callback.pod @@ -97,6 +97,9 @@ otherwise. When SCT processing is enabled, OCSP stapling will be enabled. This is because one possible source of SCTs is the OCSP response from a server. +The time returned by SSL_SESSION_get_time() will be used to evaluate whether any +presented SCTs have timestamps that are in the future (and therefore invalid). + =head1 RESTRICTIONS Certificate Transparency validation cannot be enabled and so a callback cannot @@ -124,7 +127,8 @@ L, L, L, L, -L +L, +L =head1 COPYRIGHT diff --git a/include/openssl/ct.h b/include/openssl/ct.h index 6c63265257..a87dd7f268 100644 --- 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 index a6360accea..b6f701536f 100644 --- a/ssl/ssl_lib.c +++ b/ssl/ssl_lib.c @@ -4279,6 +4279,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 index 1cfd0d17f6..223037ed39 100644 --- 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: @@ -252,7 +257,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; @@ -262,7 +267,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; } @@ -299,6 +304,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; @@ -445,7 +452,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(); } @@ -456,7 +463,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(); } @@ -545,6 +565,7 @@ int test_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 index 0b4190b2d2..a1fdc3edfc 100644 --- a/util/libcrypto.num +++ b/util/libcrypto.num @@ -4218,3 +4218,5 @@ BIO_meth_get_write_ex 4168 1_1_1 EXIST::FUNCTION: BIO_meth_set_write_ex 4169 1_1_1 EXIST::FUNCTION: 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 -- 2.34.1