Skip to content

Commit 2023178

Browse files
feat(mv3): hardware wallet support for trezor (#21765)
## **Description** Uses the Offscreen document to add an iframe for Trezor hardware wallet communication. The entry point of the trezor-iframe is trezor-iframe.ts and it is protected by Lavamoat using the build system in the same way that the offscreen document is. ## **Related issues** Fixes: #21623 ## **Manual testing steps** 1. run `yarn start:mv3` 2. load the extension into chrome, or reload it if already added. 3. If you previously had your Trezor added to the extension, remove it (or you can add a secondary account from it) 4. Click Add new hardware wallet from the add account dropdown menu. 5. See that Trezor and QR are valid options 6. Make sure you have Trezor Suite open and your device unlocked. 7. Click trezor and then connect. 8. See that you are prompted to connect your trezor and select an account to import. 9. Perform transactions, signatures, etc using the test dapp. ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <img width="1225" alt="Screenshot 2023-11-30 at 12 04 49 PM" src="https://github.com/MetaMask/metamask-extension/assets/4448075/e0300d39-561c-4a20-a494-35020c1aa6a9"> <!-- [screenshots/recordings] --> ### **After** <img width="778" alt="Screenshot 2023-11-30 at 11 59 01 AM" src="https://github.com/MetaMask/metamask-extension/assets/4448075/bbd1a50c-963e-4eed-8ffb-38b8302912a2"> <img width="1114" alt="Screenshot 2023-11-30 at 12 01 35 PM" src="https://github.com/MetaMask/metamask-extension/assets/4448075/839aed19-8cf0-4a6a-a3cd-aa6062a77231"> ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [x] I've clearly explained what problem this PR is solving and how it is solved. - [x] I've linked related issues - [x] I've included manual testing steps - [x] I've included screenshots/recordings if applicable - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. - [x] I’ve properly set the pull request status: - [ ] In case it's not yet "ready for review", I've set it to "draft". - [x] In case it's "ready for review", I've changed it from "draft" to "non-draft". ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --------- Co-authored-by: MetaMask Bot <[email protected]>
1 parent b82ef68 commit 2023178

File tree

18 files changed

+835
-757
lines changed

18 files changed

+835
-757
lines changed

.yarnrc.yml

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,11 @@ npmAuditIgnoreAdvisories:
2424
# Issue: protobufjs Prototype Pollution vulnerability
2525
# URL - https://github.com/advisories/GHSA-h755-8qp9-cq85
2626
# Not easily patched. Minimally effects the extension due to usage of
27-
# LavaMoat lockdown.
27+
# LavaMoat lockdown. Additional id added that resolves to the same advisory
28+
# but has a different entry due to it being a new dependency of
29+
# @trezor/connect-web. Upgrading
2830
- 1092429
31+
- 1095136
2932

3033
# Issue: Regular Expression Denial of Service (ReDOS)
3134
# URL: https://github.com/advisories/GHSA-257v-vj4p-3w2h

app/manifest/v3/chrome.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"content_security_policy": {
3-
"extension_pages": "script-src 'self'; object-src 'self'; frame-ancestors 'none';"
3+
"extension_pages": "script-src 'self'; object-src 'self'; frame-ancestors 'self';"
44
},
55
"externally_connectable": {
66
"matches": ["https://metamask.io/*"],
Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,136 @@
1+
import { TrezorBridge } from '@metamask/eth-trezor-keyring';
2+
import type {
3+
EthereumSignMessage,
4+
EthereumSignTransaction,
5+
Params,
6+
EthereumSignTypedDataTypes,
7+
ConnectSettings,
8+
Manifest,
9+
Response as TrezorResponse,
10+
EthereumSignedTx,
11+
PROTO,
12+
EthereumSignTypedHash,
13+
} from '@trezor/connect-web';
14+
import {
15+
OffscreenCommunicationEvents,
16+
OffscreenCommunicationTarget,
17+
TrezorAction,
18+
} from '../../../../shared/constants/offscreen-communication';
19+
20+
/**
21+
* This class is used as a custom bridge for the Trezor connection. Every
22+
* hardware wallet keyring also requires a bridge that has a known interface
23+
* that the keyring can call into for specific functions. The bridge then makes
24+
* whatever calls or requests it needs to in order to fulfill the request from
25+
* the keyring. In this case, the bridge is used to communicate with the
26+
* Offscreen Document. Inside the Offscreen document the trezor-iframe is
27+
* embedded that will listen for these calls and communicate with the
28+
* trezor/connect-web library.
29+
*/
30+
export class TrezorOffscreenBridge implements TrezorBridge {
31+
model: string | undefined;
32+
33+
init(
34+
settings: {
35+
manifest: Manifest;
36+
} & Partial<ConnectSettings>,
37+
) {
38+
chrome.runtime.onMessage.addListener((msg) => {
39+
if (
40+
msg.target === OffscreenCommunicationTarget.extension &&
41+
msg.event === OffscreenCommunicationEvents.trezorDeviceConnect
42+
) {
43+
this.model = msg.payload;
44+
}
45+
});
46+
47+
return new Promise<void>((resolve) => {
48+
chrome.runtime.sendMessage(
49+
{
50+
target: OffscreenCommunicationTarget.trezorOffscreen,
51+
action: TrezorAction.init,
52+
params: settings,
53+
},
54+
() => {
55+
resolve();
56+
},
57+
);
58+
});
59+
}
60+
61+
dispose() {
62+
return new Promise<void>((resolve) => {
63+
chrome.runtime.sendMessage(
64+
{
65+
target: OffscreenCommunicationTarget.trezorOffscreen,
66+
action: TrezorAction.dispose,
67+
},
68+
() => {
69+
resolve();
70+
},
71+
);
72+
});
73+
}
74+
75+
getPublicKey(params: { path: string; coin: string }) {
76+
return new Promise((resolve) => {
77+
chrome.runtime.sendMessage(
78+
{
79+
target: OffscreenCommunicationTarget.trezorOffscreen,
80+
action: TrezorAction.getPublicKey,
81+
params,
82+
},
83+
(response) => {
84+
resolve(response);
85+
},
86+
);
87+
}) as TrezorResponse<{ publicKey: string; chainCode: string }>;
88+
}
89+
90+
ethereumSignTransaction(params: Params<EthereumSignTransaction>) {
91+
return new Promise((resolve) => {
92+
chrome.runtime.sendMessage(
93+
{
94+
target: OffscreenCommunicationTarget.trezorOffscreen,
95+
action: TrezorAction.signTransaction,
96+
params,
97+
},
98+
(response) => {
99+
resolve(response);
100+
},
101+
);
102+
}) as TrezorResponse<EthereumSignedTx>;
103+
}
104+
105+
ethereumSignMessage(params: Params<EthereumSignMessage>) {
106+
return new Promise((resolve) => {
107+
chrome.runtime.sendMessage(
108+
{
109+
target: OffscreenCommunicationTarget.trezorOffscreen,
110+
action: TrezorAction.signMessage,
111+
params,
112+
},
113+
(response) => {
114+
resolve(response);
115+
},
116+
);
117+
}) as TrezorResponse<PROTO.MessageSignature>;
118+
}
119+
120+
ethereumSignTypedData<T extends EthereumSignTypedDataTypes>(
121+
params: Params<EthereumSignTypedHash<T>>,
122+
) {
123+
return new Promise((resolve) => {
124+
chrome.runtime.sendMessage(
125+
{
126+
target: OffscreenCommunicationTarget.trezorOffscreen,
127+
action: TrezorAction.signTypedData,
128+
params,
129+
},
130+
(response) => {
131+
resolve(response);
132+
},
133+
);
134+
}) as TrezorResponse<PROTO.EthereumTypedDataSignature>;
135+
}
136+
}

app/scripts/metamask-controller.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -285,6 +285,7 @@ import { securityProviderCheck } from './lib/security-provider-helpers';
285285
import { IndexedDBPPOMStorage } from './lib/ppom/indexed-db-backend';
286286
///: END:ONLY_INCLUDE_IF
287287
import { updateCurrentLocale } from './translate';
288+
import { TrezorOffscreenBridge } from './lib/offscreen-bridge/trezor-offscreen-bridge';
288289
///: BEGIN:ONLY_INCLUDE_IF(keyring-snaps)
289290
import { snapKeyringBuilder, getAccountsBySnapId } from './lib/snap-keyring';
290291
///: END:ONLY_INCLUDE_IF
@@ -948,6 +949,10 @@ export default class MetamaskController extends EventEmitter {
948949
);
949950
}
950951
///: END:ONLY_INCLUDE_IF
952+
} else if (isManifestV3) {
953+
additionalKeyrings.push(
954+
hardwareKeyringBuilderFactory(TrezorKeyring, TrezorOffscreenBridge),
955+
);
951956
}
952957

953958
///: BEGIN:ONLY_INCLUDE_IF(keyring-snaps)
@@ -3763,6 +3768,7 @@ export default class MetamaskController extends EventEmitter {
37633768
const keyringOverrides = this.opts.overrides?.keyrings;
37643769
let keyringName = null;
37653770
if (
3771+
deviceName !== HardwareDeviceNames.trezor &&
37663772
deviceName !== HardwareDeviceNames.QR &&
37673773
!this.canUseHardwareWallets()
37683774
) {

development/build/scripts.js

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -305,7 +305,7 @@ function createScriptTasks({
305305
// In MV3 we will need to build our offscreen entry point bundle and any
306306
// entry points for iframes that we want to lockdown with LavaMoat.
307307
if (process.env.ENABLE_MV3 === 'true') {
308-
standardEntryPoints.push('offscreen');
308+
standardEntryPoints.push('offscreen', 'trezor-iframe');
309309
}
310310

311311
const standardSubtask = createTask(
@@ -321,6 +321,8 @@ function createScriptTasks({
321321
return './app/vendor/trezor/content-script.js';
322322
case 'offscreen':
323323
return './offscreen/scripts/offscreen.ts';
324+
case 'trezor-iframe':
325+
return './offscreen/scripts/trezor-iframe.ts';
324326
default:
325327
return `./app/scripts/${label}.js`;
326328
}
@@ -805,6 +807,16 @@ function createFactoredBuild({
805807
});
806808
break;
807809
}
810+
case 'trezor-iframe': {
811+
renderJavaScriptLoader({
812+
groupSet,
813+
commonSet,
814+
browserPlatforms,
815+
applyLavaMoat,
816+
destinationFileName: 'load-trezor-iframe.js',
817+
});
818+
break;
819+
}
808820
default: {
809821
throw new Error(
810822
`build/scripts - unknown groupLabel "${groupLabel}"`,

0 commit comments

Comments
 (0)