From bc93615af21d109c0672fc7ec5995c562da2f219 Mon Sep 17 00:00:00 2001 From: "Mark S. Miller" Date: Fri, 16 Aug 2024 20:06:48 -0700 Subject: [PATCH] feat(swingset-liveslots): endow passStyleOf to liveslots guest compartment (#9874) A variant of #9431 . @warner , feel free to just adopt these changes into #9431 rather than reviewing this alternate. closes: #9781 refs: #9431 https://github.com/endojs/endo/pull/2377 https://github.com/endojs/endo/pull/2408 ## Description The code running under liveslots, i.e., user-level vat code such as contracts, must not be able to sense gc. Thus, liveslots endows them with virtual-storage-aware WeakMap and WeakSet, which treats the virtual object as the weakly held key, whereas the builtin WeakMap and WeakSet would treat the momentary representative as the weakly held key. To achieve this, the virtual-storage-aware WeakMap and WeakSet must impose a comparative storage leak. However, some WeakMaps and WeakSets are used purely as an encapsulated unobservable memo, in the sense that the clients of encapsulating abstraction cannot sense whether the memo hit or missed (modulo timing of course, which we can also deny). `passStyleOf` is such an abstraction. Measurements show that the storage leak it causes is significant. The judgement of `passStyleOf` is only to report the pass-style of its arguments, and all virtual objects that have representative have a pass-style of `'remotable'` every time any of its representatives are tested. To avoid this storage leak, https://github.com/endojs/endo/pull/2377 (merged, released, and synced with agoric-sdk) and https://github.com/endojs/endo/pull/2408 (still in review) together enable liveslots to endow the compartment it unbundles with its own efficient `passStyleOf`, built from the primitive WeakMap which it encapsulates. This PR does two things: - makes the change to liveslots to do this endowing, according to the conventions supported by https://github.com/endojs/endo/pull/2377 and https://github.com/endojs/endo/pull/2408 - because https://github.com/endojs/endo/pull/2408 is not yet synced with agoric-sdk, this PR adds an "equivalent" patch, so that we can depend on it before the next endo sync. ### Security Considerations This design *assumes* that the endowed `passStyleOf` makes the memo hits vs misses unobservable, so its dependence on these does not enable the code using it to observe gc. If there is some way to trick it into exposing the difference between a hit and miss, that would be a security concern. ### Scaling Considerations The point. With this PR, the storage leak caused by the `passStyleOf` memo should go away. For some vats, this should be a big improvement. ### Documentation Considerations For the normal developer, none. ### Testing Considerations Adapts the tests originally written by @warner in #9431 , which seem to demonstrate that this works both for node-based and for XS-based vats. ### Upgrade Considerations I don't believe there are any. When linked with an endo preceding even https://github.com/endojs/endo/pull/2377 , the only consequence should be that the storage leak remains unfixed. Likewise, if an endo with https://github.com/endojs/endo/pull/2377 and even https://github.com/endojs/endo/pull/2408 is linked with an agoric-sdk prior to the PR, the only consequence should be that the storage leak remains unfixed. --- packages/SwingSet/package.json | 1 + packages/SwingSet/test/vat-env.test.js | 2 + packages/SwingSet/test/vat-envtest.js | 10 ++- packages/swingset-liveslots/src/liveslots.js | 10 ++- .../test/vat-environment.test.js | 62 +++++++++++++++++++ .../tools/setup-vat-data.js | 13 +++- packages/vats/package.json | 1 + patches/@endo+compartment-mapper+1.2.1.patch | 29 +++++++++ 8 files changed, 118 insertions(+), 10 deletions(-) create mode 100644 packages/swingset-liveslots/test/vat-environment.test.js create mode 100644 patches/@endo+compartment-mapper+1.2.1.patch diff --git a/packages/SwingSet/package.json b/packages/SwingSet/package.json index 9f4171843c16..89d31e1e852c 100644 --- a/packages/SwingSet/package.json +++ b/packages/SwingSet/package.json @@ -49,6 +49,7 @@ "@endo/init": "^1.1.3", "@endo/marshal": "^1.5.2", "@endo/nat": "^5.0.9", + "@endo/pass-style": "^1.4.2", "@endo/patterns": "^1.4.2", "@endo/promise-kit": "^1.1.4", "@endo/ses-ava": "^1.2.4", diff --git a/packages/SwingSet/test/vat-env.test.js b/packages/SwingSet/test/vat-env.test.js index 0f1f29c9298f..1424e1641d88 100644 --- a/packages/SwingSet/test/vat-env.test.js +++ b/packages/SwingSet/test/vat-env.test.js @@ -97,6 +97,8 @@ async function testForExpectedGlobals(t, workerType) { 'VatData.makeScalarBigWeakMapStore: function', 'VatData.makeScalarBigSetStore: function', 'VatData.makeScalarBigWeakSetStore: function', + 'global has passStyleOf: true', + 'global passStyleOf is special: false', ]); } diff --git a/packages/SwingSet/test/vat-envtest.js b/packages/SwingSet/test/vat-envtest.js index e66efd4f9c08..f19c7ba2a219 100644 --- a/packages/SwingSet/test/vat-envtest.js +++ b/packages/SwingSet/test/vat-envtest.js @@ -1,6 +1,8 @@ // @ts-nocheck -/* global VatData */ +/* global VatData globalThis */ import { Far } from '@endo/far'; +import { passStyleOf } from '@endo/pass-style'; +import { PassStyleOfEndowmentSymbol } from '@endo/pass-style/endow.js'; export function buildRootObject(vatPowers) { const log = vatPowers.testLog; @@ -13,6 +15,12 @@ export function buildRootObject(vatPowers) { for (const prop of Object.keys(VatData)) { log(`VatData.${prop}: ${typeof VatData[prop]}`); } + const globalPassStyleOf = + globalThis && globalThis[PassStyleOfEndowmentSymbol]; + log(`global has passStyleOf: ${!!globalPassStyleOf}`); + log( + `global passStyleOf is special: ${globalPassStyleOf !== passStyleOf}`, + ); }, }); } diff --git a/packages/swingset-liveslots/src/liveslots.js b/packages/swingset-liveslots/src/liveslots.js index 1b5b6ca099d1..4490679c4d56 100644 --- a/packages/swingset-liveslots/src/liveslots.js +++ b/packages/swingset-liveslots/src/liveslots.js @@ -1,10 +1,7 @@ import { annotateError, assert, Fail, makeError, X } from '@endo/errors'; -import { - Remotable, - passStyleOf, - getInterfaceOf, - makeMarshal, -} from '@endo/marshal'; +import { passStyleOf } from '@endo/pass-style'; +import { PassStyleOfEndowmentSymbol } from '@endo/pass-style/endow.js'; +import { Remotable, getInterfaceOf, makeMarshal } from '@endo/marshal'; import { isPromise } from '@endo/promise-kit'; import { E, HandledPromise } from '@endo/eventual-send'; import { insistVatType, makeVatSlot, parseVatSlot } from './parseVatSlots.js'; @@ -1363,6 +1360,7 @@ function build( makeScalarBigSetStore: collectionManager.makeScalarBigSetStore, makeScalarBigWeakSetStore: collectionManager.makeScalarBigWeakSetStore, }, + [PassStyleOfEndowmentSymbol]: passStyleOf, }); const inescapableGlobalProperties = harden({ diff --git a/packages/swingset-liveslots/test/vat-environment.test.js b/packages/swingset-liveslots/test/vat-environment.test.js new file mode 100644 index 000000000000..1c1d8e2e48db --- /dev/null +++ b/packages/swingset-liveslots/test/vat-environment.test.js @@ -0,0 +1,62 @@ +// @ts-nocheck +import '@endo/init/debug.js'; +import test from 'ava'; +import { Far } from '@endo/marshal'; +import { kser } from '@agoric/kmarshal'; +import { passStyleOf } from '@endo/pass-style'; +import { PassStyleOfEndowmentSymbol } from '@endo/pass-style/endow.js'; +import { makeLiveSlots } from '../src/index.js'; +import { makeStartVat } from './util.js'; +import { buildSyscall } from './liveslots-helpers.js'; +import { makeMockGC } from './mock-gc.js'; + +test('vat globals', async t => { + const { syscall } = buildSyscall(); + const gcTools = makeMockGC(); + const buildRootObject = () => Far('root', {}); + let called = 0; + let vatGlobals; + let inescapableGlobalProperties; + const vatNS = harden({ buildRootObject }); + // buildVatNamespace + const bVN = async (vG, iGP) => { + called += 1; + vatGlobals = vG; + inescapableGlobalProperties = iGP; + return vatNS; + }; + + const ls = makeLiveSlots(syscall, 'vatA', {}, {}, gcTools, undefined, bVN); + t.is(called, 0); // not called yet + await ls.dispatch(makeStartVat(kser())); + t.is(called, 1); + t.truthy(vatGlobals); + + // 'harden' is provided by SES (installed by the lockdown bundle), + // not liveslots + t.is(typeof vatGlobals.harden, 'undefined'); + + // but liveslots provides VatData + t.is(typeof vatGlobals.VatData, 'object'); + t.is(typeof vatGlobals.VatData, 'object'); + t.is(typeof vatGlobals.VatData.defineKind, 'function'); + t.is(typeof vatGlobals.VatData.defineKindMulti, 'function'); + t.is(typeof vatGlobals.VatData.defineDurableKind, 'function'); + t.is(typeof vatGlobals.VatData.defineDurableKindMulti, 'function'); + t.is(typeof vatGlobals.VatData.makeKindHandle, 'function'); + t.is(typeof vatGlobals.VatData.canBeDurable, 'function'); + t.is(typeof vatGlobals.VatData.providePromiseWatcher, 'function'); + t.is(typeof vatGlobals.VatData.watchPromise, 'function'); + t.is(typeof vatGlobals.VatData.makeScalarBigMapStore, 'function'); + t.is(typeof vatGlobals.VatData.makeScalarBigWeakMapStore, 'function'); + t.is(typeof vatGlobals.VatData.makeScalarBigSetStore, 'function'); + t.is(typeof vatGlobals.VatData.makeScalarBigWeakSetStore, 'function'); + t.is(typeof vatGlobals[PassStyleOfEndowmentSymbol], 'function'); + // this is the passStyleOf created by liveslots, with a real WeakMap + t.is(vatGlobals[PassStyleOfEndowmentSymbol], passStyleOf); + + t.is(typeof inescapableGlobalProperties.WeakMap, 'function'); + t.not(inescapableGlobalProperties.WeakMap, WeakMap); + t.is(typeof inescapableGlobalProperties.WeakSet, 'function'); + t.not(inescapableGlobalProperties.WeakSet, WeakSet); +}); diff --git a/packages/swingset-liveslots/tools/setup-vat-data.js b/packages/swingset-liveslots/tools/setup-vat-data.js index 7ffa9e37224a..c3781d904ce9 100644 --- a/packages/swingset-liveslots/tools/setup-vat-data.js +++ b/packages/swingset-liveslots/tools/setup-vat-data.js @@ -1,8 +1,13 @@ // @ts-check /* global globalThis */ -// This file produces the globalThis.VatData property outside of a running -// SwingSet so that it can be used by '@agoric/vat-data' (which only *consumes* -// `globalThis.VatData`) in code under test. + +// This file produces the globalThis.VatData property outside of a +// running SwingSet so that it can be used by '@agoric/vat-data' +// (which only *consumes* `globalThis.VatData`) in code under test. It +// also populates the passStyleOf symbol-named property. + +import { passStyleOf } from '@endo/pass-style'; +import { PassStyleOfEndowmentSymbol } from '@endo/pass-style/endow.js'; import { makeFakeVirtualStuff } from './fakeVirtualSupport.js'; const { WeakMap, WeakSet } = globalThis; @@ -37,6 +42,8 @@ globalThis.VatData = harden({ fakeVomKit.cm.makeScalarBigWeakSetStore(...args), }); +globalThis[PassStyleOfEndowmentSymbol] = passStyleOf; + export const reincarnate = (options = {}) => { const { fakeStore = new Map(), fakeVomKit: fvk } = options; diff --git a/packages/vats/package.json b/packages/vats/package.json index 821235344a17..0b4a72a13279 100644 --- a/packages/vats/package.json +++ b/packages/vats/package.json @@ -40,6 +40,7 @@ "@endo/import-bundle": "^1.2.1", "@endo/marshal": "^1.5.2", "@endo/nat": "^5.0.9", + "@endo/pass-style": "^1.4.2", "@endo/patterns": "^1.4.2", "@endo/promise-kit": "^1.1.4", "import-meta-resolve": "^2.2.1", diff --git a/patches/@endo+compartment-mapper+1.2.1.patch b/patches/@endo+compartment-mapper+1.2.1.patch new file mode 100644 index 000000000000..549d09833274 --- /dev/null +++ b/patches/@endo+compartment-mapper+1.2.1.patch @@ -0,0 +1,29 @@ +diff --git a/node_modules/@endo/compartment-mapper/src/policy.js b/node_modules/@endo/compartment-mapper/src/policy.js +index ee2a8fb..98af69a 100644 +--- a/node_modules/@endo/compartment-mapper/src/policy.js ++++ b/node_modules/@endo/compartment-mapper/src/policy.js +@@ -10,7 +10,9 @@ import { + policyLookupHelper, + } from './policy-format.js'; + +-const { create, entries, values, assign, keys, freeze } = Object; ++const { create, entries, values, assign, freeze, getOwnPropertyDescriptors } = ++ Object; ++const { ownKeys } = Reflect; + const q = JSON.stringify; + + /** +@@ -28,7 +30,12 @@ export const ATTENUATORS_COMPARTMENT = ''; + */ + const selectiveCopy = (from, to, list) => { + if (!list) { +- list = keys(from); ++ const descs = getOwnPropertyDescriptors(from); ++ list = ownKeys(from).filter( ++ key => ++ // @ts-expect-error TypeScript still confused about a symbol as index ++ descs[key].enumerable, ++ ); + } + for (let index = 0; index < list.length; index += 1) { + const key = list[index];