Commit 0dfd6cf9 authored by Rob Percival's avatar Rob Percival Committed by Rich Salz
Browse files

Addresses review comments from richsalz



Reviewed-by: default avatarBen Laurie <ben@openssl.org>
Reviewed-by: default avatarRich Salz <rsalz@openssl.org>
parent e9fd74ac
Loading
Loading
Loading
Loading
+7 −22
Original line number Diff line number Diff line
@@ -77,18 +77,12 @@
 * A new string will be malloc'd and assigned to |out|. This will be owned by
 * the caller. Do not provide a pre-allocated string in |out|.
 */
static int CT_base64_decode(const char *in, unsigned char **out)
static int ct_base64_decode(const char *in, unsigned char **out)
{
    size_t inlen;
    size_t inlen = strlen(in);
    int outlen;
    unsigned char *outbuf = NULL;

    if (in == NULL || out == NULL) {
        CTerr(CT_F_CT_BASE64_DECODE, ERR_R_PASSED_NULL_PARAMETER);
        goto err;
    }

    inlen = strlen(in);
    if (inlen == 0) {
        *out = NULL;
        return 0;
@@ -119,18 +113,10 @@ SCT *SCT_new_from_base64(unsigned char version, const char *logid_base64,
                         const char *extensions_base64,
                         const char *signature_base64)
{
    SCT *sct;
    SCT *sct = SCT_new();
    unsigned char *dec = NULL;
    int declen;

    if (logid_base64 == NULL ||
        extensions_base64 == NULL ||
        signature_base64 == NULL) {
        CTerr(CT_F_SCT_NEW_FROM_BASE64, ERR_R_PASSED_NULL_PARAMETER);
        return NULL;
    }

    sct = SCT_new();
    if (sct == NULL) {
        CTerr(CT_F_SCT_NEW_FROM_BASE64, ERR_R_MALLOC_FAILURE);
        return NULL;
@@ -145,7 +131,7 @@ SCT *SCT_new_from_base64(unsigned char version, const char *logid_base64,
        goto err;
    }

    declen = CT_base64_decode(logid_base64, &dec);
    declen = ct_base64_decode(logid_base64, &dec);
    if (declen < 0) {
        CTerr(CT_F_SCT_NEW_FROM_BASE64, X509_R_BASE64_DECODE_ERROR);
        goto err;
@@ -154,7 +140,7 @@ SCT *SCT_new_from_base64(unsigned char version, const char *logid_base64,
        goto err;
    dec = NULL;

    declen = CT_base64_decode(extensions_base64, &dec);
    declen = ct_base64_decode(extensions_base64, &dec);
    if (declen < 0) {
        CTerr(CT_F_SCT_NEW_FROM_BASE64, X509_R_BASE64_DECODE_ERROR);
        goto err;
@@ -162,7 +148,7 @@ SCT *SCT_new_from_base64(unsigned char version, const char *logid_base64,
    SCT_set0_extensions(sct, dec, declen);
    dec = NULL;

    declen = CT_base64_decode(signature_base64, &dec);
    declen = ct_base64_decode(signature_base64, &dec);
    if (declen < 0) {
        CTerr(CT_F_SCT_NEW_FROM_BASE64, X509_R_BASE64_DECODE_ERROR);
        goto err;
@@ -188,12 +174,11 @@ SCT *SCT_new_from_base64(unsigned char version, const char *logid_base64,
CTLOG *CTLOG_new_from_base64(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;
    int pkey_der_len;
    EVP_PKEY *pkey = NULL;
    CTLOG *log = NULL;

    pkey_der_len = CT_base64_decode(pkey_base64, &pkey_der);
    if (pkey_der_len <= 0) {
        CTerr(CT_F_CTLOG_NEW_FROM_BASE64, CT_R_LOG_CONF_INVALID_KEY);
        return NULL;
+40 −45
Original line number Diff line number Diff line
@@ -53,6 +53,9 @@
 *
 */

#include <stdlib.h>
#include <string.h>

#include <openssl/conf.h>
#include <openssl/ct.h>
#include <openssl/err.h>
@@ -88,48 +91,47 @@ typedef struct ctlog_store_load_ctx_st {
 * Creates an empty context for loading a CT log store.
 * It should be populated before use.
 */
static CTLOG_STORE_LOAD_CTX *CTLOG_STORE_LOAD_CTX_new();
static CTLOG_STORE_LOAD_CTX *ctlog_store_load_ctx_new();

/*
 * Deletes a CT log store load context.
 * Does not delete any of the fields.
 */
static void CTLOG_STORE_LOAD_CTX_free(CTLOG_STORE_LOAD_CTX* ctx);
static void ctlog_store_load_ctx_free(CTLOG_STORE_LOAD_CTX* ctx);

static CTLOG_STORE_LOAD_CTX *CTLOG_STORE_LOAD_CTX_new()
static CTLOG_STORE_LOAD_CTX *ctlog_store_load_ctx_new()
{
    CTLOG_STORE_LOAD_CTX *ctx = OPENSSL_zalloc(sizeof(CTLOG_STORE_LOAD_CTX));
    CTLOG_STORE_LOAD_CTX *ctx = OPENSSL_zalloc(sizeof(*ctx));

    if (ctx == NULL) {
        CTerr(CT_F_CTLOG_STORE_LOAD_CTX_NEW, ERR_R_MALLOC_FAILURE);
        goto err;
    }

    return ctx;

err:
    CTLOG_STORE_LOAD_CTX_free(ctx);
    ctlog_store_load_ctx_free(ctx);
    return NULL;
}

static void CTLOG_STORE_LOAD_CTX_free(CTLOG_STORE_LOAD_CTX* ctx)
static void ctlog_store_load_ctx_free(CTLOG_STORE_LOAD_CTX* ctx)
{
    if (ctx == NULL)
        return;

    OPENSSL_free(ctx);
}

/* Converts a log's public key into a SHA256 log ID */
static int CT_v1_log_id_from_pkey(EVP_PKEY *pkey,
static int ct_v1_log_id_from_pkey(EVP_PKEY *pkey,
                                  unsigned char log_id[CT_V1_HASHLEN])
{
    int ret = 0;
    unsigned char *pkey_der = NULL;
    int pkey_der_len = i2d_PUBKEY(pkey, &pkey_der);

    if (pkey_der_len <= 0) {
        CTerr(CT_F_CT_V1_LOG_ID_FROM_PKEY, CT_R_LOG_KEY_INVALID);
        goto err;
    }

    SHA256(pkey_der, pkey_der_len, log_id);
    ret = 1;
err:
@@ -139,12 +141,15 @@ err:

CTLOG_STORE *CTLOG_STORE_new(void)
{
    CTLOG_STORE *ret = OPENSSL_malloc(sizeof(CTLOG_STORE));
    CTLOG_STORE *ret = OPENSSL_zalloc(sizeof(*ret));

    if (ret == NULL)
        goto err;

    ret->logs = sk_CTLOG_new_null();
    if (ret->logs == NULL)
        goto err;

    return ret;
err:
    CTLOG_STORE_free(ret);
@@ -159,17 +164,11 @@ void CTLOG_STORE_free(CTLOG_STORE *store)
    }
}

static CTLOG *CTLOG_new_from_conf(const CONF *conf, const char *section)
static CTLOG *ctlog_new_from_conf(const CONF *conf, const char *section)
{
    CTLOG *ret = NULL;
    char *description;
    char *description = NCONF_get_string(conf, section, "description");
    char *pkey_base64;
    if (conf == NULL || section == NULL) {
        CTerr(CT_F_CTLOG_NEW_FROM_CONF, ERR_R_PASSED_NULL_PARAMETER);
        goto end;
    }

    description = NCONF_get_string(conf, section, "description");

    if (description == NULL) {
        CTerr(CT_F_CTLOG_NEW_FROM_CONF, CT_R_LOG_CONF_MISSING_DESCRIPTION);
@@ -177,7 +176,6 @@ static CTLOG *CTLOG_new_from_conf(const CONF *conf, const char *section)
    }

    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;
@@ -195,20 +193,22 @@ end:

int CTLOG_STORE_load_default_file(CTLOG_STORE *store)
{
    char *fpath = (char *)getenv(CTLOG_FILE_EVP);
    const char *fpath = getenv(CTLOG_FILE_EVP);

    if (fpath == NULL)
      fpath = CTLOG_FILE;

    return CTLOG_STORE_load_file(store, fpath);
}

static int CTLOG_STORE_load_log(const char *log_name, int log_name_len, void *arg)
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;

    /* log_name may not be null-terminated, so fix that before using it */
    char *tmp = OPENSSL_strndup(log_name, log_name_len);
    ct_log = CTLOG_new_from_conf(load_ctx->conf, tmp);

    ct_log = ctlog_new_from_conf(load_ctx->conf, tmp);
    OPENSSL_free(tmp);
    if (ct_log == NULL)
        return 0;
@@ -221,7 +221,8 @@ int CTLOG_STORE_load_file(CTLOG_STORE *store, const char *file)
{
    int ret = -1;
    char *enabled_logs;
    CTLOG_STORE_LOAD_CTX* load_ctx = CTLOG_STORE_LOAD_CTX_new();
    CTLOG_STORE_LOAD_CTX* load_ctx = ctlog_store_load_ctx_new();

    load_ctx->log_store = store;
    load_ctx->conf = NCONF_new(NULL);
    if (load_ctx->conf == NULL)
@@ -234,11 +235,11 @@ 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);
    CONF_parse_list(enabled_logs, ',', 1, ctlog_store_load_log, load_ctx);

end:
    NCONF_free(load_ctx->conf);
    CTLOG_STORE_LOAD_CTX_free(load_ctx);
    ctlog_store_load_ctx_free(load_ctx);
    return ret;
}

@@ -249,20 +250,19 @@ end:
 */
CTLOG *CTLOG_new(EVP_PKEY *public_key, const char *name)
{
    CTLOG *ret = NULL;
    if (public_key == NULL || name == NULL) {
        CTerr(CT_F_CTLOG_NEW, ERR_R_PASSED_NULL_PARAMETER);
        goto err;
    }
    ret = CTLOG_new_null();
    CTLOG *ret = CTLOG_new_null();

    if (ret == NULL)
        goto err;

    ret->name = OPENSSL_strdup(name);
    if (ret->name == NULL)
        goto err;

    ret->public_key = public_key;
    if (CT_v1_log_id_from_pkey(public_key, ret->log_id) != 1)
    if (ct_v1_log_id_from_pkey(public_key, ret->log_id) != 1)
        goto err;

    return ret;
err:
    CTLOG_free(ret);
@@ -271,9 +271,11 @@ err:

CTLOG *CTLOG_new_null(void)
{
    CTLOG *ret = OPENSSL_zalloc(sizeof(CTLOG));
    CTLOG *ret = OPENSSL_zalloc(sizeof(*ret));

    if (ret == NULL)
        CTerr(CT_F_CTLOG_NEW_NULL, ERR_R_MALLOC_FAILURE);

    return ret;
}

@@ -282,11 +284,7 @@ void CTLOG_free(CTLOG *log)
{
    if (log != NULL) {
        OPENSSL_free(log->name);
        log->name = NULL;

        EVP_PKEY_free(log->public_key);
        log->public_key = NULL;

        OPENSSL_free(log);
    }
}
@@ -316,15 +314,12 @@ CTLOG *CTLOG_STORE_get0_log_by_id(const CTLOG_STORE *store,
                                  size_t log_id_len)
{
    int i;
    if (store == NULL) {
        CTerr(CT_F_CTLOG_STORE_GET0_LOG_BY_ID, ERR_R_PASSED_NULL_PARAMETER);
        goto end;
    }

    for (i = 0; i < sk_CTLOG_num(store->logs); ++i) {
        CTLOG *log = sk_CTLOG_value(store->logs, i);
        if (memcmp(log->log_id, log_id, log_id_len) == 0)
            return log;
    }
end:

    return NULL;
}
+0 −24
Original line number Diff line number Diff line
@@ -70,30 +70,6 @@

#include "ct_locl.h"

#define n2s(c,s)        ((s=(((unsigned int)((c)[0]))<< 8)| \
                            (((unsigned int)((c)[1]))    )),c+=2)

#define s2n(s,c)        ((c[0]=(unsigned char)(((s)>> 8)&0xff), \
                          c[1]=(unsigned char)(((s)    )&0xff)),c+=2)

#define n2l8(c,l)       (l =((uint64_t)(*((c)++)))<<56, \
                         l|=((uint64_t)(*((c)++)))<<48, \
                         l|=((uint64_t)(*((c)++)))<<40, \
                         l|=((uint64_t)(*((c)++)))<<32, \
                         l|=((uint64_t)(*((c)++)))<<24, \
                         l|=((uint64_t)(*((c)++)))<<16, \
                         l|=((uint64_t)(*((c)++)))<< 8, \
                         l|=((uint64_t)(*((c)++))))

#define l2n8(l,c)       (*((c)++)=(unsigned char)(((l)>>56)&0xff), \
                         *((c)++)=(unsigned char)(((l)>>48)&0xff), \
                         *((c)++)=(unsigned char)(((l)>>40)&0xff), \
                         *((c)++)=(unsigned char)(((l)>>32)&0xff), \
                         *((c)++)=(unsigned char)(((l)>>24)&0xff), \
                         *((c)++)=(unsigned char)(((l)>>16)&0xff), \
                         *((c)++)=(unsigned char)(((l)>> 8)&0xff), \
                         *((c)++)=(unsigned char)(((l)    )&0xff))

int o2i_SCT_signature(SCT *sct, const unsigned char **in, size_t len)
{
    size_t siglen;
+2 −3
Original line number Diff line number Diff line
@@ -70,7 +70,7 @@

SCT *SCT_new(void)
{
    SCT *sct = OPENSSL_zalloc(sizeof(SCT));
    SCT *sct = OPENSSL_zalloc(sizeof(*sct));

    if (sct == NULL) {
        CTerr(CT_F_SCT_NEW, ERR_R_MALLOC_FAILURE);
@@ -335,8 +335,7 @@ CTLOG *SCT_get0_log(const SCT *sct)

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);
    sct->log = CTLOG_STORE_get0_log_by_id(ct_logs, sct->log_id, sct->log_id_len);

    return sct->log != NULL;
}
+71 −40
Original line number Diff line number Diff line
@@ -71,9 +71,11 @@

SCT_CTX *SCT_CTX_new(void)
{
    SCT_CTX *sctx = OPENSSL_zalloc(sizeof(SCT_CTX));
    SCT_CTX *sctx = OPENSSL_zalloc(sizeof(*sctx));

    if (sctx == NULL)
        CTerr(CT_F_SCT_CTX_NEW, ERR_R_MALLOC_FAILURE);

    return sctx;
}

@@ -89,28 +91,44 @@ void SCT_CTX_free(SCT_CTX *sctx)
    OPENSSL_free(sctx);
}

/* retrieve extension index checking for duplicates */
static int sct_get_ext(X509 *cert, int nid)
/*
 * Finds the index of the first extension with the given NID in cert.
 * If there is more than one extension with that NID, *is_duplicated is set to
 * 1, otherwise 0 (unless it is NULL).
 */
static int ct_x509_get_ext(X509 *cert, int nid, int *is_duplicated)
{
    int rv = X509_get_ext_by_NID(cert, nid, -1);
    if (rv >= 0 && X509_get_ext_by_NID(cert, nid, rv) >= 0)
        return -2;
    return rv;
    int ret = X509_get_ext_by_NID(cert, nid, -1);

    if (is_duplicated != NULL)
        *is_duplicated = ret >= 0 && X509_get_ext_by_NID(cert, nid, ret) >= 0;

    return ret;
}

/*
 * modify certificate by deleting extensions, copying issuer
 * and AKID if necessary.
 * Modifies a certificate by deleting extensions and copying the issuer and
 * AKID from the presigner certificate, if necessary.
 * Returns 1 on success, 0 otherwise.
 */
static int sct_cert_fixup(X509 *cert, X509 *presigner)
static int ct_x509_cert_fixup(X509 *cert, X509 *presigner)
{
    int preidx, certidx;
    int pre_akid_ext_is_dup, cert_akid_ext_is_dup;

    if (presigner == NULL)
        return 1;
    preidx = sct_get_ext(presigner, NID_authority_key_identifier);
    certidx = sct_get_ext(cert, NID_authority_key_identifier);
    /* Invalid certificate if duplicate */
    if (preidx == -2 || certidx == -2)

    preidx = ct_x509_get_ext(presigner, NID_authority_key_identifier,
                             &pre_akid_ext_is_dup);
    certidx = ct_x509_get_ext(cert, NID_authority_key_identifier,
                              &cert_akid_ext_is_dup);

    /* An error occurred whilst searching for the extension */
    if (preidx < -1 || certidx < -1)
        return 0;
    /* Invalid certificate if they contain duplicate extensions */
    if (pre_akid_ext_is_dup || cert_akid_ext_is_dup)
        return 0;
    /* AKID must be present in both certificate or absent in both */
    if (preidx >= 0 && certidx == -1)
@@ -125,11 +143,13 @@ static int sct_cert_fixup(X509 *cert, X509 *presigner)
        X509_EXTENSION *preext = X509_get_ext(presigner, preidx);
        X509_EXTENSION *certext = X509_get_ext(cert, certidx);
        ASN1_OCTET_STRING *preextdata;

        /* Should never happen */
        if (preext == NULL || certext == NULL)
            return 0;
        preextdata = X509_EXTENSION_get_data(preext);
        if (preextdata == NULL || !X509_EXTENSION_set_data(certext, preextdata))
        if (preextdata == NULL ||
            !X509_EXTENSION_set_data(certext, preextdata))
            return 0;
    }
    return 1;
@@ -140,42 +160,51 @@ int SCT_CTX_set1_cert(SCT_CTX *sctx, X509 *cert, X509 *presigner)
    unsigned char *certder = NULL, *preder = NULL;
    X509 *pretmp = NULL;
    int certderlen = 0, prederlen = 0;
    int idx = -1, idxp = -1;
    idxp = sct_get_ext(cert, NID_ct_precert_poison);
    int idx = -1;
    int poison_ext_is_dup, sct_ext_is_dup;
    int poison_idx = ct_x509_get_ext(cert, NID_ct_precert_poison, &poison_ext_is_dup);

    /* Duplicate poison */
    if (idxp == -2)
    if (poison_ext_is_dup)
        goto err;
    /* If no poison store encoding */
    if (idxp == -1) {
        /* If presigner must have poison */
        if (presigner)

    /* If no poison extension, store encoding */
    if (poison_idx == -1) {
        /* presigner must have poison */
        if (presigner != NULL)
            goto err;

        certderlen = i2d_X509(cert, &certder);
        if (certderlen < 0)
            goto err;
    }

    /* See if have precert scts extension */
    idx = X509_get_ext_by_NID(cert, NID_ct_precert_scts, -1);
    idx = ct_x509_get_ext(cert, NID_ct_precert_scts, &sct_ext_is_dup);
    /* Duplicate scts */
    if (idx == -2)
    if (sct_ext_is_dup)
        goto err;

    if (idx >= 0) {
        /* Can't have both poison and scts */
        if (idxp >= 0)
        if (poison_idx >= 0)
            goto err;
    } else
        idx = idxp;
    } else {
        idx = poison_idx;
    }

    if (idx >= 0) {
        X509_EXTENSION *ext;
        /*
         * Take a copy of certificate so we don't modify passed version
         */

        /* Take a copy of certificate so we don't modify passed version */
        pretmp = X509_dup(cert);
        if (pretmp == NULL)
            goto err;

        ext = X509_delete_ext(pretmp, idx);
        X509_EXTENSION_free(ext);
        if (!sct_cert_fixup(pretmp, presigner))

        if (!ct_x509_cert_fixup(pretmp, presigner))
            goto err;

        prederlen = i2d_re_X509_tbs(pretmp, &preder);
@@ -194,7 +223,6 @@ int SCT_CTX_set1_cert(SCT_CTX *sctx, X509 *cert, X509 *presigner)
    sctx->prederlen = prederlen;

    return 1;

err:
    OPENSSL_free(certder);
    OPENSSL_free(preder);
@@ -202,15 +230,14 @@ int SCT_CTX_set1_cert(SCT_CTX *sctx, X509 *cert, X509 *presigner)
    return 0;
}

static int CT_public_key_hash(X509_PUBKEY *pkey, unsigned char **hash,
static int ct_public_key_hash(X509_PUBKEY *pkey, unsigned char **hash,
                              size_t *hash_len)
{
    int ret = -1;
    unsigned char *md = NULL, *der = NULL;
    int der_len;
    unsigned int md_len;
    if (pkey == NULL)
        goto err;

    /* Reuse buffer if possible */
    if (*hash != NULL && *hash_len >= SHA256_DIGEST_LENGTH) {
        md = *hash;
@@ -224,13 +251,16 @@ static int CT_public_key_hash(X509_PUBKEY *pkey, unsigned char **hash,
    der_len = i2d_X509_PUBKEY(pkey, &der);
    if (der_len <= 0)
        goto err;

    if (!EVP_Digest(der, der_len, md, &md_len, EVP_sha256(), NULL))
        goto err;

    if (md != *hash) {
        OPENSSL_free(*hash);
        *hash = md;
        *hash_len = SHA256_DIGEST_LENGTH;
    }

    md = NULL;
    ret = 1;
 err:
@@ -241,22 +271,23 @@ static int CT_public_key_hash(X509_PUBKEY *pkey, unsigned char **hash,

int SCT_CTX_set1_issuer(SCT_CTX *sctx, const X509 *issuer)
{
    return CT_public_key_hash(X509_get_X509_PUBKEY(issuer), &sctx->ihash,
    return ct_public_key_hash(X509_get_X509_PUBKEY(issuer), &sctx->ihash,
                              &sctx->ihashlen);
}

int SCT_CTX_set1_issuer_pubkey(SCT_CTX *sctx, X509_PUBKEY *pubkey)
{
    return CT_public_key_hash(pubkey, &sctx->ihash, &sctx->ihashlen);
    return ct_public_key_hash(pubkey, &sctx->ihash, &sctx->ihashlen);
}

int SCT_CTX_set1_pubkey(SCT_CTX *sctx, X509_PUBKEY *pubkey)
{
    EVP_PKEY *pkey = X509_PUBKEY_get(pubkey);

    if (pkey == NULL)
        return 0;

    if (!CT_public_key_hash(pubkey, &sctx->pkeyhash, &sctx->pkeyhashlen)) {
    if (!ct_public_key_hash(pubkey, &sctx->pkeyhash, &sctx->pkeyhashlen)) {
        EVP_PKEY_free(pkey);
        return 0;
    }
Loading