Improves CTLOG_STORE setters

Changes them to have clearer ownership semantics, as suggested in
https://github.com/openssl/openssl/pull/1372#discussion_r73232196.

Reviewed-by: Emilia Käsper <emilia@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/1408)

diff --git a/crypto/ct/ct_policy.c b/crypto/ct/ct_policy.c
index 4c4f9b3e3b6d13e11690040f32eef73ca8448d11..3c8411c0e427348987674ff3ed3c2d2cd90db25e 100644 (file)
--- a/crypto/ct/ct_policy.c
+++ b/crypto/ct/ct_policy.c
@@ -30,21 +30,25 @@ CT_POLICY_EVAL_CTX *CT_POLICY_EVAL_CTX_new(void)
 
 void CT_POLICY_EVAL_CTX_free(CT_POLICY_EVAL_CTX *ctx)
 {
+    X509_free(ctx->cert);
+    X509_free(ctx->issuer);
     OPENSSL_free(ctx);
 }
 
-void CT_POLICY_EVAL_CTX_set0_cert(CT_POLICY_EVAL_CTX *ctx, X509 *cert)
+void CT_POLICY_EVAL_CTX_set1_cert(CT_POLICY_EVAL_CTX *ctx, X509 *cert)
 {
-    ctx->cert = cert;
+    if (X509_up_ref(cert))
+        ctx->cert = cert;
 }
 
-void CT_POLICY_EVAL_CTX_set0_issuer(CT_POLICY_EVAL_CTX *ctx, X509 *issuer)
+void CT_POLICY_EVAL_CTX_set1_issuer(CT_POLICY_EVAL_CTX *ctx, X509 *issuer)
 {
-    ctx->issuer = issuer;
+    if (X509_up_ref(issuer))
+        ctx->issuer = issuer;
 }
 
-void CT_POLICY_EVAL_CTX_set0_log_store(CT_POLICY_EVAL_CTX *ctx,
-                                       CTLOG_STORE *log_store)
+void CT_POLICY_EVAL_CTX_set_shared_CTLOG_STORE(CT_POLICY_EVAL_CTX *ctx,
+                                               CTLOG_STORE *log_store)
 {
     ctx->log_store = log_store;
 }

diff --git a/include/openssl/ct.h b/include/openssl/ct.h
index 07068af2be66248091f0bc86bd76318a10c96cb3..52ea6a269ac835ea00f30557720d4bf5a3fcb39e 100644 (file)
--- a/include/openssl/ct.h
+++ b/include/openssl/ct.h
@@ -64,27 +64,27 @@ DEFINE_STACK_OF(CTLOG)
 /* Creates a new, empty policy evaluation context */
 CT_POLICY_EVAL_CTX *CT_POLICY_EVAL_CTX_new(void);
 
-/* Deletes a policy evaluation context */
+/* Deletes a policy evaluation context and anything it owns. */
 void CT_POLICY_EVAL_CTX_free(CT_POLICY_EVAL_CTX *ctx);
 
 /* Gets the peer certificate that the SCTs are for */
 X509* CT_POLICY_EVAL_CTX_get0_cert(const CT_POLICY_EVAL_CTX *ctx);
 
 /* Sets the certificate associated with the received SCTs */
-void CT_POLICY_EVAL_CTX_set0_cert(CT_POLICY_EVAL_CTX *ctx, X509 *cert);
+void CT_POLICY_EVAL_CTX_set1_cert(CT_POLICY_EVAL_CTX *ctx, X509 *cert);
 
 /* Gets the issuer of the aforementioned certificate */
 X509* CT_POLICY_EVAL_CTX_get0_issuer(const CT_POLICY_EVAL_CTX *ctx);
 
 /* Sets the issuer of the certificate associated with the received SCTs */
-void CT_POLICY_EVAL_CTX_set0_issuer(CT_POLICY_EVAL_CTX *ctx, X509 *issuer);
+void CT_POLICY_EVAL_CTX_set1_issuer(CT_POLICY_EVAL_CTX *ctx, X509 *issuer);
 
 /* Gets the CT logs that are trusted sources of SCTs */
 const CTLOG_STORE *CT_POLICY_EVAL_CTX_get0_log_store(const CT_POLICY_EVAL_CTX *ctx);
 
-/* Sets the log store that is in use */
-void CT_POLICY_EVAL_CTX_set0_log_store(CT_POLICY_EVAL_CTX *ctx,
-                                       CTLOG_STORE *log_store);
+/* Sets the log store that is in use. It must outlive the CT_POLICY_EVAL_CTX. */
+void CT_POLICY_EVAL_CTX_set_shared_CTLOG_STORE(CT_POLICY_EVAL_CTX *ctx,
+                                               CTLOG_STORE *log_store);
 
 /*****************
  * SCT functions *

diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index 8c3c88e227436bcbc297bc74e3c0f478d18ee50f..04bd9ee2fcbf394829b425dd4c27286b111eaf05 100644 (file)
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -4171,9 +4171,9 @@ int ssl_validate_ct(SSL *s)
     }
 
     issuer = sk_X509_value(s->verified_chain, 1);
-    CT_POLICY_EVAL_CTX_set0_cert(ctx, cert);
-    CT_POLICY_EVAL_CTX_set0_issuer(ctx, issuer);
-    CT_POLICY_EVAL_CTX_set0_log_store(ctx, s->ctx->ctlog_store);
+    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);
 
     scts = SSL_get0_peer_scts(s);
 

diff --git a/test/ct_test.c b/test/ct_test.c
index 8cc97e25ff8766bfee47d67afd557e1bf9504148..6c96268e8760189085bcf145bfc4b8db53ee6fc3 100644 (file)
--- a/test/ct_test.c
+++ b/test/ct_test.c
@@ -294,7 +294,8 @@ static int execute_cert_test(CT_TEST_FIXTURE fixture)
         expected_sct_text[sct_text_len] = '\0';
     }
 
-    CT_POLICY_EVAL_CTX_set0_log_store(ct_policy_ctx, fixture.ctlog_store);
+    CT_POLICY_EVAL_CTX_set_shared_CTLOG_STORE(
+            ct_policy_ctx, fixture.ctlog_store);
 
     if (fixture.certificate_file != NULL) {
         int sct_extension_index;
@@ -307,7 +308,7 @@ static int execute_cert_test(CT_TEST_FIXTURE fixture)
             goto end;
         }
 
-        CT_POLICY_EVAL_CTX_set0_cert(ct_policy_ctx, cert);
+        CT_POLICY_EVAL_CTX_set1_cert(ct_policy_ctx, cert);
 
         if (fixture.issuer_file != NULL) {
             issuer = load_pem_cert(fixture.certs_dir, fixture.issuer_file);
@@ -318,7 +319,7 @@ static int execute_cert_test(CT_TEST_FIXTURE fixture)
                 goto end;
             }
 
-            CT_POLICY_EVAL_CTX_set0_issuer(ct_policy_ctx, issuer);
+            CT_POLICY_EVAL_CTX_set1_issuer(ct_policy_ctx, issuer);
         }
 
         sct_extension_index =

diff --git a/util/libcrypto.num b/util/libcrypto.num
index cffb46ab686b1719ca6c9a7d29304377b6f06e2a..f9d0c20cea584644804e51c9163430620c0047cd 100644 (file)
--- a/util/libcrypto.num
+++ b/util/libcrypto.num
@@ -291,7 +291,7 @@ CRYPTO_gcm128_setiv                     291 1_1_0   EXIST::FUNCTION:
 ASN1_PCTX_set_oid_flags                 292    1_1_0   EXIST::FUNCTION:
 d2i_ASN1_INTEGER                        293    1_1_0   EXIST::FUNCTION:
 i2d_PKCS7_ENCRYPT                       294    1_1_0   EXIST::FUNCTION:
-CT_POLICY_EVAL_CTX_set0_issuer          295    1_1_0   EXIST::FUNCTION:CT
+CT_POLICY_EVAL_CTX_set0_issuer          295    1_1_0   NOEXIST::FUNCTION:
 X509_NAME_ENTRY_set                     296    1_1_0   EXIST::FUNCTION:
 PKCS8_set0_pbe                          297    1_1_0   EXIST::FUNCTION:
 PEM_write_bio_DSA_PUBKEY                298    1_1_0   EXIST::FUNCTION:DSA
@@ -457,7 +457,7 @@ DH_new_method                           457 1_1_0   EXIST::FUNCTION:DH
 BF_ecb_encrypt                          458    1_1_0   EXIST::FUNCTION:BF
 PEM_write_bio_DHparams                  459    1_1_0   EXIST::FUNCTION:DH
 EVP_DigestFinal                         460    1_1_0   EXIST::FUNCTION:
-CT_POLICY_EVAL_CTX_set0_log_store       461    1_1_0   EXIST::FUNCTION:CT
+CT_POLICY_EVAL_CTX_set0_log_store       461    1_1_0   NOEXIST::FUNCTION:
 X509v3_asid_add_id_or_range             462    1_1_0   EXIST::FUNCTION:RFC3779
 X509_NAME_ENTRY_create_by_NID           463    1_1_0   EXIST::FUNCTION:
 EC_KEY_METHOD_get_init                  464    1_1_0   EXIST::FUNCTION:EC
@@ -1390,7 +1390,7 @@ EVP_PKEY_asn1_free                      1375      1_1_0   EXIST::FUNCTION:
 ENGINE_unregister_DH                    1376   1_1_0   EXIST::FUNCTION:ENGINE
 PROXY_CERT_INFO_EXTENSION_it            1377   1_1_0   EXIST:!EXPORT_VAR_AS_FUNCTION:VARIABLE:
 PROXY_CERT_INFO_EXTENSION_it            1377   1_1_0   EXIST:EXPORT_VAR_AS_FUNCTION:FUNCTION:
-CT_POLICY_EVAL_CTX_set0_cert            1378   1_1_0   EXIST::FUNCTION:CT
+CT_POLICY_EVAL_CTX_set0_cert            1378   1_1_0   NOEXIST::FUNCTION:
 X509_NAME_hash                          1379   1_1_0   EXIST::FUNCTION:
 SCT_set_timestamp                       1380   1_1_0   EXIST::FUNCTION:CT
 UI_new                                  1381   1_1_0   EXIST::FUNCTION:UI
@@ -4190,3 +4190,6 @@ X509_get_proxy_pathlen                  4136      1_1_0   EXIST::FUNCTION:
 DSA_bits                                4137   1_1_0   EXIST::FUNCTION:DSA
 EVP_PKEY_set1_tls_encodedpoint          4138   1_1_0   EXIST::FUNCTION:
 EVP_PKEY_get1_tls_encodedpoint          4139   1_1_0   EXIST::FUNCTION:
+CT_POLICY_EVAL_CTX_set1_cert            4140   1_1_0   EXIST::FUNCTION:CT
+CT_POLICY_EVAL_CTX_set1_issuer          4141   1_1_0   EXIST::FUNCTION:CT
+CT_POLICY_EVAL_CTX_set_shared_CTLOG_STORE 4142 1_1_0   EXIST::FUNCTION:CT