check reviewer --reviewer=emilia

Remove 'log' field from SCT and related accessors

In order to still have access to an SCT's CTLOG when calling SCT_print,
SSL_CTX_get0_ctlog_store has been added.

Improved documentation for some CT functions in openssl/ssl.h.
Reviewed-by: NEmilia Käsper <emilia@openssl.org>
Reviewed-by: NRich Salz <rsalz@openssl.org>

apps/s_client.c

@@ -204,7 +204,7 @@ static int c_quiet = 0;
 static int c_ign_eof = 0;
 static int c_brief = 0;
 
-static void print_stuff(BIO *berr, SSL *con, int full);
+static void print_stuff(BIO *berr, const SSL_CTX *ctx, SSL *con, int full);
 static int ocsp_resp_cb(SSL *s, void *arg);
 
 static int saved_errno;
@@ -2184,7 +2184,7 @@ int s_client_main(int argc, char **argv)
                     print_ssl_summary(con);
                 }
 
-                print_stuff(bio_c_out, con, full_log);
+                print_stuff(bio_c_out, ctx, con, full_log);
                 if (full_log > 0)
                     full_log--;
 
@@ -2516,13 +2516,13 @@ int s_client_main(int argc, char **argv)
     ret = 0;
  shut:
     if (in_init)
-        print_stuff(bio_c_out, con, full_log);
+        print_stuff(bio_c_out, ctx, con, full_log);
     do_ssl_shutdown(con);
     BIO_closesocket(SSL_get_fd(con));
  end:
     if (con != NULL) {
         if (prexit != 0)
-            print_stuff(bio_c_out, con, 1);
+            print_stuff(bio_c_out, ctx, con, 1);
         SSL_free(con);
     }
 #if !defined(OPENSSL_NO_NEXTPROTONEG)
@@ -2554,7 +2554,7 @@ int s_client_main(int argc, char **argv)
     return (ret);
 }
 
-static void print_stuff(BIO *bio, SSL *s, int full)
+static void print_stuff(BIO *bio, const SSL_CTX *ctx, SSL *s, int full)
 {
     X509 *peer = NULL;
     char buf[BUFSIZ];
@@ -2634,7 +2634,7 @@ static void print_stuff(BIO *bio, SSL *s, int full)
 
         if (scts != NULL && sk_SCT_num(scts) > 0) {
             BIO_printf(bio, "---\n");
-            SCT_LIST_print(scts, bio, 0, "\n---\n");
+            SCT_LIST_print(scts, bio, 0, "\n---\n", SSL_CTX_get0_ctlog_store(ctx));
             BIO_printf(bio, "\n");
         }
 #endif

crypto/ct/ct_locl.h

@@ -125,8 +125,6 @@ struct sct_st {
     ct_log_entry_type_t entry_type;
     /* Where this SCT was found, e.g. certificate, OCSP response, etc. */
     sct_source_t source;
-    /* The CT log that produced this SCT. */
-    const CTLOG *log;
     /* The result of the last attempt to validate this SCT. */
     sct_validation_status_t validation_status;
 };

crypto/ct/ct_prn.c

@@ -96,7 +96,7 @@ static void timestamp_print(uint64_t timestamp, BIO *out)
     ASN1_GENERALIZEDTIME_free(gen);
 }
 
-void SCT_print(const SCT *sct, BIO *out, int indent)
+void SCT_print(const SCT *sct, BIO *out, int indent, const CTLOG *log)
 {
     BIO_printf(out, "%*sSigned Certificate Timestamp:", indent, "");
     BIO_printf(out, "\n%*sVersion   : ", indent + 4, "");
@@ -109,9 +109,9 @@ void SCT_print(const SCT *sct, BIO *out, int indent)
 
     BIO_printf(out, "v1 (0x0)");
 
-    if (sct->log != NULL) {
+    if (log != NULL) {
         BIO_printf(out, "\n%*sLog       : %s", indent + 4, "",
-                   SCT_get0_log_name(sct));
+                   CTLOG_get0_name(log));
     }
 
     BIO_printf(out, "\n%*sLog ID    : ", indent + 4, "");
@@ -133,13 +133,20 @@ void SCT_print(const SCT *sct, BIO *out, int indent)
 }
 
 void SCT_LIST_print(const STACK_OF(SCT) *sct_list, BIO *out, int indent,
-                    const char *separator)
+                    const char *separator, const CTLOG_STORE *log_store)
 {
     int i;
 
     for (i = 0; i < sk_SCT_num(sct_list); ++i) {
         SCT *sct = sk_SCT_value(sct_list, i);
-        SCT_print(sct, out, indent);
+        const CTLOG *log = NULL;
+
+        if (log_store != NULL) {
+            log = CTLOG_STORE_get0_log_by_id(log_store, sct->log_id,
+                                             sct->log_id_len);
+        }
+
+        SCT_print(sct, out, indent, log);
         if (i < sk_SCT_num(sct_list) - 1)
             BIO_printf(out, "%s", separator);
     }

crypto/ct/ct_sct.c

@@ -251,11 +251,6 @@ size_t SCT_get0_log_id(const SCT *sct, unsigned char **log_id)
     return sct->log_id_len;
 }
 
-const char *SCT_get0_log_name(const SCT *sct)
-{
-    return CTLOG_get0_name(sct->log);
-}
-
 uint64_t SCT_get_timestamp(const SCT *sct)
 {
     return sct->timestamp;
@@ -327,18 +322,6 @@ int SCT_set_source(SCT *sct, sct_source_t source)
     }
 }
 
-const CTLOG *SCT_get0_log(const SCT *sct)
-{
-    return sct->log;
-}
-
-int SCT_set0_log(SCT *sct, const CTLOG_STORE *ct_logs)
-{
-    sct->log = CTLOG_STORE_get0_log_by_id(ct_logs, sct->log_id, sct->log_id_len);
-
-    return sct->log != NULL;
-}
-
 sct_validation_status_t SCT_get_validation_status(const SCT *sct)
 {
     return sct->validation_status;
@@ -349,20 +332,17 @@ int SCT_validate(SCT *sct, const CT_POLICY_EVAL_CTX *ctx)
     int is_sct_valid = -1;
     SCT_CTX *sctx = NULL;
     X509_PUBKEY *pub = NULL, *log_pkey = NULL;
+    const CTLOG *log;
 
-    switch (sct->version) {
-    case SCT_VERSION_V1:
-        if (sct->log == NULL)
-            sct->log = CTLOG_STORE_get0_log_by_id(ctx->log_store,
-                                                  sct->log_id,
-                                                  CT_V1_HASHLEN);
-        break;
-    default:
+    if (sct->version != SCT_VERSION_V1) {
         sct->validation_status = SCT_VALIDATION_STATUS_UNKNOWN_VERSION;
         goto end;
     }
 
-    if (sct->log == NULL) {
+    log = CTLOG_STORE_get0_log_by_id(ctx->log_store,
+                                     sct->log_id, sct->log_id_len);
+
+    if (log == NULL) {
         sct->validation_status = SCT_VALIDATION_STATUS_UNKNOWN_LOG;
         goto end;
     }
@@ -371,7 +351,7 @@ int SCT_validate(SCT *sct, const CT_POLICY_EVAL_CTX *ctx)
     if (sctx == NULL)
         goto err;
 
-    if (X509_PUBKEY_set(&log_pkey, CTLOG_get0_public_key(sct->log)) != 1)
+    if (X509_PUBKEY_set(&log_pkey, CTLOG_get0_public_key(log)) != 1)
         goto err;
     if (SCT_CTX_set1_pubkey(sctx, log_pkey) != 1)
         goto err;

crypto/ct/ct_x509v3.c

@@ -75,7 +75,7 @@ static char *i2s_poison(const X509V3_EXT_METHOD *method, void *val)
 static int i2r_SCT_LIST(X509V3_EXT_METHOD *method, STACK_OF(SCT) *sct_list,
                  BIO *out, int indent)
 {
-    SCT_LIST_print(sct_list, out, indent, "\n");
+    SCT_LIST_print(sct_list, out, indent, "\n", NULL);
     return 1;
 }
 

include/openssl/ct.h

@@ -222,13 +222,6 @@ __owur int SCT_set0_log_id(SCT *sct, unsigned char *log_id, size_t log_id_len);
 __owur int SCT_set1_log_id(SCT *sct, const unsigned char *log_id,
                            size_t log_id_len);
 
-/*
- * Gets the name of the log that an SCT came from.
- * Ownership of the log name remains with the SCT.
- * Returns the log name, or NULL if it is not known.
- */
-const char *SCT_get0_log_name(const SCT *sct);
-
 /*
  * Returns the timestamp for the SCT (epoch time in milliseconds).
  */
@@ -306,33 +299,24 @@ sct_source_t SCT_get_source(const SCT *sct);
  */
 __owur int SCT_set_source(SCT *sct, sct_source_t source);
 
-/*
- * Gets information about the log the SCT came from, if set.
- */
-const CTLOG *SCT_get0_log(const SCT *sct);
-
-/*
- * Looks up information about the log the SCT came from using a CT log store.
- * The CTLOG_STORE must outlive the SCT, as ownership of the CTLOG remains with
- * the CTLOG_STORE.
- * Returns 1 if information about the log is found, 0 otherwise.
- * The information can be accessed via SCT_get0_log.
- */
-int SCT_set0_log(SCT *sct, const CTLOG_STORE* ct_logs);
-
 /*
  * Pretty-prints an |sct| to |out|.
  * It will be indented by the number of spaces specified by |indent|.
+ * If |log| is not NULL:
+ * - it should be the CT log that the SCT came from.
+ * - its name will be printed.
  */
-void SCT_print(const SCT *sct, BIO *out, int indent);
+void SCT_print(const SCT *sct, BIO *out, int indent, const CTLOG *log);
 
 /*
  * Pretty-prints an |sct_list| to |out|.
  * It will be indented by the number of spaces specified by |indent|.
  * SCTs will be delimited by |separator|.
+ * If |logs| is not NULL, it will be used to lookup the CT log that each SCT
+ * came from, so that the log names can be printed.
  */
 void SCT_LIST_print(const STACK_OF(SCT) *sct_list, BIO *out, int indent,
-                    const char *separator);
+                    const char *separator, const CTLOG_STORE *logs);
 
 /*
  * Verifies an SCT with the given context.

include/openssl/ssl.h

@@ -1932,11 +1932,39 @@ __owur ct_validation_cb SSL_CTX_get_ct_validation_callback(const SSL_CTX *ctx);
 /* Gets the SCTs received from a connection */
 const STACK_OF(SCT) *SSL_get0_peer_scts(SSL *s);
 
-/* Load the CT log list from the default location */
+/*
+ * Loads the CT log list from the default location.
+ * If a CTLOG_STORE has previously been set using SSL_CTX_set_ctlog_store,
+ * the log information loaded from this file will be appended to the
+ * CTLOG_STORE.
+ * Returns 1 on success, 0 otherwise.
+ */
 int SSL_CTX_set_default_ctlog_list_file(SSL_CTX *ctx);
-/* Load the CT log list from the specified file path */
+
+/*
+ * Loads the CT log list from the specified file path.
+ * If a CTLOG_STORE has previously been set using SSL_CTX_set_ctlog_store,
+ * the log information loaded from this file will be appended to the
+ * CTLOG_STORE.
+ * Returns 1 on success, 0 otherwise.
+ */
 int SSL_CTX_set_ctlog_list_file(SSL_CTX *ctx, const char *path);
 
+/*
+ * Sets the CT log list used by all SSL connections created from this SSL_CTX.
+ * Ownership of the CTLOG_STORE is transferred to the SSL_CTX.
+ */
+void SSL_CTX_set0_ctlog_store(SSL_CTX *ctx, CTLOG_STORE *logs);
+
+/*
+ * Gets the CT log list used by all SSL connections created from this SSL_CTX.
+ * This will be NULL unless one of the following functions has been called:
+ * - SSL_CTX_set_default_ctlog_list_file
+ * - SSL_CTX_set_ctlog_list_file
+ * - SSL_CTX_set_ctlog_store
+ */
+const CTLOG_STORE *SSL_CTX_get0_ctlog_store(const SSL_CTX *ctx);
+
 # endif /* OPENSSL_NO_CT */
 
 /* What the "other" parameter contains in security callback */

ssl/ssl_lib.c

@@ -4156,4 +4156,15 @@ int SSL_CTX_set_ctlog_list_file(SSL_CTX *ctx, const char *path)
     return CTLOG_STORE_load_file(ctx->ctlog_store, path);
 }
 
+void SSL_CTX_set0_ctlog_store(SSL_CTX *ctx, CTLOG_STORE *logs)
+{
+    CTLOG_STORE_free(ctx->ctlog_store);
+    ctx->ctlog_store = logs;
+}
+
+const CTLOG_STORE *SSL_CTX_get0_ctlog_store(const SSL_CTX *ctx)
+{
+    return ctx->ctlog_store;
+}
+
 #endif

test/ct_test.c

@@ -202,7 +202,7 @@ static int compare_sct_printout(SCT *sct,
         goto end;
     }
 
-    SCT_print(sct, text_buffer, 0);
+    SCT_print(sct, text_buffer, 0, NULL);
 
     /* Append null terminator because we're about to use the buffer contents
     * as a string. */

util/libcrypto.num

@@ -1240,7 +1240,6 @@ OBJ_obj2nid                             1202	1_1_0	EXIST::FUNCTION:
 PKCS12_SAFEBAG_free                     1203	1_1_0	EXIST::FUNCTION:
 EVP_cast5_cfb64                         1204	1_1_0	EXIST::FUNCTION:CAST
 OPENSSL_uni2asc                         1205	1_1_0	EXIST::FUNCTION:
-SCT_set0_log                            1206	1_1_0	EXIST::FUNCTION:
 PKCS7_add_attribute                     1207	1_1_0	EXIST::FUNCTION:
 ENGINE_register_DSA                     1208	1_1_0	EXIST::FUNCTION:ENGINE
 lh_node_stats                           1209	1_1_0	EXIST::FUNCTION:STDIO
@@ -1953,7 +1952,6 @@ idea_cbc_encrypt                        1890	1_1_0	EXIST::FUNCTION:IDEA
 BN_CTX_secure_new                       1891	1_1_0	EXIST::FUNCTION:
 OCSP_ONEREQ_add_ext                     1892	1_1_0	EXIST::FUNCTION:
 CMS_uncompress                          1893	1_1_0	EXIST::FUNCTION:CMS
-SCT_get0_log                            1894	1_1_0	EXIST::FUNCTION:
 CRYPTO_mem_debug_pop                    1895	1_1_0	EXIST::FUNCTION:CRYPTO_MDEBUG
 EVP_aes_192_cfb128                      1896	1_1_0	EXIST::FUNCTION:AES
 OCSP_REQ_CTX_nbio                       1897	1_1_0	EXIST::FUNCTION:
@@ -3651,7 +3649,6 @@ ENGINE_set_default_string               3532	1_1_0	EXIST::FUNCTION:ENGINE
 BIO_number_read                         3533	1_1_0	EXIST::FUNCTION:
 CRYPTO_zalloc                           3534	1_1_0	EXIST::FUNCTION:
 EVP_PKEY_cmp_parameters                 3535	1_1_0	EXIST::FUNCTION:
-SCT_get0_log_name                       3536	1_1_0	EXIST::FUNCTION:
 EVP_PKEY_CTX_new_id                     3537	1_1_0	EXIST::FUNCTION:
 TLS_FEATURE_free                        3538	1_1_0	EXIST::FUNCTION:
 d2i_BASIC_CONSTRAINTS                   3539	1_1_0	EXIST::FUNCTION:

util/libssl.num

@@ -388,3 +388,5 @@ SSL_CIPHER_get_auth_nid                 387	1_1_0	EXIST::FUNCTION:
 SSL_CIPHER_get_kx_nid                   388	1_1_0	EXIST::FUNCTION:
 SSL_CIPHER_is_aead                      389	1_1_0	EXIST::FUNCTION:
 SSL_SESSION_up_ref                      390	1_1_0	EXIST::FUNCTION:
+SSL_CTX_set0_ctlog_store                391	1_1_0	EXIST::FUNCTION:CT
+SSL_CTX_get0_ctlog_store                392	1_1_0	EXIST::FUNCTION:CT