Skip to content

Commit 4638ce6

Browse files
joyeecheungtargos
authored andcommitted
src: perform integrity checks on built-in code cache
Currently V8 only checks that the length of the source code is the same as the code used to generate the hash, so we add an additional check here: 1. During compile time, when generating node_javascript.cc and node_code_cache.cc, we compute and include the hash of the (unwrapped) JavaScript source in both. 2. At runtime, we check that the hash of the code being compiled and the hash of the code used to generate the cache (inside the wrapper) is the same. This is based on the assumptions: 1. `internalBinding('code_cache_hash')` must be in sync with `internalBinding('code_cache')` (same C++ file) 2. `internalBinding('natives_hash')` must be in sync with `process.binding('natives')` (same C++ file) 3. If `internalBinding('natives_hash')` is in sync with `internalBinding('natives_hash')`, then the (unwrapped) code used to generate `internalBinding('code_cache')` should be in sync with the (unwrapped) code in `process.binding('natives')` There will be, however, false positives if the wrapper used to generate the cache is different from the one used at run time, and the length of the wrapper somehow stays the same. But that should be rare and can be eased once we make the two bootstrappers cached and checked as well. PR-URL: #22152 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Tiancheng "Timothy" Gu <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gus Caplan <[email protected]>
1 parent eab377f commit 4638ce6

File tree

8 files changed

+100
-15
lines changed

8 files changed

+100
-15
lines changed

lib/internal/bootstrap/cache.js

+1
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ module.exports = {
4848
),
4949
builtinSource: Object.assign({}, NativeModule._source),
5050
getCodeCache,
51+
getSource: NativeModule.getSource,
5152
codeCache: internalBinding('code_cache'),
5253
compiledWithoutCache: NativeModule.compiledWithoutCache,
5354
compiledWithCache: NativeModule.compiledWithCache,

lib/internal/bootstrap/loaders.js

+37-11
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,8 @@
127127
const config = getBinding('config');
128128

129129
const codeCache = getInternalBinding('code_cache');
130+
const codeCacheHash = getInternalBinding('code_cache_hash');
131+
const sourceHash = getInternalBinding('natives_hash');
130132
const compiledWithoutCache = NativeModule.compiledWithoutCache = [];
131133
const compiledWithCache = NativeModule.compiledWithCache = [];
132134

@@ -232,32 +234,56 @@
232234
};
233235

234236
NativeModule.prototype.compile = function() {
235-
let source = NativeModule.getSource(this.id);
237+
const id = this.id;
238+
let source = NativeModule.getSource(id);
236239
source = NativeModule.wrap(source);
237240

238241
this.loading = true;
239242

240243
try {
244+
// Currently V8 only checks that the length of the source code is the
245+
// same as the code used to generate the hash, so we add an additional
246+
// check here:
247+
// 1. During compile time, when generating node_javascript.cc and
248+
// node_code_cache.cc, we compute and include the hash of the
249+
// (unwrapped) JavaScript source in both.
250+
// 2. At runtime, we check that the hash of the code being compiled
251+
// and the hash of the code used to generate the cache
252+
// (inside the wrapper) is the same.
253+
// This is based on the assumptions:
254+
// 1. `internalBinding('code_cache_hash')` must be in sync with
255+
// `internalBinding('code_cache')` (same C++ file)
256+
// 2. `internalBinding('natives_hash')` must be in sync with
257+
// `process.binding('natives')` (same C++ file)
258+
// 3. If `internalBinding('natives_hash')` is in sync with
259+
// `internalBinding('natives_hash')`, then the (unwrapped)
260+
// code used to generate `internalBinding('code_cache')`
261+
// should be in sync with the (unwrapped) code in
262+
// `process.binding('natives')`
263+
// There will be, however, false positives if the wrapper used
264+
// to generate the cache is different from the one used at run time,
265+
// and the length of the wrapper somehow stays the same.
266+
// But that should be rare and can be eased once we make the
267+
// two bootstrappers cached and checked as well.
268+
const cache = codeCacheHash[id] &&
269+
(codeCacheHash[id] === sourceHash[id]) ? codeCache[id] : undefined;
270+
241271
// (code, filename, lineOffset, columnOffset
242272
// cachedData, produceCachedData, parsingContext)
243273
const script = new ContextifyScript(
244274
source, this.filename, 0, 0,
245-
codeCache[this.id], false, undefined
275+
cache, false, undefined
246276
);
247277

278+
// This will be used to create code cache in tools/generate_code_cache.js
248279
this.script = script;
249280

250281
// One of these conditions may be false when any of the inputs
251282
// of the `node_js2c` target in node.gyp is modified.
252-
// FIXME(joyeecheung):
253-
// 1. Figure out how to resolve the dependency issue. When the
254-
// code cache was introduced we were at a point where refactoring
255-
// node.gyp may not be worth the effort.
256-
// 2. Calculate checksums in both js2c and generate_code_cache.js
257-
// and compare them before compiling the native modules since
258-
// V8 only checks the length of the source to decide whether to
259-
// reject the cache.
260-
if (!codeCache[this.id] || script.cachedDataRejected) {
283+
// FIXME(joyeecheung): Figure out how to resolve the dependency issue.
284+
// When the code cache was introduced we were at a point where refactoring
285+
// node.gyp may not be worth the effort.
286+
if (!cache || script.cachedDataRejected) {
261287
compiledWithoutCache.push(this.id);
262288
} else {
263289
compiledWithCache.push(this.id);

src/node.cc

+8
Original file line numberDiff line numberDiff line change
@@ -1732,6 +1732,14 @@ static void GetInternalBinding(const FunctionCallbackInfo<Value>& args) {
17321732
// internalBinding('code_cache')
17331733
exports = Object::New(env->isolate());
17341734
DefineCodeCache(env, exports);
1735+
} else if (!strcmp(*module_v, "code_cache_hash")) {
1736+
// internalBinding('code_cache_hash')
1737+
exports = Object::New(env->isolate());
1738+
DefineCodeCacheHash(env, exports);
1739+
} else if (!strcmp(*module_v, "natives_hash")) {
1740+
// internalBinding('natives_hash')
1741+
exports = Object::New(env->isolate());
1742+
DefineJavaScriptHash(env, exports);
17351743
} else {
17361744
return ThrowIfNoSuchModule(env, *module_v);
17371745
}

src/node_code_cache.h

+1
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
namespace node {
99

1010
void DefineCodeCache(Environment* env, v8::Local<v8::Object> target);
11+
void DefineCodeCacheHash(Environment* env, v8::Local<v8::Object> target);
1112

1213
} // namespace node
1314

src/node_code_cache_stub.cc

+6
Original file line numberDiff line numberDiff line change
@@ -11,4 +11,10 @@ void DefineCodeCache(Environment* env, v8::Local<v8::Object> target) {
1111
// (here as `target`) so this is a noop.
1212
}
1313

14+
void DefineCodeCacheHash(Environment* env, v8::Local<v8::Object> target) {
15+
// When we do not produce code cache for builtin modules,
16+
// `internalBinding('code_cache_hash')` returns an empty object
17+
// (here as `target`) so this is a noop.
18+
}
19+
1420
} // namespace node

src/node_javascript.h

+1
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
namespace node {
3030

3131
void DefineJavaScript(Environment* env, v8::Local<v8::Object> target);
32+
void DefineJavaScriptHash(Environment* env, v8::Local<v8::Object> target);
3233
v8::Local<v8::String> NodePerContextSource(v8::Isolate* isolate);
3334
v8::Local<v8::String> LoadersBootstrapperSource(Environment* env);
3435
v8::Local<v8::String> NodeBootstrapperSource(Environment* env);

tools/generate_code_cache.js

+29-3
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,17 @@
99

1010
const {
1111
getCodeCache,
12+
getSource,
1213
cachableBuiltins
1314
} = require('internal/bootstrap/cache');
1415

16+
function hash(str) {
17+
if (process.versions.openssl) {
18+
return require('crypto').createHash('sha256').update(str).digest('hex');
19+
}
20+
return '';
21+
}
22+
1523
const fs = require('fs');
1624

1725
const resultPath = process.argv[2];
@@ -51,6 +59,8 @@ function getInitalizer(key, cache) {
5159
const defName = key.replace(/\//g, '_').replace(/-/g, '_');
5260
const definition = `static uint8_t ${defName}_raw[] = {\n` +
5361
`${cache.join(',')}\n};`;
62+
const source = getSource(key);
63+
const sourceHash = hash(source);
5464
const initializer = `
5565
v8::Local<v8::ArrayBuffer> ${defName}_ab =
5666
v8::ArrayBuffer::New(isolate, ${defName}_raw, ${cache.length});
@@ -60,13 +70,19 @@ function getInitalizer(key, cache) {
6070
FIXED_ONE_BYTE_STRING(isolate, "${key}"),
6171
${defName}_array).FromJust();
6272
`;
73+
const hashIntializer = `
74+
target->Set(context,
75+
FIXED_ONE_BYTE_STRING(isolate, "${key}"),
76+
OneByteString(isolate, "${sourceHash}")).FromJust();
77+
`;
6378
return {
64-
definition, initializer
79+
definition, initializer, hashIntializer, sourceHash
6580
};
6681
}
6782

6883
const cacheDefinitions = [];
6984
const cacheInitializers = [];
85+
const cacheHashInitializers = [];
7086
let totalCacheSize = 0;
7187

7288

@@ -79,11 +95,14 @@ for (const key of cachableBuiltins) {
7995

8096
const length = cachedData.length;
8197
totalCacheSize += length;
82-
const { definition, initializer } = getInitalizer(key, cachedData);
98+
const {
99+
definition, initializer, hashIntializer, sourceHash
100+
} = getInitalizer(key, cachedData);
83101
cacheDefinitions.push(definition);
84102
cacheInitializers.push(initializer);
103+
cacheHashInitializers.push(hashIntializer);
85104
console.log(`Generated cache for '${key}', size = ${formatSize(length)}` +
86-
`, total = ${formatSize(totalCacheSize)}`);
105+
`, hash = ${sourceHash}, total = ${formatSize(totalCacheSize)}`);
87106
}
88107

89108
const result = `#include "node.h"
@@ -106,6 +125,13 @@ void DefineCodeCache(Environment* env, v8::Local<v8::Object> target) {
106125
${cacheInitializers.join('\n')}
107126
}
108127
128+
// The target here will be returned as \`internalBinding('code_cache_hash')\`
129+
void DefineCodeCacheHash(Environment* env, v8::Local<v8::Object> target) {
130+
v8::Isolate* isolate = env->isolate();
131+
v8::Local<v8::Context> context = env->context();
132+
${cacheHashInitializers.join('\n')}
133+
}
134+
109135
} // namespace node
110136
`;
111137

tools/js2c.py

+17-1
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
import re
3636
import sys
3737
import string
38+
import hashlib
3839

3940

4041
def ToCArray(elements, step=10):
@@ -205,6 +206,10 @@ def ReadMacros(lines):
205206
{initializers}
206207
}}
207208
209+
void DefineJavaScriptHash(Environment* env, v8::Local<v8::Object> target) {{
210+
{hash_initializers}
211+
}}
212+
208213
}} // namespace node
209214
"""
210215

@@ -240,6 +245,12 @@ def ReadMacros(lines):
240245
{value}.ToStringChecked(env->isolate())).FromJust());
241246
"""
242247

248+
HASH_INITIALIZER = """\
249+
CHECK(target->Set(env->context(),
250+
FIXED_ONE_BYTE_STRING(env->isolate(), "{key}"),
251+
FIXED_ONE_BYTE_STRING(env->isolate(), "{value}")).FromJust());
252+
"""
253+
243254
DEPRECATED_DEPS = """\
244255
'use strict';
245256
process.emitWarning(
@@ -280,6 +291,7 @@ def JS2C(source, target):
280291
# Build source code lines
281292
definitions = []
282293
initializers = []
294+
hash_initializers = [];
283295

284296
for name in modules:
285297
lines = ReadFile(str(name))
@@ -309,10 +321,12 @@ def JS2C(source, target):
309321
var = name.replace('-', '_').replace('/', '_')
310322
key = '%s_key' % var
311323
value = '%s_value' % var
324+
hash_value = hashlib.sha256(lines).hexdigest()
312325

313326
definitions.append(Render(key, name))
314327
definitions.append(Render(value, lines))
315328
initializers.append(INITIALIZER.format(key=key, value=value))
329+
hash_initializers.append(HASH_INITIALIZER.format(key=name, value=hash_value))
316330

317331
if deprecated_deps is not None:
318332
name = '/'.join(deprecated_deps)
@@ -324,11 +338,13 @@ def JS2C(source, target):
324338
definitions.append(Render(key, name))
325339
definitions.append(Render(value, DEPRECATED_DEPS.format(module=name)))
326340
initializers.append(INITIALIZER.format(key=key, value=value))
341+
hash_initializers.append(HASH_INITIALIZER.format(key=name, value=hash_value))
327342

328343
# Emit result
329344
output = open(str(target[0]), "w")
330345
output.write(TEMPLATE.format(definitions=''.join(definitions),
331-
initializers=''.join(initializers)))
346+
initializers=''.join(initializers),
347+
hash_initializers=''.join(hash_initializers)))
332348
output.close()
333349

334350
def main():

0 commit comments

Comments
 (0)