Lavamoat build system integration for WebApp by kumavis · Pull Request #12242 · MetaMask/metamask-extension

kumavis · 2021-09-29T07:19:59Z

🌋 🦊 🌋 🦊 🌋 🦊 🌋 🦊 🌋 🦊 🌋 🦊 🌋 🦊 🌋 🦊 🌋 🦊 🌋 🦊 🌋 🦊 🌋 🦊 🌋 🦊 🌋
the next step in incrementally locking down metamask:
providing the build process with a switch for adding lavamoat to each page
currently does not apply lavamoat to any page
🌋 🦊 🌋 🦊 🌋 🦊 🌋 🦊 🌋 🦊 🌋 🦊 🌋 🦊 🌋 🦊 🌋 🦊 🌋 🦊 🌋 🦊 🌋 🦊 🌋 🦊 🌋

summary of code changes

in app/*.html include a if-block to optionally apply lavamoat by loading the commonjs or lavamoat runtime. commonjs still needs an external lockdown script
in build/scripts.js change packer to lavapack, append policy loader, and render html with the useLavamoat switch
in lavamoat/browserify, add combined background and ui policies (ui policy is not enforced yet)
in lavamoat/node update policies to include running lavamoat and other added deps
in package.json add new scripts for generating lavamoat policies and add deps
in test/e2e/tests/contract-interactions.spec.js add delays to fix race condition
in yarn.locklavamoat dep additions

summary of work flow changes

after modifying dependencies or patches used in the background or ui, run yarn lavamoat:auto to update the policies. these policies are not applied yet, but makes PRs consistent with the future requirement where we will soon apply these policies when lavamoat is enabled.

things to be aware of

there is a change to workflow, see above
this pr was designed to be minimal and easy to revert
this version of LavaMoat was prepared to not require eval and requires no change to the manifest.
while this applies no protection to background or ui, it does changes the runtime of the ui and background (update to lavapack). All non-lavamoat pages now runs in a ses-shim like structure where a with statement is wrapping the globalThis. This is done to have a matching module initialization structure to the lavamoat protected components.
the application of this structure may have performance impacts, especially on low resource devices. This is already noticed in CI as additional timeouts were necessary. However, in QA on my machines performance has felt normal. Take this with a grain of salt.
this change is in preparation to applying lavamoat protections and bringing significant supplychain security enhancements to metamask
I am committed to polishing the developer experience in response to feedback

TODO:

add debug info for policy validation failure
ci - improve lavamoat validation debug info #12259
include lockdown-run.js additions beyond lockdown
lockdown - breakout making globalThis properties non-writable #12258
QA popup init time, esp firefox

… lavamoat-webapp-bg4

metamaskbot · 2021-09-30T18:54:48Z

Builds ready [f80c385]

builds: chrome, firefox, opera
build viz: Build System
code coverage: Report
storybook: Storybook
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5, 6, 7, 8
- common: 0, 1, 2, 3
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- inpage: inpage
- phishing-detect: phishing-detect
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 2, 3, 4, 5

Page Load Metrics (406 ± 49 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	82	547	365	86	41
		domContentLoaded	285	759	387	101	49
		load	307	773	406	102	49
		domInteractive	285	759	387	101	49

kumavis · 2021-09-30T20:25:32Z

TODO: review lockdown-run.js

Gudahtt · 2021-09-30T20:24:20Z

development/build/scripts.js

              browserPlatforms,
-            );
-            renderHtmlFile('home', groupSet, commonSet, browserPlatforms);
+              useLavamoat: false,


It would be nice to pass in useLavamoat as a CLI flag

im not sure the usecase, can add later. would need to specify it for every entry/webpage

I mean so the Lavamoat build can be tested without modifying this file. You'd have to go through and swap every useLavamoat: false, to useLavamoat: true, right?

makes sense

Gudahtt · 2021-09-30T20:28:01Z

Lavamoat policy validation is here: https://github.com/MetaMask/metamask-extension/blob/develop/.circleci/config.yml#L171
Which calls this script: https://github.com/MetaMask/metamask-extension/blob/develop/.circleci/scripts/validate-lavamoat-policy.sh

So because the lavamoat:auto script now automatically updates both, I think this PR works with that CI step already.

kumavis · 2021-09-30T20:31:15Z

request for QA on firefox esp wrt to popup init perf

development/build/scripts.js

kumavis · 2021-09-30T20:43:45Z

add debugging information if policy fails https://github.com/MetaMask/metamask-extension/blob/develop/.circleci/scripts/validate-lavamoat-policy.sh#L9

development/build/scripts.js

… lavamoat-webapp-bg4

metamaskbot · 2021-10-01T20:37:29Z

Builds ready [e464859]

builds: chrome, firefox, opera
build viz: Build System
code coverage: Report
storybook: Storybook
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5, 6, 7, 8
- common: 0, 1, 2, 3
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- inpage: inpage
- phishing-detect: phishing-detect
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 2, 3, 4, 5

Page Load Metrics (376 ± 44 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	280	582	366	90	43
		domContentLoaded	270	582	356	91	44
		load	287	582	376	92	44
		domInteractive	270	582	356	91	44

brad-decker · 2021-10-04T15:18:13Z

Other than the popup init time issue I am 👍🏻 on this

… lavamoat-webapp-bg4

kumavis · 2021-10-04T20:42:20Z

@brad-decker did you see a difference in pop perf time with lavamoat disabled as it is now?

kumavis · 2021-10-04T20:57:28Z

removing e2e test delays that are likely only required for actually turning lavamoat on

kumavis · 2021-10-04T20:57:52Z

ive gotten a few QA reports that firefox popup startup perf looks fine

brad-decker

With QA and my own testing not finding popup delays I'm good with this 💯

This reverts commit 79c3479.

kumavis · 2021-10-04T21:32:21Z

removing delays failed 😿

metamaskbot · 2021-10-04T21:52:25Z

Builds ready [0612a3f]

builds: chrome, firefox, opera
build viz: Build System
code coverage: Report
storybook: Storybook
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5, 6, 7, 8
- common: 0, 1, 2, 3
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- inpage: inpage
- phishing-detect: phishing-detect
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 2, 3, 4, 5

Page Load Metrics (371 ± 41 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	70	608	335	89	43
		domContentLoaded	272	611	349	86	41
		load	289	616	371	85	41
		domInteractive	272	611	349	86	41

brad-decker

LGTM

ryanml

lgtm after testing!

kumavis added 2 commits September 28, 2021 21:05

lavamoat - add lavamoat to webapp background

26f689c

Merge branch 'develop' of github.com:MetaMask/metamask-extension into…

6a9cd1a

… lavamoat-webapp-bg4

This was referenced Sep 29, 2021

Lavamoat Webapp Background with Precompiled Modules #12177

Closed

Enable LavaMoat for webapp background #11582

Closed

Lavamoat for webapp - UI #10732

Closed

kumavis added 2 commits September 29, 2021 19:44

test:e2e - add delay to resolve failure

cdbe923

test:e2e - add delay to resolve failure

09b0444

This comment has been minimized.

Sign in to view

Merge branch 'develop' of github.com:MetaMask/metamask-extension into…

2e1e005

… lavamoat-webapp-bg4

This comment has been minimized.

Sign in to view

kumavis marked this pull request as ready for review September 30, 2021 07:22

kumavis requested a review from a team as a code owner September 30, 2021 07:22

kumavis requested a review from NiranjanaBinoy September 30, 2021 07:22

build - add a switch for applying lavamoat, currently off for all

f80c385

kumavis force-pushed the lavamoat-webapp-bg4 branch from bbbb538 to f80c385 Compare September 30, 2021 18:31

kumavis changed the title ~~Lavamoat for WebApp background~~ Lavamoat build system integration for WebApp Sep 30, 2021

Gudahtt reviewed Sep 30, 2021

View reviewed changes

development/build/scripts.js Show resolved Hide resolved

Gudahtt reviewed Sep 30, 2021

View reviewed changes

development/build/scripts.js Show resolved Hide resolved

Merge branch 'develop' of github.com:MetaMask/metamask-extension into…

e464859

… lavamoat-webapp-bg4

Merge branch 'develop' of github.com:MetaMask/metamask-extension into…

f1df774

… lavamoat-webapp-bg4

test/e2e - remove delays added for lavamoat

79c3479

brad-decker previously approved these changes Oct 4, 2021

View reviewed changes

Revert "test/e2e - remove delays added for lavamoat"

0612a3f

This reverts commit 79c3479.

kumavis dismissed brad-decker’s stale review via 0612a3f October 4, 2021 21:25

brad-decker approved these changes Oct 5, 2021

View reviewed changes

ryanml approved these changes Oct 5, 2021

View reviewed changes

kumavis merged commit cb174ff into develop Oct 5, 2021

kumavis deleted the lavamoat-webapp-bg4 branch October 5, 2021 22:06

github-actions bot locked and limited conversation to collaborators Oct 5, 2021

tmashuang restored the lavamoat-webapp-bg4 branch October 6, 2021 16:36

tmashuang deleted the lavamoat-webapp-bg4 branch October 6, 2021 16:52

Conversation

kumavis commented Sep 29, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

summary of code changes

summary of work flow changes

things to be aware of

TODO:

Uh oh!

This comment has been minimized.

This comment has been minimized.

metamaskbot commented Sep 30, 2021

Uh oh!

kumavis commented Sep 30, 2021

Uh oh!

Gudahtt Sep 30, 2021

Choose a reason for hiding this comment

Uh oh!

kumavis Sep 30, 2021

Choose a reason for hiding this comment

Uh oh!

Gudahtt Oct 1, 2021

Choose a reason for hiding this comment

Uh oh!

kumavis Oct 4, 2021

Choose a reason for hiding this comment

Uh oh!

Gudahtt commented Sep 30, 2021

Uh oh!

kumavis commented Sep 30, 2021

Uh oh!

Uh oh!

kumavis commented Sep 30, 2021

Uh oh!

Uh oh!

metamaskbot commented Oct 1, 2021

Uh oh!

brad-decker commented Oct 4, 2021

Uh oh!

kumavis commented Oct 4, 2021

Uh oh!

kumavis commented Oct 4, 2021

Uh oh!

kumavis commented Oct 4, 2021

Uh oh!

brad-decker left a comment

Choose a reason for hiding this comment

Uh oh!

kumavis commented Oct 4, 2021

Uh oh!

metamaskbot commented Oct 4, 2021

Uh oh!

brad-decker left a comment

Choose a reason for hiding this comment

Uh oh!

ryanml left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

kumavis commented Sep 29, 2021 •

edited

Loading