refactor: remove unused evp support for md5+sha1

crypto/s2n_evp_signing.c

@@ -16,6 +16,7 @@
 #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"
@@ -50,18 +51,58 @@ static S2N_RESULT s2n_evp_pkey_set_rsa_pss_saltlen(EVP_PKEY_CTX *pctx)
 #endif
 }
 
-bool s2n_evp_signing_supported()
+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()
 {
 #ifdef S2N_LIBCRYPTO_SUPPORTS_EVP_MD_CTX_SET_PKEY_CTX
-    /* 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();
+    return true;
 #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_fips.c

@@ -17,6 +17,7 @@
 
 #include <openssl/crypto.h>
 
+#include "crypto/s2n_evp_signing.h"
 #include "utils/s2n_init.h"
 #include "utils/s2n_safety.h"
 
@@ -65,6 +66,11 @@ int s2n_fips_init(void)
     POSIX_ENSURE(!s2n_fips_mode_enabled, S2N_ERR_FIPS_MODE_UNSUPPORTED);
 #endif
 
+    /* FIPS requires the use of the FIPS-approved EVP methods */
+    if (s2n_fips_mode_enabled) {
+        POSIX_ENSURE(s2n_evp_signing_supported(), S2N_ERR_FIPS_MODE_UNSUPPORTED);
+    }
+
     /* For now, openssl is only supported for testing */
     if (s2n_libcrypto_is_openssl_fips()) {
         POSIX_ENSURE(s2n_in_unit_test(), S2N_ERR_FIPS_MODE_UNSUPPORTED);

crypto/s2n_hash.c

@@ -15,29 +15,22 @@
 
 #include "crypto/s2n_hash.h"
 
-#include "crypto/s2n_fips.h"
+#include "crypto/s2n_evp_signing.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()
 {
-    return s2n_is_in_fips_mode();
-}
-
-bool s2n_hash_evp_fully_supported()
-{
-    return s2n_use_evp_impl() && !s2n_use_custom_md5_sha1();
+    /* 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();
 }
 
 const EVP_MD *s2n_hash_alg_to_evp_md(s2n_hash_algorithm alg)
@@ -289,13 +282,8 @@ 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;
 }
 
@@ -311,13 +299,6 @@ 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;
@@ -335,12 +316,6 @@ 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;
 }
 
@@ -362,23 +337,6 @@ 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;
@@ -397,21 +355,12 @@ 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);
@@ -421,12 +370,6 @@ 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,8 +54,6 @@ 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),