refactor: add alternative EVP signing method (#5141)

codebuild/spec/buildspec_openssl3fips.yml

@@ -37,3 +37,4 @@ phases:
       # openssl3fips is still a work-in-progress. Not all tests pass.
       - make -C build test -- ARGS="-R 's2n_build_test|s2n_fips_test'"
       - make -C build test -- ARGS="-R 's2n_hash_test|s2n_hash_all_algs_test|s2n_openssl_test|s2n_init_test'"
+      - make -C build test -- ARGS="-R 's2n_evp_signing_test'"

crypto/s2n_evp_signing.c

@@ -16,6 +16,7 @@
 #include "crypto/s2n_evp_signing.h"
 
 #include "crypto/s2n_evp.h"
+#include "crypto/s2n_libcrypto.h"
 #include "crypto/s2n_pkey.h"
 #include "crypto/s2n_rsa_pss.h"
 #include "error/s2n_errno.h"
@@ -24,12 +25,6 @@
 
 DEFINE_POINTER_CLEANUP_FUNC(EVP_PKEY_CTX *, EVP_PKEY_CTX_free);
 
-/*
- * FIPS 140-3 requires that we don't pass raw digest bytes to the libcrypto signing methods.
- * In order to do that, we need to use signing methods that both calculate the digest and
- * perform the signature.
- */
-
 static S2N_RESULT s2n_evp_md_ctx_set_pkey_ctx(EVP_MD_CTX *ctx, EVP_PKEY_CTX *pctx)
 {
 #ifdef S2N_LIBCRYPTO_SUPPORTS_EVP_MD_CTX_SET_PKEY_CTX
@@ -50,28 +45,16 @@ static S2N_RESULT s2n_evp_pkey_set_rsa_pss_saltlen(EVP_PKEY_CTX *pctx)
 #endif
 }
 
-bool s2n_evp_signing_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();
-#else
-    return false;
-#endif
-}
-
-/* If using EVP signing, override the sign and verify pkey methods.
- * The EVP methods can handle all pkey types / signature algorithms.
+/* Always use EVP signing.
+ *
+ * TODO: Migrate the rest of the s2n_pkey methods to EVP and delete the legacy
+ * pkey logic and this method.
  */
 S2N_RESULT s2n_evp_signing_set_pkey_overrides(struct s2n_pkey *pkey)
 {
-    if (s2n_evp_signing_supported()) {
-        RESULT_ENSURE_REF(pkey);
-        pkey->sign = &s2n_evp_sign;
-        pkey->verify = &s2n_evp_verify;
-    }
+    RESULT_ENSURE_REF(pkey);
+    pkey->sign = &s2n_evp_sign;
+    pkey->verify = &s2n_evp_verify;
     return S2N_RESULT_OK;
 }
 
@@ -89,15 +72,123 @@ static S2N_RESULT s2n_evp_signing_validate_sig_alg(const struct s2n_pkey *key, s
     return S2N_RESULT_OK;
 }
 
+static EVP_PKEY_CTX *s2n_evp_pkey_ctx_new(EVP_PKEY *pkey, s2n_hash_algorithm hash_alg)
+{
+    PTR_ENSURE_REF(pkey);
+    switch (hash_alg) {
+#if S2N_LIBCRYPTO_SUPPORTS_PROVIDERS
+        /* For openssl-3.0, pkey methods will do an implicit fetch for the signing
+         * algorithm, which includes the hash algorithm. If using a legacy hash
+         * algorithm, specify the non-fips version.
+         */
+        case S2N_HASH_MD5:
+        case S2N_HASH_MD5_SHA1:
+        case S2N_HASH_SHA1:
+            return EVP_PKEY_CTX_new_from_pkey(NULL, pkey, "-fips");
+#endif
+        default:
+            return EVP_PKEY_CTX_new(pkey, NULL);
+    }
+}
+
+/* Our "digest-and-sign" EVP signing logic is intended to support FIPS 140-3.
+ * FIPS 140-3 does not allow signing or verifying 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.
+ *
+ * In order to avoid signing externally calculated digests, we naively would
+ * need access to the full message to be signed at the time of signing. That's
+ * a problem for TLS1.2, where the client cert verify message requires signing
+ * every handshake message sent or received before the client cert verify message.
+ * To avoid storing every single handshake message in its entirety, we instead
+ * keep a running hash of the messages in an EVP hash state. Then, instead of
+ * digesting that hash state, we pass it unmodified to EVP_DigestSignFinal.
+ * That would normally not be allowed, since the hash state was initialized without
+ * a key using EVP_DigestInit instead of with a key using EVP_DigestSignInit.
+ * We make it work by using the EVP_MD_CTX_set_pkey_ctx method to attach a key
+ * to an existing hash state.
+ *
+ * All that means that "digest-and-sign" requires two things:
+ * - A single EVP hash state to sign. So we must not use a custom MD5_SHA1 hash,
+ *   which doesn't produce a single hash state.
+ * - EVP_MD_CTX_set_pkey_ctx to exist and to behave as expected. Existence
+ *   alone is not sufficient: the method exists in openssl-3.0-fips, but
+ *   it cannot be used to setup a hash state for EVP_DigestSignFinal.
+ *
+ * Currently only awslc-fips meets both these requirements. New libcryptos
+ * should be assumed not to meet these requirements until proven otherwise.
+ */
+int s2n_evp_digest_and_sign(EVP_PKEY_CTX *pctx, s2n_signature_algorithm sig_alg,
+        struct s2n_hash_state *hash_state, struct s2n_blob *signature)
+{
+    POSIX_ENSURE_REF(pctx);
+    POSIX_ENSURE_REF(hash_state);
+    POSIX_ENSURE_REF(signature);
+
+    /* Custom MD5_SHA1 involves combining separate MD5 and SHA1 hashes.
+     * That involves two hash states instead of the single hash state this
+     * method requires.
+     */
+    POSIX_ENSURE(!s2n_hash_use_custom_md5_sha1(), S2N_ERR_SAFETY);
+
+    /* Not all implementations of EVP_MD_CTX_set_pkey_ctx behave as required
+     * by this method. Using EVP_MD_CTX_set_pkey_ctx to convert a hash initialized
+     * with EVP_DigestInit to one that can be finalized with EVP_DigestSignFinal
+     * is not entirely standard.
+     *
+     * However, this behavior is known to work with awslc-fips.
+     */
+    POSIX_ENSURE(s2n_libcrypto_is_awslc_fips(), S2N_ERR_SAFETY);
+
+    EVP_MD_CTX *ctx = hash_state->digest.high_level.evp.ctx;
+    POSIX_ENSURE_REF(ctx);
+    POSIX_GUARD_RESULT(s2n_evp_md_ctx_set_pkey_ctx(ctx, pctx));
+
+    size_t signature_size = signature->size;
+    POSIX_GUARD_OSSL(EVP_DigestSignFinal(ctx, signature->data, &signature_size), S2N_ERR_SIGN);
+    POSIX_ENSURE(signature_size <= signature->size, S2N_ERR_SIZE_MISMATCH);
+    signature->size = signature_size;
+    POSIX_GUARD_RESULT(s2n_evp_md_ctx_set_pkey_ctx(ctx, NULL));
+
+    return S2N_SUCCESS;
+}
+
+/* "digest-then-sign" means that we calculate the digest for a hash state,
+ * then sign the digest bytes. That is not allowed by FIPS 140-3, but is allowed
+ * in all other cases.
+ */
+int s2n_evp_digest_then_sign(EVP_PKEY_CTX *pctx,
+        struct s2n_hash_state *hash_state, struct s2n_blob *signature)
+{
+    POSIX_ENSURE_REF(pctx);
+    POSIX_ENSURE_REF(hash_state);
+    POSIX_ENSURE_REF(signature);
+
+    uint8_t digest_length = 0;
+    POSIX_GUARD(s2n_hash_digest_size(hash_state->alg, &digest_length));
+    POSIX_ENSURE_LTE(digest_length, S2N_MAX_DIGEST_LEN);
+
+    uint8_t digest_out[S2N_MAX_DIGEST_LEN] = { 0 };
+    POSIX_GUARD(s2n_hash_digest(hash_state, digest_out, digest_length));
+
+    size_t signature_size = signature->size;
+    POSIX_GUARD_OSSL(EVP_PKEY_sign(pctx, signature->data, &signature_size,
+                             digest_out, digest_length),
+            S2N_ERR_SIGN);
+    POSIX_ENSURE(signature_size <= signature->size, S2N_ERR_SIZE_MISMATCH);
+    signature->size = signature_size;
+
+    return S2N_SUCCESS;
+}
+
 int s2n_evp_sign(const struct s2n_pkey *priv, s2n_signature_algorithm sig_alg,
         struct s2n_hash_state *hash_state, struct s2n_blob *signature)
 {
     POSIX_ENSURE_REF(priv);
     POSIX_ENSURE_REF(hash_state);
-    POSIX_ENSURE_REF(signature);
-    POSIX_ENSURE(s2n_evp_signing_supported(), S2N_ERR_HASH_NOT_READY);
 
-    DEFER_CLEANUP(EVP_PKEY_CTX *pctx = EVP_PKEY_CTX_new(priv->pkey, NULL), EVP_PKEY_CTX_free_pointer);
+    DEFER_CLEANUP(EVP_PKEY_CTX *pctx = s2n_evp_pkey_ctx_new(priv->pkey, hash_state->alg), EVP_PKEY_CTX_free_pointer);
     POSIX_ENSURE_REF(pctx);
     POSIX_GUARD_OSSL(EVP_PKEY_sign_init(pctx), S2N_ERR_PKEY_CTX_INIT);
     POSIX_GUARD_OSSL(S2N_EVP_PKEY_CTX_set_signature_md(pctx, s2n_hash_alg_to_evp_md(hash_state->alg)), S2N_ERR_PKEY_CTX_INIT);
@@ -107,15 +198,55 @@ int s2n_evp_sign(const struct s2n_pkey *priv, s2n_signature_algorithm sig_alg,
         POSIX_GUARD_RESULT(s2n_evp_pkey_set_rsa_pss_saltlen(pctx));
     }
 
+    if (s2n_libcrypto_is_awslc_fips()) {
+        POSIX_GUARD(s2n_evp_digest_and_sign(pctx, sig_alg, hash_state, signature));
+    } else {
+        POSIX_GUARD(s2n_evp_digest_then_sign(pctx, hash_state, signature));
+    }
+
+    return S2N_SUCCESS;
+}
+
+/* See s2n_evp_digest_and_sign for more information */
+int s2n_evp_digest_and_verify(EVP_PKEY_CTX *pctx, s2n_signature_algorithm sig_alg,
+        struct s2n_hash_state *hash_state, struct s2n_blob *signature)
+{
+    POSIX_ENSURE_REF(pctx);
+    POSIX_ENSURE_REF(hash_state);
+    POSIX_ENSURE_REF(signature);
+
+    /* See digest-and-sign requirements */
+    POSIX_ENSURE(!s2n_hash_use_custom_md5_sha1(), S2N_ERR_SAFETY);
+    POSIX_ENSURE(s2n_libcrypto_is_awslc_fips(), S2N_ERR_SAFETY);
+
     EVP_MD_CTX *ctx = hash_state->digest.high_level.evp.ctx;
     POSIX_ENSURE_REF(ctx);
     POSIX_GUARD_RESULT(s2n_evp_md_ctx_set_pkey_ctx(ctx, pctx));
 
-    size_t signature_size = signature->size;
-    POSIX_GUARD_OSSL(EVP_DigestSignFinal(ctx, signature->data, &signature_size), S2N_ERR_SIGN);
-    POSIX_ENSURE(signature_size <= signature->size, S2N_ERR_SIZE_MISMATCH);
-    signature->size = signature_size;
+    POSIX_GUARD_OSSL(EVP_DigestVerifyFinal(ctx, signature->data, signature->size), S2N_ERR_VERIFY_SIGNATURE);
     POSIX_GUARD_RESULT(s2n_evp_md_ctx_set_pkey_ctx(ctx, NULL));
+
+    return S2N_SUCCESS;
+}
+
+/* See s2n_evp_digest_then_sign for more information */
+int s2n_evp_digest_then_verify(EVP_PKEY_CTX *pctx,
+        struct s2n_hash_state *hash_state, struct s2n_blob *signature)
+{
+    POSIX_ENSURE_REF(pctx);
+    POSIX_ENSURE_REF(hash_state);
+    POSIX_ENSURE_REF(signature);
+
+    uint8_t digest_length = 0;
+    POSIX_GUARD(s2n_hash_digest_size(hash_state->alg, &digest_length));
+    POSIX_ENSURE_LTE(digest_length, S2N_MAX_DIGEST_LEN);
+
+    uint8_t digest_out[S2N_MAX_DIGEST_LEN] = { 0 };
+    POSIX_GUARD(s2n_hash_digest(hash_state, digest_out, digest_length));
+
+    POSIX_GUARD_OSSL(EVP_PKEY_verify(pctx, signature->data, signature->size,
+                             digest_out, digest_length),
+            S2N_ERR_VERIFY_SIGNATURE);
     return S2N_SUCCESS;
 }
 
@@ -125,10 +256,9 @@ int s2n_evp_verify(const struct s2n_pkey *pub, s2n_signature_algorithm sig_alg,
     POSIX_ENSURE_REF(pub);
     POSIX_ENSURE_REF(hash_state);
     POSIX_ENSURE_REF(signature);
-    POSIX_ENSURE(s2n_evp_signing_supported(), S2N_ERR_HASH_NOT_READY);
     POSIX_GUARD_RESULT(s2n_evp_signing_validate_sig_alg(pub, sig_alg));
 
-    DEFER_CLEANUP(EVP_PKEY_CTX *pctx = EVP_PKEY_CTX_new(pub->pkey, NULL), EVP_PKEY_CTX_free_pointer);
+    DEFER_CLEANUP(EVP_PKEY_CTX *pctx = s2n_evp_pkey_ctx_new(pub->pkey, hash_state->alg), EVP_PKEY_CTX_free_pointer);
     POSIX_ENSURE_REF(pctx);
     POSIX_GUARD_OSSL(EVP_PKEY_verify_init(pctx), S2N_ERR_PKEY_CTX_INIT);
     POSIX_GUARD_OSSL(S2N_EVP_PKEY_CTX_set_signature_md(pctx, s2n_hash_alg_to_evp_md(hash_state->alg)), S2N_ERR_PKEY_CTX_INIT);
@@ -138,11 +268,11 @@ int s2n_evp_verify(const struct s2n_pkey *pub, s2n_signature_algorithm sig_alg,
         POSIX_GUARD_RESULT(s2n_evp_pkey_set_rsa_pss_saltlen(pctx));
     }
 
-    EVP_MD_CTX *ctx = hash_state->digest.high_level.evp.ctx;
-    POSIX_ENSURE_REF(ctx);
-    POSIX_GUARD_RESULT(s2n_evp_md_ctx_set_pkey_ctx(ctx, pctx));
+    if (s2n_libcrypto_is_awslc_fips()) {
+        POSIX_GUARD(s2n_evp_digest_and_verify(pctx, sig_alg, hash_state, signature));
+    } else {
+        POSIX_GUARD(s2n_evp_digest_then_verify(pctx, hash_state, signature));
+    }
 
-    POSIX_GUARD_OSSL(EVP_DigestVerifyFinal(ctx, signature->data, signature->size), S2N_ERR_VERIFY_SIGNATURE);
-    POSIX_GUARD_RESULT(s2n_evp_md_ctx_set_pkey_ctx(ctx, NULL));
     return S2N_SUCCESS;
 }

crypto/s2n_evp_signing.h

@@ -20,7 +20,6 @@
 #include "crypto/s2n_signature.h"
 #include "utils/s2n_blob.h"
 
-bool s2n_evp_signing_supported();
 S2N_RESULT s2n_evp_signing_set_pkey_overrides(struct s2n_pkey *pkey);
 int s2n_evp_sign(const struct s2n_pkey *priv, s2n_signature_algorithm sig_alg,
         struct s2n_hash_state *digest, struct s2n_blob *signature);

crypto/s2n_hash.c

@@ -26,7 +26,7 @@ static EVP_MD *s2n_evp_mds[S2N_HASH_ALGS_COUNT] = { 0 };
 static const EVP_MD *s2n_evp_mds[S2N_HASH_ALGS_COUNT] = { 0 };
 #endif
 
-static bool s2n_use_custom_md5_sha1()
+bool s2n_hash_use_custom_md5_sha1()
 {
 #if defined(S2N_LIBCRYPTO_SUPPORTS_EVP_MD5_SHA1_HASH)
     return false;
@@ -35,16 +35,6 @@ static bool s2n_use_custom_md5_sha1()
 #endif
 }
 
-/* This is currently only used by s2n_evp_signing
- * to determine whether or not to use EVP signing.
- * Historically we only used EVP signing for FIPS.
- * To avoid a premature behavior change, consider FIPS a requirement.
- */
-bool s2n_hash_evp_fully_supported()
-{
-    return s2n_is_in_fips_mode() && !s2n_use_custom_md5_sha1();
-}
-
 S2N_RESULT s2n_hash_algorithms_init()
 {
 #if S2N_LIBCRYPTO_SUPPORTS_PROVIDERS
@@ -177,7 +167,7 @@ int s2n_hash_is_ready_for_input(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()) {
+    if (s2n_hash_use_custom_md5_sha1()) {
         POSIX_ENSURE_REF(state->digest.high_level.evp_md5_secondary.ctx = S2N_EVP_MD_CTX_NEW());
     }
 
@@ -199,7 +189,7 @@ 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()) {
+    if (alg == S2N_HASH_MD5_SHA1 && s2n_hash_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,
                                  s2n_hash_alg_to_evp_md(S2N_HASH_SHA1), NULL),
@@ -231,7 +221,7 @@ 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()) {
+    if (state->alg == S2N_HASH_MD5_SHA1 && s2n_hash_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);
     }
@@ -257,7 +247,7 @@ 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()) {
+    if (state->alg == S2N_HASH_MD5_SHA1 && s2n_hash_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;
@@ -293,7 +283,7 @@ 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()) {
+    if (from->alg == S2N_HASH_MD5_SHA1 && s2n_hash_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);
     }
@@ -304,7 +294,7 @@ static int s2n_evp_hash_copy(struct s2n_hash_state *to, struct s2n_hash_state *f
 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()) {
+    if (state->alg == S2N_HASH_MD5_SHA1 && s2n_hash_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);
     }
 
@@ -317,7 +307,7 @@ 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()) {
+    if (s2n_hash_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;
     }

crypto/s2n_hash.h

@@ -72,7 +72,7 @@ struct s2n_hash {
 
 S2N_RESULT s2n_hash_algorithms_init();
 S2N_RESULT s2n_hash_algorithms_cleanup();
-bool s2n_hash_evp_fully_supported();
+bool s2n_hash_use_custom_md5_sha1();
 const EVP_MD *s2n_hash_alg_to_evp_md(s2n_hash_algorithm alg);
 int s2n_hash_digest_size(s2n_hash_algorithm alg, uint8_t *out);
 int s2n_hash_block_size(s2n_hash_algorithm alg, uint64_t *block_size);

crypto/s2n_rsa_pss.c

@@ -106,8 +106,8 @@ static int s2n_rsa_pss_validate_sign_verify_match(const struct s2n_pkey *pub, co
 
     /* Sign and Verify the Hash of the Random Blob */
     s2n_stack_blob(signature_data, RSA_PSS_SIGN_VERIFY_SIGNATURE_SIZE, RSA_PSS_SIGN_VERIFY_SIGNATURE_SIZE);
-    POSIX_GUARD(s2n_rsa_pss_key_sign(priv, S2N_SIGNATURE_RSA_PSS_PSS, &sign_hash, &signature_data));
-    POSIX_GUARD(s2n_rsa_pss_key_verify(pub, S2N_SIGNATURE_RSA_PSS_PSS, &verify_hash, &signature_data));
+    POSIX_GUARD(s2n_pkey_sign(priv, S2N_SIGNATURE_RSA_PSS_PSS, &sign_hash, &signature_data));
+    POSIX_GUARD(s2n_pkey_verify(pub, S2N_SIGNATURE_RSA_PSS_PSS, &verify_hash, &signature_data));
 
     return 0;
 }