diff --git a/dependency-graph.json b/dependency-graph.json index 9309b18f86e..b9d4b73a5cd 100644 --- a/dependency-graph.json +++ b/dependency-graph.json @@ -111,7 +111,8 @@ "@celo/base", "@celo/contractkit", "@celo/identity", - "@celo/utils" + "@celo/utils", + "@celo/wallet-local" ] }, "@celo/phone-number-privacy-monitor": { diff --git a/packages/phone-number-privacy/common/package.json b/packages/phone-number-privacy/common/package.json index a806d493b45..0c2e129c635 100644 --- a/packages/phone-number-privacy/common/package.json +++ b/packages/phone-number-privacy/common/package.json @@ -34,6 +34,7 @@ "libphonenumber-js": "^1.9.11" }, "devDependencies": { + "@celo/wallet-local": "1.3.1-dev", "@types/btoa": "^1.2.3", "@types/bunyan": "1.8.4", "@types/elliptic": "^6.4.12", diff --git a/packages/phone-number-privacy/common/src/interfaces/requests.ts b/packages/phone-number-privacy/common/src/interfaces/requests.ts index f7e66468160..c8b14032df9 100644 --- a/packages/phone-number-privacy/common/src/interfaces/requests.ts +++ b/packages/phone-number-privacy/common/src/interfaces/requests.ts @@ -5,12 +5,15 @@ import { domainOptionsEIP712Types, KnownDomain, KnownDomainOptions, + SequentialDelayDomain, } from '@celo/identity/lib/odis/domains' import { EIP712Optional, eip712OptionalType, EIP712TypedData, + noString, } from '@celo/utils/lib/sign-typed-data-utils' +import { verifyEIP712TypedDataSigner } from '@celo/utils/lib/signatureUtils' export interface GetBlindedMessageSigRequest { /** Celo account address. Query is charged against this account's quota. */ @@ -39,7 +42,7 @@ export interface GetQuotaRequest { } /** - * Domain resitricted signature request to get a pOPRF evaluation on the given message in a given + * Domain restricted signature request to get a pOPRF evaluation on the given message in a given * domain, as specified by CIP-40. * * @remarks Concrete request types are created by specifying the type parameters for Domain and @@ -67,6 +70,17 @@ export type DomainRestrictedSignatureRequest< 'options' > +/** + * Request to get the quota status of the given domain. ODIS will respond with the current state + * relevant to calculating quota under the associated rate limiting rules. + * + * Options may be provided for authentication in case the quota state is non-public information. + * E.g. Quota state may reveal whether or not a user has attempted to recover a given account. + * + * @remarks Concrete request types are created by specifying the type parameters for Domain and + * DomainOptions. If a DomainOptions type parameter is specified, then the options field is + * required. If not, it must not be provided. + */ export type DomainQuotaStatusRequest< D extends Domain = Domain, O extends DomainOptions = D extends KnownDomain ? KnownDomainOptions : never @@ -82,6 +96,17 @@ export type DomainQuotaStatusRequest< 'options' > +/** + * Request to disable a domain such that not further requests for signatures in the given domain + * will be served. Available for domains which need to option to prevent further requests for + * security. + * + * Options may be provided for authentication to prevent unintended parties from disabling a domain. + * + * @remarks Concrete request types are created by specifying the type parameters for Domain and + * DomainOptions. If a DomainOptions type parameter is specified, then the options field is + * required. If not, it must not be provided. + */ export type DisableDomainRequest< D extends Domain = Domain, O extends DomainOptions = D extends KnownDomain ? KnownDomainOptions : never @@ -97,8 +122,18 @@ export type DisableDomainRequest< 'options' > +/** Union type of Domain API requests */ +export type DomainRequest< + D extends Domain = Domain, + O extends DomainOptions = D extends KnownDomain ? KnownDomainOptions : never +> = + | DomainRestrictedSignatureRequest + | DomainQuotaStatusRequest + | DisableDomainRequest + +/** Wraps the signature request as an EIP-712 typed data structure for hashing and signing */ export function domainRestrictedSignatureRequestEIP712( - request: DomainRestrictedSignatureRequest> + request: DomainRestrictedSignatureRequest ): EIP712TypedData { const domainTypes = domainEIP712Types(request.domain) const optionsTypes = domainOptionsEIP712Types(request.domain) @@ -128,8 +163,9 @@ export function domainRestrictedSignatureRequestEIP712( } } +/** Wraps the domain quota request as an EIP-712 typed data structure for hashing and signing */ export function domainQuotaStatusRequestEIP712( - request: DomainQuotaStatusRequest> + request: DomainQuotaStatusRequest ): EIP712TypedData { const domainTypes = domainEIP712Types(request.domain) const optionsTypes = domainOptionsEIP712Types(request.domain) @@ -158,8 +194,9 @@ export function domainQuotaStatusRequestEIP712( } } +/** Wraps the disable domain request as an EIP-712 typed data structure for hashing and signing */ export function disableDomainRequestEIP712( - request: DisableDomainRequest> + request: DisableDomainRequest ): EIP712TypedData { const domainTypes = domainEIP712Types(request.domain) const optionsTypes = domainOptionsEIP712Types(request.domain) @@ -188,6 +225,91 @@ export function disableDomainRequestEIP712( } } +/** + * Generic function to verify the signature on a Domain API request. + * + * @remarks Passing in the builder allows the caller to handle the differences of EIP-712 types + * between request types. Requests cannot be fully differentiated at runtime. In particular, + * DomainQuotaStatusRequest and DisableDomainRequest are indistinguishable at runtime. + * + * @privateRemarks Function is currently defined explicitly in terms of SequentialDelayDomain. It + * should be generalized to other authenticated domain types as they are standardized. + */ +function verifyRequestSignature>( + typedDataBuilder: (request: R) => EIP712TypedData, + request: R +): boolean { + // If the address field is undefined, then this domain is unauthenticated. + // Return true as the signature does not need to be checked. + if (!request.domain.address.defined) { + return true + } + const signer = request.domain.address.value + + // If not signature is provided, return false. + if (!request.options.signature.defined) { + return false + } + const signature = request.options.signature.value + + // Requests are signed over the message excluding the signature. CIP-40 specifies that the + // signature in the signed message should be the zero value. When the signature type is + // EIP712Optional, this is { defined: false, value: "" } (i.e. `noString`) + const message: R = { + ...request, + options: { + ...request.options, + signature: noString, + }, + } + + // Build the typed data then return the result of signature verification. + const typedData = typedDataBuilder(message) + return verifyEIP712TypedDataSigner(typedData, signature, signer) +} + +/** + * Verifies the signature over a signature request for authenticated domains. + * If the domain is unauthenticated, this function returns true. + * + * @remarks As specified in CIP-40, the signed message is the full request interpretted as EIP-712 + * typed data with the signature field in the domain options set to its zero value (i.e. It is set + * to the undefined value for type EIP712Optional). + */ +export function verifyDomainRestrictedSignatureRequestSignature( + request: DomainRestrictedSignatureRequest +): boolean { + return verifyRequestSignature(domainRestrictedSignatureRequestEIP712, request) +} + +/** + * Verifies the signature over a domain quota status request for authenticated domains. + * If the domain is unauthenticated, this function returns true. + * + * @remarks As specified in CIP-40, the signed message is the full request interpretted as EIP-712 + * typed data with the signature field in the domain options set to its zero value (i.e. It is set + * to the undefined value for type EIP712Optional). + */ +export function verifyDomainQuotaStatusRequestSignature( + request: DomainQuotaStatusRequest +): boolean { + return verifyRequestSignature(domainQuotaStatusRequestEIP712, request) +} + +/** + * Verifies the signature over a disable domain request for authenticated domains. + * If the domain is unauthenticated, this function returns true. + * + * @remarks As specified in CIP-40, the signed message is the full request interpretted as EIP-712 + * typed data with the signature field in the domain options set to its zero value (i.e. It is set + * to the undefined value for type EIP712Optional). + */ +export function verifyDisableDomainRequestSignature( + request: DisableDomainRequest +): boolean { + return verifyRequestSignature(disableDomainRequestEIP712, request) +} + // Use distributive conditional types to extract from the keys of T, keys with value type != never. // Eg. AssignableKeys<{ foo: string, bar: never }, 'foo'|'bar'> = 'foo' type AssignableKeys = K extends (T[K] extends never ? never : K) ? K : never diff --git a/packages/phone-number-privacy/common/test/interfaces/requests.test.ts b/packages/phone-number-privacy/common/test/interfaces/requests.test.ts index 071521c5f2c..a4f0b77d09a 100644 --- a/packages/phone-number-privacy/common/test/interfaces/requests.test.ts +++ b/packages/phone-number-privacy/common/test/interfaces/requests.test.ts @@ -4,8 +4,10 @@ import { generateTypedDataHash, noBool, noString, + noNumber, } from '@celo/utils/lib/sign-typed-data-utils' import { Domain, DomainOptions, SequentialDelayDomain } from '@celo/identity/lib/odis/domains' +import { LocalWallet } from '@celo/wallet-local' import { DomainRestrictedSignatureRequest, domainRestrictedSignatureRequestEIP712, @@ -13,6 +15,9 @@ import { domainQuotaStatusRequestEIP712, DisableDomainRequest, disableDomainRequestEIP712, + verifyDisableDomainRequestSignature, + verifyDomainQuotaStatusRequestSignature, + verifyDomainRestrictedSignatureRequestSignature, } from '../../src/interfaces/requests' // Compile-time check that DomainRestrictedSignatureRequest can be cast to type EIP712Object. @@ -111,3 +116,149 @@ describe('disableDomainRequestEIP712()', () => { expect(generateTypedDataHash(typedData).toString('hex')).toEqual(expectedHash) }) }) + +const wallet = new LocalWallet() +wallet.addAccount('0x00000000000000000000000000000000000000000000000000000000deadbeef') +wallet.addAccount('0x00000000000000000000000000000000000000000000000000000000bad516e9') +const walletAddress = wallet.getAccounts()[0]! +const badAddress = wallet.getAccounts()[1]! + +const authenticatedDomain: SequentialDelayDomain = { + name: 'ODIS Sequential Delay Domain', + version: '1', + stages: [{ delay: 0, resetTimer: noBool, batchSize: defined(2), repetitions: defined(10) }], + address: defined(walletAddress), + salt: noString, +} + +const unauthenticatedDomain: SequentialDelayDomain = { + name: 'ODIS Sequential Delay Domain', + version: '1', + stages: [{ delay: 0, resetTimer: noBool, batchSize: defined(2), repetitions: defined(10) }], + address: noString, + salt: noString, +} + +const manipulatedDomain: SequentialDelayDomain = { + name: 'ODIS Sequential Delay Domain', + version: '1', + stages: [{ delay: 0, resetTimer: noBool, batchSize: defined(100), repetitions: defined(10) }], + address: defined(walletAddress), + salt: noString, +} + +const cases = [ + { + request: { + domain: authenticatedDomain, + options: { + signature: noString, + nonce: defined(0), + }, + blindedMessage: '', + sessionID: noString, + } as DomainRestrictedSignatureRequest, + typedDataBuilder: domainRestrictedSignatureRequestEIP712, + verifier: verifyDomainRestrictedSignatureRequestSignature, + name: 'verifyDomainRestrictedSignatureRequestSignature()', + }, + { + request: { + domain: authenticatedDomain, + options: { + signature: noString, + nonce: defined(0), + }, + sessionID: noString, + } as DomainQuotaStatusRequest, + typedDataBuilder: domainQuotaStatusRequestEIP712, + verifier: verifyDomainQuotaStatusRequestSignature, + name: 'verifyDomainQuotaStatusRequestSignature()', + }, + { + request: { + domain: authenticatedDomain, + options: { + signature: noString, + nonce: defined(0), + }, + sessionID: noString, + } as DisableDomainRequest, + typedDataBuilder: disableDomainRequestEIP712, + verifier: verifyDisableDomainRequestSignature, + name: 'verifyDisableDomainRequestSignature()', + }, +] + +for (const { request, verifier, typedDataBuilder, name } of cases) { + describe(name, () => { + it('should report a correctly signed request as verified', async () => { + //@ts-ignore type checking does not correctly infer types. + const typedData = typedDataBuilder(request) + const signature = await wallet.signTypedData(walletAddress, typedData) + const signed = { + ...request, + options: { + ...request.options, + signature: defined(signature), + }, + } + + //@ts-ignore type checking does not correctly infer types. + expect(verifier(signed)).toBe(true) + }) + + it('should report an unsigned message as unverified', async () => { + //@ts-ignore type checking does not correctly infer types. + expect(verifier(request)).toBe(false) + }) + + it('should report a manipulated message as unverified', async () => { + //@ts-ignore type checking does not correctly infer types. + const typedData = typedDataBuilder(request) + const signature = await wallet.signTypedData(walletAddress, typedData) + const signed = { + ...request, + options: { + ...request.options, + signature: defined(signature), + }, + } + //@ts-ignore type checking does not correctly infer types. + expect(verifier(signed)).toBe(true) + + const manipulated = { ...request, domain: manipulatedDomain } + //@ts-ignore type checking does not correctly infer types. + expect(verifier(manipulated)).toBe(false) + }) + + it('should report an incorrectly signed request as unverified', async () => { + //@ts-ignore type checking does not correctly infer types. + const typedData = typedDataBuilder(request) + const signature = await wallet.signTypedData(badAddress, typedData) + const signed = { + ...request, + options: { + ...request.options, + signature: defined(signature), + }, + } + + //@ts-ignore type checking does not correctly infer types. + expect(verifier(signed)).toBe(false) + }) + + it('should report requests against unauthenticated domains to be verified', async () => { + const unauthenticatedRequest = { + ...request, + domain: unauthenticatedDomain, + options: { + signature: noString, + nonce: noNumber, + }, + } + //@ts-ignore type checking does not correctly infer types. + expect(verifier(unauthenticatedRequest)).toBe(true) + }) + }) +} diff --git a/packages/sdk/identity/src/odis/domains.ts b/packages/sdk/identity/src/odis/domains.ts index 3481f3f9f90..56dab2805ab 100644 --- a/packages/sdk/identity/src/odis/domains.ts +++ b/packages/sdk/identity/src/odis/domains.ts @@ -34,17 +34,25 @@ export interface Domain { export type DomainOptions = EIP712Object export type SequentialDelayStage = { - // How many seconds each batch of attempts in this stage is delayed with - // respect to the timer. + /** + * How many seconds each batch of attempts in this stage is delayed with + * respect to the timer. + */ delay: number - // Whether the timer should be reset between attempts during this stage. - // Defaults to true. + /** + * Whether the timer should be reset between attempts during this stage. + * Defaults to true. + */ resetTimer: EIP712Optional - // The number of continuous attempts a user gets before the next delay - // in each repetition of this stage. Defaults to 1. + /** + * The number of continuous attempts a user gets before the next delay + * in each repetition of this stage. Defaults to 1. + */ batchSize: EIP712Optional - // The number of times this stage repeats before continuing to the next stage - // in the RateLimit array. Defaults to 1. + /** + * The number of times this stage repeats before continuing to the next stage + * in the RateLimit array. Defaults to 1. + */ repetitions: EIP712Optional } @@ -52,20 +60,32 @@ export type SequentialDelayDomain = { name: 'ODIS Sequential Delay Domain' version: '1' stages: SequentialDelayStage[] - // Optional Celo address against which signed requests must be authenticated. - // In the case of Cloud Backup, this will be derived from a one-time key stored with the ciphertext. + /** + * Optional Celo address against which signed requests must be authenticated. + * In the case of Cloud Backup, this will be derived from a one-time key stored with the ciphertext. + * Encoded as a checksummed address with leading "0x". + */ address: EIP712Optional - // Optional string to distinguish the output of this domain instance from - // other SequentialDelayDomain instances + /** + * Optional string to distinguish the output of this domain instance from + * other SequentialDelayDomain instances + */ salt: EIP712Optional } export type SequentialDelayDomainOptions = { - // EIP-712 signature over the entire request by the address specified in the domain. - // Required if `address` is defined in the domain instance. If `address` is - // not defined in the domain instance, then a signature must not be provided. + /** + * EIP-712 signature over the entire request by the address specified in the domain. + * Required if `address` is defined in the domain instance. If `address` is + * not defined in the domain instance, then a signature must not be provided. + * Encoded as a hex string with leading 0x. + */ signature: EIP712Optional - // Used to prevent replay attacks. Required if a signature is provided. + /** + * Used to prevent replay attacks. Required if a signature is provided. + * Code verifying the signature for rate limiting should check this nonce against a counter of + * applied requests. E.g. Ensure the nonce is 0 on the first request and 2 on the third. + */ nonce: EIP712Optional }