If a CT log entry in CTLOG_FILE is invalid, skip it and continue loading

Previously, the remaining CT log entries would not be loaded.
Also, CTLOG_STORE_load_file would return 1 even if a log entry was
invalid, resulting in no errors being shown.

Reviewed-by: Ben Laurie <ben@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>

diff --git a/crypto/ct/ct_log.c b/crypto/ct/ct_log.c
index 14f3bcc1fd8c1e6c93700dce194fad946353cd21..f5a01fdbd329e385d5caf2af0eba9155cc2edd80 100644 (file)
--- a/crypto/ct/ct_log.c
+++ b/crypto/ct/ct_log.c
@@ -85,6 +85,7 @@ struct ctlog_store_st {
 typedef struct ctlog_store_load_ctx_st {
     CTLOG_STORE *log_store;
     CONF *conf;
+    size_t invalid_log_entries;
 } CTLOG_STORE_LOAD_CTX;
 
 /*
@@ -201,7 +202,13 @@ int CTLOG_STORE_load_default_file(CTLOG_STORE *store)
     return CTLOG_STORE_load_file(store, fpath);
 }
 
-static int ctlog_store_load_log(const char *log_name, int log_name_len, void *arg)
+/*
+ * Called by CONF_parse_list, which stops if this returns <= 0, so don't unless
+ * something very bad happens. Otherwise, one bad log entry would stop loading
+ * of any of the following log entries.
+ */
+static int ctlog_store_load_log(const char *log_name, int log_name_len,
+                                void *arg)
 {
     CTLOG_STORE_LOAD_CTX *load_ctx = arg;
     CTLOG *ct_log;
@@ -210,8 +217,11 @@ static int ctlog_store_load_log(const char *log_name, int log_name_len, void *ar
 
     ct_log = ctlog_new_from_conf(load_ctx->conf, tmp);
     OPENSSL_free(tmp);
-    if (ct_log == NULL)
-        return 0;
+    if (ct_log == NULL) {
+        /* If we can't load this log, record that fact and skip it */
+        ++load_ctx->invalid_log_entries;
+        return 1;
+    }
 
     sk_CTLOG_push(load_ctx->log_store->logs, ct_log);
     return 1;
@@ -219,7 +229,7 @@ static int ctlog_store_load_log(const char *log_name, int log_name_len, void *ar
 
 int CTLOG_STORE_load_file(CTLOG_STORE *store, const char *file)
 {
-    int ret = -1;
+    int ret = 0;
     char *enabled_logs;
     CTLOG_STORE_LOAD_CTX* load_ctx = ctlog_store_load_ctx_new();
 
@@ -235,7 +245,12 @@ int CTLOG_STORE_load_file(CTLOG_STORE *store, const char *file)
     }
 
     enabled_logs = NCONF_get_string(load_ctx->conf, NULL, "enabled_logs");
-    CONF_parse_list(enabled_logs, ',', 1, ctlog_store_load_log, load_ctx);
+    ret = CONF_parse_list(enabled_logs, ',', 1, ctlog_store_load_log, load_ctx);
+    if (ret == 1 && load_ctx->invalid_log_entries > 0) {
+        ret = 0;
+        CTerr(CT_F_CTLOG_STORE_LOAD_FILE, CT_R_LOG_CONF_INVALID);
+        goto end;
+    }
 
 end:
     NCONF_free(load_ctx->conf);

diff --git a/include/openssl/ct.h b/include/openssl/ct.h
index 7ea7ff236c330544bf992ac78a27ae08494ad1be..6d2182f796f154c5f3de7a822aa25d4809d41272 100644 (file)
--- a/include/openssl/ct.h
+++ b/include/openssl/ct.h
@@ -542,7 +542,7 @@ CTLOG *CTLOG_STORE_get0_log_by_id(const CTLOG_STORE *store,
 
 /*
  * Loads a CT log list into a |store| from a |file|.
- * Returns 1 if loading is successful, or a non-positive integer otherwise.
+ * Returns 1 if loading is successful, or 0 otherwise.
  */
 int CTLOG_STORE_load_file(CTLOG_STORE *store, const char *file);
 
@@ -550,7 +550,7 @@ int CTLOG_STORE_load_file(CTLOG_STORE *store, const char *file);
  * Loads the default CT log list into a |store|.
  * See internal/cryptlib.h for the environment variable and file path that are
  * consulted to find the default file.
- * Returns 1 if loading is successful, or a non-positive integer otherwise.
+ * Returns 1 if loading is successful, or 0 otherwise.
  */
 int CTLOG_STORE_load_default_file(CTLOG_STORE *store);