endojs / endo

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

Terminate CapTP in non-hardened Realm without SES shim #1686

Open kriskowal opened 1 year ago

kriskowal commented 1 year ago

@danfinlay raises an issue we have struggled with: it’s currently not possible to terminate CapTP in a realm that does not also use the SES shim, because these layers depend upon harden. We have yet to find a nice compromise that allows us to write these libraries in a way that works well in both hardened and unhardened JavaScript without compromising the readability or audit-ability of that code in the hardened case. Some of the difficulty lies in the dichotomy:

However, it is clearly the responsibility of the creator of instances to harden the instance and also clearly the responsibility of a library to harden its classes, constructors, prototypes, return values, and so on. Currently, harden does both as a safeguard against a library that failed to harden itself.

So, I propose we can break this logjam with the following strategy:

This would allow us to refactor CapTP and all its dependencies to depend on the @endo/harden ponyfill, making CapTP usable without shims, equally safe with shims, and otherwise identical verbatim to the current implementation.

kriskowal commented 1 year ago

For review by @erights and @michaelfig

mhofman commented 1 year ago

So there is the question of getters and setters in this split world: who is responsible for hardening them?

I believe that in most cases these are not shared with other objects, and that a "surface" harden should thus freeze them as part of the "walking transitive properties" step.

Btw in this world, it's not really the harden implementation that changes , but whether isHardened() recursively checks the prototype chain or not. In fact isHardened() could skip the prototype walk before lockdown(), but enforce it after. Taken an other way, lockdown() replaces isHardened() to add a recursive hardening check for the prototype.

gibson042 commented 1 year ago

It looks to me like initialization of the marshal package and use for marshalling/unmarshalling CapData currently has three firm dependencies that are satisfied by @endo/init:

The first two dependencies seem relevant, although they could in principle be parameterized, and the third can be addressed by #1687.

The bar is actually rather low:

run with node --input-type=module -e '…' ```js // STUB DEPENDENCIES import "data:text/javascript, globalThis.harden ||= Object.freeze;"; import "data:text/javascript, const q = JSON.stringify, { raw } = String, X = (T, ...args) => raw(T, ...args.map(v => q(v) || String(v))); const Fail = (...args) => { throw Error(X(...args)); }; const assert = (globalThis.assert ||= (ok, msg) => ok || Fail([msg])); assert.Fail ||= Fail; assert.fail ||= msg => Fail([msg]); assert.typeof ||= (v, t) => typeof v === t || Fail`not a ${t}: ${v}`; assert.details ||= X; assert.quote ||= q; assert.note ||= () => {};"; import "data:text/javascript, globalThis.HandledPromise ||= false;"; import { makeMarshal } from "@endo/marshal"; import { Far } from "@endo/pass-style"; const { defineProperties, freeze } = Object; const slotAssignments = new Map(); const getSlotAssignment = slotAssignments.get.bind(slotAssignments); const setSlotAssignment = slotAssignments.set.bind(slotAssignments); let nextSlotAssignment = 10; const convertValToSlot = obj => { let slot = getSlotAssignment(obj); if (slot === undefined) { setSlotAssignment(obj, slot = `id1:${nextSlotAssignment++}`); } return slot; }; const convertSlotToVal = (id, iface = "") => Far(`${iface}<${id}>`); const m = makeMarshal(convertValToSlot, convertSlotToVal); // Demonstrate successful unmarshalling and marshalling. const o = m.fromCapData({ body: `#{"bigint":"+42","remotable":"$0.Foo","repeat":"$0"}`, slots: ["id0:1"] }); console.log(o); // => { // bigint: 42n, // remotable: Object [Alleged: Foo] {}, // repeat: Object [Alleged: Foo] {} // } console.log(m.toCapData(o)); // => { // body: '{"bigint":{"@qclass":"bigint","digits":"42"},"remotable":{"@qclass":"slot","iface":"Alleged: Foo","index":0},"repeat":{"@qclass":"slot","index":0}}', // slots: [ 'id1:10' ] // } // Demonstrate unhardened environment. Object.prototype.expando = "Object.prototype is mutable"; console.log({}.expando); // => Object.prototype is mutable // Demonstrate input validation. const badError = freeze(defineProperties(Error(), { message: { value: freeze(["not", "a", "string"]) } })); console.log(m.toCapData(freeze({ badError }))); // => Error: Passed Error "message" {"value":["not","a","string"],"writable":false,"enumerable":false,"configurable":false} must be a string-valued data property. ```
michaelfig commented 1 year ago

a deconstructable global HandledPromise (e.g., even globalThis.HandledPromise = false suffices)

I'm confused as to what @endo/marshal buys us in isolation. For "terminating CapTP", all we need are functioning versions of E() and Far() and implementing their correct behaviour currently relies pretty deeply on HandledPromise, which is not so easy to stub.

I'm in wholehearted agreement that the @endo/eventual-send layers can be structured better, but I don't see any cheap way to make do in their total absence.

kriskowal commented 1 year ago

Ah, yes. We can’t avoid shimming HandledPromise. I assume that’s not possible to ponyfill (is it?). I do assume that HandledPromise can be used before lockdown (can it?). If not in its current form, I’m expecting things to get easier when SES supports vetted shims.

michaelfig commented 1 year ago

I assume that's not possible to ponyfill (is it?).

The problem with not installing HandledPromise (or its spiritual successors) somewhere in globalThis is that it really needs a few per-Realm WeakMaps to work correctly. Making it a shim is how we avoid its eval twins.

kriskowal commented 1 year ago

That’ll be even more true with CapTP when we support 3-party-handoff. That will take a dependency on a shared nonce locator, and that in turn will require WeakRef and FinalizationRegistry to track the retention of objects that were transported between sessions (if I grok right).

mhofman commented 8 months ago

I have been giving more thoughts about the different harden semantics before and after lockdown. I had been uneasy about the possibility of an object to be considered hardened before lockdown, but not satisfy the prototype checks post lockdown. I think this can be remediated by a weak list of prototype objects to check during lockdown.

Assuming that harden is implemented as a flag on the object (not as a transitive frozen check), here would be the suggested semantics:

There is a single remaining pre/post lockdown discrepancy with this scheme: pre lockdown, an object can be hardened without its prototype ever being hardened. If the prototype is hardened after the object, hardening of the prototype fails. If lockdown is called without the prototype being an intrinsic, or having been manually hardened (before the objects using it as prototype), lockdown fails. As such the possibility of code working pre lockdown but not post lockdown is reduced, and can easily be fixed by adding the harden() call for the missing prototypes (and only at the right place).

Regarding the cost of maintaining the "CheckIsHardenedAtLockdownList". For user code correctly using hardening, only the intrinsics should ever be present in this list. Even if lockdown is never called, the cost to maintain this list should be minimal. As a specification construct, the content of the list is never observable, and its entries can be garbage collected. As a shim, to avoid leaks it may have to be implemented using WeakRef and FinalizationRegistry.

A shim implementing harden() without lockdown() should probably provide a lockdown() stub that only throws. In that case the "CheckIsHardenedAtLockdownList" can be implemented as a simple WeakSet as it never needs to be iterated. Technically a lockdown+harden shim does not strictly need to iterate "CheckIsHardenedAtLockdownList" unless it wants to provide information about which prototypes were not hardened.