Fixed bug on usage of GeneralJwsVerifier and improved its usability…

… & performance (#594)

* Fixed bug where a new cache is created every time the DWN performs a signature check
* Removed the need to instantiate a new instance of GeneralJwsVerifier every time DWN performs a signature check
* Improved perf by not doing DID resolution when the signature check result is already cached
* Minor error code naming improvements

src/core/auth.ts

@@ -23,7 +23,7 @@ export async function validateMessageSignatureIntegrity(
 ): Promise<{ descriptorCid: CID, [key: string]: any }> {
 
   if (messageSignature.signatures.length !== 1) {
-    throw new DwnError(DwnErrorCode.AuthenticateMoreThanOneAuthoriation, 'expected no more than 1 signature for authorization purpose');
+    throw new DwnError(DwnErrorCode.AuthenticationMoreThanOneSignatureNotSupported, 'expected no more than 1 signature for authorization purpose');
   }
 
   // validate payload integrity
@@ -55,19 +55,16 @@ export async function authenticate(authorizationModel: AuthorizationModel | unde
     throw new DwnError(DwnErrorCode.AuthenticateJwsMissing, 'Missing JWS.');
   }
 
-  const signatureVerifier = new GeneralJwsVerifier(authorizationModel.signature);
-  await signatureVerifier.verify(didResolver);
+  await GeneralJwsVerifier.verifySignatures(authorizationModel.signature, didResolver);
 
   if (authorizationModel.ownerSignature !== undefined) {
-    const ownerSignatureVerifier = new GeneralJwsVerifier(authorizationModel.ownerSignature);
-    await ownerSignatureVerifier.verify(didResolver);
+    await GeneralJwsVerifier.verifySignatures(authorizationModel.ownerSignature, didResolver);
   }
 
   if (authorizationModel.authorDelegatedGrant !== undefined) {
     // verify the signature of the grantor of the delegated grant
     const authorDelegatedGrant = await PermissionsGrant.parse(authorizationModel.authorDelegatedGrant);
-    const grantedBySignatureVerifier = new GeneralJwsVerifier(authorDelegatedGrant.message.authorization.signature);
-    await grantedBySignatureVerifier.verify(didResolver);
+    await GeneralJwsVerifier.verifySignatures(authorDelegatedGrant.message.authorization.signature, didResolver);
   }
 }
 

src/core/dwn-error.ts

@@ -14,8 +14,8 @@ export class DwnError extends Error {
  */
 export enum DwnErrorCode {
   AuthenticateJwsMissing = 'AuthenticateJwsMissing',
-  AuthenticateMoreThanOneAuthoriation = 'AuthenticateMoreThanOneAuthoriation',
   AuthenticateDescriptorCidMismatch = 'AuthenticateDescriptorCidMismatch',
+  AuthenticationMoreThanOneSignatureNotSupported = 'AuthenticationMoreThanOneSignatureNotSupported',
   AuthorizationUnknownAuthor = 'AuthorizationUnknownAuthor',
   AuthorizationNotGrantedToAuthor = 'AuthorizationNotGrantedToAuthor',
   ComputeCidCodecNotSupported = 'ComputeCidCodecNotSupported',
@@ -25,6 +25,7 @@ export enum DwnErrorCode {
   DidNotValid = 'DidNotValid',
   DidResolutionFailed = 'DidResolutionFailed',
   Ed25519InvalidJwk = 'Ed25519InvalidJwk',
+  GeneralJwsVerifierGetPublicKeyNotFound = 'GeneralJwsVerifierGetPublicKeyNotFound',
   GeneralJwsVerifierInvalidSignature = 'GeneralJwsVerifierInvalidSignature',
   GrantAuthorizationGrantExpired = 'GrantAuthorizationGrantExpired',
   GrantAuthorizationGrantMissing = 'GrantAuthorizationGrantMissing',
@@ -76,7 +77,6 @@ export enum DwnErrorCode {
   ProtocolsQueryUnauthorized = 'ProtocolsQueryUnauthorized',
   RecordsDecryptNoMatchingKeyEncryptedFound = 'RecordsDecryptNoMatchingKeyEncryptedFound',
   RecordsDeleteAuthorizationFailed = 'RecordsDeleteAuthorizationFailed',
-
   RecordsGrantAuthorizationConditionPublicationProhibited = 'RecordsGrantAuthorizationConditionPublicationProhibited',
   RecordsGrantAuthorizationConditionPublicationRequired = 'RecordsGrantAuthorizationConditionPublicationRequired',
   RecordsGrantAuthorizationScopeContextIdMismatch = 'RecordsGrantAuthorizationScopeContextIdMismatch',
@@ -131,5 +131,4 @@ export enum DwnErrorCode {
   UrlProtocolNotNormalizable = 'UrlProtocolNotNormalizable',
   UrlSchemaNotNormalized = 'UrlSchemaNotNormalized',
   UrlSchemaNotNormalizable = 'UrlSchemaNotNormalizable',
-  VerifierValidPublicKeyNotFound = 'VerifierValidPublicKeyNotFound',
 };

src/jose/jws/general/verifier.ts

@@ -13,29 +13,53 @@ type VerificationResult = {
   signers: string[];
 };
 
+/**
+ * Verifies the signature(s) of a General JWS.
+ */
 export class GeneralJwsVerifier {
-  jws: GeneralJws;
+
+  private static _singleton: GeneralJwsVerifier;
+
   cache: Cache;
 
-  constructor(jws: GeneralJws, cache?: Cache) {
-    this.jws = jws;
+  private constructor(cache?: Cache) {
     this.cache = cache || new MemoryCache(600);
   }
 
-  async verify(didResolver: DidResolver): Promise<VerificationResult> {
+  private static get singleton(): GeneralJwsVerifier {
+    if (GeneralJwsVerifier._singleton === undefined) {
+      GeneralJwsVerifier._singleton = new GeneralJwsVerifier();
+    }
+
+    return GeneralJwsVerifier._singleton;
+  }
+
+  /**
+   * Verifies the signatures of the given General JWS.
+   * @returns the list of signers that have valid signatures.
+   */
+  public static async verifySignatures(jws: GeneralJws, didResolver: DidResolver): Promise<VerificationResult> {
+    return await GeneralJwsVerifier.singleton.verifySignatures(jws, didResolver);
+  }
+
+  /**
+   * Verifies the signatures of the given General JWS.
+   * @returns the list of signers that have valid signatures.
+   */
+  public async verifySignatures(jws: GeneralJws, didResolver: DidResolver): Promise<VerificationResult> {
     const signers: string[] = [];
 
-    for (const signatureEntry of this.jws.signatures) {
+    for (const signatureEntry of jws.signatures) {
       let isVerified: boolean;
-      const cacheKey = `${signatureEntry.protected}.${this.jws.payload}.${signatureEntry.signature}`;
       const kid = Jws.getKid(signatureEntry);
-      const publicJwk = await GeneralJwsVerifier.getPublicKey(kid, didResolver);
 
+      const cacheKey = `${signatureEntry.protected}.${jws.payload}.${signatureEntry.signature}`;
       const cachedValue = await this.cache.get(cacheKey);
 
-      // explicit strict equality check to avoid potential buggy cache implementation causing incorrect truthy compare e.g. "false"
+      // explicit `undefined` check to differentiate `false`
       if (cachedValue === undefined) {
-        isVerified = await Jws.verifySignature(this.jws.payload, signatureEntry, publicJwk);
+        const publicJwk = await GeneralJwsVerifier.getPublicKey(kid, didResolver);
+        isVerified = await Jws.verifySignature(jws.payload, signatureEntry, publicJwk);
         await this.cache.set(cacheKey, isVerified);
       } else {
         isVerified = cachedValue;
@@ -54,9 +78,9 @@ export class GeneralJwsVerifier {
   }
 
   /**
-   * Gets the public key given a fully qualified key ID (`kid`).
+   * Gets the public key given a fully qualified key ID (`kid`) by resolving the DID to its DID Document.
    */
-  public static async getPublicKey(kid: string, didResolver: DidResolver): Promise<PublicJwk> {
+  private static async getPublicKey(kid: string, didResolver: DidResolver): Promise<PublicJwk> {
     // `resolve` throws exception if DID is invalid, DID method is not supported,
     // or resolving DID fails
     const did = Jws.extractDid(kid);
@@ -65,18 +89,18 @@ export class GeneralJwsVerifier {
 
     let verificationMethod: VerificationMethod | undefined;
 
-    for (const vm of verificationMethods) {
+    for (const method of verificationMethods) {
       // consider optimizing using a set for O(1) lookups if needed
       // key ID in DID Document may or may not be fully qualified. e.g.
       // `did:ion:alice#key1` or `#key1`
-      if (kid.endsWith(vm.id)) {
-        verificationMethod = vm;
+      if (kid.endsWith(method.id)) {
+        verificationMethod = method;
         break;
       }
     }
 
     if (!verificationMethod) {
-      throw new DwnError(DwnErrorCode.VerifierValidPublicKeyNotFound, 'public key needed to verify signature not found in DID Document');
+      throw new DwnError(DwnErrorCode.GeneralJwsVerifierGetPublicKeyNotFound, 'public key needed to verify signature not found in DID Document');
     }
 
     validateJsonSchema('JwkVerificationMethod', verificationMethod);

src/utils/jws.ts

@@ -8,7 +8,7 @@ import isPlainObject from 'lodash/isPlainObject.js';
 import { Encoder } from './encoder.js';
 import { PrivateKeySigner } from './private-key-signer.js';
 import { signatureAlgorithms } from '../jose/algorithms/signing/signature-algorithms.js';
-import { DwnError, DwnErrorCode } from '../index.js';
+import { DwnError, DwnErrorCode } from '../core/dwn-error.js';
 
 
 /**

tests/jose/jws/general.spec.ts

@@ -46,10 +46,7 @@ describe('General JWS Sign/Verify', () => {
       resolve: sinon.stub().withArgs('did:jank:alice').resolves(mockResolutionResult)
     });
 
-    const verifier = new GeneralJwsVerifier(jws);
-
-    const verificationResult = await verifier.verify(resolverStub);
-
+    const verificationResult = await GeneralJwsVerifier.verifySignatures(jws, resolverStub);
     expect(verificationResult.signers.length).to.equal(1);
     expect(verificationResult.signers).to.include('did:jank:alice');
   });
@@ -80,12 +77,9 @@ describe('General JWS Sign/Verify', () => {
       resolve: sinon.stub().withArgs('did:jank:alice').resolves(mockResolutionResult)
     });
 
-    const verifier = new GeneralJwsVerifier(jws);
-
-    const verificatonResult = await verifier.verify(resolverStub);
-
-    expect(verificatonResult.signers.length).to.equal(1);
-    expect(verificatonResult.signers).to.include('did:jank:alice');
+    const verificationResult = await GeneralJwsVerifier.verifySignatures(jws, resolverStub);
+    expect(verificationResult.signers.length).to.equal(1);
+    expect(verificationResult.signers).to.include('did:jank:alice');
   });
 
   it('should support multiple signatures using different key types', async () => {
@@ -148,15 +142,16 @@ describe('General JWS Sign/Verify', () => {
       resolve: resolveStub
     });
 
-    const verifier = new GeneralJwsVerifier(jws);
-    const verificationResult = await verifier.verify(resolverStub);
-
+    const verificationResult = await GeneralJwsVerifier.verifySignatures(jws, resolverStub);
     expect(verificationResult.signers.length).to.equal(2);
     expect(verificationResult.signers).to.include(alice.did);
     expect(verificationResult.signers).to.include(bob.did);
   });
 
   it('should not verify the same signature more than once', async () => {
+    // scenario: include two signatures in the JWS,
+    // repeated calls to verifySignature should only verify each of the signature once,
+    // resulting total of 2 calls to `Jws.verifySignature()` and 2 calls to cache the results.
     const { privateJwk: privateJwkEd25519, publicJwk: publicJwkEd25519 } = await Ed25519.generateKeyPair();
     const { privateJwk: privateJwkSecp256k1, publicJwk: publicJwkSecp256k1 } = await secp256k1.generateKeyPair();
     const payloadBytes = new TextEncoder().encode('anyPayloadValue');
@@ -195,16 +190,16 @@ describe('General JWS Sign/Verify', () => {
       resolve: sinon.stub().withArgs('did:jank:alice').resolves(mockResolutionResult)
     });
 
-    const verifier = new GeneralJwsVerifier(jws);
-
     const verifySignatureSpy = sinon.spy(Jws, 'verifySignature');
-    const cacheSetSpy = sinon.spy(verifier.cache, 'set');
+    const cacheSetSpy = sinon.spy((GeneralJwsVerifier as any).singleton.cache, 'set');
 
-    await verifier.verify(resolverStub);
-    await verifier.verify(resolverStub);
+    // intentionally calling verifySignatures() multiple times on the same JWS
+    await GeneralJwsVerifier.verifySignatures(jws, resolverStub);
+    await GeneralJwsVerifier.verifySignatures(jws, resolverStub);
+    await GeneralJwsVerifier.verifySignatures(jws, resolverStub);
 
-    sinon.assert.calledTwice(cacheSetSpy);
     sinon.assert.calledTwice(verifySignatureSpy);
+    sinon.assert.calledTwice(cacheSetSpy);
   });
 
 });