Avoid hashing payload before signing (#1391)

Please provide a paragraph or two giving a summary of the change, including relevant motivation and context. # Checklist: Remove the checklist to signal you've completed it. Enable auto-merge if the PR is ready to merge. - [x] If the pull request requires a cryptography review (e.g. cryptographic algorithm implementations) I have added the 'crypto' tag. - [x] I have reviewed my diff in github, line by line and removed unexpected formatting changes, testing logs, or commented-out code. - [x] Every change is related to the PR description. - [x] I have [linked](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue) this pull request to relevant issues (if any exist).
AztecProtocol · Aug 7, 2023 · a2c37c6 · a2c37c6
1 parent 4b9b3fe
commit a2c37c6
Show file tree

Hide file tree

Showing 16 changed files with 70 additions and 28 deletions.
diff --git a/circuits/cpp/barretenberg/cpp/src/barretenberg/crypto/pedersen_commitment/c_bind.cpp b/circuits/cpp/barretenberg/cpp/src/barretenberg/crypto/pedersen_commitment/c_bind.cpp
@@ -54,6 +54,16 @@ WASM_EXPORT void pedersen__compress_with_hash_index(uint8_t const* inputs_buffer
     barretenberg::fr::serialize_to_buffer(r, output);
 }
 
+WASM_EXPORT void pedersen_plookup_compress_with_hash_index(uint8_t const* inputs_buffer,
+                                                           uint8_t* output,
+                                                           uint32_t hash_index)
+{
+    std::vector<grumpkin::fq> to_compress;
+    read(inputs_buffer, to_compress);
+    auto r = crypto::pedersen_commitment::lookup::compress_native(to_compress, hash_index);
+    barretenberg::fr::serialize_to_buffer(r, output);
+}
+
 WASM_EXPORT void pedersen__commit(uint8_t const* inputs_buffer, uint8_t* output)
 {
     std::vector<grumpkin::fq> to_compress;

diff --git a/circuits/cpp/barretenberg/cpp/src/barretenberg/crypto/pedersen_commitment/c_bind.hpp b/circuits/cpp/barretenberg/cpp/src/barretenberg/crypto/pedersen_commitment/c_bind.hpp
@@ -16,6 +16,9 @@ WASM_EXPORT void pedersen__compress(uint8_t const* inputs_buffer, uint8_t* outpu
 WASM_EXPORT void pedersen_plookup_compress(uint8_t const* inputs_buffer, uint8_t* output);
 
 WASM_EXPORT void pedersen__compress_with_hash_index(uint8_t const* inputs_buffer, uint8_t* output, uint32_t hash_index);
+WASM_EXPORT void pedersen_plookup_compress_with_hash_index(uint8_t const* inputs_buffer,
+                                                           uint8_t* output,
+                                                           uint32_t hash_index);
 
 WASM_EXPORT void pedersen__commit(uint8_t const* inputs_buffer, uint8_t* output);
 WASM_EXPORT void pedersen_plookup_commit(uint8_t const* inputs_buffer, uint8_t* output);

diff --git a/circuits/cpp/src/aztec3/circuits/abis/packers.hpp b/circuits/cpp/src/aztec3/circuits/abis/packers.hpp
@@ -130,6 +130,7 @@ struct GeneratorIndexPacker {
         int GLOBAL_VARIABLES = GeneratorIndex::GLOBAL_VARIABLES;
         int PARTIAL_CONTRACT_ADDRESS = GeneratorIndex::PARTIAL_CONTRACT_ADDRESS;
         int TX_REQUEST = GeneratorIndex::TX_REQUEST;
+        int SIGNATURE_PAYLOAD = GeneratorIndex::SIGNATURE_PAYLOAD;
         int VK = GeneratorIndex::VK;
         int PRIVATE_CIRCUIT_PUBLIC_INPUTS = GeneratorIndex::PRIVATE_CIRCUIT_PUBLIC_INPUTS;
         int PUBLIC_CIRCUIT_PUBLIC_INPUTS = GeneratorIndex::PUBLIC_CIRCUIT_PUBLIC_INPUTS;
@@ -166,6 +167,7 @@ struct GeneratorIndexPacker {
                  GLOBAL_VARIABLES,
                  PARTIAL_CONTRACT_ADDRESS,
                  TX_REQUEST,
+                 SIGNATURE_PAYLOAD,
                  VK,
                  PRIVATE_CIRCUIT_PUBLIC_INPUTS,
                  PUBLIC_CIRCUIT_PUBLIC_INPUTS,

diff --git a/circuits/cpp/src/aztec3/constants.hpp b/circuits/cpp/src/aztec3/constants.hpp
@@ -169,7 +169,8 @@ enum GeneratorIndex {
     /**
      * Indices with size ≤ 16
      */
-    TX_REQUEST = 33,  // Size = 14
+    TX_REQUEST = 33,    // Size = 14
+    SIGNATURE_PAYLOAD,  // Size = 13
     /**
      * Indices with size ≤ 44
      */

diff --git a/yarn-project/aztec.js/src/abis/ecdsa_account_contract.json b/yarn-project/aztec.js/src/abis/ecdsa_account_contract.json
diff --git a/yarn-project/aztec.js/src/abis/schnorr_multi_key_account_contract.json b/yarn-project/aztec.js/src/abis/schnorr_multi_key_account_contract.json
diff --git a/yarn-project/aztec.js/src/abis/schnorr_single_key_account_contract.json b/yarn-project/aztec.js/src/abis/schnorr_single_key_account_contract.json
diff --git a/yarn-project/aztec.js/src/account/entrypoint/entrypoint_payload.ts b/yarn-project/aztec.js/src/account/entrypoint/entrypoint_payload.ts
@@ -1,6 +1,7 @@
-import { CircuitsWasm, Fr } from '@aztec/circuits.js';
+import { CircuitsWasm, Fr, GeneratorIndex } from '@aztec/circuits.js';
+import { pedersenPlookupCompressWithHashIndex } from '@aztec/circuits.js/barretenberg';
 import { padArrayEnd } from '@aztec/foundation/collection';
-import { sha256 } from '@aztec/foundation/crypto';
+import { IWasmModule } from '@aztec/foundation/wasm';
 import { FunctionCall, PackedArguments, emptyFunctionCall } from '@aztec/types';
 
 // These must match the values defined in yarn-project/noir-libs/noir-aztec/src/entrypoint.nr
@@ -60,10 +61,13 @@ export async function buildPayload(
   };
 }
 
-/** Hashes an entrypoint payload (useful for signing) */
-export function hashPayload(payload: EntrypointPayload) {
-  // TODO: Switch to keccak when avaiable in Noir
-  return sha256(Buffer.concat(flattenPayload(payload).map(fr => fr.toBuffer())));
+/** Compresses an entrypoint payload to a 32-byte buffer (useful for signing) */
+export function hashPayload(payload: EntrypointPayload, wasm: IWasmModule) {
+  return pedersenPlookupCompressWithHashIndex(
+    wasm,
+    flattenPayload(payload).map(fr => fr.toBuffer()),
+    GeneratorIndex.SIGNATURE_PAYLOAD,
+  );
 }
 
 /** Flattens an entrypoint payload */

diff --git a/yarn-project/aztec.js/src/account/entrypoint/single_key_account_entrypoint.ts b/yarn-project/aztec.js/src/account/entrypoint/single_key_account_entrypoint.ts
@@ -44,9 +44,9 @@ export class SingleKeyAccountEntrypoint implements Entrypoint {
     const [privateCalls, publicCalls] = partition(executions, exec => exec.functionData.isPrivate);
     const wasm = await CircuitsWasm.get();
     const { payload, packedArguments: callsPackedArguments } = await buildPayload(privateCalls, publicCalls);
-    const hash = hashPayload(payload);
+    const message = hashPayload(payload, wasm);
 
-    const signature = this.signer.constructSignature(hash, this.privateKey).toBuffer();
+    const signature = this.signer.constructSignature(message, this.privateKey).toBuffer();
     const publicKey = await generatePublicKey(this.privateKey);
     const args = [payload, publicKey.toBuffer(), signature, this.partialContractAddress];
     const abi = this.getEntrypointAbi();

diff --git a/yarn-project/aztec.js/src/account/entrypoint/stored_key_account_entrypoint.ts b/yarn-project/aztec.js/src/account/entrypoint/stored_key_account_entrypoint.ts
@@ -39,9 +39,9 @@ export class StoredKeyAccountEntrypoint implements Entrypoint {
     const [privateCalls, publicCalls] = partition(executions, exec => exec.functionData.isPrivate);
     const wasm = await CircuitsWasm.get();
     const { payload, packedArguments: callsPackedArguments } = await buildPayload(privateCalls, publicCalls);
-    const hash = hashPayload(payload);
-    const signature = this.signer.constructSignature(hash, this.privateKey).toBuffer();
-    this.log(`Signed challenge ${hash.toString('hex')} as ${signature.toString('hex')}`);
+    const message = hashPayload(payload, wasm);
+    const signature = this.signer.constructSignature(message, this.privateKey).toBuffer();
+    this.log(`Signed challenge ${message.toString('hex')} as ${signature.toString('hex')}`);
 
     const args = [payload, signature];
     const abi = this.getEntrypointAbi();

diff --git a/yarn-project/circuits.js/src/barretenberg/crypto/pedersen/pedersen.ts b/yarn-project/circuits.js/src/barretenberg/crypto/pedersen/pedersen.ts
@@ -101,6 +101,22 @@ export function pedersenCompressWithHashIndex(wasm: IWasmModule, inputs: Buffer[
   return Buffer.from(wasm.getMemorySlice(0, 32));
 }
 
+/**
+ * Compresses an array of buffers.
+ * @param wasm - The barretenberg module.
+ * @param inputs - The array of buffers to compress.
+ * @param hashIndex - Hash index of the generator to use (See GeneratorIndex enum).
+ * @returns The resulting 32-byte hash.
+ */
+export function pedersenPlookupCompressWithHashIndex(wasm: IWasmModule, inputs: Buffer[], hashIndex: number): Buffer {
+  // If not done already, precompute constants.
+  wasm.call('pedersen__init');
+  const inputVectors = serializeBufferArrayToVector(inputs);
+  wasm.writeMemory(0, inputVectors);
+  wasm.call('pedersen_plookup_compress_with_hash_index', 0, 0, hashIndex);
+  return Buffer.from(wasm.getMemorySlice(0, 32));
+}
+
 /**
  * Get a 32-byte pedersen hash from a buffer.
  * @param wasm - The barretenberg module.

diff --git a/yarn-project/circuits.js/src/cbind/constants.gen.ts b/yarn-project/circuits.js/src/cbind/constants.gen.ts
@@ -103,6 +103,7 @@ export enum GeneratorIndex {
   GLOBAL_VARIABLES = 26,
   PARTIAL_CONTRACT_ADDRESS = 27,
   TX_REQUEST = 33,
+  SIGNATURE_PAYLOAD = 34,
   VK = 41,
   PRIVATE_CIRCUIT_PUBLIC_INPUTS = 42,
   PUBLIC_CIRCUIT_PUBLIC_INPUTS = 43,

diff --git a/yarn-project/noir-contracts/src/contracts/ecdsa_account_contract/src/main.nr b/yarn-project/noir-contracts/src/contracts/ecdsa_account_contract/src/main.nr
@@ -17,6 +17,7 @@ contract EcdsaAccount {
     use dep::aztec::public_call_stack_item::PublicCallStackItem;
     use dep::aztec::types::vec::BoundedVec;
     use dep::aztec::types::point::Point;
+    use dep::aztec::constants_gen::GENERATOR_INDEX__SIGNATURE_PAYLOAD;
 
     use dep::aztec::constants_gen::MAX_NOTE_FIELDS_LENGTH;
     use dep::aztec::note::{
@@ -49,10 +50,11 @@ contract EcdsaAccount {
 
         // Verify payload signature using Ethereum's signing scheme
         // Note that noir expects the hash of the message/challenge as input to the ECDSA verification.
-        let payload_bytes: [u8; entrypoint::ENTRYPOINT_PAYLOAD_SIZE_IN_BYTES] = payload.to_be_bytes();
-        let challenge: [u8; 32] = std::hash::sha256(payload_bytes);
-        let hashed_challenge: [u8; 32] = std::hash::sha256(challenge);
-        let verification = std::ecdsa_secp256k1::verify_signature(public_key.x, public_key.y, signature, hashed_challenge);
+        let payload_fields: [Field; entrypoint::ENTRYPOINT_PAYLOAD_SIZE] = payload.serialize();
+        let message_field: Field = std::hash::pedersen_with_separator(payload_fields, GENERATOR_INDEX__SIGNATURE_PAYLOAD)[0];
+        let message_bytes = message_field.to_be_bytes(32);
+        let hashed_message: [u8; 32] = std::hash::sha256(message_bytes);
+        let verification = std::ecdsa_secp256k1::verify_signature(public_key.x, public_key.y, signature, hashed_message);
         assert(verification == true);
 
         payload.execute_calls(&mut context);

diff --git a/yarn-project/noir-contracts/src/contracts/schnorr_multi_key_account_contract/src/main.nr b/yarn-project/noir-contracts/src/contracts/schnorr_multi_key_account_contract/src/main.nr
@@ -21,6 +21,7 @@ contract SchnorrMultiKeyAccount {
     use dep::aztec::note::note_header::NoteHeader;
     use dep::aztec::constants_gen::MAX_NOTE_FIELDS_LENGTH;
     use dep::aztec::constants_gen::GENERATOR_INDEX__CONTRACT_ADDRESS;
+    use dep::aztec::constants_gen::GENERATOR_INDEX__SIGNATURE_PAYLOAD;
 
     use crate::storage::Storage;
     use crate::public_key_note::PublicKeyNote;
@@ -44,12 +45,12 @@ contract SchnorrMultiKeyAccount {
         let public_key = storage.signing_public_key.get_note(&mut context);
 
         // Verify payload signature
-        // TODO: Use pedersen to make the payload hash cheaper to compute
-        let payload_bytes: [u8; entrypoint::ENTRYPOINT_PAYLOAD_SIZE_IN_BYTES] = payload.to_be_bytes();
-        let payload_hash: [u8; 32] = std::hash::sha256(payload_bytes);
+        let payload_fields: [Field; entrypoint::ENTRYPOINT_PAYLOAD_SIZE] = payload.serialize();
+        let message_field: Field = std::hash::pedersen_with_separator(payload_fields, GENERATOR_INDEX__SIGNATURE_PAYLOAD)[0];
+        let message_bytes = message_field.to_be_bytes(32);
 
-        // Verify signature of the payload hash
-        let verification = std::schnorr::verify_signature(public_key.x, public_key.y, signature, payload_hash);
+        // Verify signature of the payload bytes
+        let verification = std::schnorr::verify_signature(public_key.x, public_key.y, signature, message_bytes);
         assert(verification == true);
 
         // Execute calls

diff --git a/yarn-project/noir-contracts/src/contracts/schnorr_single_key_account_contract/src/main.nr b/yarn-project/noir-contracts/src/contracts/schnorr_single_key_account_contract/src/main.nr
@@ -14,6 +14,7 @@ contract SchnorrSingleKeyAccount {
     use dep::aztec::types::vec::BoundedVec;
     use dep::aztec::types::point::Point;
     use dep::aztec::constants_gen::GENERATOR_INDEX__CONTRACT_ADDRESS;
+    use dep::aztec::constants_gen::GENERATOR_INDEX__SIGNATURE_PAYLOAD;
 
     fn entrypoint(
         inputs: pub PrivateContextInputs,
@@ -32,9 +33,9 @@ contract SchnorrSingleKeyAccount {
         let mut context = Context::new(inputs, abi::hash_args(args.storage));
 
         // Verify payload signature
-        // TODO: Use pedersen to make the payload hash cheaper to compute
-        let payload_bytes: [u8; entrypoint::ENTRYPOINT_PAYLOAD_SIZE_IN_BYTES] = payload.to_be_bytes();
-        let payload_hash: [u8; 32] = std::hash::sha256(payload_bytes);
+        let payload_fields: [Field; entrypoint::ENTRYPOINT_PAYLOAD_SIZE] = payload.serialize();
+        let message_field: Field = std::hash::pedersen_with_separator(payload_fields, GENERATOR_INDEX__SIGNATURE_PAYLOAD)[0];
+        let message_bytes = message_field.to_be_bytes(32);
 
         // Convert owner pubkey into fields
         let mut x: Field = 0;
@@ -50,7 +51,7 @@ contract SchnorrSingleKeyAccount {
 
         // Verify signature of the payload hash
         // TODO: Find out why this signature verification never fails
-        let verification = std::schnorr::verify_signature(x, y, signature, payload_hash);
+        let verification = std::schnorr::verify_signature(x, y, signature, message_bytes);
         assert(verification == true);
 
         // Verify public key against address

diff --git a/yarn-project/noir-libs/noir-aztec/src/constants_gen.nr b/yarn-project/noir-libs/noir-aztec/src/constants_gen.nr
@@ -101,6 +101,7 @@ global GENERATOR_INDEX__SIGNED_TX_REQUEST = 25;
 global GENERATOR_INDEX__GLOBAL_VARIABLES = 26;
 global GENERATOR_INDEX__PARTIAL_CONTRACT_ADDRESS = 27;
 global GENERATOR_INDEX__TX_REQUEST = 33;
+global GENERATOR_INDEX__SIGNATURE_PAYLOAD = 34;
 global GENERATOR_INDEX__VK = 41;
 global GENERATOR_INDEX__PRIVATE_CIRCUIT_PUBLIC_INPUTS = 42;
 global GENERATOR_INDEX__PUBLIC_CIRCUIT_PUBLIC_INPUTS = 43;