From 5da65ef23ce30285e87652469298ce6513560032 Mon Sep 17 00:00:00 2001 From: Rob Percival Date: Fri, 4 Mar 2016 19:51:43 +0000 Subject: [PATCH] Extensive application of __owur to CT functions that return a boolean MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Also improves some documentation of those functions. Reviewed-by: Emilia Käsper Reviewed-by: Rich Salz --- apps/apps.h | 7 +++--- crypto/ct/ct_locl.h | 41 +++++++++++++++++++++++-------- crypto/ct/ct_sct_ctx.c | 11 ++++----- include/openssl/ct.h | 56 ++++++++++++++++++++++-------------------- test/ct_test.c | 36 +++++++++++++++++++++------ 5 files changed, 98 insertions(+), 53 deletions(-) diff --git a/apps/apps.h b/apps/apps.h index ebf696b81b..0fcac07351 100644 --- a/apps/apps.h +++ b/apps/apps.h @@ -487,9 +487,10 @@ int load_crls(const char *file, STACK_OF(X509_CRL) **crls, int format, const char *pass, const char *cert_descrip); X509_STORE *setup_verify(char *CAfile, char *CApath, int noCAfile, int noCApath); -int ctx_set_verify_locations(SSL_CTX *ctx, const char *CAfile, - const char *CApath, int noCAfile, int noCApath); -int ctx_set_ctlog_list_file(SSL_CTX *ctx, const char *path); +__owur int ctx_set_verify_locations(SSL_CTX *ctx, const char *CAfile, + const char *CApath, int noCAfile, + int noCApath); +__owur int ctx_set_ctlog_list_file(SSL_CTX *ctx, const char *path); # ifdef OPENSSL_NO_ENGINE # define setup_engine(engine, debug) NULL diff --git a/crypto/ct/ct_locl.h b/crypto/ct/ct_locl.h index b82fabc4f0..eb1d377b4a 100644 --- a/crypto/ct/ct_locl.h +++ b/crypto/ct/ct_locl.h @@ -167,20 +167,41 @@ SCT_CTX *SCT_CTX_new(void); */ void SCT_CTX_free(SCT_CTX *sctx); -/* Sets the certificate that the SCT is related to */ -int SCT_CTX_set1_cert(SCT_CTX *sctx, X509 *cert, X509 *presigner); -/* Sets the issuer of the certificate that the SCT is related to */ -int SCT_CTX_set1_issuer(SCT_CTX *sctx, const X509 *issuer); -/* Sets the public key of the issuer of the certificate that the SCT relates to */ -int SCT_CTX_set1_issuer_pubkey(SCT_CTX *sctx, X509_PUBKEY *pubkey); -/* Sets the public key of the CT log that the SCT is from */ -int SCT_CTX_set1_pubkey(SCT_CTX *sctx, X509_PUBKEY *pubkey); +/* + * Sets the certificate that the SCT is being verified against. + * This will fail if the certificate is invalid. + * Returns 1 on success, 0 on failure. + */ +__owur int SCT_CTX_set1_cert(SCT_CTX *sctx, X509 *cert, X509 *presigner); + +/* + * Sets the issuer of the certificate that the SCT is being verified against. + * This is just a convenience method to save extracting the public key and + * calling SCT_CTX_set1_issuer_pubkey(). + * Issuer must not be NULL. + * Returns 1 on success, 0 on failure. + */ +__owur int SCT_CTX_set1_issuer(SCT_CTX *sctx, const X509 *issuer); + +/* + * Sets the public key of the issuer of the certificate that the SCT is being + * verified against. + * The public key must not be NULL. + * Returns 1 on success, 0 on failure. + */ +__owur int SCT_CTX_set1_issuer_pubkey(SCT_CTX *sctx, X509_PUBKEY *pubkey); + +/* + * Sets the public key of the CT log that the SCT is from. + * Returns 1 on success, 0 on failure. + */ +__owur int SCT_CTX_set1_pubkey(SCT_CTX *sctx, X509_PUBKEY *pubkey); /* * Does this SCT have the minimum fields populated to be usuable? * Returns 1 if so, 0 otherwise. */ -int SCT_is_complete(const SCT *sct); +__owur int SCT_is_complete(const SCT *sct); /* * Does this SCT have the signature-related fields populated? @@ -188,6 +209,6 @@ int SCT_is_complete(const SCT *sct); * This checks that the signature and hash algorithms are set to supported * values and that the signature field is set. */ -int SCT_signature_is_complete(const SCT *sct); +__owur int SCT_signature_is_complete(const SCT *sct); diff --git a/crypto/ct/ct_sct_ctx.c b/crypto/ct/ct_sct_ctx.c index 7c50c91d69..89051d2f7c 100644 --- a/crypto/ct/ct_sct_ctx.c +++ b/crypto/ct/ct_sct_ctx.c @@ -111,7 +111,7 @@ static int ct_x509_get_ext(X509 *cert, int nid, int *is_duplicated) * AKID from the presigner certificate, if necessary. * Returns 1 on success, 0 otherwise. */ -static int ct_x509_cert_fixup(X509 *cert, X509 *presigner) +__owur static int ct_x509_cert_fixup(X509 *cert, X509 *presigner) { int preidx, certidx; int pre_akid_ext_is_dup, cert_akid_ext_is_dup; @@ -230,10 +230,10 @@ err: return 0; } -static int ct_public_key_hash(X509_PUBKEY *pkey, unsigned char **hash, - size_t *hash_len) +__owur static int ct_public_key_hash(X509_PUBKEY *pkey, unsigned char **hash, + size_t *hash_len) { - int ret = -1; + int ret = 0; unsigned char *md = NULL, *der = NULL; int der_len; unsigned int md_len; @@ -271,8 +271,7 @@ 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, - &sctx->ihashlen); + return SCT_CTX_set1_issuer_pubkey(sctx, X509_get_X509_PUBKEY(issuer)); } int SCT_CTX_set1_issuer_pubkey(SCT_CTX *sctx, X509_PUBKEY *pubkey) diff --git a/include/openssl/ct.h b/include/openssl/ct.h index 7a19053018..1225242177 100644 --- a/include/openssl/ct.h +++ b/include/openssl/ct.h @@ -187,7 +187,7 @@ sct_version_t SCT_get_version(const SCT *sct); * Set the version of an SCT. * Returns 1 on success, 0 if the version is unrecognized. */ -int SCT_set_version(SCT *sct, sct_version_t version); +__owur int SCT_set_version(SCT *sct, sct_version_t version); /* * Returns the log entry type of the SCT. @@ -196,9 +196,9 @@ ct_log_entry_type_t SCT_get_log_entry_type(const SCT *sct); /* * Set the log entry type of an SCT. - * Returns 1 on success. + * Returns 1 on success, 0 otherwise. */ -int SCT_set_log_entry_type(SCT *sct, ct_log_entry_type_t entry_type); +__owur int SCT_set_log_entry_type(SCT *sct, ct_log_entry_type_t entry_type); /* * Gets the ID of the log that an SCT came from. @@ -210,16 +210,17 @@ size_t SCT_get0_log_id(const SCT *sct, unsigned char **log_id); /* * Set the log ID of an SCT to point directly to the *log_id specified. * The SCT takes ownership of the specified pointer. - * Returns 1 on success. + * Returns 1 on success, 0 otherwise. */ -int SCT_set0_log_id(SCT *sct, unsigned char *log_id, size_t log_id_len); +__owur int SCT_set0_log_id(SCT *sct, unsigned char *log_id, size_t log_id_len); /* * Set the log ID of an SCT. * This makes a copy of the log_id. - * Returns 1 on success. + * Returns 1 on success, 0 otherwise. */ -int SCT_set1_log_id(SCT *sct, const unsigned char *log_id, size_t log_id_len); +__owur int SCT_set1_log_id(SCT *sct, const unsigned char *log_id, + size_t log_id_len); /* * Gets the name of the log that an SCT came from. @@ -249,9 +250,9 @@ int SCT_get_signature_nid(const SCT *sct); * Set the signature type of an SCT * For CT v1, this should be either NID_sha256WithRSAEncryption or * NID_ecdsa_with_SHA256. - * Returns 1 on success. + * Returns 1 on success, 0 otherwise. */ -int SCT_set_signature_nid(SCT *sct, int nid); +__owur int SCT_set_signature_nid(SCT *sct, int nid); /* * Set *ext to point to the extension data for the SCT. ext must not be NULL. @@ -269,9 +270,10 @@ void SCT_set0_extensions(SCT *sct, unsigned char *ext, size_t ext_len); /* * Set the extensions of an SCT. * This takes a copy of the ext. - * Returns 1 on success. + * Returns 1 on success, 0 otherwise. */ -int SCT_set1_extensions(SCT *sct, const unsigned char *ext, size_t ext_len); +__owur int SCT_set1_extensions(SCT *sct, const unsigned char *ext, + size_t ext_len); /* * Set *sig to point to the signature for the SCT. sig must not be NULL. @@ -288,9 +290,10 @@ void SCT_set0_signature(SCT *sct, unsigned char *sig, size_t sig_len); /* * Set the signature of an SCT to be a copy of the *sig specified. - * Returns 1 on success. + * Returns 1 on success, 0 otherwise. */ -int SCT_set1_signature(SCT *sct, const unsigned char *sig, size_t sig_len); +__owur int SCT_set1_signature(SCT *sct, const unsigned char *sig, + size_t sig_len); /* * The origin of this SCT, e.g. TLS extension, OCSP response, etc. @@ -301,13 +304,13 @@ sct_source_t SCT_get_source(const SCT *sct); * Set the origin of this SCT, e.g. TLS extension, OCSP response, etc. * Returns 1 on success, 0 otherwise. */ -int SCT_set_source(SCT *sct, sct_source_t source); +__owur int SCT_set_source(SCT *sct, sct_source_t source); /* * Sets the source of all of the SCTs to the same value. * Returns the number of SCTs whose source was set successfully. */ -int SCT_LIST_set_source(const STACK_OF(SCT) *scts, sct_source_t source); +__owur int SCT_LIST_set_source(const STACK_OF(SCT) *scts, sct_source_t source); /* * Gets information about the log the SCT came from, if set. @@ -347,14 +350,14 @@ void SCT_LIST_print(const STACK_OF(SCT) *sct_list, BIO *out, int indent, * Returns 1 if the SCT verifies successfully, 0 if it cannot be verified and a * negative integer if an error occurs. */ -int SCT_verify(const SCT_CTX *sctx, const SCT *sct); +__owur int SCT_verify(const SCT_CTX *sctx, const SCT *sct); /* * Verifies an SCT against the provided data. * Returns 1 if the SCT verifies successfully, 0 if it cannot be verified and a * negative integer if an error occurs. */ -int SCT_verify_v1(SCT *sct, X509 *cert, X509 *preissuer, +__owur int SCT_verify_v1(SCT *sct, X509 *cert, X509 *preissuer, X509_PUBKEY *log_pubkey, X509 *issuer_cert); /* @@ -370,7 +373,7 @@ sct_validation_status_t SCT_get_validation_status(const SCT *sct); * Returns 0 if the SCT is invalid or could not be verified. * Returns -1 if an error occurs. */ -int SCT_validate(SCT *sct, const CT_POLICY_EVAL_CTX *ctx); +__owur int SCT_validate(SCT *sct, const CT_POLICY_EVAL_CTX *ctx); /* * Validates the given list of SCTs with the provided context. @@ -379,7 +382,8 @@ int SCT_validate(SCT *sct, const CT_POLICY_EVAL_CTX *ctx); * Returns 0 if at least one SCT is invalid or could not be verified. * Returns a negative integer if an error occurs. */ -int SCT_LIST_validate(const STACK_OF(SCT) *scts, CT_POLICY_EVAL_CTX *ctx); +__owur int SCT_LIST_validate(const STACK_OF(SCT) *scts, + CT_POLICY_EVAL_CTX *ctx); /********************************* @@ -398,7 +402,7 @@ int SCT_LIST_validate(const STACK_OF(SCT) *scts, CT_POLICY_EVAL_CTX *ctx); * Returns < 0 on error, >= 0 indicating bytes written (or would have been) * on success. */ -int i2o_SCT_LIST(const STACK_OF(SCT) *a, unsigned char **pp); +__owur int i2o_SCT_LIST(const STACK_OF(SCT) *a, unsigned char **pp); /* * Convert TLS format SCT list to a stack of SCTs. @@ -425,7 +429,7 @@ STACK_OF(SCT) *o2i_SCT_LIST(STACK_OF(SCT) **a, const unsigned char **pp, * Returns < 0 on error, >= 0 indicating bytes written (or would have been) * on success. */ -int i2d_SCT_LIST(STACK_OF(SCT) *a, unsigned char **pp); +__owur int i2d_SCT_LIST(STACK_OF(SCT) *a, unsigned char **pp); /* * Parses an SCT list in DER format and returns it. @@ -449,7 +453,7 @@ STACK_OF(SCT) *d2i_SCT_LIST(STACK_OF(SCT) **a, const unsigned char **pp, * to it. * The length of the SCT in TLS format will be returned. */ -int i2o_SCT(const SCT *sct, unsigned char **out); +__owur int i2o_SCT(const SCT *sct, unsigned char **out); /* * Parses an SCT in TLS format and returns it. @@ -472,7 +476,7 @@ SCT *o2i_SCT(SCT **psct, const unsigned char **in, size_t len); * If |out| points to an allocated string, the signature will be written to it. * The length of the signature in TLS format will be returned. */ -int i2o_SCT_signature(const SCT *sct, unsigned char **out); +__owur int i2o_SCT_signature(const SCT *sct, unsigned char **out); /* * Parses an SCT signature in TLS format and populates the |sct| with it. @@ -481,7 +485,7 @@ int i2o_SCT_signature(const SCT *sct, unsigned char **out); * |len| should be the length of the signature in |in|. * Returns the number of bytes parsed, or a negative integer if an error occurs. */ -int o2i_SCT_signature(SCT *sct, const unsigned char **in, size_t len); +__owur int o2i_SCT_signature(SCT *sct, const unsigned char **in, size_t len); /******************** * CT log functions * @@ -544,7 +548,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 0 otherwise. */ -int CTLOG_STORE_load_file(CTLOG_STORE *store, const char *file); +__owur int CTLOG_STORE_load_file(CTLOG_STORE *store, const char *file); /* * Loads the default CT log list into a |store|. @@ -552,7 +556,7 @@ int CTLOG_STORE_load_file(CTLOG_STORE *store, const char *file); * consulted to find the default file. * Returns 1 if loading is successful, or 0 otherwise. */ -int CTLOG_STORE_load_default_file(CTLOG_STORE *store); +__owur int CTLOG_STORE_load_default_file(CTLOG_STORE *store); /* BEGIN ERROR CODES */ /* diff --git a/test/ct_test.c b/test/ct_test.c index f60be6022b..d5eb5ecbe0 100644 --- a/test/ct_test.c +++ b/test/ct_test.c @@ -341,7 +341,12 @@ static int execute_cert_test(CT_TEST_FIXTURE fixture) if (fixture.test_validity) { int are_scts_validated = 0; scts = X509V3_EXT_d2i(sct_extension); - SCT_LIST_set_source(scts, SCT_SOURCE_X509V3_EXTENSION); + if (SCT_LIST_set_source(scts, SCT_SOURCE_X509V3_EXTENSION) != + sk_SCT_num(scts)) { + fprintf(stderr, + "Error setting SCT source to X509v3 extension\n"); + test_failed = 1; + } are_scts_validated = SCT_LIST_validate(scts, ct_policy_ctx); if (are_scts_validated < 0) { @@ -529,19 +534,34 @@ static int test_encode_tls_sct() SETUP_CT_TEST_FIXTURE(); SCT *sct = SCT_new(); - SCT_set_version(sct, 0); - SCT_set1_log_id(sct, (unsigned char *) + if (!SCT_set_version(sct, 0)) { + fprintf(stderr, "Failed to set SCT version\n"); + return 1; + } + if (!SCT_set1_log_id(sct, (unsigned char *) "\xDF\x1C\x2E\xC1\x15\x00\x94\x52\x47\xA9\x61\x68\x32\x5D\xDC\x5C\x79" - "\x59\xE8\xF7\xC6\xD3\x88\xFC\x00\x2E\x0B\xBD\x3F\x74\xD7\x64", 32); + "\x59\xE8\xF7\xC6\xD3\x88\xFC\x00\x2E\x0B\xBD\x3F\x74\xD7\x64", 32)) { + fprintf(stderr, "Failed to set SCT log ID\n"); + return 1; + } SCT_set_timestamp(sct, 1); - SCT_set1_extensions(sct, (unsigned char *)"", 0); - SCT_set_signature_nid(sct, NID_ecdsa_with_SHA256); - SCT_set1_signature(sct, (unsigned char *) + if (!SCT_set1_extensions(sct, (unsigned char *)"", 0)) { + fprintf(stderr, "Failed to set SCT extensions\n"); + return 1; + } + if (!SCT_set_signature_nid(sct, NID_ecdsa_with_SHA256)) { + fprintf(stderr, "Failed to set SCT signature NID\n"); + return 1; + } + if (!SCT_set1_signature(sct, (unsigned char *) "\x45\x02\x20\x48\x2F\x67\x51\xAF\x35\xDB\xA6\x54\x36\xBE" "\x1F\xD6\x64\x0F\x3D\xBF\x9A\x41\x42\x94\x95\x92\x45\x30\x28\x8F\xA3" "\xE5\xE2\x3E\x06\x02\x21\x00\xE4\xED\xC0\xDB\x3A\xC5\x72\xB1\xE2\xF5" "\xE8\xAB\x6A\x68\x06\x53\x98\x7D\xCF\x41\x02\x7D\xFE\xFF\xA1\x05\x51" - "\x9D\x89\xED\xBF\x08", 71); + "\x9D\x89\xED\xBF\x08", 71)) { + fprintf(stderr, "Failed to set SCT signature\n"); + return 1; + } fixture.sct = sct; fixture.sct_dir = ct_dir; fixture.sct_text_file = "tls1.sct"; -- 2.25.1