Skip to content

crypto: pass all WebCryptoAPI WPTs #43656

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
panva wants to merge 17 commits into from
Closed

crypto: pass all WebCryptoAPI WPTs #43656

Changes from 1 commit
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
9952d1f
crypto: pass all webcrypto WPTs
panva Jul 2, 2022
491ea8d
address comments
panva Jul 3, 2022
d9cd385
make format-cpp
panva Jul 3, 2022
f0c0a9c
apply review feedback
panva Jul 3, 2022
3720974
mark test-webcrypto SLOW
panva Jul 3, 2022
4fcfa50
Revert "mark test-webcrypto SLOW"
panva Jul 3, 2022
e305875
apply review feedback
panva Jul 5, 2022
7745cb0
apply review feedback
panva Jul 6, 2022
41d849d
see if increasing wpt timeout helps with ci
panva Jul 12, 2022
07a8676
test if runing wpt js files one by one helps with test-webcrypto
panva Jul 13, 2022
fb5eb54
fix lint
panva Jul 13, 2022
03275d0
apply review feedback
panva Jul 23, 2022
8c05855
update hkdf docs
panva Jul 23, 2022
2561171
add 0-length key hmac and equals tests
panva Jul 23, 2022
09a6e50
add yaml change entries
panva Jul 23, 2022
d51c30c
apply review feedback
panva Jul 23, 2022
c8d7b2c
make lint happy
panva Jul 23, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
apply review feedback
  • Loading branch information
@panva
panva committed Jul 23, 2022
commit d51c30c349b95ab0ed72a53cdbc6679c63f1c101
4 changes: 4 additions & 0 deletions src/crypto/crypto_hkdf.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,9 @@ bool HKDFTraits::DeriveBits(
return false;
}

// TODO: Once support for OpenSSL 1.1.1 is dropped the whole
// of HKDFTraits::DeriveBits can be refactored to use
// EVP_KDF which does handle zero length key.
if (params.key->GetSymmetricKeySize() != 0) {
if (!EVP_PKEY_CTX_hkdf_mode(ctx.get(),
EVP_PKEY_HKDEF_MODE_EXTRACT_AND_EXPAND) ||
Expand All @@ -121,6 +124,7 @@ bool HKDFTraits::DeriveBits(
return false;
}
} else {
Copy link
Copy Markdown
Member

@tniessen tniessen Jul 23, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a brief comment to the beginning of the else branch to explain why it exists for future readers?

If we accept that this branch must exist (until we fully drop OpenSSL 1.1.1), then we should probably either

  • remove the other branch that uses EVP_PKEY_HKDEF_MODE_EXTRACT_AND_EXPAND and pass the key to HMAC in this new branch (that should be equivalent as far as I can tell), or
  • add a TODO comment saying to remove this branch once we have dropped OpenSSL 1.1.1.

The first option would give us improved coverage because all HMAC operations would go through it.

Copy link
Copy Markdown
Member Author

@panva panva Jul 23, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a brief comment to the beginning of the else branch to explain why it exists for future readers?

Sure.

If we accept that this branch must exist (until we fully drop OpenSSL 1.1.1), then we should probably either

  • remove the other branch that uses EVP_PKEY_HKDEF_MODE_EXTRACT_AND_EXPAND and pass the key to HMAC in this new branch (that should be equivalent as far as I can tell)

I think that can be done as a follow-up, I am not up for such challenge myself.

  • add a TODO comment saying to remove this branch once we have dropped OpenSSL 1.1.1.

You mean to change the implementation to use EVP_KDF-HKDF when 1.1.1 is dropped?

// Workaround for EVP_PKEY_derive HKDF not handling zero length keys.
unsigned char temp_key[EVP_MAX_MD_SIZE];
unsigned int len = sizeof(temp_key);
if (params.salt.size() != 0) {
Expand Down