Revert "refactor: remove unused evp support for md5+sha1 (#5106)" (#5118)

Author: lrstewart

crypto/s2n_evp_signing.c

@@ -16,7 +16,6 @@
 #include "crypto/s2n_evp_signing.h"
 
 #include "crypto/s2n_evp.h"
-#include "crypto/s2n_fips.h"
 #include "crypto/s2n_pkey.h"
 #include "crypto/s2n_rsa_pss.h"
 #include "error/s2n_errno.h"
@@ -51,58 +50,18 @@ static S2N_RESULT s2n_evp_pkey_set_rsa_pss_saltlen(EVP_PKEY_CTX *pctx)
 #endif
 }
 
-static bool s2n_evp_md5_sha1_is_supported()
-{
-#if defined(S2N_LIBCRYPTO_SUPPORTS_EVP_MD5_SHA1_HASH)
-    return true;
-#else
-    return false;
-#endif
-}
-
-static bool s2n_evp_md_ctx_set_pkey_ctx_is_supported()
+bool s2n_evp_signing_supported()
 {
 #ifdef S2N_LIBCRYPTO_SUPPORTS_EVP_MD_CTX_SET_PKEY_CTX
-    return true;
+    /* We can only use EVP signing if the hash state has an EVP_MD_CTX
+     * that we can pass to the EVP signing methods.
+     */
+    return s2n_hash_evp_fully_supported();
 #else
     return false;
 #endif
 }
 
-bool s2n_evp_signing_supported()
-{
-    /* We must use the FIPS-approved EVP APIs in FIPS mode,
-     * but we could also use the EVP APIs outside of FIPS mode.
-     * Only using the EVP APIs in FIPS mode was a choice made to reduce
-     * the impact of adding support for the EVP APIs.
-     * We should consider instead making the EVP APIs the default.
-     */
-    if (!s2n_is_in_fips_mode()) {
-        return false;
-    }
-
-    /* Our EVP signing logic is intended to support FIPS 140-3.
-     * FIPS 140-3 does not allow externally calculated digests (except for
-     * signing, but not verifying, with ECDSA).
-     * See https://csrc.nist.gov/Projects/Cryptographic-Algorithm-Validation-Program/Digital-Signatures,
-     * and note that "component" tests only exist for ECDSA sign.
-     *
-     * We currently work around that restriction by calling EVP_MD_CTX_set_pkey_ctx,
-     * which lets us set a key on an existing hash state. This is important
-     * when we need to handle signing the TLS1.2 client cert verify message,
-     * which requires signing the entire message transcript. If EVP_MD_CTX_set_pkey_ctx
-     * is unavailable (true for openssl-1.0.2), our current EVP logic will not work.
-     *
-     * FIPS 140-3 is also not possible if EVP_md5_sha1() isn't available
-     * (again true for openssl-1.0.2). In that case, we use two separate hash
-     * states to track the md5 and sha1 parts of the hash separately. That means
-     * that we also have to calculate the digests separately, then combine the
-     * result. We therefore only have an externally calculated digest available
-     * for signing or verifying.
-     */
-    return s2n_evp_md_ctx_set_pkey_ctx_is_supported() && s2n_evp_md5_sha1_is_supported();
-}
-
 /* If using EVP signing, override the sign and verify pkey methods.
  * The EVP methods can handle all pkey types / signature algorithms.
  */

crypto/s2n_hash.c

@@ -15,22 +15,29 @@
 
 #include "crypto/s2n_hash.h"
 
-#include "crypto/s2n_evp_signing.h"
+#include "crypto/s2n_fips.h"
 #include "crypto/s2n_hmac.h"
 #include "crypto/s2n_openssl.h"
 #include "error/s2n_errno.h"
 #include "utils/s2n_safety.h"
 
+static bool s2n_use_custom_md5_sha1()
+{
+#if defined(S2N_LIBCRYPTO_SUPPORTS_EVP_MD5_SHA1_HASH)
+    return false;
+#else
+    return true;
+#endif
+}
+
 static bool s2n_use_evp_impl()
 {
-    /* Our current EVP signing implementation requires EVP hashing.
-     *
-     * We could use the EVP hashing impl with legacy signing, but that would
-     * unnecessarily complicate the logic. The only known libcrypto that
-     * doesn't support EVP signing is openssl-1.0.2. Just let legacy
-     * libcryptos use legacy methods.
-     */
-    return s2n_evp_signing_supported();
+    return s2n_is_in_fips_mode();
+}
+
+bool s2n_hash_evp_fully_supported()
+{
+    return s2n_use_evp_impl() && !s2n_use_custom_md5_sha1();
 }
 
 const EVP_MD *s2n_hash_alg_to_evp_md(s2n_hash_algorithm alg)
@@ -282,8 +289,13 @@ static int s2n_low_level_hash_free(struct s2n_hash_state *state)
 static int s2n_evp_hash_new(struct s2n_hash_state *state)
 {
     POSIX_ENSURE_REF(state->digest.high_level.evp.ctx = S2N_EVP_MD_CTX_NEW());
+    if (s2n_use_custom_md5_sha1()) {
+        POSIX_ENSURE_REF(state->digest.high_level.evp_md5_secondary.ctx = S2N_EVP_MD_CTX_NEW());
+    }
+
     state->is_ready_for_input = 0;
     state->currently_in_hash = 0;
+
     return S2N_SUCCESS;
 }
 
@@ -299,6 +311,13 @@ static int s2n_evp_hash_init(struct s2n_hash_state *state, s2n_hash_algorithm al
         return S2N_SUCCESS;
     }
 
+    if (alg == S2N_HASH_MD5_SHA1 && s2n_use_custom_md5_sha1()) {
+        POSIX_ENSURE_REF(state->digest.high_level.evp_md5_secondary.ctx);
+        POSIX_GUARD_OSSL(EVP_DigestInit_ex(state->digest.high_level.evp.ctx, EVP_sha1(), NULL), S2N_ERR_HASH_INIT_FAILED);
+        POSIX_GUARD_OSSL(EVP_DigestInit_ex(state->digest.high_level.evp_md5_secondary.ctx, EVP_md5(), NULL), S2N_ERR_HASH_INIT_FAILED);
+        return S2N_SUCCESS;
+    }
+
     POSIX_ENSURE_REF(s2n_hash_alg_to_evp_md(alg));
     POSIX_GUARD_OSSL(EVP_DigestInit_ex(state->digest.high_level.evp.ctx, s2n_hash_alg_to_evp_md(alg), NULL), S2N_ERR_HASH_INIT_FAILED);
     return S2N_SUCCESS;
@@ -316,6 +335,12 @@ static int s2n_evp_hash_update(struct s2n_hash_state *state, const void *data, u
 
     POSIX_ENSURE_REF(EVP_MD_CTX_md(state->digest.high_level.evp.ctx));
     POSIX_GUARD_OSSL(EVP_DigestUpdate(state->digest.high_level.evp.ctx, data, size), S2N_ERR_HASH_UPDATE_FAILED);
+
+    if (state->alg == S2N_HASH_MD5_SHA1 && s2n_use_custom_md5_sha1()) {
+        POSIX_ENSURE_REF(EVP_MD_CTX_md(state->digest.high_level.evp_md5_secondary.ctx));
+        POSIX_GUARD_OSSL(EVP_DigestUpdate(state->digest.high_level.evp_md5_secondary.ctx, data, size), S2N_ERR_HASH_UPDATE_FAILED);
+    }
+
     return S2N_SUCCESS;
 }
 
@@ -337,6 +362,23 @@ static int s2n_evp_hash_digest(struct s2n_hash_state *state, void *out, uint32_t
 
     POSIX_ENSURE_REF(EVP_MD_CTX_md(state->digest.high_level.evp.ctx));
 
+    if (state->alg == S2N_HASH_MD5_SHA1 && s2n_use_custom_md5_sha1()) {
+        POSIX_ENSURE_REF(EVP_MD_CTX_md(state->digest.high_level.evp_md5_secondary.ctx));
+
+        uint8_t sha1_digest_size = 0;
+        POSIX_GUARD(s2n_hash_digest_size(S2N_HASH_SHA1, &sha1_digest_size));
+
+        unsigned int sha1_primary_digest_size = sha1_digest_size;
+        unsigned int md5_secondary_digest_size = digest_size - sha1_primary_digest_size;
+
+        POSIX_ENSURE(EVP_MD_CTX_size(state->digest.high_level.evp.ctx) <= sha1_digest_size, S2N_ERR_HASH_DIGEST_FAILED);
+        POSIX_ENSURE((size_t) EVP_MD_CTX_size(state->digest.high_level.evp_md5_secondary.ctx) <= md5_secondary_digest_size, S2N_ERR_HASH_DIGEST_FAILED);
+
+        POSIX_GUARD_OSSL(EVP_DigestFinal_ex(state->digest.high_level.evp.ctx, ((uint8_t *) out) + MD5_DIGEST_LENGTH, &sha1_primary_digest_size), S2N_ERR_HASH_DIGEST_FAILED);
+        POSIX_GUARD_OSSL(EVP_DigestFinal_ex(state->digest.high_level.evp_md5_secondary.ctx, out, &md5_secondary_digest_size), S2N_ERR_HASH_DIGEST_FAILED);
+        return S2N_SUCCESS;
+    }
+
     POSIX_ENSURE((size_t) EVP_MD_CTX_size(state->digest.high_level.evp.ctx) <= digest_size, S2N_ERR_HASH_DIGEST_FAILED);
     POSIX_GUARD_OSSL(EVP_DigestFinal_ex(state->digest.high_level.evp.ctx, out, &digest_size), S2N_ERR_HASH_DIGEST_FAILED);
     return S2N_SUCCESS;
@@ -355,12 +397,21 @@ static int s2n_evp_hash_copy(struct s2n_hash_state *to, struct s2n_hash_state *f
 
     POSIX_ENSURE_REF(to->digest.high_level.evp.ctx);
     POSIX_GUARD_OSSL(EVP_MD_CTX_copy_ex(to->digest.high_level.evp.ctx, from->digest.high_level.evp.ctx), S2N_ERR_HASH_COPY_FAILED);
+
+    if (from->alg == S2N_HASH_MD5_SHA1 && s2n_use_custom_md5_sha1()) {
+        POSIX_ENSURE_REF(to->digest.high_level.evp_md5_secondary.ctx);
+        POSIX_GUARD_OSSL(EVP_MD_CTX_copy_ex(to->digest.high_level.evp_md5_secondary.ctx, from->digest.high_level.evp_md5_secondary.ctx), S2N_ERR_HASH_COPY_FAILED);
+    }
+
     return S2N_SUCCESS;
 }
 
 static int s2n_evp_hash_reset(struct s2n_hash_state *state)
 {
     POSIX_GUARD_OSSL(S2N_EVP_MD_CTX_RESET(state->digest.high_level.evp.ctx), S2N_ERR_HASH_WIPE_FAILED);
+    if (state->alg == S2N_HASH_MD5_SHA1 && s2n_use_custom_md5_sha1()) {
+        POSIX_GUARD_OSSL(S2N_EVP_MD_CTX_RESET(state->digest.high_level.evp_md5_secondary.ctx), S2N_ERR_HASH_WIPE_FAILED);
+    }
 
     /* hash_init resets the ready_for_input and currently_in_hash fields. */
     return s2n_evp_hash_init(state, state->alg);
@@ -370,6 +421,12 @@ static int s2n_evp_hash_free(struct s2n_hash_state *state)
 {
     S2N_EVP_MD_CTX_FREE(state->digest.high_level.evp.ctx);
     state->digest.high_level.evp.ctx = NULL;
+
+    if (s2n_use_custom_md5_sha1()) {
+        S2N_EVP_MD_CTX_FREE(state->digest.high_level.evp_md5_secondary.ctx);
+        state->digest.high_level.evp_md5_secondary.ctx = NULL;
+    }
+
     state->is_ready_for_input = 0;
     return S2N_SUCCESS;
 }

crypto/s2n_hash.h

@@ -54,6 +54,8 @@ union s2n_hash_low_level_digest {
 /* The evp_digest stores all OpenSSL structs to be used with OpenSSL's EVP hash API's. */
 struct s2n_hash_evp_digest {
     struct s2n_evp_digest evp;
+    /* Always store a secondary evp_digest to allow resetting a hash_state to MD5_SHA1 from another alg. */
+    struct s2n_evp_digest evp_md5_secondary;
 };
 
 /* s2n_hash_state stores the s2n_hash implementation being used (low-level or EVP),

tests/cbmc/include/cbmc_proof/cbmc_utils.h

@@ -47,8 +47,13 @@ struct rc_keys_from_evp_pkey_ctx {
     int pkey_eckey_refs;
 };
 
+/**
+ * In the `rc_keys_from_hash_state`, we store two `rc_keys_from_evp_pkey_ctx` objects:
+ * one for the `evp` field and another for `evp_md5_secondary` field.
+ */
 struct rc_keys_from_hash_state {
     struct rc_keys_from_evp_pkey_ctx evp;
+    struct rc_keys_from_evp_pkey_ctx evp_md5;
 };
 
 /**

tests/cbmc/proofs/s2n_hash_const_time_get_currently_in_hash_block/Makefile

@@ -22,7 +22,7 @@ HARNESS_FILE = $(HARNESS_ENTRY).c
 PROOF_SOURCES += $(PROOFDIR)/$(HARNESS_FILE)
 PROOF_SOURCES += $(PROOF_SOURCE)/make_common_datastructures.c
 PROOF_SOURCES += $(PROOF_STUB)/s2n_calculate_stacktrace.c
-PROOF_SOURCES += $(PROOF_STUB)/s2n_evp_signing_supported.c
+PROOF_SOURCES += $(PROOF_STUB)/s2n_is_in_fips_mode.c
 
 PROJECT_SOURCES += $(SRCDIR)/crypto/s2n_hash.c
 

tests/cbmc/proofs/s2n_hash_copy/Makefile

@@ -25,7 +25,7 @@ PROOF_SOURCES += $(OPENSSL_SOURCE)/ec_override.c
 PROOF_SOURCES += $(OPENSSL_SOURCE)/evp_override.c
 PROOF_SOURCES += $(PROOF_SOURCE)/make_common_datastructures.c
 PROOF_SOURCES += $(PROOF_STUB)/s2n_calculate_stacktrace.c
-PROOF_SOURCES += $(PROOF_STUB)/s2n_evp_signing_supported.c
+PROOF_SOURCES += $(PROOF_STUB)/s2n_is_in_fips_mode.c
 
 PROJECT_SOURCES += $(SRCDIR)/crypto/s2n_hash.c
 PROJECT_SOURCES += $(SRCDIR)/utils/s2n_ensure.c

tests/cbmc/proofs/s2n_hash_digest/Makefile

@@ -27,7 +27,7 @@ PROOF_SOURCES += $(OPENSSL_SOURCE)/md5_override.c
 PROOF_SOURCES += $(OPENSSL_SOURCE)/sha_override.c
 PROOF_SOURCES += $(PROOF_SOURCE)/make_common_datastructures.c
 PROOF_SOURCES += $(PROOF_STUB)/s2n_calculate_stacktrace.c
-PROOF_SOURCES += $(PROOF_STUB)/s2n_evp_signing_supported.c
+PROOF_SOURCES += $(PROOF_STUB)/s2n_is_in_fips_mode.c
 
 PROJECT_SOURCES += $(SRCDIR)/crypto/s2n_hash.c
 PROJECT_SOURCES += $(SRCDIR)/utils/s2n_safety.c

tests/cbmc/proofs/s2n_hash_free/Makefile

@@ -24,7 +24,7 @@ PROOF_SOURCES += $(OPENSSL_SOURCE)/ec_override.c
 PROOF_SOURCES += $(OPENSSL_SOURCE)/evp_override.c
 PROOF_SOURCES += $(PROOF_SOURCE)/cbmc_utils.c
 PROOF_SOURCES += $(PROOF_SOURCE)/make_common_datastructures.c
-PROOF_SOURCES += $(PROOF_STUB)/s2n_evp_signing_supported.c
+PROOF_SOURCES += $(PROOF_STUB)/s2n_is_in_fips_mode.c
 PROOF_SOURCES += $(PROOFDIR)/$(HARNESS_FILE)
 
 PROJECT_SOURCES += $(SRCDIR)/crypto/s2n_hash.c

tests/cbmc/proofs/s2n_hash_free/s2n_hash_free_harness.c

@@ -13,7 +13,7 @@
  * permissions and limitations under the License.
  */
 
-#include "crypto/s2n_evp_signing.h"
+#include "crypto/s2n_fips.h"
 #include "crypto/s2n_hash.h"
 
 #include <cbmc_proof/make_common_datastructures.h>
@@ -35,8 +35,9 @@ void s2n_hash_free_harness()
     assert(s2n_hash_free(state) == S2N_SUCCESS);
     if (state != NULL) {
         assert(state->hash_impl->free != NULL);
-        if (s2n_evp_signing_supported()) {
+        if (s2n_is_in_fips_mode()) {
             assert(state->digest.high_level.evp.ctx == NULL);
+            assert(state->digest.high_level.evp_md5_secondary.ctx == NULL);
             assert_rc_decrement_on_hash_state(&saved_hash_state);
         } else {
             assert_rc_unchanged_on_hash_state(&saved_hash_state);
@@ -46,10 +47,11 @@ void s2n_hash_free_harness()
 
     /* Cleanup after expected error cases, for memory leak check. */
     if (state != NULL) {
-        /* 1. `free` leftover EVP_MD_CTX objects if `s2n_evp_signing_supported`,
+        /* 1. `free` leftover EVP_MD_CTX objects if `s2n_is_in_fips_mode`,
               since `s2n_hash_free` is a NO-OP in that case. */
-        if (!s2n_evp_signing_supported()) {
+        if (!s2n_is_in_fips_mode()) {
             S2N_EVP_MD_CTX_FREE(state->digest.high_level.evp.ctx);
+            S2N_EVP_MD_CTX_FREE(state->digest.high_level.evp_md5_secondary.ctx);
         }
 
         /* 2. `free` leftover reference-counted keys (i.e. those with non-zero ref-count),

tests/cbmc/proofs/s2n_hash_get_currently_in_hash_total/Makefile

@@ -22,7 +22,7 @@ HARNESS_FILE = $(HARNESS_ENTRY).c
 PROOF_SOURCES += $(PROOFDIR)/$(HARNESS_FILE)
 PROOF_SOURCES += $(PROOF_SOURCE)/make_common_datastructures.c
 PROOF_SOURCES += $(PROOF_STUB)/s2n_calculate_stacktrace.c
-PROOF_SOURCES += $(PROOF_STUB)/s2n_evp_signing_supported.c
+PROOF_SOURCES += $(PROOF_STUB)/s2n_is_in_fips_mode.c
 
 PROJECT_SOURCES += $(SRCDIR)/crypto/s2n_hash.c