kris/boringssl
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Fix and test EVP_PKEY_CTX copying.

Browse Source 
The RSA-PSS salt length was not being copied, and copying an Ed25519
EVP_MD_CTX did not work.

This is rather pointless (an EVP_PKEY_CTX is just a bundle of
parameters), and it's unlikely anyone ever will use this. But since
OpenSSL's EVP_PKEY signing API reuses EVP_MD_CTX and EVP_MD_CTX_copy_ex
is plausible in that scenario, we're stuck making EVP_MD_CTX_copy_ex
reachable for EVP_PKEY too. That then implies EVP_PKEY_dup should exist,
and if it exists we should be testing it.

Change-Id: I189435d0c716a83f58e1d8ac4abc2c409ecfea64
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/35626
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
This commit is contained in:
David Benjamin 2019-04-15 13:55:07 -05:00 committed by CQ bot account: commit-bot@chromium.org
parent d1a6d23686
commit 2e26348e25
4 changed files with 123 additions and 80 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

152
crypto/evp/evp_test.cc
View File

@ -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<EVP_PKEY_CTX> 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<EVP_PKEY_CTX> 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<EVP_PKEY_CTX> 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;
}

11
crypto/evp/evp_tests.txt
View File

@ -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

1
crypto/evp/p_rsa.c
View File

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

39
crypto/fipsmodule/digest/digest.c
View File

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