Skip to content

Commit

Permalink
Merge pull request #3111 from cloudflare/jsnell/node-crypto-kdf-use-b…
Browse files Browse the repository at this point in the history
…uffersource
  • Loading branch information
jasnell authored Nov 14, 2024
2 parents 69099b3 + 73b5097 commit 24b9a56
Show file tree
Hide file tree
Showing 6 changed files with 47 additions and 28 deletions.
16 changes: 10 additions & 6 deletions src/workerd/api/crypto/hkdf.c++
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,11 @@ private:

auto derivedLengthBytes = length / 8;

return JSG_REQUIRE_NONNULL(hkdf(derivedLengthBytes, hashType, keyData, salt, info),
auto bs = JSG_REQUIRE_NONNULL(hkdf(js, derivedLengthBytes, hashType, keyData, salt, info),
DOMOperationError, "HKDF deriveBits failed.");

// TODO(cleanup): deriveBits should return a BufferSource and will do so soon.
return bs.asArrayPtr().attach(kj::mv(bs));
}

kj::StringPtr getAlgorithmName() const override {
Expand All @@ -80,17 +83,18 @@ private:

} // namespace

kj::Maybe<kj::Array<kj::byte>> hkdf(size_t length,
kj::Maybe<jsg::BufferSource> hkdf(jsg::Lock& js,
size_t length,
const EVP_MD* digest,
kj::ArrayPtr<const kj::byte> key,
kj::ArrayPtr<const kj::byte> salt,
kj::ArrayPtr<const kj::byte> info) {
auto buf = kj::heapArray<kj::byte>(length);
if (HKDF(buf.begin(), length, digest, key.begin(), key.size(), salt.begin(), salt.size(),
info.begin(), info.size()) != 1) {
auto buf = jsg::BackingStore::alloc<v8::ArrayBuffer>(js, length);
if (HKDF(buf.asArrayPtr().begin(), length, digest, key.begin(), key.size(), salt.begin(),
salt.size(), info.begin(), info.size()) != 1) {
return kj::none;
}
return kj::mv(buf);
return jsg::BufferSource(js, kj::mv(buf));
}

kj::Own<CryptoKey::Impl> CryptoKey::Impl::importHkdf(jsg::Lock& js,
Expand Down
14 changes: 11 additions & 3 deletions src/workerd/api/crypto/kdf.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,24 +9,32 @@

typedef struct env_md_st EVP_MD;

namespace workerd::jsg {
class Lock;
class BufferSource;
} // namespace workerd::jsg

namespace workerd::api {

// Perform HKDF key derivation.
kj::Maybe<kj::Array<kj::byte>> hkdf(size_t length,
kj::Maybe<jsg::BufferSource> hkdf(jsg::Lock& js,
size_t length,
const EVP_MD* digest,
kj::ArrayPtr<const kj::byte> key,
kj::ArrayPtr<const kj::byte> salt,
kj::ArrayPtr<const kj::byte> info);

// Perform PBKDF2 key derivation.
kj::Maybe<kj::Array<kj::byte>> pbkdf2(size_t length,
kj::Maybe<jsg::BufferSource> pbkdf2(jsg::Lock& js,
size_t length,
size_t iterations,
const EVP_MD* digest,
kj::ArrayPtr<const kj::byte> password,
kj::ArrayPtr<const kj::byte> salt);

// Perform Scrypt key derivation.
kj::Maybe<kj::Array<kj::byte>> scrypt(size_t length,
kj::Maybe<jsg::BufferSource> scrypt(jsg::Lock& js,
size_t length,
uint32_t N,
uint32_t r,
uint32_t p,
Expand Down
16 changes: 10 additions & 6 deletions src/workerd/api/crypto/pbkdf2.c++
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,11 @@ private:
// wisest.
checkPbkdfLimits(js, iterations);

return JSG_REQUIRE_NONNULL(pbkdf2(length / 8, iterations, hashType, keyData, salt), Error,
"PBKDF2 deriveBits failed.");
auto buf = JSG_REQUIRE_NONNULL(pbkdf2(js, length / 8, iterations, hashType, keyData, salt),
Error, "PBKDF2 deriveBits failed.");
// TODO(cleanup): This is a bit of a hack. deriveBits should return a BufferSource and
// will do so soon.
return buf.asArrayPtr().attach(kj::mv(buf));
}

// TODO(bug): Possibly by mistake, PBKDF2 was historically not on the allow list of
Expand Down Expand Up @@ -101,17 +104,18 @@ private:

} // namespace

kj::Maybe<kj::Array<kj::byte>> pbkdf2(size_t length,
kj::Maybe<jsg::BufferSource> pbkdf2(jsg::Lock& js,
size_t length,
size_t iterations,
const EVP_MD* digest,
kj::ArrayPtr<const kj::byte> password,
kj::ArrayPtr<const kj::byte> salt) {
auto buf = kj::heapArray<kj::byte>(length);
auto buf = jsg::BackingStore::alloc<v8::ArrayBuffer>(js, length);
if (PKCS5_PBKDF2_HMAC(password.asChars().begin(), password.size(), salt.begin(), salt.size(),
iterations, digest, length, buf.begin()) != 1) {
iterations, digest, length, buf.asArrayPtr().begin()) != 1) {
return kj::none;
}
return kj::mv(buf);
return jsg::BufferSource(js, kj::mv(buf));
}

kj::Own<CryptoKey::Impl> CryptoKey::Impl::importPbkdf2(jsg::Lock& js,
Expand Down
9 changes: 5 additions & 4 deletions src/workerd/api/crypto/scrypt.c++
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,18 @@

namespace workerd::api {

kj::Maybe<kj::Array<kj::byte>> scrypt(size_t length,
kj::Maybe<jsg::BufferSource> scrypt(jsg::Lock& js,
size_t length,
uint32_t N,
uint32_t r,
uint32_t p,
uint32_t maxmem,
kj::ArrayPtr<const kj::byte> pass,
kj::ArrayPtr<const kj::byte> salt) {
ClearErrorOnReturn clearErrorOnReturn;
auto buf = kj::heapArray<kj::byte>(length);
auto buf = jsg::BackingStore::alloc<v8::ArrayBuffer>(js, length);
if (!EVP_PBE_scrypt(pass.asChars().begin(), pass.size(), salt.begin(), salt.size(), N, r, p,
maxmem, buf.begin(), length)) {
maxmem, buf.asArrayPtr().begin(), length)) {
// This does not currently handle the errors in exactly the same way as
// the Node.js implementation but that's probably ok? We can update the
// error thrown to match Node.js more closely later if necessary. There
Expand All @@ -30,7 +31,7 @@ kj::Maybe<kj::Array<kj::byte>> scrypt(size_t length,
}
return kj::none;
}
return kj::mv(buf);
return jsg::BufferSource(js, kj::mv(buf));
}

} // namespace workerd::api
13 changes: 7 additions & 6 deletions src/workerd/api/node/crypto.c++
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@

namespace workerd::api::node {

kj::Array<kj::byte> CryptoImpl::getHkdf(kj::String hash,
jsg::BufferSource CryptoImpl::getHkdf(jsg::Lock& js,
kj::String hash,
kj::Array<const kj::byte> key,
kj::Array<const kj::byte> salt,
kj::Array<const kj::byte> info,
Expand Down Expand Up @@ -43,10 +44,10 @@ kj::Array<kj::byte> CryptoImpl::getHkdf(kj::String hash,
JSG_REQUIRE(
length <= EVP_MD_size(digest) * kMaxDigestMultiplier, RangeError, "Invalid Hkdf key length");

return JSG_REQUIRE_NONNULL(hkdf(length, digest, key, salt, info), Error, "Hkdf failed");
return JSG_REQUIRE_NONNULL(hkdf(js, length, digest, key, salt, info), Error, "Hkdf failed");
}

kj::Array<kj::byte> CryptoImpl::getPbkdf(jsg::Lock& js,
jsg::BufferSource CryptoImpl::getPbkdf(jsg::Lock& js,
kj::Array<const kj::byte> password,
kj::Array<const kj::byte> salt,
uint32_t num_iterations,
Expand All @@ -70,10 +71,10 @@ kj::Array<kj::byte> CryptoImpl::getPbkdf(jsg::Lock& js,

// Both pass and salt may be zero length here.
return JSG_REQUIRE_NONNULL(
pbkdf2(keylen, num_iterations, digest, password, salt), Error, "Pbkdf2 failed");
pbkdf2(js, keylen, num_iterations, digest, password, salt), Error, "Pbkdf2 failed");
}

kj::Array<kj::byte> CryptoImpl::getScrypt(jsg::Lock& js,
jsg::BufferSource CryptoImpl::getScrypt(jsg::Lock& js,
kj::Array<const kj::byte> password,
kj::Array<const kj::byte> salt,
uint32_t N,
Expand All @@ -86,7 +87,7 @@ kj::Array<kj::byte> CryptoImpl::getScrypt(jsg::Lock& js,
JSG_REQUIRE(salt.size() <= INT32_MAX, RangeError, "Scrypt failed: salt is too large");

return JSG_REQUIRE_NONNULL(
scrypt(keylen, N, r, p, maxmem, password, salt), Error, "Scrypt failed");
scrypt(js, keylen, N, r, p, maxmem, password, salt), Error, "Scrypt failed");
}

bool CryptoImpl::verifySpkac(kj::Array<const kj::byte> input) {
Expand Down
7 changes: 4 additions & 3 deletions src/workerd/api/node/crypto.h
Original file line number Diff line number Diff line change
Expand Up @@ -115,22 +115,23 @@ class CryptoImpl final: public jsg::Object {
};

// Hkdf
kj::Array<kj::byte> getHkdf(kj::String hash,
jsg::BufferSource getHkdf(jsg::Lock& js,
kj::String hash,
kj::Array<const kj::byte> key,
kj::Array<const kj::byte> salt,
kj::Array<const kj::byte> info,
uint32_t length);

// Pbkdf2
kj::Array<kj::byte> getPbkdf(jsg::Lock& js,
jsg::BufferSource getPbkdf(jsg::Lock& js,
kj::Array<const kj::byte> password,
kj::Array<const kj::byte> salt,
uint32_t num_iterations,
uint32_t keylen,
kj::String name);

// Scrypt
kj::Array<kj::byte> getScrypt(jsg::Lock& js,
jsg::BufferSource getScrypt(jsg::Lock& js,
kj::Array<const kj::byte> password,
kj::Array<const kj::byte> salt,
uint32_t N,
Expand Down

0 comments on commit 24b9a56

Please sign in to comment.