Extensive application of __owur to CT functions that return a boolean

Also improves some documentation of those functions.
Reviewed-by: NEmilia Käsper <emilia@openssl.org>
Reviewed-by: NRich Salz <rsalz@openssl.org>

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

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);
 
 

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,
+__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)

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 */
 /*

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";