Verify recovery byte for signature

src/api/bft/UnicitySeal.ts

@@ -1,6 +1,7 @@
 import { DataHash } from '../../crypto/hash/DataHash.js';
 import { DataHasher } from '../../crypto/hash/DataHasher.js';
 import { HashAlgorithm } from '../../crypto/hash/HashAlgorithm.js';
+import { Signature } from '../../crypto/secp256k1/Signature.js';
 import { SigningService } from '../../crypto/secp256k1/SigningService.js';
 import { CborDeserializer } from '../../serialization/cbor/CborDeserializer.js';
 import { CborError } from '../../serialization/cbor/CborError.js';
@@ -25,7 +26,7 @@ export class UnicitySeal {
     public readonly timestamp: bigint,
     private readonly _previousHash: Uint8Array | null,
     private readonly _hash: Uint8Array,
-    private readonly _signatures: Map<string, Uint8Array> | null,
+    private readonly _signatures: Map<string, Signature> | null,
   ) {}
 
   /**
@@ -43,12 +44,10 @@ export class UnicitySeal {
   }
 
   /**
-   * @returns {Map<string, Uint8Array>|null} Copy of the signer-to-signature map, or `null` if absent.
+   * @returns {Map<string, Signature>|null} Copy of the signer-to-signature map, or `null` if absent.
    */
-  public get signatures(): Map<string, Uint8Array> | null {
-    return this._signatures
-      ? new Map(Array.from(this._signatures.entries()).map(([key, value]) => [key, new Uint8Array(value)]))
-      : null;
+  public get signatures(): Map<string, Signature> | null {
+    return this._signatures ? new Map(this._signatures) : null;
   }
 
   /**
@@ -93,9 +92,9 @@ export class UnicitySeal {
       new Map(
         (await Promise.all(
           Array.from(signers.entries()).map(([key, signingService]) =>
-            signingService.sign(hash).then((signature) => [key, signature.encode()]),
+            signingService.sign(hash).then((signature) => [key, signature]),
           ),
-        )) as [string, Uint8Array][],
+        )) as [string, Signature][],
       ),
     );
   }
@@ -128,7 +127,7 @@ export class UnicitySeal {
       new Map(
         CborDeserializer.decodeMap(data[7]).map((entry) => [
           CborDeserializer.decodeTextString(entry.key),
-          CborDeserializer.decodeByteString(entry.value),
+          Signature.fromCBOR(entry.value),
         ]),
       ),
     );
@@ -173,8 +172,7 @@ export class UnicitySeal {
           CborSerializer.encodeMap(
             new CborMap(
               Array.from(signatures.entries()).map(
-                ([key, value]) =>
-                  new CborMapEntry(CborSerializer.encodeTextString(key), CborSerializer.encodeByteString(value)),
+                ([key, signature]) => new CborMapEntry(CborSerializer.encodeTextString(key), signature.toCBOR()),
               ),
             ),
           ),
@@ -199,7 +197,7 @@ export class UnicitySeal {
         Hash: ${HexConverter.encode(this._hash)}
         Signatures: [
           ${Array.from(this._signatures?.entries() ?? [])
-            .map(([key, value]) => `${key}: ${HexConverter.encode(value)}`)
+            .map(([key, value]) => `${key}: ${value.toString()}`)
             .join('\n')}
         ]`;
   }

src/api/bft/verification/rule/UnicitySealQuorumSignaturesVerificationRule.ts

@@ -1,4 +1,5 @@
 import { DataHash } from '../../../../crypto/hash/DataHash.js';
+import { Signature } from '../../../../crypto/secp256k1/Signature.js';
 import { SigningService } from '../../../../crypto/secp256k1/SigningService.js';
 import { VerificationResult } from '../../../../verification/VerificationResult.js';
 import { VerificationStatus } from '../../../../verification/VerificationStatus.js';
@@ -53,7 +54,7 @@ export class UnicitySealQuorumSignaturesVerificationRule {
   private static async verifySignature(
     trustBase: RootTrustBase,
     nodeId: string,
-    signature: Uint8Array,
+    signature: Signature,
     hash: DataHash,
   ): Promise<VerificationResult<VerificationStatus>> {
     const node = trustBase.rootNodes.get(nodeId) ?? null;
@@ -65,7 +66,7 @@ export class UnicitySealQuorumSignaturesVerificationRule {
       );
     }
 
-    const result = await SigningService.verifyWithPublicKey(hash, signature.slice(0, -1), node.signingKey);
+    const result = await SigningService.verifyWithPublicKey(hash, signature, node.signingKey);
     if (!result) {
       return new VerificationResult(
         `SignatureVerificationRule[${nodeId}]`,

src/crypto/secp256k1/SigningService.ts

@@ -2,6 +2,7 @@ import { secp256k1 } from '@noble/curves/secp256k1.js';
 
 import { ISigningService } from '../ISigningService.js';
 import { Signature } from './Signature.js';
+import { areUint8ArraysEqual } from '../../util/TypedArrayUtils.js';
 import { DataHash } from '../hash/DataHash.js';
 
 /**
@@ -61,34 +62,75 @@ export class SigningService implements ISigningService<Signature> {
     return secp256k1.utils.isValidPublicKey(publicKey, true);
   }
 
+  /**
+   * Verify secp256k1 signature against the given public key.
+   *
+   * @param {DataHash} hash Signed hash.
+   * @param {Uint8Array} signature Compact signature bytes.
+   * @param {Uint8Array} publicKey Compressed public key.
+   * @returns {Promise<boolean>} True if the signature verifies.
+   */
+  public static verify(hash: DataHash, signature: Uint8Array, publicKey: Uint8Array): Promise<boolean> {
+    return Promise.resolve(secp256k1.verify(signature, hash.data, publicKey, { format: 'compact', prehash: false }));
+  }
+
+  /**
+   * Verify a recoverable signature against an expected public key.
+   *
+   * Unlike {@link verify}, this binds the signature's recovery byte: the
+   * public key is recovered from the signature and must equal `publicKey`,
+   * so a tampered recovery byte fails verification.
+   *
+   * @param {DataHash} hash Signed hash.
+   * @param {Signature} signature Recoverable signature.
+   * @param {Uint8Array} publicKey Expected compressed public key.
+   * @returns {Promise<boolean>} True if the signature verifies and the
+   *   recovered public key matches `publicKey`.
+   */
+  public static verifyWithPublicKey(hash: DataHash, signature: Signature, publicKey: Uint8Array): Promise<boolean> {
+    const expectedPublicKey = new Uint8Array(publicKey);
+    if (!areUint8ArraysEqual(expectedPublicKey, SigningService.recoverPublicKey(hash, signature))) {
+      return Promise.resolve(false);
+    }
+
+    return SigningService.verify(hash, signature.bytes, expectedPublicKey);
+  }
+
   /**
    * Recover the public key from the signature's recovery byte and verify the
-   * signature against `hash`.
+   * signature against `hash`. The recovered key defines the signer's identity;
+   * no expected key is supplied.
    *
    * @param {DataHash} hash Hash that was signed.
    * @param {Signature} signature Recoverable signature.
    * @returns {Promise<boolean>} True if the signature verifies.
    */
-  public static verifySignatureWithRecoveredPublicKey(hash: DataHash, signature: Signature): Promise<boolean> {
-    const publicKey = secp256k1.Signature.fromBytes(
-      new Uint8Array([signature.recovery, ...signature.bytes]),
-      'recovered',
-    )
-      .recoverPublicKey(hash.data)
-      .toBytes();
-    return SigningService.verifyWithPublicKey(hash, signature.bytes, publicKey);
+  public static verifyWithRecoveredPublicKey(hash: DataHash, signature: Signature): Promise<boolean> {
+    const recoveredPublicKey = SigningService.recoverPublicKey(hash, signature);
+    if (recoveredPublicKey === null) {
+      return Promise.resolve(false);
+    }
+
+    return SigningService.verify(hash, signature.bytes, recoveredPublicKey);
   }
 
   /**
-   * Verify secp256k1 signature against the given public key.
+   * Recover the compressed public key that produced `signature` over `hash`,
+   * using the signature's recovery byte.
    *
-   * @param {DataHash} hash Signed hash.
-   * @param {Uint8Array} signature Compact signature bytes.
-   * @param {Uint8Array} publicKey Compressed public key.
-   * @returns {Promise<boolean>} True if the signature verifies.
+   * @param {DataHash} hash Hash that was signed.
+   * @param {Signature} signature Recoverable signature.
+   * @returns {Uint8Array|null} Recovered compressed public key, or `null` if the
+   *   signature is not recoverable (e.g. `r`/`s` out of range).
    */
-  public static verifyWithPublicKey(hash: DataHash, signature: Uint8Array, publicKey: Uint8Array): Promise<boolean> {
-    return Promise.resolve(secp256k1.verify(signature, hash.data, publicKey, { format: 'compact', prehash: false }));
+  private static recoverPublicKey(hash: DataHash, signature: Signature): Uint8Array | null {
+    try {
+      return secp256k1.Signature.fromBytes(new Uint8Array([signature.recovery, ...signature.bytes]), 'recovered')
+        .recoverPublicKey(hash.data)
+        .toBytes();
+    } catch {
+      return null;
+    }
   }
 
   /**
@@ -110,6 +152,6 @@ export class SigningService implements ISigningService<Signature> {
    * @returns {Promise<boolean>} True if the signature verifies.
    */
   public verify(hash: DataHash, signature: Signature): Promise<boolean> {
-    return SigningService.verifyWithPublicKey(hash, signature.bytes, this._publicKey);
+    return SigningService.verifyWithPublicKey(hash, signature, this._publicKey);
   }
 }

src/predicate/builtin/verification/SignaturePredicateVerifier.ts

@@ -1,7 +1,6 @@
 import { DataHash } from '../../../crypto/hash/DataHash.js';
 import { DataHasher } from '../../../crypto/hash/DataHasher.js';
 import { HashAlgorithm } from '../../../crypto/hash/HashAlgorithm.js';
-import { Signature } from '../../../crypto/secp256k1/Signature.js';
 import { SigningService } from '../../../crypto/secp256k1/SigningService.js';
 import { CborSerializer } from '../../../serialization/cbor/CborSerializer.js';
 import { VerificationResult } from '../../../verification/VerificationResult.js';
@@ -10,6 +9,7 @@ import { EncodedPredicate } from '../../EncodedPredicate.js';
 import { SignaturePredicate } from '../SignaturePredicate.js';
 import { IBuiltInPredicateVerifier } from './IBuiltInPredicateVerifier.js';
 import { BuiltInPredicateType } from '../BuiltInPredicateType.js';
+import { SignaturePredicateUnlockScript } from '../SignaturePredicateUnlockScript.js';
 
 /**
  * Verifier for {@link SignaturePredicate}: recomputes the
@@ -26,9 +26,10 @@ export class SignaturePredicateVerifier implements IBuiltInPredicateVerifier {
     encodedPredicate: EncodedPredicate,
     sourceStateHash: DataHash,
     transactionHash: DataHash,
-    unlockScript: Uint8Array,
+    unlockScriptBytes: Uint8Array,
   ): Promise<VerificationResult<VerificationStatus>> {
     const predicate = SignaturePredicate.fromPredicate(encodedPredicate);
+    const unlockScript = SignaturePredicateUnlockScript.decode(unlockScriptBytes);
 
     const result = await SigningService.verifyWithPublicKey(
       await new DataHasher(HashAlgorithm.SHA256)
@@ -39,7 +40,7 @@ export class SignaturePredicateVerifier implements IBuiltInPredicateVerifier {
           ),
         )
         .digest(),
-      Signature.decode(unlockScript).bytes,
+      unlockScript.signature,
       predicate.publicKey,
     );
 

tests/unit/predicate/builtin/verification/SignaturePredicateVerifierTest.ts

@@ -0,0 +1,63 @@
+import { DataHash } from '../../../../../src/crypto/hash/DataHash.js';
+import { DataHasher } from '../../../../../src/crypto/hash/DataHasher.js';
+import { HashAlgorithm } from '../../../../../src/crypto/hash/HashAlgorithm.js';
+import { Signature } from '../../../../../src/crypto/secp256k1/Signature.js';
+import { SigningService } from '../../../../../src/crypto/secp256k1/SigningService.js';
+import { SignaturePredicate } from '../../../../../src/predicate/builtin/SignaturePredicate.js';
+import { SignaturePredicateVerifier } from '../../../../../src/predicate/builtin/verification/SignaturePredicateVerifier.js';
+import { EncodedPredicate } from '../../../../../src/predicate/EncodedPredicate.js';
+import { CborSerializer } from '../../../../../src/serialization/cbor/CborSerializer.js';
+import { VerificationStatus } from '../../../../../src/verification/VerificationStatus.js';
+
+describe('SignaturePredicateVerifier', () => {
+  const verifier = new SignaturePredicateVerifier();
+  const signingService = SigningService.generate();
+  const encodedPredicate = EncodedPredicate.fromPredicate(SignaturePredicate.fromSigningService(signingService));
+
+  const hash = (data: Uint8Array): Promise<DataHash> => new DataHasher(HashAlgorithm.SHA256).update(data).digest();
+
+  const signUnlock = async (): Promise<{
+    signature: Signature;
+    sourceStateHash: DataHash;
+    transactionHash: DataHash;
+  }> => {
+    const sourceStateHash = await hash(new Uint8Array([1]));
+    const transactionHash = await hash(new Uint8Array([2]));
+    const digest = await hash(
+      CborSerializer.encodeArray(
+        CborSerializer.encodeByteString(sourceStateHash.data),
+        CborSerializer.encodeByteString(transactionHash.data),
+      ),
+    );
+
+    return { signature: await signingService.sign(digest), sourceStateHash, transactionHash };
+  };
+
+  it('should accept a valid unlock script', async () => {
+    const { sourceStateHash, transactionHash, signature } = await signUnlock();
+
+    const result = await verifier.verify(encodedPredicate, sourceStateHash, transactionHash, signature.encode());
+
+    expect(result.status).toBe(VerificationStatus.OK);
+  });
+
+  it('should reject an unlock script whose recovery byte has been tampered with', async () => {
+    const { sourceStateHash, transactionHash, signature } = await signUnlock();
+
+    const tampered = new Signature(signature.bytes, signature.recovery ^ 1);
+
+    const result = await verifier.verify(encodedPredicate, sourceStateHash, transactionHash, tampered.encode());
+
+    expect(result.status).toBe(VerificationStatus.FAIL);
+  });
+
+  it('should fail (not throw) when the recovery byte makes the signature unrecoverable', async () => {
+    const { sourceStateHash, transactionHash, signature } = await signUnlock();
+
+    const tampered = new Signature(signature.bytes, 2);
+
+    const result = await verifier.verify(encodedPredicate, sourceStateHash, transactionHash, tampered.encode());
+
+    expect(result.status).toBe(VerificationStatus.FAIL);
+  });
+});