Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(ses): hostEvaluators lockdown option #2723

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 68 additions & 1 deletion packages/ses/docs/lockdown.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ Each option is explained in its own section below.
| `reporting` | `'platform'` | `'console'` `'none'` | where to report warnings ([details](#reporting-options))
| `unhandledRejectionTrapping` | `'report'` | `'none'` | handling of finalized unhandled rejections ([details](#unhandledrejectiontrapping-options)) |
| `evalTaming` | `'safe-eval'` | `'unsafe-eval'` `'no-eval'` | `eval` and `Function` of the start compartment ([details](#evaltaming-options)) |
| `hostEvaluators` | `'all'` | `'none'` `'no-direct'` | handling of sloppy and indirect eval ([details](#hostevaluators-options)) |
| `stackFiltering` | `'concise'` | `'verbose'` | deep stacks signal/noise ([details](#stackfiltering-options)) |
| `overrideTaming` | `'moderate'` | `'min'` or `'severe'` | override mistake antidote ([details](#overridetaming-options)) |
| `overrideDebug` | `[]` | array of property names | detect override mistake ([details](#overridedebug-options)) |
Expand All @@ -51,6 +52,7 @@ for threading environment variables into a JavaScript program.
| `reporting` | `LOCKDOWN_REPORTING` | |
| `unhandledRejectionTrapping` | `LOCKDOWN_UNHANDLED_REJECTION_TRAPPING` | |
| `evalTaming` | `LOCKDOWN_EVAL_TAMING` | |
| `hostEvaluators` | `LOCKDOWN_HOST_EVALUATORS` | |
| `stackFiltering` | `LOCKDOWN_STACK_FILTERING` | |
| `overrideTaming` | `LOCKDOWN_OVERRIDE_TAMING` | |
| `overrideDebug` | `LOCKDOWN_OVERRIDE_DEBUG` | comma separated names |
Expand Down Expand Up @@ -463,7 +465,7 @@ the container to exit explicitly, and we highly recommend setting

## `reporting` Options

**Background**: Lockdown and `repairIntrinsics` report warnings if they
**Background**: `lockdown` and `repairIntrinsics` report warnings if they
encounter unexpected but repairable variations on the shared intrinsics, which
regularly occurs if the version of `ses` predates the introduction of new
language features.
Expand Down Expand Up @@ -617,6 +619,71 @@ LOCKDOWN_EVAL_TAMING=no-eval
LOCKDOWN_EVAL_TAMING=unsafe-eval
```

## `hostEvaluators` Options

**Background**: Hermes is a JavaScript engine that does not yet support direct `eval()` nor the `with` statement. The SES `evalTaming` default option `"safe-eval"` uses multiple nested `with` statements to create a restricted scope chain, so on Hermes we must run under the `"unsafe-eval"` option. However SES cannot initialize unless 'eval' is the original intrinsic 'eval', suitable for direct-eval (dynamically scoped eval), which is where we introduce the `hostEvaluators` option `"no-direct"`.

```js
lockdown(); // hostEvaluators defaults to 'all' and warns `SES Please now use the 'hostEvaluators' option. In the future not specifying 'none' will error under strict CSP.` if not set
// or
lockdown({ hostEvaluators: 'all' }); // SES fails to initialize if direct-eval is not the original intrinsic 'eval'
// vs
lockdown({ hostEvaluators: 'none' }); // SES initializes when evaluators are not allowed to execute (e.g. a strict CSP)
// vs
lockdown({ hostEvaluators: 'no-direct' }); // SES initializes if direct-eval is not the original intrinsic 'eval' (e.g. Hermes)
```

Further

* `'all'`: asserts evaluators are allowed to execute
* `'none'`: asserts evaluators are *not* allowed to execute
* `'no-direct'`: asserts direct-eval is not available

Hermes examples

```js
lockdown({ evalTaming: 'unsafe-eval', hostEvaluators: 'no-direct' });
lockdown({ evalTaming: 'no-eval', hostEvaluators: 'no-direct' });
```

However, attempting `"safe-eval"` with `"no-direct"` we fail early, due to Hermes' lack of `with` statement

```js
lockdown({ evalTaming: 'safe-eval', hostEvaluators: 'no-direct' }); // Fail: `lockdown(): option { evalTaming: 'safe-eval' } is incompatible with { hostEvaluators: 'no-direct' }`
```

Since Compartments currently evaluate using `safe-eval` by default, we throw a descriptive error on attempt to `.evaluate`: `'Compartment evaluation not supported without direct eval.'` However this will only be surfaced once Compartment creation is supported on Hermes under `lockdown` (currently incompatible with `removeUnpermittedIntrinsics`).

For users with a strict CSP, `hostEvaluators` defaulting to `all` is a breaking change and will error (SES_DIRECT_EVAL), so for backward-compatibility we warn instead to set it to `none`.

If `lockdown` does not receive a `hostEvaluators` option, it will respect
`process.env.LOCKDOWN_HOST_EVALUATORS`.

```console
LOCKDOWN_HOST_EVALUATORS=all
LOCKDOWN_HOST_EVALUATORS=none
LOCKDOWN_HOST_EVALUATORS=no-direct
```

Once Hermes engine supports direct-eval, `'no-direct'` will no longer be required.
Currently there is an open feature request and open pull request targeting Static Hermes.

* <https://github.com/facebook/hermes/issues/957>
* <https://github.com/facebook/hermes/pull/1515>

You can also test and verify `lockdown` completing on this change by building and running Hermes on the following fork for example:
<https://github.com/leotm/hermes/tree/ses-lockdown-test-static-hermes-compiler-vm>

Once Hermes engine supports the `with` statement, `evalTaming: 'safe-eval'` will be possible.
Currently there is an open feature request and open pull request targeting Static Hermes.

* <https://github.com/facebook/hermes/issues/1056>
* <https://github.com/facebook/hermes/pull/1571>

There is also an open alternate idea to sandbox `Compartment` _without_ the `with` statement.

* <https://github.com/endojs/endo/discussions/1944>

## `stackFiltering` Options

**Background**: The error stacks shown by many JavaScript engines are
Expand Down
32 changes: 31 additions & 1 deletion packages/ses/error-codes/SES_DIRECT_EVAL.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,37 @@
The SES Hardened JavaScript shim captures the `eval` function when it is
initialized.
The `eval` function it finds must be the original `eval` because SES uses its
dynamic scope to implement its isolated eval.
dynamic scope to implement its isolated eval.

If you see this error, something running before `ses` initialized, most likely
another instance of `ses`, has replaced `eval` with something else.

If you're running under an environment that doesn't support direct eval (Hermes),
try setting `hostEvaluators` to `no-direct`.

If you're running under CSP, try setting `hostEvaluators` to `none`.

# 'hostEvaluators' was set to 'all', but the Function() constructor and eval() are not allowed to execute (SES_DIRECT_EVAL)

The default option 'all' expects all evaluators to be allowed to execute, but they seem blocked.

If you're running under CSP, try setting `hostEvaluators` to `none`.

# 'hostEvaluators' was set to 'none', but the Function() constructor or eval() are allowed to execute (SES_DIRECT_EVAL)

You indicated that you expected evaluators to be blocked (e.g. by Content Security Policy),
but they actually work. It's either a misunderstanding of `hostEvaluators` option or whatever
method was used to block evaluators no longer works.

# 'hostEvaluators' was set to 'no-direct', but direct eval is available (SES_DIRECT_EVAL)

If evaluators are working, including direct eval, it seems you've upgraded to a version of
the host environment that now supports them (future Hermes). Or you're running code intended
for Hermes in a different host.

# 'hostEvaluators' was set to 'no-direct', but all evaluators seem blocked (SES_DIRECT_EVAL)

You indicated that you expected a host with only indirect eval available (Hermes), but
all evaluators appear to be blocked.

If you're running under CSP, try setting `hostEvaluators` to `none`.
8 changes: 3 additions & 5 deletions packages/ses/scripts/hermes-test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,9 @@ $HERMESC test/_hermes-smoke-dist.js -emit-binary -out test/_hermes-smoke-dist.hb
echo "Generated: test/_hermes-smoke-dist.hbc"
echo "Hermes compiler done"

# TODO: Disabled until https://github.com/endojs/endo/issues/1891 complete
# echo "Executing generated bytecode file on Hermes VM"
# $HERMES -b test/_hermes-smoke-dist.hbc
# echo "Hermes VM done"
echo "Skipping: Hermes VM"
echo "Executing generated bytecode file on Hermes VM"
$HERMES -b test/_hermes-smoke-dist.hbc
echo "Hermes VM done"

echo "Hermes tests complete"

Expand Down
22 changes: 20 additions & 2 deletions packages/ses/src/compartment.js
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,7 @@ defineProperties(InertCompartment, {
* @param {object} [options]
* @param {Compartment} [options.parentCompartment]
* @param {boolean} [options.enforceNew]
* @param {string} [options.hostEvaluators]
* @returns {Compartment['constructor']}
*/

Expand Down Expand Up @@ -270,7 +271,12 @@ export const makeCompartmentConstructor = (
targetMakeCompartmentConstructor,
intrinsics,
markVirtualizedNativeFunction,
{ parentCompartment = undefined, enforceNew = false } = {},
// eslint-disable-next-line default-param-last
{
parentCompartment = undefined,
enforceNew = false,
hostEvaluators = undefined,
} = {},
) => {
function Compartment(...args) {
if (enforceNew && new.target === undefined) {
Expand Down Expand Up @@ -326,6 +332,18 @@ export const makeCompartmentConstructor = (
sloppyGlobalsMode: false,
});

let evaluator;

if (hostEvaluators === 'no-direct') {
evaluator = () => {
throw TypeError(
'Compartment evaluation not supported without direct eval.',
);
};
} else {
evaluator = safeEvaluate;
}

setGlobalObjectMutableProperties(globalObject, {
intrinsics,
newGlobalPropertyNames: sharedGlobalPropertyNames,
Expand All @@ -337,7 +355,7 @@ export const makeCompartmentConstructor = (
// TODO: maybe add evalTaming to the Compartment constructor 3rd options?
setGlobalObjectEvaluators(
globalObject,
safeEvaluate,
evaluator,
markVirtualizedNativeFunction,
);

Expand Down
5 changes: 4 additions & 1 deletion packages/ses/src/global-object.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import { constantProperties, universalPropertyNames } from './permits.js';
* guest programs, we cannot emulate the proper behavior.
* With this shim, assigning Symbol.unscopables causes the given lexical
* names to fall through to the terminal scope proxy.
* But, we can install this setter to prevent a program from proceding on
* But, we can install this setter to prevent a program from proceeding on
* this false assumption.
*
* @param {object} globalObject
Expand Down Expand Up @@ -75,6 +75,7 @@ export const setGlobalObjectConstantProperties = globalObject => {
* @param {Function} args.makeCompartmentConstructor
* @param {(object) => void} args.markVirtualizedNativeFunction
* @param {Compartment} [args.parentCompartment]
* @param {string} [args.hostEvaluators]
*/
export const setGlobalObjectMutableProperties = (
globalObject,
Expand All @@ -84,6 +85,7 @@ export const setGlobalObjectMutableProperties = (
makeCompartmentConstructor,
markVirtualizedNativeFunction,
parentCompartment,
hostEvaluators,
},
) => {
for (const [name, intrinsicName] of entries(universalPropertyNames)) {
Expand Down Expand Up @@ -121,6 +123,7 @@ export const setGlobalObjectMutableProperties = (
parentCompartment,
enforceNew: true,
},
hostEvaluators,
),
);

Expand Down
Loading
Loading