Skip to content

Commit

Permalink
Ensure that crypto allocations are using BufferSource
Browse files Browse the repository at this point in the history 
... ensures that all ArrayBuffer instances are using sandboxed allocations
rather than external allocations.
  • Loading branch information
@jasnell
jasnell committed Nov 19, 2024
1 parent d1badf9 commit 8ce6016
Show file tree
Hide file tree
Showing 21 changed files with 440 additions and 302 deletions.
45 changes: 23 additions & 22 deletions src/workerd/api/crypto/aes-test.c++
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,6 @@ KJ_TEST("AES-KW key wrap") {
// ASAN/valgrind than using our conformance tests with test-runner.
jsg::test::Evaluator<CryptoContext, CryptoIsolate> e(v8System);
e.getIsolate().runInLockScope([&](CryptoIsolate::Lock& isolateLock) {
auto isolate = isolateLock.v8Isolate;
auto& js = jsg::Lock::from(isolate);

auto rawWrappingKeys = std::array<kj::Array<kj::byte>, 3>({
kj::heapArray<kj::byte>({0xe6, 0x95, 0xea, 0xe3, 0xa8, 0xc0, 0x30, 0xf1, 0x76, 0xe3, 0x0e,
0x8e, 0x36, 0xf8, 0xf4, 0x31}),
Expand All @@ -51,32 +48,35 @@ KJ_TEST("AES-KW key wrap") {
};
bool extractable = false;

return CryptoKey::Impl::importAes(js, "AES-KW", "raw", kj::mv(rawKey), kj::mv(algorithm),
extractable, {kj::str("wrapKey"), kj::str("unwrapKey")});
return CryptoKey::Impl::importAes(isolateLock, "AES-KW", "raw", kj::mv(rawKey),
kj::mv(algorithm), extractable, {kj::str("wrapKey"), kj::str("unwrapKey")});
};

auto keyMaterial = kj::heapArray<const kj::byte>(
{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24});

for (const auto& aesKey: aesKeys) {
SubtleCrypto::EncryptAlgorithm params;
params.name = kj::str("AES-KW");
JSG_WITHIN_CONTEXT_SCOPE(isolateLock,
isolateLock.newContext<CryptoContext>().getHandle(isolateLock), [&](jsg::Lock& js) {
for (const auto& aesKey: aesKeys) {
SubtleCrypto::EncryptAlgorithm params;
params.name = kj::str("AES-KW");

auto wrapped = aesKey->wrapKey(kj::mv(params), keyMaterial.asPtr());
auto wrapped = aesKey->wrapKey(js, kj::mv(params), keyMaterial.asPtr());

params = {};
params.name = kj::str("AES-KW");
params = {};
params.name = kj::str("AES-KW");

auto unwrapped = aesKey->unwrapKey(kj::mv(params), wrapped);
auto unwrapped = aesKey->unwrapKey(js, kj::mv(params), wrapped);

KJ_EXPECT(unwrapped == keyMaterial);
KJ_EXPECT(unwrapped.asArrayPtr() == keyMaterial);

// Corruption of wrapped key material should throw.
params = {};
params.name = kj::str("AES-KW");
wrapped[5] += 1;
KJ_EXPECT_THROW_MESSAGE("[24 == -1]", aesKey->unwrapKey(kj::mv(params), wrapped));
}
// Corruption of wrapped key material should throw.
params = {};
params.name = kj::str("AES-KW");
wrapped.asArrayPtr()[5] += 1;
KJ_EXPECT_THROW_MESSAGE("[24 == -1]", aesKey->unwrapKey(js, kj::mv(params), wrapped));
}
});
});
}

Expand Down Expand Up @@ -136,14 +136,15 @@ KJ_TEST("AES-CTR key wrap") {
return subtle.wrapKey(js, kj::str("raw"), *toWrap, *wrappingKey, getEnc(), *jwkHandler);
})
.then(js,
[&](jsg::Lock&, kj::Array<kj::byte> wrapped) {
return subtle.unwrapKey(js, kj::str("raw"), kj::mv(wrapped), *wrappingKey, getEnc(),
[&](jsg::Lock&, jsg::BufferSource wrapped) {
auto data = kj::heapArray(wrapped.asArrayPtr());
return subtle.unwrapKey(js, kj::str("raw"), kj::mv(data), *wrappingKey, getEnc(),
getImportKeyAlg(), true, kj::arr(kj::str("encrypt")), *jwkHandler);
})
.then(js, [&](jsg::Lock& js, jsg::Ref<CryptoKey> unwrapped) {
return subtle.exportKey(js, kj::str("raw"), *unwrapped);
}).then(js, [&](jsg::Lock&, api::SubtleCrypto::ExportKeyData roundTrippedKeyMaterial) {
KJ_ASSERT(roundTrippedKeyMaterial.get<kj::Array<kj::byte>>() == KEY_DATA);
KJ_ASSERT(roundTrippedKeyMaterial.get<jsg::BufferSource>() == KEY_DATA);
completed = true;
});

Expand Down
117 changes: 66 additions & 51 deletions src/workerd/api/crypto/aes.c++
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,11 @@ protected:
CRYPTO_memcmp(keyData.begin(), other.begin(), keyData.size()) == 0;
}

bool equals(const jsg::BufferSource& other) const override final {
return keyData.size() == other.size() &&
CRYPTO_memcmp(keyData.begin(), other.asArrayPtr().begin(), keyData.size()) == 0;
}

kj::StringPtr jsgGetMemoryName() const override {
return "AesKeyBase"_kjc;
}
Expand All @@ -149,7 +154,7 @@ private:
return keyAlgorithm;
}

SubtleCrypto::ExportKeyData exportKey(kj::StringPtr format) const override final {
SubtleCrypto::ExportKeyData exportKey(jsg::Lock& js, kj::StringPtr format) const override final {
JSG_REQUIRE(format == "raw" || format == "jwk", DOMNotSupportedError, getAlgorithmName(),
" key only supports exporting \"raw\" & \"jwk\", not \"", format, "\".");

Expand Down Expand Up @@ -184,7 +189,10 @@ private:
return jwk;
}

return kj::heapArray(keyData.asPtr());
// Every export should be a separate copy.
auto backing = jsg::BackingStore::alloc<v8::ArrayBuffer>(js, keyData.size());
backing.asArrayPtr().copyFrom(keyData);
return jsg::BufferSource(js, kj::mv(backing));
}

protected:
Expand All @@ -201,7 +209,8 @@ public:
: AesKeyBase(kj::mv(keyData), kj::mv(keyAlgorithm), extractable, usages) {}

private:
kj::Array<kj::byte> encrypt(SubtleCrypto::EncryptAlgorithm&& algorithm,
jsg::BufferSource encrypt(jsg::Lock& js,
SubtleCrypto::EncryptAlgorithm&& algorithm,
kj::ArrayPtr<const kj::byte> plainText) const override {
kj::ArrayPtr<kj::byte> iv =
JSG_REQUIRE_NONNULL(algorithm.iv, TypeError, "Missing field \"iv\" in \"algorithm\".");
Expand Down Expand Up @@ -242,31 +251,32 @@ private:
// a stream cipher in that it does not add padding and can process partial blocks, meaning that
// we know the exact ciphertext size in advance.
auto tagByteSize = tagLength / 8;
auto cipherText = kj::heapArray<kj::byte>(plainText.size() + tagByteSize);
auto cipherText = jsg::BackingStore::alloc<v8::ArrayBuffer>(js, plainText.size() + tagByteSize);

// Perform the actual encryption.

int cipherSize = 0;
OSSLCALL(EVP_EncryptUpdate(
cipherCtx.get(), cipherText.begin(), &cipherSize, plainText.begin(), plainText.size()));
OSSLCALL(EVP_EncryptUpdate(cipherCtx.get(), cipherText.asArrayPtr().begin(), &cipherSize,
plainText.begin(), plainText.size()));
KJ_ASSERT(cipherSize == plainText.size(), "EVP_EncryptUpdate should encrypt all at once");

int finalCipherSize = 0;
OSSLCALL(
EVP_EncryptFinal_ex(cipherCtx.get(), cipherText.begin() + cipherSize, &finalCipherSize));
OSSLCALL(EVP_EncryptFinal_ex(
cipherCtx.get(), cipherText.asArrayPtr().begin() + cipherSize, &finalCipherSize));
KJ_ASSERT(finalCipherSize == 0, "EVP_EncryptFinal_ex should not output any data");

// Concatenate the tag onto the cipher text.
KJ_ASSERT(cipherSize + tagByteSize == cipherText.size(), "imminent buffer overrun");
OSSLCALL(EVP_CIPHER_CTX_ctrl(
cipherCtx.get(), EVP_CTRL_GCM_GET_TAG, tagByteSize, cipherText.begin() + cipherSize));
OSSLCALL(EVP_CIPHER_CTX_ctrl(cipherCtx.get(), EVP_CTRL_GCM_GET_TAG, tagByteSize,
cipherText.asArrayPtr().begin() + cipherSize));
cipherSize += tagByteSize;
KJ_ASSERT(cipherSize == cipherText.size(), "buffer overrun");

return cipherText;
return jsg::BufferSource(js, kj::mv(cipherText));
}

kj::Array<kj::byte> decrypt(SubtleCrypto::EncryptAlgorithm&& algorithm,
jsg::BufferSource decrypt(jsg::Lock& js,
SubtleCrypto::EncryptAlgorithm&& algorithm,
kj::ArrayPtr<const kj::byte> cipherText) const override {
kj::ArrayPtr<kj::byte> iv =
JSG_REQUIRE_NONNULL(algorithm.iv, TypeError, "Missing field \"iv\" in \"algorithm\".");
Expand Down Expand Up @@ -303,10 +313,10 @@ private:
auto actualCipherText = cipherText.first(cipherText.size() - tagLength / 8);
auto tagText = cipherText.slice(actualCipherText.size(), cipherText.size());

auto plainText = kj::heapArray<kj::byte>(actualCipherText.size());
auto plainText = jsg::BackingStore::alloc<v8::ArrayBuffer>(js, actualCipherText.size());

// Perform the actual decryption.
OSSLCALL(EVP_DecryptUpdate(cipherCtx.get(), plainText.begin(), &plainSize,
OSSLCALL(EVP_DecryptUpdate(cipherCtx.get(), plainText.asArrayPtr().begin(), &plainSize,
actualCipherText.begin(), actualCipherText.size()));
KJ_ASSERT(plainSize == plainText.size());

Expand All @@ -322,10 +332,10 @@ private:
const_cast<kj::byte*>(tagText.begin())));

plainSize += decryptFinalHelper(getAlgorithmName(), actualCipherText.size(), plainSize,
cipherCtx.get(), plainText.begin() + plainSize);
cipherCtx.get(), plainText.asArrayPtr().begin() + plainSize);
KJ_ASSERT(plainSize == plainText.size());

return plainText;
return jsg::BufferSource(js, kj::mv(plainText));
}
};

Expand All @@ -338,7 +348,8 @@ public:
: AesKeyBase(kj::mv(keyData), kj::mv(keyAlgorithm), extractable, usages) {}

private:
kj::Array<kj::byte> encrypt(SubtleCrypto::EncryptAlgorithm&& algorithm,
jsg::BufferSource encrypt(jsg::Lock& js,
SubtleCrypto::EncryptAlgorithm&& algorithm,
kj::ArrayPtr<const kj::byte> plainText) const override {
kj::ArrayPtr<kj::byte> iv =
JSG_REQUIRE_NONNULL(algorithm.iv, TypeError, "Missing field \"iv\" in \"algorithm\".");
Expand All @@ -355,29 +366,30 @@ private:

auto blockSize = EVP_CIPHER_CTX_block_size(cipherCtx.get());
size_t paddingSize = blockSize - (plainText.size() % blockSize);
auto cipherText = kj::heapArray<kj::byte>(plainText.size() + paddingSize);
auto cipherText = jsg::BackingStore::alloc<v8::ArrayBuffer>(js, plainText.size() + paddingSize);

// Perform the actual encryption.
//
// Note: We don't worry about PKCS padding (see RFC2315 section 10.3 step 2) because BoringSSL
// takes care of it for us by default in EVP_EncryptFinal_ex().

int cipherSize = 0;
OSSLCALL(EVP_EncryptUpdate(
cipherCtx.get(), cipherText.begin(), &cipherSize, plainText.begin(), plainText.size()));
OSSLCALL(EVP_EncryptUpdate(cipherCtx.get(), cipherText.asArrayPtr().begin(), &cipherSize,
plainText.begin(), plainText.size()));
KJ_ASSERT(cipherSize <= cipherText.size(), "buffer overrun");

KJ_ASSERT(cipherSize + blockSize <= cipherText.size(), "imminent buffer overrun");
int finalCipherSize = 0;
OSSLCALL(
EVP_EncryptFinal_ex(cipherCtx.get(), cipherText.begin() + cipherSize, &finalCipherSize));
OSSLCALL(EVP_EncryptFinal_ex(
cipherCtx.get(), cipherText.asArrayPtr().begin() + cipherSize, &finalCipherSize));
cipherSize += finalCipherSize;
KJ_ASSERT(cipherSize == cipherText.size(), "buffer overrun");

return cipherText;
return jsg::BufferSource(js, kj::mv(cipherText));
}

kj::Array<kj::byte> decrypt(SubtleCrypto::EncryptAlgorithm&& algorithm,
jsg::BufferSource decrypt(jsg::Lock& js,
SubtleCrypto::EncryptAlgorithm&& algorithm,
kj::ArrayPtr<const kj::byte> cipherText) const override {
kj::ArrayPtr<kj::byte> iv =
JSG_REQUIRE_NONNULL(algorithm.iv, TypeError, "Missing field \"iv\" in \"algorithm\".");
Expand Down Expand Up @@ -408,7 +420,9 @@ private:
KJ_ASSERT(plainSize <= plainText.size());

// TODO(perf): Avoid this copy, see comment in the encrypt implementation functions.
return kj::heapArray(plainText.begin(), plainSize);
auto backing = jsg::BackingStore::alloc<v8::ArrayBuffer>(js, plainSize);
backing.asArrayPtr().copyFrom(plainText.first(plainSize));
return jsg::BufferSource(js, kj::mv(backing));
}
};

Expand All @@ -422,14 +436,16 @@ public:
CryptoKeyUsageSet usages)
: AesKeyBase(kj::mv(keyData), kj::mv(keyAlgorithm), extractable, usages) {}

kj::Array<kj::byte> encrypt(SubtleCrypto::EncryptAlgorithm&& algorithm,
jsg::BufferSource encrypt(jsg::Lock& js,
SubtleCrypto::EncryptAlgorithm&& algorithm,
kj::ArrayPtr<const kj::byte> plainText) const override {
return encryptOrDecrypt(kj::mv(algorithm), plainText);
return encryptOrDecrypt(js, kj::mv(algorithm), plainText);
}

kj::Array<kj::byte> decrypt(SubtleCrypto::EncryptAlgorithm&& algorithm,
jsg::BufferSource decrypt(jsg::Lock& js,
SubtleCrypto::EncryptAlgorithm&& algorithm,
kj::ArrayPtr<const kj::byte> cipherText) const override {
return encryptOrDecrypt(kj::mv(algorithm), cipherText);
return encryptOrDecrypt(js, kj::mv(algorithm), cipherText);
}

protected:
Expand All @@ -448,8 +464,9 @@ protected:
KJ_FAIL_ASSERT("CryptoKey has invalid data length");
}

kj::Array<kj::byte> encryptOrDecrypt(
SubtleCrypto::EncryptAlgorithm&& algorithm, kj::ArrayPtr<const kj::byte> data) const {
jsg::BufferSource encryptOrDecrypt(jsg::Lock& js,
SubtleCrypto::EncryptAlgorithm&& algorithm,
kj::ArrayPtr<const kj::byte> data) const {
auto& counter = JSG_REQUIRE_NONNULL(
algorithm.counter, TypeError, "Missing \"counter\" member in \"algorithm\".");
JSG_REQUIRE(counter.size() == expectedCounterByteSize, DOMOperationError,
Expand All @@ -472,9 +489,8 @@ protected:

const auto& cipher = lookupAesType(keyData.size());

kj::Vector<kj::byte> result;
// The output of AES-CTR is the same size as the input.
result.resize(data.size());
auto result = jsg::BackingStore::alloc<v8::ArrayBuffer>(js, data.size());

auto numCounterValues = newBignum();
JSG_REQUIRE(BN_lshift(numCounterValues.get(), BN_value_one(), counterBitLength),
Expand Down Expand Up @@ -505,15 +521,15 @@ protected:

if (BN_cmp(numBlocksUntilReset.get(), numOutputBlocks.get()) >= 0) {
// If the counter doesn't need any wrapping, can evaluate this as a single call.
process(&cipher, data, counter, result.asPtr());
return result.releaseAsArray();
process(&cipher, data, counter, result.asArrayPtr());
return jsg::BufferSource(js, kj::mv(result));
}

// Need this to be done in 2 parts using the current counter block and then resetting the
// counter portion of the block back to zero.
auto inputSizePart1 = BN_get_word(numBlocksUntilReset.get()) * AES_BLOCK_SIZE;

process(&cipher, data.first(inputSizePart1), counter, result.asPtr());
process(&cipher, data.first(inputSizePart1), counter, result.asArrayPtr());

// Zero the counter bits of the block. Chromium creates a copy but we own our buffer.
{
Expand All @@ -528,9 +544,9 @@ protected:
}

process(&cipher, data.slice(inputSizePart1, data.size()), counter,
result.slice(inputSizePart1, result.size()));
result.asArrayPtr().slice(inputSizePart1, result.size()));

return result.releaseAsArray();
return jsg::BufferSource(js, kj::mv(result));
}

private:
Expand Down Expand Up @@ -620,7 +636,8 @@ public:
CryptoKeyUsageSet usages)
: AesKeyBase(kj::mv(keyData), kj::mv(keyAlgorithm), extractable, usages) {}

kj::Array<kj::byte> wrapKey(SubtleCrypto::EncryptAlgorithm&& algorithm,
jsg::BufferSource wrapKey(jsg::Lock& js,
SubtleCrypto::EncryptAlgorithm&& algorithm,
kj::ArrayPtr<const kj::byte> unwrappedKey) const override {
// Resources used to implement this:
// https://www.ietf.org/rfc/rfc3394.txt
Expand All @@ -636,8 +653,7 @@ public:
"equal to 16 and less than or equal to ",
SIZE_MAX - 8);

kj::Vector<kj::byte> wrapped(unwrappedKey.size() + 8);
wrapped.resize(unwrappedKey.size() + 8);
auto wrapped = jsg::BackingStore::alloc<v8::ArrayBuffer>(js, unwrappedKey.size() + 8);
// Wrapping adds 8 bytes of overhead for storing the IV which we check on decryption.

AES_KEY aesKey;
Expand All @@ -646,14 +662,15 @@ public:
internalDescribeOpensslErrors());

JSG_REQUIRE(wrapped.size() ==
AES_wrap_key(
&aesKey, nullptr, wrapped.begin(), unwrappedKey.begin(), unwrappedKey.size()),
AES_wrap_key(&aesKey, nullptr, wrapped.asArrayPtr().begin(), unwrappedKey.begin(),
unwrappedKey.size()),
DOMOperationError, getAlgorithmName(), " key wrapping failed", tryDescribeOpensslErrors());

return wrapped.releaseAsArray();
return jsg::BufferSource(js, kj::mv(wrapped));
}

kj::Array<kj::byte> unwrapKey(SubtleCrypto::EncryptAlgorithm&& algorithm,
jsg::BufferSource unwrapKey(jsg::Lock& js,
SubtleCrypto::EncryptAlgorithm&& algorithm,
kj::ArrayPtr<const kj::byte> wrappedKey) const override {
// Resources used to implement this:
// https://www.ietf.org/rfc/rfc3394.txt
Expand All @@ -667,9 +684,7 @@ public:
"Provided a wrapped key to unwrap this is ", wrappedKey.size() * 8,
" bits that is less than the minimal length of 192 bits.");

kj::Vector<kj::byte> unwrapped(wrappedKey.size() - 8);
// Key wrap adds 8 bytes of overhead because it mixes in the IV.
unwrapped.resize(wrappedKey.size() - 8);
auto unwrapped = jsg::BackingStore::alloc<v8::ArrayBuffer>(js, wrappedKey.size() - 8);

AES_KEY aesKey;
JSG_REQUIRE(0 == AES_set_decrypt_key(keyData.begin(), keyData.size() * 8, &aesKey),
Expand All @@ -679,12 +694,12 @@ public:
// null for the IV value here will tell OpenSSL to validate using the default IV from RFC3394.
// https://github.com/openssl/openssl/blob/13a574d8bb2523181f8150de49bc041c9841f59d/crypto/modes/wrap128.c
JSG_REQUIRE(unwrapped.size() ==
AES_unwrap_key(
&aesKey, nullptr, unwrapped.begin(), wrappedKey.begin(), wrappedKey.size()),
AES_unwrap_key(&aesKey, nullptr, unwrapped.asArrayPtr().begin(), wrappedKey.begin(),
wrappedKey.size()),
DOMOperationError, getAlgorithmName(), " key unwrapping failed",
tryDescribeOpensslErrors());

return unwrapped.releaseAsArray();
return jsg::BufferSource(js, kj::mv(unwrapped));
}
};

Expand Down
Loading

0 comments on commit 8ce6016

Please sign in to comment.