Skip to content

Persist profile storage keys to e2ee backend #5630

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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ package-lock.json
.eslintcache

.DS_STORE
.idea/

# Build preview message
preview-build-message.txt
Expand All @@ -34,4 +35,4 @@ scripts/coverage
!.yarn/versions

# typescript
packages/*/*.tsbuildinfo
packages/*/*.tsbuildinfo
1 change: 1 addition & 0 deletions packages/profile-sync-controller/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@
},
"dependencies": {
"@metamask/base-controller": "^8.0.1",
"@metamask/eth-sig-util": "^8.2.0",
"@metamask/keyring-api": "^17.4.0",
"@metamask/snaps-sdk": "^6.22.0",
"@metamask/snaps-utils": "^9.2.0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,3 +41,41 @@ export function createSnapSignMessageRequest(
},
};
}

/**
* Constructs Request to Message Signing Snap to get Public Key
*
* @returns Snap getEncryptionPublicKey Key Request
*/
export function createSnapEncryptionPublicKeyRequest(): SnapRPCRequest {
return {
snapId,
origin: '',
handler: 'onRpcRequest' as any,
request: {
method: 'getEncryptionPublicKey',
},
};
}

/**
* Constructs Request to Message Signing Snap to Decrypt a message
*
* @param ciphertext - The encrypted text to decrypt
* @returns Snap decryptMessage Request
*/
export function createSnapDecryptMessageRequest(
ciphertext: string,
): SnapRPCRequest {
return {
snapId,
origin: '',
handler: 'onRpcRequest' as any,
request: {
method: 'decryptMessage',
params: {
data: JSON.parse(ciphertext),
},
},
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -1024,7 +1024,7 @@ describe('user-storage/user-storage-controller - snap handling', () => {
});

await expect(controller.getStorageKey()).rejects.toThrow(
'#snapSignMessage - unable to call snap, wallet is locked',
/.*unable to call snap, wallet is locked/u,
);
});

Expand All @@ -1044,7 +1044,7 @@ describe('user-storage/user-storage-controller - snap handling', () => {
messengerMocks.baseMessenger.publish('KeyringController:lock');

await expect(controller.getStorageKey()).rejects.toThrow(
'#snapSignMessage - unable to call snap, wallet is locked',
/.*unable to call snap, wallet is locked/u,
);

messengerMocks.baseMessenger.publish('KeyringController:unlock');
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import type {
AccountsControllerAccountAddedEvent,
AccountsControllerAccountRenamedEvent,
AccountsControllerListAccountsAction,
AccountsControllerUpdateAccountMetadataAction,
AccountsControllerAccountRenamedEvent,
AccountsControllerAccountAddedEvent,
} from '@metamask/accounts-controller';
import type {
ControllerGetStateAction,
Expand All @@ -11,6 +11,7 @@ import type {
StateMetadata,
} from '@metamask/base-controller';
import { BaseController } from '@metamask/base-controller';
import { encrypt as ERC1024Encrypt } from '@metamask/eth-sig-util';
import {
type KeyringControllerGetStateAction,
type KeyringControllerLockEvent,
Expand All @@ -26,6 +27,7 @@ import type {
NetworkControllerUpdateNetworkAction,
} from '@metamask/network-controller';
import type { HandleSnapRequest } from '@metamask/snaps-controllers';
import { hexToBytes } from '@noble/hashes/utils';

import {
saveInternalAccountToUserStorage,
Expand All @@ -38,19 +40,24 @@ import {
startNetworkSyncing,
} from './network-syncing/controller-integration';
import { Env, UserStorage } from '../../sdk';
import { byteArrayToBase64 } from '../../shared/encryption/utils';
import type { UserStorageFeatureKeys } from '../../shared/storage-schema';
import {
type UserStoragePathWithFeatureAndKey,
type UserStoragePathWithFeatureOnly,
} from '../../shared/storage-schema';
import type { NativeScrypt } from '../../shared/types/encryption';
import { createSnapSignMessageRequest } from '../authentication/auth-snap-requests';
import type {
AuthenticationControllerGetBearerToken,
AuthenticationControllerGetSessionProfile,
AuthenticationControllerIsSignedIn,
AuthenticationControllerPerformSignIn,
} from '../authentication/AuthenticationController';
} from '../authentication';
import {
createSnapDecryptMessageRequest,
createSnapEncryptionPublicKeyRequest,
createSnapSignMessageRequest,
} from '../authentication/auth-snap-requests';

const controllerName = 'UserStorageController';

Expand Down Expand Up @@ -371,6 +378,23 @@ export default class UserStorageController extends BaseController<
signMessage: (message) =>
this.#snapSignMessage(message as `metamask:${string}`),
},
encryption: {
getEncryptionPublicKey: async () => {
return await this.#snapGetEncryptionPublicKey();
},
decryptMessage: async (ciphertext: string) => {
return await this.#snapDecryptMessage(ciphertext);
},
encryptMessage: async (message: string, publicKeyHex: string) => {
const erc1024Payload = ERC1024Encrypt({
// eth-sig-util expects the public key to be in base64 format
publicKey: byteArrayToBase64(hexToBytes(publicKeyHex)),
data: message,
version: 'x25519-xsalsa20-poly1305',
});
return JSON.stringify(erc1024Payload);
},
},
},
{
storage: {
Expand Down Expand Up @@ -607,6 +631,65 @@ export default class UserStorageController extends BaseController<
return result;
}

#_snapEncryptionKeyCache: string | null = null;

/**
* Get an encryption public key from the snap
*
* @returns The encryption public key used by the snap
*/
async #snapGetEncryptionPublicKey(): Promise<string> {
if (this.#_snapEncryptionKeyCache) {
return this.#_snapEncryptionKeyCache;
}

if (!this.#isUnlocked) {
throw new Error(
'#snapGetEncryptionPublicKey - unable to call snap, wallet is locked',
);
}

const result = (await this.messagingSystem.call(
'SnapController:handleRequest',
createSnapEncryptionPublicKeyRequest(),
)) as string;

this.#_snapEncryptionKeyCache = result;

return result;
}

#_snapDecryptMessageCache: Record<string, string> = {};

/**
* Calls the snap to attempt to decrypt the message.
*
* @param ciphertext - the encrypted text to decrypt.
* @returns The decrypted message, if decryption was possible.
*/
async #snapDecryptMessage(ciphertext: string): Promise<string> {
if (this.#_snapDecryptMessageCache[ciphertext]) {
return this.#_snapDecryptMessageCache[ciphertext];
}

if (!this.#isUnlocked) {
throw new Error(
'#snapDecryptMessage - unable to call snap, wallet is locked',
);
}

const result = (await this.messagingSystem.call(
'SnapController:handleRequest',
createSnapDecryptMessageRequest(ciphertext),
)) as string;

if (result) {
this.#_snapDecryptMessageCache[ciphertext] = result;
}

return result;
}

Comment on lines +641 to +692
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: I feel there's a lot of overlap between those two methods and #snapSignMessage, should we try to refactor and have one snap interaction method that handles designated caches and params?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose it could provide some marginal readability benefits, but at the cost of increasing the complexity of this PR.
For now, we have 4 methods using this pattern, all in one file, all rather small, so I don't think it's a very big deal.
Let's do this in a separate issue/PR, WDYT?

public async setIsBackupAndSyncFeatureEnabled(
feature: keyof typeof BACKUPANDSYNC_FEATURES,
enabled: boolean,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
import type { NotNamespacedBy } from '@metamask/base-controller';
import { Messenger } from '@metamask/base-controller';
import {
decrypt as ERC1024Decrypt,
type EthEncryptedData,
} from '@metamask/eth-sig-util';
import type { EthKeyring } from '@metamask/keyring-internal-api';

import type {
Expand All @@ -8,7 +12,11 @@ import type {
UserStorageControllerMessenger,
} from '..';
import { MOCK_LOGIN_RESPONSE } from '../../authentication/mocks';
import { MOCK_STORAGE_KEY_SIGNATURE } from '../mocks';
import {
MOCK_ENCRYPTION_PRIVATE_KEY,
MOCK_ENCRYPTION_PUBLIC_KEY,
MOCK_STORAGE_KEY_SIGNATURE,
} from '../mocks';

type GetHandler<ActionType extends AllowedActions['type']> = Extract<
AllowedActions,
Expand Down Expand Up @@ -103,6 +111,16 @@ export function mockUserStorageMessenger(
.fn()
.mockResolvedValue(MOCK_STORAGE_KEY_SIGNATURE);

const mockSnapGetEncryptionPublicKey = jest.fn(() =>
Promise.resolve(MOCK_ENCRYPTION_PUBLIC_KEY),
);
const mockDecryptMessage = jest.fn((data: EthEncryptedData) => {
return ERC1024Decrypt({
encryptedData: data,
privateKey: MOCK_ENCRYPTION_PRIVATE_KEY,
});
});

const mockAuthGetBearerToken = typedMockFn(
'AuthenticationController:getBearerToken',
).mockResolvedValue('MOCK_BEARER_TOKEN');
Expand Down Expand Up @@ -167,19 +185,23 @@ export function mockUserStorageMessenger(

if (actionType === 'SnapController:handleRequest') {
const [, params] = typedArgs;
if (params.request.method === 'getPublicKey') {
return mockSnapGetPublicKey();
switch (params.request.method) {
case 'getPublicKey':
return mockSnapGetPublicKey();
case 'signMessage':
return mockSnapSignMessage();
case 'getEncryptionPublicKey':
return mockSnapGetEncryptionPublicKey();
case 'decryptMessage':
// eslint-disable-next-line @typescript-eslint/no-explicit-any
return mockDecryptMessage((params.request.params as any).data);
default:
throw new Error(
`MOCK_FAIL - unsupported SnapController:handleRequest call: ${
params.request.method as string
}`,
);
}

if (params.request.method === 'signMessage') {
return mockSnapSignMessage();
}

throw new Error(
`MOCK_FAIL - unsupported SnapController:handleRequest call: ${
params.request.method as string
}`,
);
}

if (actionType === 'AuthenticationController:getBearerToken') {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ export function mockUserStorageMessengerForAccountSyncing(options?: {
messengerMocks.mockKeyringGetAccounts.mockImplementation(async () => {
return (
options?.accounts?.accountsList
// eslint-disable-next-line @typescript-eslint/no-unsafe-enum-comparison
?.filter((a) => a.metadata.keyring.type === KeyringTypes.hd)
.map((a) => a.address) ??
MOCK_INTERNAL_ACCOUNTS.ALL.map((a) => a.address)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@ import encryption, { createSHA256Hash } from '../../../shared/encryption';
export const MOCK_STORAGE_KEY_SIGNATURE = 'mockStorageKey';
export const MOCK_STORAGE_KEY = createSHA256Hash(MOCK_STORAGE_KEY_SIGNATURE);
export const MOCK_STORAGE_DATA = JSON.stringify({ hello: 'world' });
export const MOCK_ENCRYPTION_PRIVATE_KEY =
'29411742c5cf3d7eb1a0cf57230488900e86778c876078fb87164ec6864841cd';
export const MOCK_ENCRYPTION_PUBLIC_KEY =
'37a3160dda3e0086a89974854f1ba7c3e6b1bc960e06311bcc4c15264d95ee36';

// NOTE - using encryption.encryptString directly in fixtures causes issues on mobile.
// This is because this fixture is getting added in at run time. Will be improved once we support multiple exports
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,6 @@ export function startNetworkSyncing(props: StartNetworkSyncingProps) {
try {
messenger.subscribe(
'NetworkController:networkRemoved',

// eslint-disable-next-line @typescript-eslint/no-misused-promises
async (networkConfiguration) => {
try {
// If blocked (e.g. we have not yet performed a main-sync), then we should not perform any mutations
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ export const MOCK_STORAGE_URL_ALL_FEATURE_ENTRIES = STORAGE_URL(
USER_STORAGE_FEATURE_NAMES.notifications,
);

export const MOCK_STORAGE_KEY = createSHA256Hash('mockStorageKey');
export const MOCK_STORAGE_KEY_SIGNATURE = 'mockStorageKey';
export const MOCK_STORAGE_KEY = createSHA256Hash(MOCK_STORAGE_KEY_SIGNATURE);
export const MOCK_NOTIFICATIONS_DATA = '{ is_compact: false }';
export const MOCK_NOTIFICATIONS_DATA_ENCRYPTED = async (data?: string) =>
await encryption.encryptString(
Expand Down
Loading
Loading