Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Vetted shims #422

Closed
kriskowal opened this issue Aug 19, 2020 · 10 comments · Fixed by #1705
Closed

Vetted shims #422

kriskowal opened this issue Aug 19, 2020 · 10 comments · Fixed by #1705
Assignees
Labels

Comments

@kriskowal
Copy link
Member

kriskowal commented Aug 19, 2020

To improve compatibility between Hardened JavaScript and shims, programs that upgrade the underlying language, some shims need to be in a position where they can introduce new properties to shared intrinsics and for those properties to remain visible after lockdown.

Enable Vetted Shims

In the first phase of this design, we would introduce a separate repair(options) such that the following programs are equivalent:

lockdown(options);
repair(options);
lockdown();

In the latter form, shims can be executed between repair and lockdown that alter the shared intrinsics in ways that will not be erased by calling lockdown. These are vetted shims.

repair() will introduce the global harden(), but between repair() and lockdown(), harden will simply mark objects that must be hardened by lockdown(). Currently, initializing the ses module introduces harden(), and it throws an error until after calling lockdown(). Introducing harden at time of repair is consistent with the idea that repair is effectively a shim that upgrades the base JavaScript to Almost-Hardened JavaScript, such that shims can run, then lockdown is effectively a shim that completes the job.

In the following program, lockdown() will throw an error because repair options were already applied.

repair();
lockdown(options);

Allowing lockdown(options) to serve as both repair and lockdown if repair has not already been called gives us a graceful migration path from the status quo, and may even be desirable as a feature of the Hardened JavaScript standard since it simplifies usage in scenarios that do not have vetted shims.

Enable Hardened Vetted Shims

The above design is consistent with further work, to be tracked in subsequent issues:

repair introduces new shared intrinsics but does not expose them in a way that vetted shims would be able to alter them. For this to become possible, repair would need to upgrade getIntrinsic and getIntrinsics in place such that, for example, getIntrinsic('%SharedMath%') could be upgraded in place. This would presumably also introduce a %sharedGetIntrinsic% and %sharedGetIntrinsics% function that could be safely shared with compartments, providing access only to the shared intrinsics and no others. Further intrinsics would be introduced to compartments by either writing a wrapper for these or possibly additional compartment constructor options with the same effect.

Eject Internal Shims

Separating repair and lockdown will allow us to refactor ses such that some of the internal layers can become separate shims. For example, the internal compartment-shim could be refactored into a layers: compartment-as-module-loader and compartment-as-confinement. We can remove HandledPromise from the permits, since the handled-promise-shim can be applied between repair and lockdown instead. The console, assert, and Error shims could be separated, and alternate implementations can be used to provide the same services.

Alternatives Considered

The other major design family that we considered and rejected was a variant on this design that lacked a separable repair() phase, also allowed but deferred harden until after lockdown(), and had an additional API for submitting plans for changes to the permits structure that lockdown() would apply. The crux of the design decision is the emergent behavior of shims that must have a follow-up shim to accommodate Hardened JavaScript. In both design spaces, these shims would need to use getIntrinsic to make changes to shared shims and mark new intrinsics with harden. The only material difference is that these hardening-shims would also need to use permit objects to plan for behaviors to be applied during lockdown instead of immediately. We elected to preserve the current convention, that shims modify the environment in place, at the expense of having a separate repair() call that serves as a shim for taming and repairing the JavaScript environment, upgrading it to Almost-Hardened JavaScript.


[original description, 2020-08-18 by kriskowal:]

Endo needs the ability to promote packages and modules that serve as vetted shims, such that they execute after repairs and before lockdown, before any non-shim modules execute. This is a form of dependency “hoisting”. We will need to incorporate the allow-list metadata into the package.json of either the shim, the module that depends upon the shim, or create shim envelope packages to host the metadata. The metadata would be collected by Endo and captured in the compartment map.

@kriskowal
Copy link
Member Author

cc @erights

@kriskowal kriskowal mentioned this issue Aug 22, 2020
36 tasks
@kriskowal kriskowal changed the title Endo: Facilitate shims Vetted shims Jul 10, 2023
@erights
Copy link
Contributor

erights commented Jul 10, 2023

LGTM

@naugtur
Copy link
Member

naugtur commented Jul 11, 2023

Some thoughts I want to persist so I don't forget to bring them up in the meeting. May or may not be discussed asynchronously here.

  1. The text implies that all options we know as lockdown options are actually repair options.
  2. If not entirely, would lockdown have access to the options earlier passed to repair?
  3. If we're splitting lockdown into two phases, would repair and thatFreezeSynonymWeDecideToUse (potentially as methods of a namespace) be cleaner than reusing lockdown for 2 behaviors and codifying passing the options to repair even if some lockdown-specific options are a possibility?

@mhofman
Copy link
Contributor

mhofman commented Jul 11, 2023

In theory the final "lockdown" phase is basically:

for (const [name, intrinsic] of getIntrinsics()) {
  harden(intrinsic);
}

@mhofman
Copy link
Contributor

mhofman commented Jul 11, 2023

We could make a gross overload of harden which when called with no argument, and harden was never called with no argument before, it performs the lockdown step above.

@erights
Copy link
Contributor

erights commented Jul 11, 2023

We could make a gross overload of harden which when called with no argument, and harden was never called with no argument before, it performs the lockdown step above.

Please no. That implies that harden has access to getIntrinsics, whereas its normal behavior does not imply that.

@kriskowal
Copy link
Member Author

Some thoughts I want to persist so I don't forget to bring them up in the meeting. May or may not be discussed asynchronously here.

  1. The text implies that all options we know as lockdown options are actually repair options.
  2. If not entirely, would lockdown have access to the options earlier passed to repair?
  3. If we're splitting lockdown into two phases, would repair and thatFreezeSynonymWeDecideToUse (potentially as methods of a namespace) be cleaner than reusing lockdown for 2 behaviors and codifying passing the options to repair even if some lockdown-specific options are a possibility?

I’m open to the alternative:

Security.repair(options)
Security.lockdown()

With backward-compatibility preserved only in the form lockdown(options?) provided that we can arrive at a name that is less bad than Security before we add vetted shim support. We will have to eventually namespace the methods, but I didn’t want to block vetted shims on that bike-shed.

@naugtur
Copy link
Member

naugtur commented Jul 12, 2023

I didn’t want to block vetted shims on that bike-shed.

+1 to that. I only brought up the context of a future namespace to back up the idea of adding another 2 methods instead of 1.

@weizman
Copy link
Contributor

weizman commented Jul 12, 2023

How do we assure vetted shims are trust worthy?

And yes, I do understand that this is the whole point behind a vetted shim - that you can trust it, and that if you can't than it isn't a vetted shim and that lockdown/SES can be justifiably compromised out of our control.

However, is there a reason to consider making the user of repair declare the intrinsics (and props) the following vetted shims are going to modify, so that those which aren't properly declared would make a breaking exception thrown?

Or does that approach introduces some fundamental wrongs I'm not grasping?

EDIT

It was made clear to me that vetted shims are shims the user is willing to vet on, and that can and will have the power to leave your program fully vulnerable by definition, and therefore the answer to my question is: No. There's no use for what I propose in this comment.

@mhofman
Copy link
Contributor

mhofman commented Jul 23, 2023

Once we have vetted shims, I'd like to migrate the async_hooks monkey patch to that approach to remove the permits hack we added, so I can relax what the patch does in the case of @endo/init/unsafe-fast.js

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants