Commit 4aed8756 authored by FdaSilvaYY's avatar FdaSilvaYY Committed by Matt Caswell
Browse files

Improve some error management code in CT 



Separate invalid input case from any internal (malloc) failure

Reviewed-by: default avatarRich Salz <rsalz@openssl.org>
Reviewed-by: default avatarMatt Caswell <matt@openssl.org>
parent e57036f2

crypto/ct/ct_b64.c

+17 −8
Original line number Diff line number Diff line
@@ -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;

}

crypto/ct/ct_log.c

+16 −18
Original line number Diff line number Diff line
@@ -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;

include/openssl/ct.h

+3 −2
Original line number Diff line number Diff line
@@ -419,10 +419,11 @@ CTLOG *CTLOG_new(EVP_PKEY *public_key, const char *name); 
CTLOG *CTLOG_new_null(void);



/*

 * Creates a new CT log instance with the given base64 public_key and |name|.

 * Creates a new CT |ct_log| instance with the given base64 public_key and |name|.

 * Should be deleted by the caller using CTLOG_free when no longer needed.

 */

CTLOG *CTLOG_new_from_base64(const char *pkey_base64, const char *name);

int CTLOG_new_from_base64(CTLOG ** ct_log,

                          const char *pkey_base64, const char *name);



/*

 * Deletes a CT log instance and its fields.