search

endojs / endo

Endo is a distributed secure JavaScript sandbox, based on SES
Apache License 2.0
762 stars 68 forks source link

SES lockdown Hermes compat tracking issue #1891

Open leotm opened 6 months ago

leotm commented 6 months ago

Briefly demo'd https://github.com/MetaMask/metamask-mobile/pull/8009, a minimally transformed ses@0.18.8 wip that runs on Hermes (latest React Native default JS engine)

here's the omnibus of issues/mods below, while figuring which parts we definitely need (vs can skip/remove in a codemod) cc @erights would appreciate any thoughts

Bundling (Metro, release)

Runtime

Tested across bundled Hermes for RN

Nb: latest minor version tags above, side-by-side with Hermes tarballs downloaded during build

async functions

# ...

metamask-mobile/android/app/build/ASSETS/createBundleProdDebugJsAndAssets/index.android.bundle: error: async functions are unsupported
      const loadWithoutErrorAnnotation = async (compartmentPrivateFields, moduleAliases, compartment, moduleSpecifier, pendingJobs, moduleLoads, errors) => {

metamask-mobile/android/app/build/ASSETS/createBundleProdDebugJsAndAssets/index.android.bundle: error: async functions are unsupported
      const memoizedLoadWithErrorAnnotation = async (compartmentPrivateFields, moduleAliases, compartment, moduleSpecifier, pendingJobs, moduleLoads, errors) => {

metamask-mobile/android/app/build/ASSETS/createBundleProdDebugJsAndAssets/index.android.bundle: error: async functions are unsupported
      const load = async (compartmentPrivateFields, moduleAliases, compartment, moduleSpecifier) => {

mobile/android/app/build/ASSETS/createBundleProdDebugJsAndAssets/index.android.bundle:1233 error: async generators are unsupported
        async function* AsyncGeneratorFunctionInstance() {}

# ...

nb: interestingly, Hermes seems to bundle fine and not complain about async function AsyncFunctionInstance() { }

encountered bundling the app with Metro (considering testing Re.Pack)

# via RN CLI
yarn watch:clean
yarn start:android # react-native run-android --variant=prodDebug
# or via Gradle Wrapper
./gradlew clean # after changes to SES
./gradlew :app:createBundleProdDebugJsAndAssets # bundle only
./gradlew :app:installProdDebug -PreactNativeArchitectures=arm64-v8a # bundle then build for M2

Since Hermes async and await support is In Progress (let alone async generators)

so Hermes doesn't support async arrow functions directly

but we're getting indirect support later via @babel/preset-env

# regenerator-runtime
metamask@7.15.0 /Users/leo/Documents/GitHub/metamask-mobile
├─┬ @babel/runtime@7.23.2
│ └── regenerator-runtime@0.14.0
# ...
# no longer babel-plugin-transform-async-to-generator
metamask@7.15.0 /Users/leo/Documents/GitHub/metamask-mobile
├─┬ @babel/preset-env@7.23.2
│ └── @babel/plugin-transform-async-to-generator@7.22.5
└─┬ metro-react-native-babel-preset@0.73.10
  └── @babel/plugin-transform-async-to-generator@7.22.5 deduped
# @babel/plugin-transform-arrow-functions
metamask@7.15.0 /Users/leo/Documents/GitHub/metamask-mobile
├─┬ @babel/preset-env@7.23.2
│ └── @babel/plugin-transform-arrow-functions@7.22.5
├─┬ fbjs-scripts@3.0.1
│ └─┬ babel-preset-fbjs@3.4.0
│   └── @babel/plugin-transform-arrow-functions@7.22.5 deduped
├─┬ metro-react-native-babel-preset@0.73.10
│ └── @babel/plugin-transform-arrow-functions@7.22.5 deduped
└─┬ react-native-reanimated@3.1.0
  └── @babel/plugin-transform-arrow-functions@7.22.5 deduped
# @babel/plugin-transform-async-generator-functions
└─┬ @babel/preset-env@7.24.4
  └── @babel/plugin-transform-async-generator-functions@7.24.3
# @babel/plugin-async-generator-functions
metamask@7.15.0 /Users/leo/Documents/GitHub/metamask-mobile
└── (empty)

nb: metro-react-native-babel-preset includes these 37 babel plugins too

In order to call lockdown only, do we still need

If so, re-write to Promises, otherwise skip parts or remove and refactor, or is there a better solution?

TODO: check refactor from async arrow fns to async fns; check latest bundled Hermes versions of RN

No longer an issue on bundled Hermes for RN 0.71.17+ (i.e. not a custom build from source)

assertDirectEvalAvailable

SES cannot initialize unless 'eval' is the

Since eval is Excluded From Support

tameRegExpConstructor

no RegExpSymbol species descriptor

no RegExp[Symbol.species] descriptor, no stack

ses.cjs#4472: TypeError('no RegExp[Symbol.species] descriptor');

Screenshot 2023-11-29 at 11 05 28 am 
// node_modules/typescript/lib/lib.es2015.symbol.wellknown.d.ts
// ...
/// <reference lib="es2015.symbol" />

interface SymbolConstructor {
// ...
    /**
     * A function valued property that is the constructor function that is used to create
     * derived objects.
     */
    readonly species: unique symbol;
// ...
}

This is because it is Excluded From Support on Hermes

Symbol.species and its interactions with JS library functions

For a good reason it seems, noting both warnings

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Symbol/species

Screenshot 2024-02-28 at 6 51 52 pm

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/RegExp/@@species

Screenshot 2024-03-04 at 11 49 57 am

So we likely want this part skipped or tameRegExpConstructor option to skip (plus remaining changes, then upstream)

    if( !speciesDesc) {
      throw TypeError('no RegExp[Symbol.species] descriptor'); // Thrown on Hermes since excluded from support
     }

fixed in

tameSymbolConstructor

property is not configurable Screenshot 2024-03-04 at 2 23 27 pm

property is not configurable, no stack / at [object CallSite]

ses.cjs#9481: defineProperties(SharedSymbol, descs);

Screenshot 2023-12-05 at 1 41 49 pm Screenshot 2023-12-05 at 3 41 43 pm

We can fix this simply by omitting configurable: true on Hermes

  const descs = fromEntries(
    arrayMap(originalDescsEntries, ([name, desc]) => [
      name,
      { ...desc }, // configurable: true, omitted
    ]),
  );

in order to tame the Symbol constructor (used on constructed compartments), we attempt to temporarily make all props configurable: true so that we can whitelist (remove non-standard) props, then harden them (back to non-configurable)

Hermes has 15 Symbol props on all versions (contrasted to e.g. Node with 18)

on metamask-mobile (Android) when we observe Object.getOwnPropertyDescriptor(originalDescsEntries, name)?.configurable we get 15 non-true props: 1 false prop (length), 14 undefined

nb: on Hermes v0.12.0 built from scratch

➜  bin ./hermes --version
LLVM (http://llvm.org/):
  LLVH version 8.0.0svn
  Optimized build

Hermes JavaScript compiler and Virtual Machine.
  Hermes release version: 0.12.0
  HBC bytecode version: 96

  Features:
    Debugger
    Zip file input
➜  bin ./hermes
>> Object.getOwnPropertyDescriptors(Symbol)
{ length: { value: 0, writable: false, enumerable: false, configurable: true }, name: { value: "Symbol", writable: false, enumerable: false, configurable: true }, prototype: { value: Symbol { [constructor]: [Function Symbol], [description]: [accessor], [toString]: [Function toString], [valueOf]: [Function valueOf], [Symbol(Symbol.toStringTag)]: "Symbol", [Symbol(Symbol.toPrimitive)]: [Function [Symbol.toPrimitive]] }, writable: false, enumerable: false, configurable: false }, for: { value: [Function for], writable: true, enumerable: false, configurable: true }, keyFor: { value: [Function keyFor], writable: true, enumerable: false, configurable: true }, hasInstance: { value: Symbol(Symbol.hasInstance), writable: false, enumerable: false, configurable: false }, iterator: { value: Symbol(Symbol.iterator), writable: false, enumerable: false, configurable: false }, isConcatSpreadable: { value: Symbol(Symbol.isConcatSpreadable), writable: false, enumerable: false, configurable: false }, toPrimitive: { value: Symbol(Symbol.toPrimitive), writable: false, enumerable: false, configurable: false }, toStringTag: { value: Symbol(Symbol.toStringTag), writable: false, enumerable: false, configurable: false }, match: { value: Symbol(Symbol.match), writable: false, enumerable: false, configurable: false }, matchAll: { value: Symbol(Symbol.matchAll), writable: false, enumerable: false, configurable: false }, search: { value: Symbol(Symbol.search), writable: false, enumerable: false, configurable: false }, replace: { value: Symbol(Symbol.replace), writable: false, enumerable: false, configurable: false }, split: { value: Symbol(Symbol.split), writable: false, enumerable: false, configurable: false } }

(i built/ran Hermes from scratch to observe above, since Hermes via eshost-cli on m2 is unsupported via jsvu(mac64arm) and doesn't fetch via esvu(darwin-arm64) and built Hermes via eshost complains hermes: Unknown command line argument '-fenable-tdz' and Hermes playground only prints [object Object] - but thx for demo'ing @gibson042, jealous how it worked so eloquently on your machine :p)

nb: some Hermes Language Features

nb: our tameSymbolConstructor fn

// https://github.com/endojs/endo/blob/master/packages/ses/src/tame-symbol-constructor.js
/**
 * This taming provides a tamed alternative to the original `Symbol` constructor
 * that starts off identical, except that all its properties are "temporarily"
 * configurable. The original `Symbol` constructor remains unmodified on
 * the start compartment's global. The tamed alternative is used as the shared
 * `Symbol` constructor on constructed compartments.
 *
 * Starting these properties as configurable assumes two succeeding phases of
 * processing: A whitelisting phase, that
 * removes all properties not on the whitelist (which requires them to be
 * configurable) and a global hardening step that freezes all primordials,
 * returning these properties to their expected non-configurable status.
 *
 * The ses shim is constructed to eventually enable vetted shims to run between
 * repair and global hardening. However, such vetted shims would normally
 * run in the start compartment, which continues to use the original unmodified
 * `Symbol`, so they should not normally be affected by the temporary
 * configurability of these properties.
 *
 * Note that the spec refers to the global `Symbol` function as the
 * ["Symbol Constructor"](https://tc39.es/ecma262/multipage/fundamental-objects.html#sec-symbol-constructor)
 * even though it has a call behavior (can be called as a function) and does not
 * not have a construct behavior (cannot be called with `new`). Accordingly,
 * to tame it, we must replace it with a function without a construct
 * behavior.
 */

nb: a SymbolTaming lockdown option doesn't exist yet (same with FunctionTaming on tameFunctionConstructors)

solutions

extra

security considerations pending further thought

getAnonymousIntrinsics

Conflicting definitions of InertAsyncFunction

Conflicting definitions of %InertAsyncFunction%, no stack

ses.cjs#3688: TypeError( `Conflicting definitions of ${name}`);

Screenshot 2023-12-05 at 3 14 18 pm

No longer an issue (was only occurring in initial ses.cjs iteration)

completePrototypes

lockdown prototype property not whitelisted

lockdown.prototype property not whitelisted, no stack

ses.cjs#3730: TypeError( `${name}.prototype property not whitelisted`)

whitelistIntrinsics

Unexpected intrinsic intrinsics isFinite _proto_

Unexpected intrinsic intrinsics.isFinite.__proto__ at %FunctionPrototype%, no stack

ses.cjs#3953: TypeError( `Unexpected intrinsic ${path}.__proto__ at ${protoName}`)

Screenshot 2023-12-05 at 2 51 49 pm

isFinite appears to be the first of many intrinsics

No more SES TypeErrors thrown after whitelisting intrinsics

hardenIntrinsics

No errors visible, but app hangs loading (under investigation)

Solution: __hardenTaming__: 'unsafe' (default safe)

https://github.com/endojs/endo/blob/master/packages/ses/docs/lockdown.md#__hardentaming__-options

kriskowal commented 6 months ago

My understanding of this issue is that we’re tracking integration problems with Hermes and some of these may motivate recommendations to change the SES shim to broaden its portability. (I’m driving similar work with XS.) You also are getting by with some transformations and manual edits of the SES shim that might upstream.

@leotm Do you plan to propose changes?

One of the things I am thinking of doing for XS is elaborate the "exports" in package.json to use a custom variant of SES for that platform. That amounts to including only certain layers, and having some XS-specific layers. I imagine something similar will be in order for Hermes.

{
  "exports": {
    "hermes": {},
    "xsnap": {},
    "default": {},
  }
}
leotm commented 5 months ago

My understanding of this issue is that we’re tracking integration problems with Hermes and some of these may motivate recommendations to change the SES shim to broaden its portability. (I’m driving similar work with XS.) You also are getting by with some transformations and manual edits of the SES shim that might upstream.

@leotm Do you plan to propose changes?

One of the things I am thinking of doing for XS is elaborate the "exports" in package.json to use a custom variant of SES for that platform. That amounts to including only certain layers, and having some XS

Exactly ^ renamed to tracking issue, was initially only planning on running a local codemod once functional, but a new separate export (non-async SES shim) for Hermes makes sense 👍 like you're thinking for XS

leotm commented 4 months ago

Updated description with deeper dive on tameSymbolConstructor with couple solutions ^ security considerations pending further thought