From 4aed8756d86e2b934e83d916e57bee91c83c4b28 Mon Sep 17 00:00:00 2001 From: FdaSilvaYY Date: Mon, 27 Jun 2016 22:42:25 +0200 Subject: Improve some error management code in CT Separate invalid input case from any internal (malloc) failure Reviewed-by: Rich Salz Reviewed-by: Matt Caswell --- crypto/ct/ct_b64.c | 25 +++++++++++++++++-------- crypto/ct/ct_log.c | 34 ++++++++++++++++------------------ 2 files changed, 33 insertions(+), 26 deletions(-) (limited to 'crypto') diff --git a/crypto/ct/ct_b64.c b/crypto/ct/ct_b64.c index d6279d2298..9cf7c51be3 100644 --- a/crypto/ct/ct_b64.c +++ b/crypto/ct/ct_b64.c @@ -115,17 +115,26 @@ SCT *SCT_new_from_base64(unsigned char version, const char *logid_base64, return NULL; } -CTLOG *CTLOG_new_from_base64(const char *pkey_base64, const char *name) +/* + * This methods returns: 1 on Success, + * 0 on decoding failure, + * -1 on internal (malloc) failure, or invalid parameter if any. + */ +int CTLOG_new_from_base64(CTLOG **ct_log, const char *pkey_base64, const char *name) { unsigned char *pkey_der = NULL; int pkey_der_len = ct_base64_decode(pkey_base64, &pkey_der); const unsigned char *p; EVP_PKEY *pkey = NULL; - CTLOG *log = NULL; + + if (ct_log == NULL) { + CTerr(CT_F_CTLOG_NEW_FROM_BASE64, ERR_R_PASSED_INVALID_ARGUMENT); + return 0; + } if (pkey_der_len <= 0) { CTerr(CT_F_CTLOG_NEW_FROM_BASE64, CT_R_LOG_CONF_INVALID_KEY); - return NULL; + return 0; } p = pkey_der; @@ -133,14 +142,14 @@ CTLOG *CTLOG_new_from_base64(const char *pkey_base64, const char *name) OPENSSL_free(pkey_der); if (pkey == NULL) { CTerr(CT_F_CTLOG_NEW_FROM_BASE64, CT_R_LOG_CONF_INVALID_KEY); - return NULL; + return 0; } - log = CTLOG_new(pkey, name); - if (log == NULL) { + *ct_log = CTLOG_new(pkey, name); + if (*ct_log == NULL) { EVP_PKEY_free(pkey); - return NULL; + return -1; } - return log; + return 1; } diff --git a/crypto/ct/ct_log.c b/crypto/ct/ct_log.c index b01bb5693e..1874d9121c 100644 --- a/crypto/ct/ct_log.c +++ b/crypto/ct/ct_log.c @@ -116,31 +116,23 @@ void CTLOG_STORE_free(CTLOG_STORE *store) } } -static CTLOG *ctlog_new_from_conf(const CONF *conf, const char *section) +static int ctlog_new_from_conf(CTLOG **ct_log, const CONF *conf, const char *section) { - CTLOG *ret = NULL; const char *description = NCONF_get_string(conf, section, "description"); char *pkey_base64; if (description == NULL) { CTerr(CT_F_CTLOG_NEW_FROM_CONF, CT_R_LOG_CONF_MISSING_DESCRIPTION); - goto end; + return 0; } pkey_base64 = NCONF_get_string(conf, section, "key"); if (pkey_base64 == NULL) { CTerr(CT_F_CTLOG_NEW_FROM_CONF, CT_R_LOG_CONF_MISSING_KEY); - goto end; - } - - ret = CTLOG_new_from_base64(pkey_base64, description); - if (ret == NULL) { - CTerr(CT_F_CTLOG_NEW_FROM_CONF, CT_R_LOG_CONF_INVALID); - goto end; + return 0; } -end: - return ret; + return CTLOG_new_from_base64(ct_log, pkey_base64, description); } int CTLOG_STORE_load_default_file(CTLOG_STORE *store) @@ -154,9 +146,10 @@ int CTLOG_STORE_load_default_file(CTLOG_STORE *store) } /* - * 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. + * Called by CONF_parse_list, which stops if this returns <= 0, + * Otherwise, one bad log entry would stop loading of any of + * the following log entries. + * It may stop parsing and returns -1 on any internal (malloc) error. */ static int ctlog_store_load_log(const char *log_name, int log_name_len, void *arg) @@ -165,6 +158,7 @@ static int ctlog_store_load_log(const char *log_name, int log_name_len, CTLOG *ct_log = NULL; /* log_name may not be null-terminated, so fix that before using it */ char *tmp; + int ret = 0; /* log_name will be NULL for empty list entries */ if (log_name == NULL) @@ -174,10 +168,14 @@ static int ctlog_store_load_log(const char *log_name, int log_name_len, if (tmp == NULL) goto mem_err; - ct_log = ctlog_new_from_conf(load_ctx->conf, tmp); + ret = ctlog_new_from_conf(&ct_log, load_ctx->conf, tmp); OPENSSL_free(tmp); - if (ct_log == NULL) { - /* TODO: split invalid input case from internal failure */ + + if (ret < 0) { + /* Propagate any internal error */ + return ret; + } + if (ret == 0) { /* If we can't load this log, record that fact and skip it */ ++load_ctx->invalid_log_entries; return 1; -- cgit v1.2.3