fix(ses): Add Node.js domain hazard mitigation (merge #850)

packages/ses/NEWS.md

@@ -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)
 

packages/ses/error-codes/SES_NO_DOMAINS.md

@@ -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

packages/ses/index.d.ts

@@ -25,6 +25,7 @@ export interface LockdownOptions {
   stackFiltering?: 'concise' | 'verbose';
   overrideTaming?: 'moderate' | 'min' | 'severe';
   overrideDebug?: Array<string>;
+  domainTaming?: 'safe' | 'unsafe';
   __allowUnsafeMonkeyPatching__?: 'safe' | 'unsafe';
 }
 

packages/ses/index.js

@@ -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';

packages/ses/lockdown-options.md

@@ -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
@@ -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

packages/ses/src/lockdown-shim.js

@@ -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';
@@ -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
@@ -173,6 +175,7 @@ export const repairIntrinsics = (
     overrideTaming,
     overrideDebug,
     stackFiltering,
+    domainTaming,
     __allowUnsafeMonkeyPatching__,
   };
 
@@ -220,6 +223,9 @@ export const repairIntrinsics = (
   /**
    * 1. TAME powers & gather intrinsics first.
    */
+
+  tameDomains(domainTaming);
+
   const {
     addIntrinsics,
     completePrototypes,

packages/ses/src/tame-domains.js

@@ -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,
+    });
+  }
+}

packages/ses/test/test-tame-domains-after-lockdown.js

@@ -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();
+  }
+});

packages/ses/test/test-tame-domains-before-lockdown.js

@@ -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' }));
+});