Skip to content

Commit

Permalink
src: perform integrity checks on built-in code cache
Browse files Browse the repository at this point in the history
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 <anna@addaleax.net>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
  • Loading branch information
joyeecheung committed Aug 17, 2018
1 parent 478a78b commit 7cbbb27
Show file tree
Hide file tree
Showing 8 changed files with 100 additions and 15 deletions.
1 change: 1 addition & 0 deletions lib/internal/bootstrap/cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ module.exports = {
),
builtinSource: Object.assign({}, NativeModule._source),
getCodeCache,
getSource: NativeModule.getSource,
codeCache: internalBinding('code_cache'),
compiledWithoutCache: NativeModule.compiledWithoutCache,
compiledWithCache: NativeModule.compiledWithCache,
Expand Down
48 changes: 37 additions & 11 deletions lib/internal/bootstrap/loaders.js
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,8 @@
const config = getBinding('config');

const codeCache = getInternalBinding('code_cache');
const codeCacheHash = getInternalBinding('code_cache_hash');
const sourceHash = getInternalBinding('natives_hash');
const compiledWithoutCache = NativeModule.compiledWithoutCache = [];
const compiledWithCache = NativeModule.compiledWithCache = [];

Expand Down Expand Up @@ -232,32 +234,56 @@
};

NativeModule.prototype.compile = function() {
let source = NativeModule.getSource(this.id);
const id = this.id;
let source = NativeModule.getSource(id);
source = NativeModule.wrap(source);

this.loading = true;

try {
// 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.
const cache = codeCacheHash[id] &&
(codeCacheHash[id] === sourceHash[id]) ? codeCache[id] : undefined;

// (code, filename, lineOffset, columnOffset
// cachedData, produceCachedData, parsingContext)
const script = new ContextifyScript(
source, this.filename, 0, 0,
codeCache[this.id], false, undefined
cache, false, undefined
);

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

// One of these conditions may be false when any of the inputs
// of the `node_js2c` target in node.gyp is modified.
// FIXME(joyeecheung):
// 1. Figure out how to resolve the dependency issue. When the
// code cache was introduced we were at a point where refactoring
// node.gyp may not be worth the effort.
// 2. Calculate checksums in both js2c and generate_code_cache.js
// and compare them before compiling the native modules since
// V8 only checks the length of the source to decide whether to
// reject the cache.
if (!codeCache[this.id] || script.cachedDataRejected) {
// FIXME(joyeecheung): Figure out how to resolve the dependency issue.
// When the code cache was introduced we were at a point where refactoring
// node.gyp may not be worth the effort.
if (!cache || script.cachedDataRejected) {
compiledWithoutCache.push(this.id);
} else {
compiledWithCache.push(this.id);
Expand Down
8 changes: 8 additions & 0 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1731,6 +1731,14 @@ static void GetInternalBinding(const FunctionCallbackInfo<Value>& args) {
// internalBinding('code_cache')
exports = Object::New(env->isolate());
DefineCodeCache(env, exports);
} else if (!strcmp(*module_v, "code_cache_hash")) {
// internalBinding('code_cache_hash')
exports = Object::New(env->isolate());
DefineCodeCacheHash(env, exports);
} else if (!strcmp(*module_v, "natives_hash")) {
// internalBinding('natives_hash')
exports = Object::New(env->isolate());
DefineJavaScriptHash(env, exports);
} else {
return ThrowIfNoSuchModule(env, *module_v);
}
Expand Down
1 change: 1 addition & 0 deletions src/node_code_cache.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
namespace node {

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

} // namespace node

Expand Down
6 changes: 6 additions & 0 deletions src/node_code_cache_stub.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,10 @@ void DefineCodeCache(Environment* env, v8::Local<v8::Object> target) {
// (here as `target`) so this is a noop.
}

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

} // namespace node
1 change: 1 addition & 0 deletions src/node_javascript.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
namespace node {

void DefineJavaScript(Environment* env, v8::Local<v8::Object> target);
void DefineJavaScriptHash(Environment* env, v8::Local<v8::Object> target);
v8::Local<v8::String> NodePerContextSource(v8::Isolate* isolate);
v8::Local<v8::String> LoadersBootstrapperSource(Environment* env);
v8::Local<v8::String> NodeBootstrapperSource(Environment* env);
Expand Down
32 changes: 29 additions & 3 deletions tools/generate_code_cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,17 @@

const {
getCodeCache,
getSource,
cachableBuiltins
} = require('internal/bootstrap/cache');

function hash(str) {
if (process.versions.openssl) {
return require('crypto').createHash('sha256').update(str).digest('hex');
}
return '';
}

const fs = require('fs');

const resultPath = process.argv[2];
Expand Down Expand Up @@ -51,6 +59,8 @@ function getInitalizer(key, cache) {
const defName = key.replace(/\//g, '_').replace(/-/g, '_');
const definition = `static uint8_t ${defName}_raw[] = {\n` +
`${cache.join(',')}\n};`;
const source = getSource(key);
const sourceHash = hash(source);
const initializer = `
v8::Local<v8::ArrayBuffer> ${defName}_ab =
v8::ArrayBuffer::New(isolate, ${defName}_raw, ${cache.length});
Expand All @@ -60,13 +70,19 @@ function getInitalizer(key, cache) {
FIXED_ONE_BYTE_STRING(isolate, "${key}"),
${defName}_array).FromJust();
`;
const hashIntializer = `
target->Set(context,
FIXED_ONE_BYTE_STRING(isolate, "${key}"),
OneByteString(isolate, "${sourceHash}")).FromJust();
`;
return {
definition, initializer
definition, initializer, hashIntializer, sourceHash
};
}

const cacheDefinitions = [];
const cacheInitializers = [];
const cacheHashInitializers = [];
let totalCacheSize = 0;


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

const length = cachedData.length;
totalCacheSize += length;
const { definition, initializer } = getInitalizer(key, cachedData);
const {
definition, initializer, hashIntializer, sourceHash
} = getInitalizer(key, cachedData);
cacheDefinitions.push(definition);
cacheInitializers.push(initializer);
cacheHashInitializers.push(hashIntializer);
console.log(`Generated cache for '${key}', size = ${formatSize(length)}` +
`, total = ${formatSize(totalCacheSize)}`);
`, hash = ${sourceHash}, total = ${formatSize(totalCacheSize)}`);
}

const result = `#include "node.h"
Expand All @@ -106,6 +125,13 @@ void DefineCodeCache(Environment* env, v8::Local<v8::Object> target) {
${cacheInitializers.join('\n')}
}
// The target here will be returned as \`internalBinding('code_cache_hash')\`
void DefineCodeCacheHash(Environment* env, v8::Local<v8::Object> target) {
v8::Isolate* isolate = env->isolate();
v8::Local<v8::Context> context = env->context();
${cacheHashInitializers.join('\n')}
}
} // namespace node
`;

Expand Down
18 changes: 17 additions & 1 deletion tools/js2c.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import re
import sys
import string
import hashlib


def ToCArray(elements, step=10):
Expand Down Expand Up @@ -205,6 +206,10 @@ def ReadMacros(lines):
{initializers}
}}
void DefineJavaScriptHash(Environment* env, v8::Local<v8::Object> target) {{
{hash_initializers}
}}
}} // namespace node
"""

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

HASH_INITIALIZER = """\
CHECK(target->Set(env->context(),
FIXED_ONE_BYTE_STRING(env->isolate(), "{key}"),
FIXED_ONE_BYTE_STRING(env->isolate(), "{value}")).FromJust());
"""

DEPRECATED_DEPS = """\
'use strict';
process.emitWarning(
Expand Down Expand Up @@ -280,6 +291,7 @@ def JS2C(source, target):
# Build source code lines
definitions = []
initializers = []
hash_initializers = [];

for name in modules:
lines = ReadFile(str(name))
Expand Down Expand Up @@ -309,10 +321,12 @@ def JS2C(source, target):
var = name.replace('-', '_').replace('/', '_')
key = '%s_key' % var
value = '%s_value' % var
hash_value = hashlib.sha256(lines).hexdigest()

definitions.append(Render(key, name))
definitions.append(Render(value, lines))
initializers.append(INITIALIZER.format(key=key, value=value))
hash_initializers.append(HASH_INITIALIZER.format(key=name, value=hash_value))

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

# Emit result
output = open(str(target[0]), "w")
output.write(TEMPLATE.format(definitions=''.join(definitions),
initializers=''.join(initializers)))
initializers=''.join(initializers),
hash_initializers=''.join(hash_initializers)))
output.close()

def main():
Expand Down

0 comments on commit 7cbbb27

Please sign in to comment.