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

Use Argon2 for encrypted vaults #3502

Merged
merged 14 commits into from
Jul 4, 2023
Merged
Show file tree
Hide file tree
Changes from 13 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
8 changes: 7 additions & 1 deletion .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,13 @@ module.exports = {
],
},
],
"@typescript-eslint/no-unused-vars": "error",
"@typescript-eslint/no-unused-vars": [
"error",
{
argsIgnorePattern: "^_",
varsIgnorePattern: "^_",
},
],
"no-unused-vars": "off",
},
ignorePatterns: [
Expand Down
2 changes: 2 additions & 0 deletions background/lib/posthog.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ export enum OneTimeAnalyticsEvent {
ONBOARDING_STARTED = "Onboarding Started",
ONBOARDING_FINISHED = "Onboarding Finished",
CHAIN_ADDED = "Chain Added",
ARGON_MIGRATION = "Migrate to Argon2",
ARGON_MIGRATION_FAILED = "Migrate to Argon2 failed",
}

export const isOneTimeAnalyticsEvent = (
Expand Down
2 changes: 1 addition & 1 deletion background/lib/token-lists.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import { DeepWriteable } from "../types"
const cleanTokenListResponse = (json: any, url: string) => {
if (url.includes("api-polygon-tokens.polygon.technology")) {
if (typeof json === "object" && json !== null && "tags" in json) {
const { tags, ...cleanedJson } = json
const { tags: _, ...cleanedJson } = json
return cleanedJson
}
}
Expand Down
4 changes: 2 additions & 2 deletions background/lib/utils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,14 +108,14 @@ export function gweiToWei(value: number | bigint): bigint {
return BigInt(utils.parseUnits(value.toString(), "gwei").toString())
}

export function convertToEth(value: string | number | bigint): string {
export function convertToEth(value: bigint): string {
if (value && value >= 1) {
return utils.formatUnits(BigInt(value))
}
return ""
}

export function weiToGwei(value: string | number | bigint): string {
export function weiToGwei(value: bigint): string {
if (value && value >= 1) {
return truncateDecimalAmount(utils.formatUnits(BigInt(value), "gwei"), 2)
}
Expand Down
2 changes: 1 addition & 1 deletion background/lib/validate/prices.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export const coingeckoPriceSchema: JSONSchemaType<CoingeckoPriceData> = {
additionalProperties: { type: "number", nullable: true },
nullable: true,
},
}
} as const

export type CoingeckoPriceData = {
[coinId: string]:
Expand Down
10 changes: 6 additions & 4 deletions background/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -303,8 +303,12 @@ export default class Main extends BaseService<never> {

static create: ServiceCreatorFunction<never, Main, []> = async () => {
const preferenceService = PreferenceService.create()
const internalSignerService =
InternalSignerService.create(preferenceService)
const analyticsService = AnalyticsService.create(preferenceService)

const internalSignerService = InternalSignerService.create(
preferenceService,
analyticsService
)
const chainService = ChainService.create(
preferenceService,
internalSignerService
Expand Down Expand Up @@ -337,8 +341,6 @@ export default class Main extends BaseService<never> {
chainService
)

const analyticsService = AnalyticsService.create(preferenceService)

const nftsService = NFTsService.create(chainService)

const abilitiesService = AbilitiesService.create(
Expand Down
3 changes: 3 additions & 0 deletions background/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,9 @@
"@walletconnect/utils": "^2.1.4",
"ajv": "^8.6.2",
"ajv-formats": "^2.1.0",
"argon2-browser": "^1.18.0",
"assert": "^2.0.0",
"base64-loader": "^1.0.0",
"bnc-sdk": "^3.4.1",
"dayjs": "^1.10.7",
"dexie": "^3.0.3",
Expand All @@ -64,6 +66,7 @@
},
"devDependencies": {
"@reduxjs/toolkit": "^1.6.1",
"@types/argon2-browser": "^1.18.1",
"@types/sinon": "^10.0.12",
"@types/uuid": "^8.3.4",
"@types/webextension-polyfill": "^0.8.0",
Expand Down
2 changes: 1 addition & 1 deletion background/redux-slices/migrations/to-14.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
export default (
prevState: Record<string, unknown>
): Record<string, unknown> => {
const { assets, ...newState } = prevState
const { assets: _, ...newState } = prevState

// Clear assets collection; these should be immediately repopulated by the
// IndexingService in startService.
Expand Down
7 changes: 5 additions & 2 deletions background/redux-slices/migrations/to-16.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,11 @@ export default (prevState: Record<string, unknown>): NewState => {
: {},
}

const { spenderName, spenderAddress, ...oldAnnotationProps } =
annotation
const {
spenderName: __,
spenderAddress: ___,
...oldAnnotationProps
} = annotation

newState.activities[address][chainID].entities[
activityItem.hash
Expand Down
2 changes: 1 addition & 1 deletion background/redux-slices/migrations/to-19.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
export default (
prevState: Record<string, unknown>
): Record<string, unknown> => {
const { activities, ...newState } = prevState
const { activities: _, ...newState } = prevState

// Clear activities slice as we now have new activities slice instead
newState.activities = {}
Expand Down
2 changes: 1 addition & 1 deletion background/redux-slices/migrations/to-22.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
export default (
prevState: Record<string, unknown>
): Record<string, unknown> => {
const { assets, ...newState } = prevState
const { assets: _, ...newState } = prevState

// Clear assets collection; these should be immediately repopulated by the
// IndexingService in startService.
Expand Down
2 changes: 1 addition & 1 deletion background/redux-slices/migrations/to-29.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
export default (
prevState: Record<string, unknown>
): Record<string, unknown> => {
const { assets, ...newState } = prevState
const { assets: _, ...newState } = prevState

// Clear assets collection; these should be immediately repopulated by the
// IndexingService in startService.
Expand Down
2 changes: 1 addition & 1 deletion background/redux-slices/migrations/to-3.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
export default (
prevState: Record<string, unknown>
): Record<string, unknown> => {
const { assets, ...newState } = prevState
const { assets: _, ...newState } = prevState

// Clear assets collection; these should be immediately repopulated by the
// IndexingService in startService.
Expand Down
2 changes: 1 addition & 1 deletion background/redux-slices/migrations/to-30.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ type NewState = {

// Remove old nfts slice and rename updated nfts slice
export default (prevState: Record<string, unknown>): NewState => {
const { nfts, nftsUpdate, ...otherState } = prevState as OldState
const { nfts: _, nftsUpdate, ...otherState } = prevState as OldState

return {
...otherState,
Expand Down
2 changes: 1 addition & 1 deletion background/redux-slices/migrations/to-32.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ type NewState = {
export default (prevState: Record<string, unknown>): NewState => {
const oldState = prevState as OldState
const {
keyrings: { keyringMetadata, importing, ...keyringsState },
keyrings: { keyringMetadata, importing: _, ...keyringsState },
...stateWithoutKeyrings
} = oldState

Expand Down
2 changes: 1 addition & 1 deletion background/redux-slices/migrations/to-4.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export default (
},
}

const { blocks, ...oldStateAccountWithoutBlocks } = oldState.account ?? {
const { blocks: _, ...oldStateAccountWithoutBlocks } = oldState.account ?? {
blocks: undefined,
}

Expand Down
12 changes: 11 additions & 1 deletion background/redux-slices/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,16 @@ const asyncThunkProperties = (() => {
return exhaustiveList
})()

// Extracts a @reduxjs/toolkit internal type for type alignment in the below
// function types.
type AsyncThunkConfig = ReturnType<typeof createAsyncThunk> extends AsyncThunk<
infer _,
infer __,
infer T
>
? T
: never

/**
* Create an async thunk action that will always run in the background script,
* and dispatches lifecycle actions (pending, fulfilled, rejected) on the
Expand Down Expand Up @@ -108,7 +118,7 @@ export function createBackgroundAsyncThunk<
TypePrefix extends string,
Returned,
ThunkArg = void,
ThunkApiConfig = { extra: { main: Main } }
ThunkApiConfig extends AsyncThunkConfig = { extra: { main: Main } }
>(
typePrefix: TypePrefix,
payloadCreator: AsyncThunkPayloadCreator<Returned, ThunkArg, ThunkApiConfig>,
Expand Down
5 changes: 4 additions & 1 deletion background/services/chain/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1642,7 +1642,7 @@ export default class ChainService extends BaseService<Events> {
// Don't override an already-persisted successful status with
// an expiration-based failed status, but do set status to
// failure if no transaction was seen.
{ status: 0, ...existingTransaction },
{ status: 0, ...existingTransaction } as AnyEVMTransaction,
"local"
)
}
Expand Down Expand Up @@ -1710,6 +1710,9 @@ export default class ChainService extends BaseService<Events> {
logger.error(`Error emitting tx ${finalTransaction}`, error)
}
if (error) {
// We don't control the errors in the whole stack, but we do want to
// rethrow them regardless.
// eslint-disable-next-line @typescript-eslint/no-throw-literal
throw error
}
}
Expand Down
76 changes: 66 additions & 10 deletions background/services/internal-signer/encryption.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import argon2 from "argon2-browser"
/**
* An encrypted vault which can be safely serialized and stored.
*/
Expand All @@ -10,6 +11,11 @@ export type EncryptedVault = {
cipherText: string
}

export enum VaultVersion {
Copy link
Contributor

Choose a reason for hiding this comment

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

Re: my note about not leaking vault version information: that basically means we should try to be able to not export this.

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 see but if we need to know and save (in the service) on which version we are then we need this exported, right?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ultimately discussed in #3502 (comment), since what I was pushing for is precisely not saving in the service which version we're on—but the need for that has been clarified.

PBKDF2 = 1,
Argon2 = 2,
}

/*
* A key with a salt that can be combined with a password to re-derive the key.
*
Expand All @@ -21,7 +27,7 @@ export type SaltedKey = {
key: CryptoKey
}

function bufferToBase64(array: Uint8Array): string {
function bufferToBase64(array: Uint8Array | ArrayBuffer): string {
return Buffer.from(array).toString("base64")
}

Expand Down Expand Up @@ -64,7 +70,7 @@ function requireCryptoGlobal(message?: string) {
* material using AES GCM mode, as well as the salt required to derive
* the key again later.
*/
export async function deriveSymmetricKeyFromPassword(
export async function deprecatedDerivePbkdf2KeyFromPassword(
password: string,
existingSalt?: string
): Promise<SaltedKey> {
Expand Down Expand Up @@ -101,6 +107,48 @@ export async function deriveSymmetricKeyFromPassword(
}
}

export async function deriveArgon2KeyFromPassword(
password: string,
existingSalt?: string
): Promise<SaltedKey> {
const { crypto } = global

const salt = existingSalt || (await generateSalt())

const { hash } = await argon2.hash({
pass: password,
salt,
hashLen: 32,
})

const key = await crypto.subtle.importKey(
"raw",
hash,
{ name: "AES-GCM", length: 256 },
false,
["encrypt", "decrypt"]
)

return {
key,
salt,
}
}

export async function deriveSymmetricKeyFromPassword(
version: VaultVersion,
password: string,
existingSalt?: string
): Promise<SaltedKey> {
switch (version) {
case VaultVersion.PBKDF2:
return deprecatedDerivePbkdf2KeyFromPassword(password, existingSalt)
case VaultVersion.Argon2:
return deriveArgon2KeyFromPassword(password, existingSalt)
default:
throw new Error(`Unsupported vault version: ${version}`)
}
}
/**
* Encrypt a JSON-serializable object with a supplied password using AES GCM
* mode.
Expand All @@ -111,16 +159,18 @@ export async function deriveSymmetricKeyFromPassword(
* @returns the ciphertext and all non-password material required for later
* decryption, including the salt and AES initialization vector.
*/
export async function encryptVault<V>(
vault: V,
export async function encryptVault<V>(vaultData: {
vault: V
passwordOrSaltedKey: string | SaltedKey
): Promise<EncryptedVault> {
version: VaultVersion
}): Promise<EncryptedVault> {
requireCryptoGlobal("Encrypting a vault")
const { crypto } = global
const { vault, passwordOrSaltedKey, version } = vaultData

const { key, salt } =
typeof passwordOrSaltedKey === "string"
? await deriveSymmetricKeyFromPassword(passwordOrSaltedKey)
? await deriveSymmetricKeyFromPassword(version, passwordOrSaltedKey)
: passwordOrSaltedKey

const encoder = new TextEncoder()
Expand Down Expand Up @@ -159,18 +209,24 @@ export async function encryptVault<V>(
* most objects `decryptVault(encryptVault(o, password), password)`
* should deeply equal `o`.
*/
export async function decryptVault<V>(
vault: EncryptedVault,
export async function decryptVault<V>(vaultData: {
vault: EncryptedVault
passwordOrSaltedKey: string | SaltedKey
): Promise<V> {
version: VaultVersion
}): Promise<V> {
const { vault, passwordOrSaltedKey } = vaultData
requireCryptoGlobal("Decrypting a vault")
const { crypto } = global

const { initializationVector, salt, cipherText } = vault

const { key } =
typeof passwordOrSaltedKey === "string"
? await deriveSymmetricKeyFromPassword(passwordOrSaltedKey, salt)
? await deriveSymmetricKeyFromPassword(
vaultData.version,
passwordOrSaltedKey,
salt
)
: passwordOrSaltedKey

const plaintext = await crypto.subtle.decrypt(
Expand Down
Loading