diff --git a/crypto/evp/evp_test.cc b/crypto/evp/evp_test.cc index 6571c302..4d742920 100644 --- a/crypto/evp/evp_test.cc +++ b/crypto/evp/evp_test.cc @@ -297,16 +297,19 @@ static bool TestEVP(FileTest *t, KeyMap *key_map) { } if (md_op_init) { - bssl::ScopedEVP_MD_CTX ctx; + bssl::ScopedEVP_MD_CTX ctx, copy; EVP_PKEY_CTX *pctx; if (!md_op_init(ctx.get(), &pctx, digest, nullptr, key) || - !SetupContext(t, pctx)) { + !SetupContext(t, pctx) || + !EVP_MD_CTX_copy_ex(copy.get(), ctx.get())) { return false; } if (is_verify) { - return !!EVP_DigestVerify(ctx.get(), output.data(), output.size(), - input.data(), input.size()); + return EVP_DigestVerify(ctx.get(), output.data(), output.size(), + input.data(), input.size()) && + EVP_DigestVerify(copy.get(), output.data(), output.size(), + input.data(), input.size()); } size_t len; @@ -321,6 +324,21 @@ static bool TestEVP(FileTest *t, KeyMap *key_map) { } actual.resize(len); EXPECT_EQ(Bytes(output), Bytes(actual)); + + // Repeat the test with |copy|, to check |EVP_MD_CTX_copy_ex| duplicated + // everything. + if (!EVP_DigestSign(copy.get(), nullptr, &len, input.data(), + input.size())) { + return false; + } + actual.resize(len); + if (!EVP_DigestSign(copy.get(), actual.data(), &len, input.data(), + input.size()) || + !t->GetBytes(&output, "Output")) { + return false; + } + actual.resize(len); + EXPECT_EQ(Bytes(output), Bytes(actual)); return true; } @@ -333,72 +351,78 @@ static bool TestEVP(FileTest *t, KeyMap *key_map) { return false; } + bssl::UniquePtr copy(EVP_PKEY_CTX_dup(ctx.get())); + if (!copy) { + return false; + } + if (is_verify) { - return !!EVP_PKEY_verify(ctx.get(), output.data(), output.size(), - input.data(), input.size()); + return EVP_PKEY_verify(ctx.get(), output.data(), output.size(), + input.data(), input.size()) && + EVP_PKEY_verify(copy.get(), output.data(), output.size(), + input.data(), input.size()); } - size_t len; - if (!key_op(ctx.get(), nullptr, &len, input.data(), input.size())) { - return false; - } - actual.resize(len); - if (!key_op(ctx.get(), actual.data(), &len, input.data(), input.size())) { - return false; - } + for (EVP_PKEY_CTX *pctx : {ctx.get(), copy.get()}) { + size_t len; + if (!key_op(pctx, nullptr, &len, input.data(), input.size())) { + return false; + } + actual.resize(len); + if (!key_op(pctx, actual.data(), &len, input.data(), input.size())) { + return false; + } - // Encryption is non-deterministic, so we check by decrypting. - if (t->HasAttribute("CheckDecrypt")) { - size_t plaintext_len; - ctx.reset(EVP_PKEY_CTX_new(key, nullptr)); - if (!ctx || - !EVP_PKEY_decrypt_init(ctx.get()) || - (digest != nullptr && - !EVP_PKEY_CTX_set_signature_md(ctx.get(), digest)) || - !SetupContext(t, ctx.get()) || - !EVP_PKEY_decrypt(ctx.get(), nullptr, &plaintext_len, actual.data(), - actual.size())) { - return false; + if (t->HasAttribute("CheckDecrypt")) { + // Encryption is non-deterministic, so we check by decrypting. + size_t plaintext_len; + bssl::UniquePtr decrypt_ctx(EVP_PKEY_CTX_new(key, nullptr)); + if (!decrypt_ctx || + !EVP_PKEY_decrypt_init(decrypt_ctx.get()) || + (digest != nullptr && + !EVP_PKEY_CTX_set_signature_md(decrypt_ctx.get(), digest)) || + !SetupContext(t, decrypt_ctx.get()) || + !EVP_PKEY_decrypt(decrypt_ctx.get(), nullptr, &plaintext_len, + actual.data(), actual.size())) { + return false; + } + output.resize(plaintext_len); + if (!EVP_PKEY_decrypt(decrypt_ctx.get(), output.data(), &plaintext_len, + actual.data(), actual.size())) { + ADD_FAILURE() << "Could not decrypt result."; + return false; + } + output.resize(plaintext_len); + EXPECT_EQ(Bytes(input), Bytes(output)) << "Decrypted result mismatch."; + } else if (t->HasAttribute("CheckVerify")) { + // Some signature schemes are non-deterministic, so we check by verifying. + bssl::UniquePtr verify_ctx(EVP_PKEY_CTX_new(key, nullptr)); + if (!verify_ctx || + !EVP_PKEY_verify_init(verify_ctx.get()) || + (digest != nullptr && + !EVP_PKEY_CTX_set_signature_md(verify_ctx.get(), digest)) || + !SetupContext(t, verify_ctx.get())) { + return false; + } + if (t->HasAttribute("VerifyPSSSaltLength")) { + if (!EVP_PKEY_CTX_set_rsa_pss_saltlen( + verify_ctx.get(), + atoi(t->GetAttributeOrDie("VerifyPSSSaltLength").c_str()))) { + return false; + } + } + EXPECT_TRUE(EVP_PKEY_verify(verify_ctx.get(), actual.data(), + actual.size(), input.data(), input.size())) + << "Could not verify result."; + } else { + // By default, check by comparing the result against Output. + if (!t->GetBytes(&output, "Output")) { + return false; + } + actual.resize(len); + EXPECT_EQ(Bytes(output), Bytes(actual)); } - output.resize(plaintext_len); - if (!EVP_PKEY_decrypt(ctx.get(), output.data(), &plaintext_len, - actual.data(), actual.size())) { - ADD_FAILURE() << "Could not decrypt result."; - return false; - } - output.resize(plaintext_len); - EXPECT_EQ(Bytes(input), Bytes(output)) << "Decrypted result mismatch."; - return true; } - - // Some signature schemes are non-deterministic, so we check by verifying. - if (t->HasAttribute("CheckVerify")) { - ctx.reset(EVP_PKEY_CTX_new(key, nullptr)); - if (!ctx || - !EVP_PKEY_verify_init(ctx.get()) || - (digest != nullptr && - !EVP_PKEY_CTX_set_signature_md(ctx.get(), digest)) || - !SetupContext(t, ctx.get())) { - return false; - } - if (t->HasAttribute("VerifyPSSSaltLength") && - !EVP_PKEY_CTX_set_rsa_pss_saltlen( - ctx.get(), - atoi(t->GetAttributeOrDie("VerifyPSSSaltLength").c_str()))) { - return false; - } - EXPECT_TRUE(EVP_PKEY_verify(ctx.get(), actual.data(), actual.size(), - input.data(), input.size())) - << "Could not verify result."; - return true; - } - - // By default, check by comparing the result against Output. - if (!t->GetBytes(&output, "Output")) { - return false; - } - actual.resize(len); - EXPECT_EQ(Bytes(output), Bytes(actual)); return true; } diff --git a/crypto/evp/evp_tests.txt b/crypto/evp/evp_tests.txt index ff08ee76..9dbe1cb7 100644 --- a/crypto/evp/evp_tests.txt +++ b/crypto/evp/evp_tests.txt @@ -274,6 +274,17 @@ Digest = SHA256 Input = "0123456789ABCDEF0123456789ABCDEF" CheckVerify +# Check a salt length with a non-standard digest length, to verify things are +# not just working due to defaults. (The current default is a maximum salt +# length, but the ecosystem has converged on matching the digest length, so we +# may change this in the future.) +Sign = RSA-2048 +RSAPadding = PSS +PSSSaltLength = 42 +Digest = SHA256 +Input = "0123456789ABCDEF0123456789ABCDEF" +CheckVerify + # Auto-detected salt length Verify = RSA-2048-SPKI RSAPadding = PSS diff --git a/crypto/evp/p_rsa.c b/crypto/evp/p_rsa.c index eb59901e..865b36af 100644 --- a/crypto/evp/p_rsa.c +++ b/crypto/evp/p_rsa.c @@ -132,6 +132,7 @@ static int pkey_rsa_copy(EVP_PKEY_CTX *dst, EVP_PKEY_CTX *src) { dctx->pad_mode = sctx->pad_mode; dctx->md = sctx->md; dctx->mgf1md = sctx->mgf1md; + dctx->saltlen = sctx->saltlen; if (sctx->oaep_label) { OPENSSL_free(dctx->oaep_label); dctx->oaep_label = BUF_memdup(sctx->oaep_label, sctx->oaep_labellen); diff --git a/crypto/fipsmodule/digest/digest.c b/crypto/fipsmodule/digest/digest.c index e49d552d..6705867c 100644 --- a/crypto/fipsmodule/digest/digest.c +++ b/crypto/fipsmodule/digest/digest.c @@ -116,7 +116,9 @@ void EVP_MD_CTX_free(EVP_MD_CTX *ctx) { void EVP_MD_CTX_destroy(EVP_MD_CTX *ctx) { EVP_MD_CTX_free(ctx); } int EVP_MD_CTX_copy_ex(EVP_MD_CTX *out, const EVP_MD_CTX *in) { - if (in == NULL || in->digest == NULL) { + // |in->digest| may be NULL if this is a signing |EVP_MD_CTX| for, e.g., + // Ed25519 which does not hash with |EVP_MD_CTX|. + if (in == NULL || (in->pctx == NULL && in->digest == NULL)) { OPENSSL_PUT_ERROR(DIGEST, DIGEST_R_INPUT_NOT_INITIALIZED); return 0; } @@ -131,29 +133,34 @@ int EVP_MD_CTX_copy_ex(EVP_MD_CTX *out, const EVP_MD_CTX *in) { } } - uint8_t *tmp_buf; - if (out->digest != in->digest) { - assert(in->digest->ctx_size != 0); - tmp_buf = OPENSSL_malloc(in->digest->ctx_size); - if (tmp_buf == NULL) { - if (pctx) { - in->pctx_ops->free(pctx); + uint8_t *tmp_buf = NULL; + if (in->digest != NULL) { + if (out->digest != in->digest) { + assert(in->digest->ctx_size != 0); + tmp_buf = OPENSSL_malloc(in->digest->ctx_size); + if (tmp_buf == NULL) { + if (pctx) { + in->pctx_ops->free(pctx); + } + OPENSSL_PUT_ERROR(DIGEST, ERR_R_MALLOC_FAILURE); + return 0; } - OPENSSL_PUT_ERROR(DIGEST, ERR_R_MALLOC_FAILURE); - return 0; + } else { + // |md_data| will be the correct size in this case. It's removed from + // |out| so that |EVP_MD_CTX_cleanup| doesn't free it, and then it's + // reused. + tmp_buf = out->md_data; + out->md_data = NULL; } - } else { - // |md_data| will be the correct size in this case. It's removed from |out| - // so that |EVP_MD_CTX_cleanup| doesn't free it, and then it's reused. - tmp_buf = out->md_data; - out->md_data = NULL; } EVP_MD_CTX_cleanup(out); out->digest = in->digest; out->md_data = tmp_buf; - OPENSSL_memcpy(out->md_data, in->md_data, in->digest->ctx_size); + if (in->digest != NULL) { + OPENSSL_memcpy(out->md_data, in->md_data, in->digest->ctx_size); + } out->pctx = pctx; out->pctx_ops = in->pctx_ops; assert(out->pctx == NULL || out->pctx_ops != NULL);