Skip to content
New issue

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

#

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

Already on GitHub? # to your account

Disallow importing public ECDH keys with non-empty usage #433

Merged
merged 1 commit into from
Mar 7, 2023
Merged

Disallow importing public ECDH keys with non-empty usage #433

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
17 changes: 11 additions & 6 deletions src/workerd/api/crypto-impl-asymmetric.c++
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -255,12 +255,13 @@ ImportAsymmetricResult importAsymmetric(kj::StringPtr format,
JSG_REQUIRE(keyDataJwk.oth == nullptr, DOMNotSupportedError,
"Multi-prime private keys not supported.");
} else {
// Public key. If the given key is an ECDH key allow usages to contain deriveBits and
// deriveKey. While these usages appear to not be needed in the public key for the
// operations, users may be confused when they can't import a public key to be used alongside
// a private key for derive*() operations when such a usage is specified.
// The derive* functions check that baseKey is a private key, so imported public keys can't
// be mistakenly used as private keys regardless of the usages field.
// Public key.
// TODO(soon): The usage set is required to be empty for public ECDH keys, which is not being
// checked for currently. See if any code relies on the current behavior and require an empty
// an empty set otherwise (i.e. change to allowedUsages = CryptoKeyUsageSet()). Same applies
// for spki-based and raw ECDH key imports.
JSG_WARN_ONCE_IF(normalizedName == "ECDH" && allowedUsages.size() != 0,
"importing jwk-based public ECDH key with non-empty usages");
keyType = "public";
usages =
CryptoKeyUsageSet::validate(normalizedName, CryptoKeyUsageSet::Context::importPublic,
Expand Down Expand Up @@ -337,6 +338,8 @@ ImportAsymmetricResult importAsymmetric(kj::StringPtr format,
JSG_FAIL_REQUIRE(DOMDataError, "Invalid ", keyBytes.end() - ptr,
" trailing bytes after SPKI input.");
}
JSG_WARN_ONCE_IF(normalizedName == "ECDH" && allowedUsages.size() != 0,
"importing spki-based public ECDH key with non-empty usages");
usages =
CryptoKeyUsageSet::validate(normalizedName, CryptoKeyUsageSet::Context::importPublic,
keyUsages, allowedUsages & (normalizedName == "ECDH" ?
Expand Down Expand Up @@ -1558,6 +1561,8 @@ ImportAsymmetricResult importEllipticRaw(SubtleCrypto::ImportKeyData keyData, in

const auto& raw = keyData.get<kj::Array<kj::byte>>();

JSG_WARN_ONCE_IF(normalizedName == "ECDH" && allowedUsages.size() != 0,
"importing raw ECDH key with non-empty usages");
auto usages = CryptoKeyUsageSet::validate(normalizedName,
CryptoKeyUsageSet::Context::importPublic, keyUsages, allowedUsages);
// TODO(revisit once this is standardized): NodeJS appears to support importing raw for private
Expand Down