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

[multi] Enable contextIsolation in main window #3492

Merged
merged 6 commits into from Jun 9, 2021

Conversation

matheusd
Copy link
Member

@matheusd matheusd commented Jun 2, 2021

Rebased on top of #3486

This makes the final changes and switches on contextIsolation for the main window.

contextIsolation is a recommended security feature for electron apps which makes the vm that runs the preload script layer code completely independent from the vm that runs the ipc renderer code. This prevents UI code from modifying data stored in the preload layer.

contextIsolation is currently on by default in electron apps starting on version 12, but we had to turn it off due to the mixed usage of node and electron APIs across Decrediton's UI code. This PR finally completes the migrations necessary to turn it on and enable Decrediton to leverage the security features offered by it.

The main change necessary to enable contextIsolation introduced by this PR is to change all calls in the wallet package that involve gRPC to use primitive js Object instances instead of the gRPC and protobuf class instances. This is necessary because the preload <-> renderer boundary conversion does not copy prototype definitions, only simple values.

Care was taken to make the commits reasonably self-contained, so that reviewers can have an easier time reviewing the code changes.

Main changes in this PR:

  • Remove outstanding import Promise lines which are no longer necessary due to electron version natively suporting it.
  • Shim the config related calls to return a standard Object instead of ElectronLoader instances.
  • Introduce a gRPC client tracking feature to enable the preload script to store the native gRPC client while UI code only deals with an opaque string identifier.
  • Switch all gRPC related calls to return native objects instead of protobuf instances; this mostly involves switching every getXXXX() call to using the corresponding XXXX field and modifying wallet functions to use toObject() to return the response data directly
  • Close The privacy option could not be saved on the Privacy Options view #3499 (this was caused by an incomplete code move in the previous PR)

Promise is now a built-in js class, so no need to import.
This is needed once contextIsolation is enabled due to electronStore instances
not being proxied.
This adds a client tracking layer to gRPC clients returned by the wallet pkg.

Instead of directly returning the gRPC client instance, the wallet keeps track
of the client and returns a random string ID to the UI code. Subsequent calls
to gRPC functions fetch the actual client from the tracked map.

This is needed due to contextIsolation not allowing non-primitive values from
crossing the preload -> ipcRenderer layers.
This changes the wallet package and UI code so that calls to the wallet pkg
no longer return instances of the protobuf objects but rather standard js
objects.

This is needed because when contextIsolation is turned on, the protobuf objects
cannot cross the preload -> ipcRenderer barrier.
This sets the contextIsolation flag to true when starting the main wallet
window. This improves security by isolating the preload and ipcRenderer scripts
from each other.
This should've been moved on an earlier commit.
@matheusd matheusd marked this pull request as ready for review June 8, 2021 19:34
@alexlyp alexlyp merged commit c39e3c1 into decred:master Jun 9, 2021
@matheusd matheusd deleted the enable-context-isolation branch June 9, 2021 21:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The privacy option could not be saved on the Privacy Options view
2 participants