Add signature verification code for CIP-40 requests

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": {

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",

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<D> : 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<D> : 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<D> : never
+> =
+  | DomainRestrictedSignatureRequest<D, O>
+  | DomainQuotaStatusRequest<D, O>
+  | DisableDomainRequest<D, O>
+
+/** Wraps the signature request as an EIP-712 typed data structure for hashing and signing */
 export function domainRestrictedSignatureRequestEIP712<D extends KnownDomain>(
-  request: DomainRestrictedSignatureRequest<D, KnownDomainOptions<D>>
+  request: DomainRestrictedSignatureRequest<D>
 ): EIP712TypedData {
   const domainTypes = domainEIP712Types(request.domain)
   const optionsTypes = domainOptionsEIP712Types(request.domain)
@@ -128,8 +163,9 @@ export function domainRestrictedSignatureRequestEIP712<D extends KnownDomain>(
   }
 }
 
+/** Wraps the domain quota request as an EIP-712 typed data structure for hashing and signing */
 export function domainQuotaStatusRequestEIP712<D extends KnownDomain>(
-  request: DomainQuotaStatusRequest<D, KnownDomainOptions<D>>
+  request: DomainQuotaStatusRequest<D>
 ): EIP712TypedData {
   const domainTypes = domainEIP712Types(request.domain)
   const optionsTypes = domainOptionsEIP712Types(request.domain)
@@ -158,8 +194,9 @@ export function domainQuotaStatusRequestEIP712<D extends KnownDomain>(
   }
 }
 
+/** Wraps the disable domain request as an EIP-712 typed data structure for hashing and signing */
 export function disableDomainRequestEIP712<D extends KnownDomain>(
-  request: DisableDomainRequest<D, KnownDomainOptions<D>>
+  request: DisableDomainRequest<D>
 ): EIP712TypedData {
   const domainTypes = domainEIP712Types(request.domain)
   const optionsTypes = domainOptionsEIP712Types(request.domain)
@@ -188,6 +225,91 @@ export function disableDomainRequestEIP712<D extends KnownDomain>(
   }
 }
 
+/**
+ * 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<R extends DomainRequest<SequentialDelayDomain>>(
+  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<string>, 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<string>).
+ */
+export function verifyDomainRestrictedSignatureRequestSignature(
+  request: DomainRestrictedSignatureRequest<SequentialDelayDomain>
+): 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<string>).
+ */
+export function verifyDomainQuotaStatusRequestSignature(
+  request: DomainQuotaStatusRequest<SequentialDelayDomain>
+): 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<string>).
+ */
+export function verifyDisableDomainRequestSignature(
+  request: DisableDomainRequest<SequentialDelayDomain>
+): 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<T, K extends keyof T> = K extends (T[K] extends never ? never : K) ? K : never

packages/phone-number-privacy/common/test/interfaces/requests.test.ts

@@ -4,15 +4,20 @@ 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,
   DomainQuotaStatusRequest,
   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: '<blinded message>',
+      sessionID: noString,
+    } as DomainRestrictedSignatureRequest<SequentialDelayDomain>,
+    typedDataBuilder: domainRestrictedSignatureRequestEIP712,
+    verifier: verifyDomainRestrictedSignatureRequestSignature,
+    name: 'verifyDomainRestrictedSignatureRequestSignature()',
+  },
+  {
+    request: {
+      domain: authenticatedDomain,
+      options: {
+        signature: noString,
+        nonce: defined(0),
+      },
+      sessionID: noString,
+    } as DomainQuotaStatusRequest<SequentialDelayDomain>,
+    typedDataBuilder: domainQuotaStatusRequestEIP712,
+    verifier: verifyDomainQuotaStatusRequestSignature,
+    name: 'verifyDomainQuotaStatusRequestSignature()',
+  },
+  {
+    request: {
+      domain: authenticatedDomain,
+      options: {
+        signature: noString,
+        nonce: defined(0),
+      },
+      sessionID: noString,
+    } as DisableDomainRequest<SequentialDelayDomain>,
+    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)
+    })
+  })
+}