endojs / endo

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

refactor(ses): remove unused expensive diagnostic from harden #2235

Closed erights closed 5 months ago

erights commented 5 months ago

closes: #XXXX refs: #XXXX

Description

All this path maintenance within harden was there in case we encountered failures to harden that were hard to diagnose. They accumulate the path accumulated from the harden root to the thing that failed to be hardened. But it was never used to produce a better error message. Rather, they could have been useful from a debugger's eye view. But in all the years we've paid the price for this diagnostic, we have not made use of it. The operations used to accumulate the path are expensive string operations and WeakSet operations.

Because the diagnostic information was never actually shown in any diagnostic, this change should be completely without any observable effect. Hence, this PR is a pure refactor.

Security Considerations

Mostly none. By making harden cheaper, perhaps it'll be used more, which would help security. But we have not generally avoided harden due to its runtime cost, with the exception of the SwingSet kernel.

Scaling Considerations

The point.

Note though that on XS we're already using the native harden that does not pay this price. The SwingSet kernel harden is already suppressed. So this PR only affects use of harden not on XS and not within the SwingSet kernel itself.

@gibson042 , before merging this, I'd like to work with you to quantify its performance impact. If it has none, perhaps it isn't work doing and we should keep the diagnostic info, just in case?

Documentation Considerations

none.

Testing Considerations

none

Compatibility Considerations

none

However, I noticed this while working on #2232 because I had to modify the same code for different purposes. There will be minor merge conflicts coming between these two PRs which I'll resolve after one of them is merged.

Upgrade Considerations

none