Skip to content

Commit

Permalink
feat(ses): Lockdown option domainTaming
Browse files Browse the repository at this point in the history
  • Loading branch information
kriskowal committed Sep 9, 2021
1 parent 3a3afdd commit ee3e4c3
Show file tree
Hide file tree
Showing 9 changed files with 147 additions and 9 deletions.
9 changes: 9 additions & 0 deletions packages/ses/NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,15 @@ User-visible changes in SES:
setting. At this setting, assigning to the `name` property of a mutable error
instance should work. It will continue not to work at the `'min'` setting, so
use the default `'moderate'` setting if you need to.
- Adds a lockdown option `domainTaming` to detect whether Node.js domains have
been initialized and prevents them from being initialized afterward.
Domains otherwise presented a hazard to the integrity of SES containment on
Node.js.
The option defaults to `"unsafe"` in this version and will be switched to
`"safe"` by default in the next release that allows for breaking-changes, to
afford a gradual migration.
Thank you to @dominictarr with [Least Authority](https://leastauthority.com/)
for devising this feature.

# 0.14.1 (2021-08-12)

Expand Down
21 changes: 21 additions & 0 deletions packages/ses/error-codes/SES_NO_DOMAINS.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# SES failed to lockdown, Node.js domains have been initialized (SES_NO_DOMAINS)

The SES shim cannot guarantee the containment of tenant programs if a program
uses the deprecated [Node.js domains](https://nodejs.org/api/domain.html)
feature.

To work-around this restriction and explicitly remain vulnerable to domains,
call `lockdown` with the [domainTaming][] option set to `'unsafe'`.

SES attempts to detect whether Node.js domains have been initialized, and also
attempts to prevent the domains module from initializing in the future, by
testing and breaking the `process.domain` property.

Domains introduce a `domain` property on various objects, but most
perniciously, every `Promise` object, in a way that SES is unable to intercept
or prevent.
These domain properties are unfrozen, so they can be used for covert
communication between tenant programs, and provide a view into dynamic scope
that any program can directly manipulate to confuse other programs.

[domainTaming]: https://github.com/endojs/endo/blob/master/packages/ses/lockdown-options.md#domaintaming-options
1 change: 1 addition & 0 deletions packages/ses/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ export interface LockdownOptions {
stackFiltering?: 'concise' | 'verbose';
overrideTaming?: 'moderate' | 'min' | 'severe';
overrideDebug?: Array<string>;
domainTaming?: 'safe' | 'unsafe';
__allowUnsafeMonkeyPatching__?: 'safe' | 'unsafe';
}

Expand Down
1 change: 1 addition & 0 deletions packages/ses/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
// limitations under the License.

import { globalThis, TypeError, assign } from './src/commons.js';

import { tameFunctionToString } from './src/tame-function-tostring.js';
import { getGlobalIntrinsics } from './src/intrinsics.js';
import { getAnonymousIntrinsics } from './src/get-anonymous-intrinsics.js';
Expand Down
49 changes: 40 additions & 9 deletions packages/ses/lockdown-options.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,16 @@ Each option is explained in its own section below.

| option | default setting | other settings | about |
|------------------|------------------|----------------|-------|
| `regExpTaming` | `'safe'` | `'unsafe'` | `RegExp.prototype.compile` |
| `localeTaming` | `'safe'` | `'unsafe'` | `toLocaleString` |
| `consoleTaming` | `'safe'` | `'unsafe'` | deep stacks |
| `errorTaming` | `'safe'` | `'unsafe'` | `errorInstance.stack` |
| `errorTrapping` | `'platform'` | `'exit'` `'abort'` `'report'` | handling of uncaught exceptions |
| `stackFiltering` | `'concise'` | `'verbose'` | deep stacks signal/noise |
| `overrideTaming` | `'moderate'` | `'min'` or `'severe'` | override mistake antidote |
| `overrideDebug` | `[]` | array of property names | detect override mistake |
| `__allowUnsafeMonkeyPatching__` | `'safe'` | `'unsafe'` | run unsafe code unsafely |
| `regExpTaming` | `'safe'` | `'unsafe'` | `RegExp.prototype.compile` |
| `localeTaming` | `'safe'` | `'unsafe'` | `toLocaleString` |
| `consoleTaming` | `'safe'` | `'unsafe'` | deep stacks |
| `errorTaming` | `'safe'` | `'unsafe'` | `errorInstance.stack` |
| `errorTrapping` | `'platform'` | `'exit'` `'abort'` `'report'` | handling of uncaught exceptions |
| `stackFiltering` | `'concise'` | `'verbose'` | deep stacks signal/noise |
| `overrideTaming` | `'moderate'` | `'min'` or `'severe'` | override mistake antidote |
| `overrideDebug` | `[]` | array of property names | detect override mistake |
| `domainTaming` | `'unsafe'` | `'safe'` | bans the Node.js `domain` module, to be `'safe'` by default in the next breaking-changes release |
| `__allowUnsafeMonkeyPatching__` | `'safe'` | `'unsafe'` | run unsafe code unsafely |

The options `mathTaming` and `dateTaming` are deprecated.
`Math.random`, `Date.now`, and the `new Date()` are disabled within
Expand Down Expand Up @@ -604,6 +605,36 @@ Error#1: Override property constructor
at packages/ses/test/test-enable-property-overrides-severe-debug.js:14:3
```

## `domainTaming` Options

The deprecated Node.js `domain` module adds `domain` properties to callbacks
and promises.
These `domain` properties allow communication between client programs that
`lockdown` otherwise isolates.

To disable this safety feature, call `lockdown` with `domainTaming` set to
`'unsafe'` explicitly.

```js
lockdown({
domainTaming: 'unsafe'
});
```

The `domainTaming` option, when set to `'safe'`, protects programs
by detecting whether the `'domain'` module has been initialized, and by laying
a trap that prevents it from initializing later.

The `domain` module adds a `domain` object to the `process` global object,
so it's possible to detect without consulting the module system.
Defining a non-configurable `domain` property on the `process` object
causes any later attempt to initialize domains to fail loudly.

Unfortunately, some modules ultimately depend on the `domain` module,
even when they do not actively use its features.
To run multi-tenant applications safely, these dependencies must be carefully
fixed or avoided.

## `__allowUnsafeMonkeyPatching__` Options

Sometimes SES is used where SES's safety is not required. Some libraries
Expand Down
6 changes: 6 additions & 0 deletions packages/ses/src/lockdown-shim.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import tameLocaleMethods from './tame-locale-methods.js';
import { initGlobalObject } from './global-object.js';
import { initialGlobalPropertyNames } from './whitelist.js';
import { tameFunctionToString } from './tame-function-tostring.js';
import { tameDomains } from './tame-domains.js';

import { tameConsole } from './error/tame-console.js';
import tameErrorConstructor from './error/tame-error-constructor.js';
Expand Down Expand Up @@ -139,6 +140,7 @@ export const repairIntrinsics = (
overrideTaming = 'moderate',
overrideDebug = [],
stackFiltering = 'concise',
domainTaming = 'unsafe', // TODO become 'safe' by default in next-breaking-release.
__allowUnsafeMonkeyPatching__ = 'safe',

...extraOptions
Expand Down Expand Up @@ -173,6 +175,7 @@ export const repairIntrinsics = (
overrideTaming,
overrideDebug,
stackFiltering,
domainTaming,
__allowUnsafeMonkeyPatching__,
};

Expand Down Expand Up @@ -220,6 +223,9 @@ export const repairIntrinsics = (
/**
* 1. TAME powers & gather intrinsics first.
*/

tameDomains(domainTaming);

const {
addIntrinsics,
completePrototypes,
Expand Down
46 changes: 46 additions & 0 deletions packages/ses/src/tame-domains.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
// @ts-check

import {
TypeError,
globalThis,
getOwnPropertyDescriptor,
defineProperty,
} from './commons.js';

export function tameDomains(domainTaming = 'safe') {
if (domainTaming !== 'safe' && domainTaming !== 'unsafe') {
throw new TypeError(`unrecognized domainTaming ${domainTaming}`);
}

if (domainTaming === 'unsafe') {
return;
}

// Protect against the hazard presented by Node.js domains.
if (typeof globalThis.process === 'object' && globalThis.process !== null) {
// Check whether domains were initialized.
const domainDescriptor = getOwnPropertyDescriptor(
globalThis.process,
'domain',
);
if (domainDescriptor !== undefined && domainDescriptor.get !== undefined) {
// The domain descriptor on Node.js initially has value: null, which
// becomes a get, set pair after domains initialize.
throw new TypeError(
`SES failed to lockdown, Node.js domains have been initialized (SES_NO_DOMAINS)`,
);
}
// Prevent domains from initializing.
// This is clunky because the exception thrown from the domains package does
// not direct the user's gaze toward a knowledge base about the problem.
// The domain module merely throws an exception when it attempts to define
// the domain property of the process global during its initialization.
// We have no better recourse because Node.js uses defineProperty too.
defineProperty(globalThis.process, 'domain', {
value: null,
configurable: false,
writable: false,
enumerable: false,
});
}
}
16 changes: 16 additions & 0 deletions packages/ses/test/test-tame-domains-after-lockdown.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import '../index.js';
import test from 'ava';

lockdown({ domainTaming: 'safe' });

test('import domains after lockdown', async t => {
try {
await import('domain');
t.fail('importing domain should throw');
} catch (error) {
// This assertion omitted to avoid coupling to a specific engine.
// t.is(error.message, 'Cannot redefine property: domain');
t.log(error);
t.pass();
}
});
7 changes: 7 additions & 0 deletions packages/ses/test/test-tame-domains-before-lockdown.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import '../index.js';
import test from 'ava';
import 'domain';

test('lockdown after domains introduced', async t => {
t.throws(() => lockdown({ domainTaming: 'safe' }));
});

0 comments on commit ee3e4c3

Please sign in to comment.