diff --git a/lib/internal/policy/manifest.js b/lib/internal/policy/manifest.js index fab7d132ca6023..fdba0c9bd98f08 100644 --- a/lib/internal/policy/manifest.js +++ b/lib/internal/policy/manifest.js @@ -15,7 +15,6 @@ const { StringPrototypeEndsWith, StringPrototypeStartsWith, Symbol, - uncurryThis, } = primordials; const { ERR_MANIFEST_ASSERT_INTEGRITY, @@ -27,13 +26,8 @@ let debug = require('internal/util/debuglog').debuglog('policy', (fn) => { debug = fn; }); const SRI = require('internal/policy/sri'); -const crypto = require('crypto'); -const { Buffer } = require('buffer'); const { URL } = require('internal/url'); -const { createHash, timingSafeEqual } = crypto; -const HashUpdate = uncurryThis(crypto.Hash.prototype.update); -const HashDigest = uncurryThis(crypto.Hash.prototype.digest); -const BufferToString = uncurryThis(Buffer.prototype.toString); +const { internalVerifyIntegrity } = internalBinding('crypto'); const kRelativeURLStringPattern = /^\.{0,2}\//; const { getOptionValue } = require('internal/options'); const shouldAbortOnUncaughtException = getOptionValue( @@ -589,16 +583,13 @@ class Manifest { // Avoid clobbered Symbol.iterator for (let i = 0; i < integrityEntries.length; i++) { const { algorithm, value: expected } = integrityEntries[i]; - const hash = createHash(algorithm); // TODO(tniessen): the content should not be passed as a string in the // first place, see https://github.com/nodejs/node/issues/39707 - HashUpdate(hash, content, 'utf8'); - const digest = HashDigest(hash, 'buffer'); - if (digest.length === expected.length && - timingSafeEqual(digest, expected)) { + const mismatchedIntegrity = internalVerifyIntegrity(algorithm, content, expected); + if (mismatchedIntegrity === undefined) { return true; } - realIntegrities.set(algorithm, BufferToString(digest, 'base64')); + realIntegrities.set(algorithm, mismatchedIntegrity); } } diff --git a/src/crypto/crypto_hash.cc b/src/crypto/crypto_hash.cc index 5627bac590f2b3..12dd5de3bd7de7 100644 --- a/src/crypto/crypto_hash.cc +++ b/src/crypto/crypto_hash.cc @@ -67,6 +67,9 @@ void Hash::Initialize(Environment* env, Local target) { SetMethodNoSideEffect(context, target, "getHashes", GetHashes); HashJob::Initialize(env, target); + + SetMethodNoSideEffect( + context, target, "internalVerifyIntegrity", InternalVerifyIntegrity); } void Hash::RegisterExternalReferences(ExternalReferenceRegistry* registry) { @@ -76,6 +79,8 @@ void Hash::RegisterExternalReferences(ExternalReferenceRegistry* registry) { registry->Register(GetHashes); HashJob::RegisterExternalReferences(registry); + + registry->Register(InternalVerifyIntegrity); } void Hash::New(const FunctionCallbackInfo& args) { @@ -308,5 +313,50 @@ bool HashTraits::DeriveBits( return true; } +void InternalVerifyIntegrity(const v8::FunctionCallbackInfo& args) { + Environment* env = Environment::GetCurrent(args); + + CHECK_EQ(args.Length(), 3); + + CHECK(args[0]->IsString()); + Utf8Value algorithm(env->isolate(), args[0]); + + CHECK(args[1]->IsString() || IsAnyBufferSource(args[1])); + ByteSource content = ByteSource::FromStringOrBuffer(env, args[1]); + + CHECK(args[2]->IsArrayBufferView()); + ArrayBufferOrViewContents expected(args[2]); + + const EVP_MD* md_type = EVP_get_digestbyname(*algorithm); + unsigned char digest[EVP_MAX_MD_SIZE]; + unsigned int digest_size; + if (md_type == nullptr || EVP_Digest(content.data(), + content.size(), + digest, + &digest_size, + md_type, + nullptr) != 1) { + return ThrowCryptoError( + env, ERR_get_error(), "Digest method not supported"); + } + + if (digest_size != expected.size() || + CRYPTO_memcmp(digest, expected.data(), digest_size) != 0) { + Local error; + MaybeLocal rc = + StringBytes::Encode(env->isolate(), + reinterpret_cast(digest), + digest_size, + BASE64, + &error); + if (rc.IsEmpty()) { + CHECK(!error.IsEmpty()); + env->isolate()->ThrowException(error); + return; + } + args.GetReturnValue().Set(rc.FromMaybe(Local())); + } +} + } // namespace crypto } // namespace node diff --git a/src/crypto/crypto_hash.h b/src/crypto/crypto_hash.h index 96a9804420db63..2d17c3510ed214 100644 --- a/src/crypto/crypto_hash.h +++ b/src/crypto/crypto_hash.h @@ -82,6 +82,8 @@ struct HashTraits final { using HashJob = DeriveBitsJob; +void InternalVerifyIntegrity(const v8::FunctionCallbackInfo& args); + } // namespace crypto } // namespace node diff --git a/test/fixtures/policy/crypto-hash-tampering/.gitattributes b/test/fixtures/policy/crypto-hash-tampering/.gitattributes new file mode 100644 index 00000000000000..cbdcbbc258e6e7 --- /dev/null +++ b/test/fixtures/policy/crypto-hash-tampering/.gitattributes @@ -0,0 +1 @@ +*.js text eol=lf diff --git a/test/fixtures/policy/crypto-hash-tampering/main.js b/test/fixtures/policy/crypto-hash-tampering/main.js new file mode 100644 index 00000000000000..2ee233fe75461b --- /dev/null +++ b/test/fixtures/policy/crypto-hash-tampering/main.js @@ -0,0 +1,8 @@ +const h = require('crypto').createHash('sha384'); +const fakeDigest = h.digest(); + +const kHandle = Object.getOwnPropertySymbols(h) + .find((s) => s.description === 'kHandle'); +h[kHandle].constructor.prototype.digest = () => fakeDigest; + +require('./protected.js'); diff --git a/test/fixtures/policy/crypto-hash-tampering/policy.json b/test/fixtures/policy/crypto-hash-tampering/policy.json new file mode 100644 index 00000000000000..3d981911533f56 --- /dev/null +++ b/test/fixtures/policy/crypto-hash-tampering/policy.json @@ -0,0 +1,15 @@ +{ + "resources": { + "./main.js": { + "integrity": true, + "dependencies": { + "./protected.js": true, + "crypto": true + } + }, + "./protected.js": { + "integrity": "sha384-OLBgp1GsljhM2TJ+sbHjaiH9txEUvgdDTAzHv2P24donTt6/529l+9Ua0vFImLlb", + "dependencies": true + } + } +} diff --git a/test/fixtures/policy/crypto-hash-tampering/protected.js b/test/fixtures/policy/crypto-hash-tampering/protected.js new file mode 100644 index 00000000000000..2b57adba5b191a --- /dev/null +++ b/test/fixtures/policy/crypto-hash-tampering/protected.js @@ -0,0 +1 @@ +console.log(require('fs').readFileSync('/etc/passwd').length); diff --git a/test/parallel/test-policy-crypto-hash-tampering.js b/test/parallel/test-policy-crypto-hash-tampering.js new file mode 100644 index 00000000000000..96066defb59a1b --- /dev/null +++ b/test/parallel/test-policy-crypto-hash-tampering.js @@ -0,0 +1,21 @@ +'use strict'; + +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); +common.requireNoPackageJSONAbove(); + +const fixtures = require('../common/fixtures'); + +const assert = require('assert'); +const { spawnSync } = require('child_process'); + +const mainPath = fixtures.path('policy', 'crypto-hash-tampering', 'main.js'); +const policyPath = fixtures.path( + 'policy', + 'crypto-hash-tampering', + 'policy.json'); +const { status, stderr } = + spawnSync(process.execPath, ['--experimental-policy', policyPath, mainPath], { encoding: 'utf8' }); +assert.strictEqual(status, 1); +assert(stderr.includes('sha384-Bnp/T8gFNzT9mHj2G/AeuMH8LcAQ4mljw15nxBNl5yaGM7VgbMzDT7O4+dXZTJJn'));