Skip to content

Commit

Permalink
Mask secret value passed between extension scripts (#557)
Browse files Browse the repository at this point in the history
* Mask secret value passed between extension scripts
* Use Opaque type, mask privateKeys along the way
* Mask imported wallets
  • Loading branch information
everdimension authored Sep 6, 2024
1 parent b5844c0 commit c775234
Show file tree
Hide file tree
Showing 25 changed files with 234 additions and 64 deletions.
22 changes: 15 additions & 7 deletions src/background/Wallet/Wallet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,8 @@ import { FEATURE_PAYMASTER_ENABLED } from 'src/env/config';
import { createTypedData } from 'src/modules/ethereum/account-abstraction/createTypedData';
import { getDefiSdkClient } from 'src/modules/defi-sdk/background';
import type { NetworkConfig } from 'src/modules/networks/NetworkConfig';
import type { LocallyEncoded } from 'src/shared/wallet/encode-locally';
import { decodeMasked } from 'src/shared/wallet/encode-locally';
import type { DaylightEventParams, ScreenViewParams } from '../events';
import { emitter } from '../events';
import type { Credentials, SessionCredentials } from '../account/Credentials';
Expand All @@ -93,7 +95,7 @@ import { lastUsedAddressStore } from '../user-activity';
import { toEthersWallet } from './helpers/toEthersWallet';
import { maskWallet, maskWalletGroup, maskWalletGroups } from './helpers/mask';
import type { PendingWallet, WalletRecord } from './model/types';
import type { BareWallet } from './model/BareWallet';
import type { MaskedBareWallet } from './model/BareWallet';
import {
MnemonicWalletContainer,
PrivateKeyWalletContainer,
Expand Down Expand Up @@ -343,8 +345,12 @@ export class Wallet {
return maskWallet(walletContainer.getFirstWallet());
}

async uiImportPrivateKey({ params: privateKey }: WalletMethodParams<string>) {
const walletContainer = new PrivateKeyWalletContainer([{ privateKey }]);
async uiImportPrivateKey({
params: privateKey,
}: WalletMethodParams<LocallyEncoded>) {
const walletContainer = new PrivateKeyWalletContainer([
{ privateKey: decodeMasked(privateKey) },
]);
this.pendingWallet = {
origin: WalletOrigin.imported,
groupId: null,
Expand All @@ -355,10 +361,12 @@ export class Wallet {

async uiImportSeedPhrase({
params: mnemonics,
}: WalletMethodParams<NonNullable<BareWallet['mnemonic']>[]>) {
}: WalletMethodParams<NonNullable<MaskedBareWallet['mnemonic']>[]>) {
this.ensureActiveSession(this.userCredentials);
const walletContainer = await MnemonicWalletContainer.create({
wallets: mnemonics.map((mnemonic) => ({ mnemonic })),
wallets: mnemonics.map((mnemonic) => ({
mnemonic: { ...mnemonic, phrase: decodeMasked(mnemonic.phrase) },
})),
credentials: this.userCredentials,
});
this.pendingWallet = {
Expand Down Expand Up @@ -436,7 +444,7 @@ export class Wallet {
async verifyRecoveryPhrase({
params: { groupId, value },
context,
}: WalletMethodParams<{ groupId: string; value: string }>) {
}: WalletMethodParams<{ groupId: string; value: LocallyEncoded }>) {
this.verifyInternalOrigin(context);
this.ensureRecord(this.record);
this.ensureActiveSession(this.userCredentials);
Expand All @@ -460,7 +468,7 @@ export class Wallet {
async verifyPrivateKey({
params: { address, value },
context,
}: WalletMethodParams<{ address: string; value: string }>) {
}: WalletMethodParams<{ address: string; value: LocallyEncoded }>) {
this.verifyInternalOrigin(context);
this.ensureRecord(this.record);
this.ensureActiveSession(this.userCredentials); // require anyway
Expand Down
18 changes: 12 additions & 6 deletions src/background/Wallet/WalletRecord.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ import {
} from 'src/shared/types/validators';
import { capitalize } from 'capitalize-ts';
import { upgradeRecord } from 'src/shared/type-utils/versions';
import type { LocallyEncoded } from 'src/shared/wallet/encode-locally';
import { encodeForMasking } from 'src/shared/wallet/encode-locally';
import type { Credentials, SessionCredentials } from '../account/Credentials';
import { emitter } from '../events';
import type {
Expand Down Expand Up @@ -467,7 +469,7 @@ export class WalletRecordModel {
groupId,
credentials,
}: { groupId: string; credentials: SessionCredentials }
) {
): Promise<{ phrase: LocallyEncoded; path: string }> {
const group = record.walletManager.groups.find(
(group) => group.id === groupId
);
Expand All @@ -483,20 +485,23 @@ export class WalletRecordModel {
}
const isNotEncrypted = !isEncryptedMnemonic(encryptedMnemonic.phrase);
if (isNotEncrypted) {
return encryptedMnemonic;
return {
...encryptedMnemonic,
phrase: encodeForMasking(encryptedMnemonic.phrase),
};
}

const phrase = await decryptMnemonic(encryptedMnemonic.phrase, credentials);
return {
...encryptedMnemonic,
phrase,
phrase: encodeForMasking(phrase),
};
}

static async getPendingRecoveryPhrase(
pendingWallet: PendingWallet,
credentials: SessionCredentials
) {
): Promise<LocallyEncoded> {
const walletContainer = pendingWallet.walletContainer;
if (!walletContainer || !isMnemonicContainer(walletContainer)) {
throw new Error('Pending wallet is not a Mnemonic Container');
Expand All @@ -505,7 +510,8 @@ export class WalletRecordModel {
if (!encryptedPhrase) {
throw new Error('Pending wallet does not have a seed phrase');
}
return decryptMnemonic(encryptedPhrase, credentials);
const phrase = await decryptMnemonic(encryptedPhrase, credentials);
return encodeForMasking(phrase);
}

static async getPrivateKey(
Expand All @@ -526,7 +532,7 @@ export class WalletRecordModel {
if (!wallet) {
throw new Error('Wallet with given address not found');
}
return wallet.privateKey;
return encodeForMasking(wallet.privateKey);
}

static setCurrentAddress(
Expand Down
5 changes: 4 additions & 1 deletion src/background/Wallet/helpers/mask.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@ function maskMnemonic(
return mnemonic ? { phrase: '<phrase>', path: mnemonic.path } : null;
}

export function maskWallet(wallet: ExternallyOwnedAccount | BareWallet) {
/** TODO: rename to "removeSensitiveValues"? or something better? */
export function maskWallet<T extends ExternallyOwnedAccount | BareWallet>(
wallet: T
) {
return produce(wallet, (draft) => {
if ('privateKey' in draft) {
draft.privateKey = '<privateKey>';
Expand Down
11 changes: 6 additions & 5 deletions src/background/Wallet/model/BareWallet.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import type { ethers } from 'ethers';
import type { ExternallyOwnedAccount } from './AccountContainer';
import type { MaskedSignerObject, SignerObject } from './SignerObject';

// TODO: rename BareWallet to SignerWallet?
export interface BareWallet extends ExternallyOwnedAccount {
mnemonic: { phrase: string; path: string } | null;
privateKey: ethers.Wallet['privateKey'];
}
export interface BareWallet extends ExternallyOwnedAccount, SignerObject {}

export interface MaskedBareWallet
extends ExternallyOwnedAccount,
MaskedSignerObject {}
12 changes: 12 additions & 0 deletions src/background/Wallet/model/SignerObject.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import type { ethers } from 'ethers';
import type { LocallyEncoded } from 'src/shared/wallet/encode-locally';

export interface SignerObject {
mnemonic: { phrase: string; path: string } | null;
privateKey: ethers.Wallet['privateKey'];
}

export interface MaskedSignerObject {
mnemonic: { phrase: LocallyEncoded; path: string } | null;
privateKey: LocallyEncoded;
}
20 changes: 20 additions & 0 deletions src/shared/type-utils/Opaque.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// eslint-disable-next-line @typescript-eslint/no-namespace
namespace OpaqueSymbols {
export declare const type: unique symbol;
export declare const name: unique symbol;
}

export type Opaque<T, Name> = {
readonly [OpaqueSymbols.type]: T;
readonly [OpaqueSymbols.name]: Name;
};

export function opaqueType<T extends Opaque<unknown, unknown>>(
value: T[typeof OpaqueSymbols.type]
) {
return value as T;
}

export function unwrapOpaqueType<T extends Opaque<unknown, unknown>>(value: T) {
return value as T[typeof OpaqueSymbols.type];
}
1 change: 1 addition & 0 deletions src/shared/types/BareWallet.ts
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
export type { BareWallet } from 'src/background/Wallet/model/BareWallet';
export type { MaskedBareWallet } from 'src/background/Wallet/model/BareWallet';
31 changes: 31 additions & 0 deletions src/shared/wallet/encode-locally.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import { unwrapOpaqueType } from '../type-utils/Opaque';
import { encodeForMasking, decodeMasked } from './encode-locally';

describe.only('encode-locally.ts', () => {
test('encodeForMasking', () => {
const value = 'hello';
expect(decodeMasked(encodeForMasking(value))).toBe(value);
});

test('maskedValue is not equal to input value', () => {
const value = 'something';
const encoded = encodeForMasking(value);
expect(encoded).not.toBe(value);
});

test('maskedValue and value are not substrings of each other', () => {
const values = ['one two three', '000000', '#$%@#$=='];
values.forEach((value) => {
const encoded = encodeForMasking(value);
const encodedString = unwrapOpaqueType(encoded);
expect(value.includes(encodedString)).toBe(false);
expect(encodedString.includes(value)).toBe(false);
});
});

test('encodeForMasking longer text', () => {
const value =
"What is Lorem Ipsum?\nLorem Ipsum is simply dummy text of the printing and typesetting industry. Lorem Ipsum has been the industry's standard dummy text ever since the 1500s!";
expect(decodeMasked(encodeForMasking(value))).toBe(value);
});
});
37 changes: 37 additions & 0 deletions src/shared/wallet/encode-locally.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import { type Opaque } from '../type-utils/Opaque';
import { opaqueType, unwrapOpaqueType } from '../type-utils/Opaque';

export type LocallyEncoded = Opaque<string, 'LocallyEncoded'>;

const NONSECRET_KEY_FOR_INTERNAL_USE = '2024-06-18';

/**
* This function is intended to mask a secret value that is being
* passed between extension scripts (ui, service-worker and web-worker)
* and may live in memory for some time.
* The purpose is to prevent passing clear text.
* This "encoding" is simply one step less trivial than base64
* and should not be considered encryption.
*/
export function encodeForMasking(value: string) {
const key = NONSECRET_KEY_FOR_INTERNAL_USE;
let encoded = '';
for (let i = 0; i < value.length; i++) {
encoded += String.fromCharCode(
value.charCodeAt(i) ^ key.charCodeAt(i % key.length)
);
}
return opaqueType<LocallyEncoded>(btoa(encoded));
}

export function decodeMasked(encoded: LocallyEncoded) {
const key = NONSECRET_KEY_FOR_INTERNAL_USE;
const decoded = atob(unwrapOpaqueType(encoded));
let data = '';
for (let i = 0; i < decoded.length; i++) {
data += String.fromCharCode(
decoded.charCodeAt(i) ^ key.charCodeAt(i % key.length)
);
}
return data;
}
17 changes: 17 additions & 0 deletions src/ui/Onboarding/shared/usePendingRecoveryPhrase.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import { useQuery } from '@tanstack/react-query';
import { walletPort } from 'src/ui/shared/channels';

export function usePendingRecoveryPhrase() {
return useQuery({
queryKey: ['getPendingRecoveryPhrase'],
queryFn: () => {
return walletPort.request('getPendingRecoveryPhrase');
},
cacheTime: 0 /** sensitive value, prevent from being cached */,
suspense: false,
retry: 0,
refetchOnMount: false,
refetchOnWindowFocus: true,
useErrorBoundary: false,
});
}
10 changes: 5 additions & 5 deletions src/ui/features/onboarding/Import/Import.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import { UnstyledButton } from 'src/ui/ui-kit/UnstyledButton';
import { HStack } from 'src/ui/ui-kit/HStack';
import { VStack } from 'src/ui/ui-kit/VStack';
import { UIText } from 'src/ui/ui-kit/UIText';
import type { BareWallet } from 'src/shared/types/BareWallet';
import type { BareWallet, MaskedBareWallet } from 'src/shared/types/BareWallet';
import type { ExternallyOwnedAccount } from 'src/shared/types/ExternallyOwnedAccount';
import { setCurrentAddress } from 'src/ui/shared/requests/setCurrentAddress';
import { accountPublicRPCPort, walletPort } from 'src/ui/shared/channels';
Expand Down Expand Up @@ -73,10 +73,10 @@ function ImportWallet() {

const goBack = useGoBack();

const [wallets, setWallets] = useState<BareWallet[] | null>(null);
const [wallets, setWallets] = useState<MaskedBareWallet[] | null>(null);
const [mnemonic, setMnemonic] = useState<string | null>(null);
const handleWallets = useCallback(
(wallets: BareWallet[]) => {
(wallets: MaskedBareWallet[]) => {
setWallets(wallets);
setSearchParams(`view=${ViewParam.password}`);
},
Expand All @@ -89,7 +89,7 @@ function ImportWallet() {
wallets,
}: {
password: string | null;
wallets: BareWallet[];
wallets: MaskedBareWallet[];
}) => {
setShowError(false);
return Promise.race([
Expand Down Expand Up @@ -211,7 +211,7 @@ function ImportWallet() {
type === 'private-key' ? (
<ImportKey
onWalletCreate={(wallet) =>
handleWallets([wallet as BareWallet])
handleWallets([wallet as MaskedBareWallet])
}
/>
) : type === 'mnemonic' ? (
Expand Down
4 changes: 3 additions & 1 deletion src/ui/features/onboarding/Import/ImportKey.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { zeroizeAfterSubmission } from 'src/ui/shared/zeroize-submission';
import { Input } from 'src/ui/ui-kit/Input';
import type { ExternallyOwnedAccount } from 'src/shared/types/ExternallyOwnedAccount';
import { useWindowSizeStore } from 'src/ui/shared/useWindowSizeStore';
import { encodeForMasking } from 'src/shared/wallet/encode-locally';
import { SecretKeyFAQ } from '../FAQ';

export function ImportKey({
Expand All @@ -34,7 +35,8 @@ export function ImportKey({
if (!validity.valid) {
return;
}
return walletPort.request('uiImportPrivateKey', secretKey);
const encoded = encodeForMasking(secretKey);
return walletPort.request('uiImportPrivateKey', encoded);
},
onSuccess: (wallet) => {
if (wallet) {
Expand Down
10 changes: 6 additions & 4 deletions src/ui/features/onboarding/Import/SelectWallets.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { Content } from 'react-area';
import { useQuery } from '@tanstack/react-query';
import groupBy from 'lodash/groupBy';
import { invariant } from 'src/shared/invariant';
import type { BareWallet } from 'src/shared/types/BareWallet';
import type { MaskedBareWallet } from 'src/shared/types/BareWallet';
import { getFirstNMnemonicWallets } from 'src/ui/pages/GetStarted/ImportWallet/MnemonicImportView/getFirstNMnemonicWallets';
import { normalizeAddress } from 'src/shared/normalizeAddress';
import { VStack } from 'src/ui/ui-kit/VStack';
Expand All @@ -15,6 +15,7 @@ import { wait } from 'src/shared/wait';
import { useAllExistingMnemonicAddresses } from 'src/ui/shared/requests/useAllExistingAddresses';
import { useAddressActivity } from 'src/ui/shared/requests/useAddressActivity';
import { useStaleTime } from 'src/ui/pages/GetStarted/ImportWallet/MnemonicImportView/useStaleTime';
import { encodeForMasking } from 'src/shared/wallet/encode-locally';
import * as helperStyles from '../shared/helperStyles.module.css';
import { SelectWalletsFAQ } from '../FAQ';

Expand All @@ -23,7 +24,7 @@ export function SelectWallets({
onSelect,
}: {
mnemonic: string | null;
onSelect(wallets: BareWallet[]): void;
onSelect(wallets: MaskedBareWallet[]): void;
}) {
const [count] = useState(100);
invariant(mnemonic, 'Seed phrase is empty');
Expand All @@ -33,7 +34,8 @@ export function SelectWallets({
queryKey: ['getFirstNMnemonicWallets', mnemonic, count],
queryFn: async () => {
await wait(1000);
return getFirstNMnemonicWallets({ phrase: mnemonic, n: count });
const phrase = encodeForMasking(mnemonic);
return getFirstNMnemonicWallets({ phrase, n: count });
},
useErrorBoundary: true,
suspense: false,
Expand All @@ -53,7 +55,7 @@ export function SelectWallets({
);
const { active, rest } = grouped as Record<
'active' | 'rest',
BareWallet[] | undefined
MaskedBareWallet[] | undefined
>;

const [values, setValue] = useState<Set<string>>(() => new Set());
Expand Down
Loading

0 comments on commit c775234

Please sign in to comment.