From 9b29bef371a7b92b4d2aaa71fd0a10b3d9f1f0f7 Mon Sep 17 00:00:00 2001 From: Joy Wang <108701016+joyqvq@users.noreply.github.com> Date: Mon, 20 Mar 2023 19:37:51 -0400 Subject: [PATCH] crypto: pass in hashed message for sign and verify (#9561) ## Description Pass in hashed message for sign and verify API for user signatures. Closes https://github.com/MystenLabs/sui/issues/9512 ## Test Plan Existing unit tests and e2e test. --- If your changes are not user-facing and not a breaking change, you can skip the following section. Otherwise, please indicate what changed, and then add to the Release Notes section as highlighted during the release process. ### Type of Change (Check all that apply) - [ ] user-visible impact - [ ] breaking change for a client SDKs - [ ] breaking change for FNs (FN binary must upgrade) - [ ] breaking change for validators or node operators (must upgrade binaries) - [ ] breaking change for on-chain data layout - [ ] necessitate either a data wipe or data migration ### Release notes --- .changeset/orange-birds-tan.md | 5 +++ crates/sui-keys/src/keystore.rs | 36 +++++++++----------- crates/sui-open-rpc/spec/openrpc.json | 10 +++--- crates/sui-rosetta/src/construction.rs | 9 +++-- crates/sui-rosetta/tests/rosetta_client.rs | 6 ++-- crates/sui-types/src/crypto.rs | 37 ++++++++------------- crates/sui-types/src/multisig.rs | 12 ++++--- doc/src/learn/exchange-integration-guide.md | 2 +- sdk/typescript/src/signers/raw-signer.ts | 4 ++- sdk/typescript/src/utils/verify.ts | 7 ++-- sdk/typescript/test/e2e/raw-signer.test.ts | 7 ++-- 11 files changed, 68 insertions(+), 67 deletions(-) create mode 100644 .changeset/orange-birds-tan.md diff --git a/.changeset/orange-birds-tan.md b/.changeset/orange-birds-tan.md new file mode 100644 index 0000000000000..ee759a5685bb3 --- /dev/null +++ b/.changeset/orange-birds-tan.md @@ -0,0 +1,5 @@ +--- +"@mysten/sui.js": minor +--- + +Pass blake2b hash to signer API diff --git a/crates/sui-keys/src/keystore.rs b/crates/sui-keys/src/keystore.rs index a8e44aaf4e393..535c0b64025bc 100644 --- a/crates/sui-keys/src/keystore.rs +++ b/crates/sui-keys/src/keystore.rs @@ -18,7 +18,7 @@ use std::path::{Path, PathBuf}; use sui_types::base_types::SuiAddress; use sui_types::crypto::{ enum_dispatch, get_key_pair_from_rng, EncodeDecodeBase64, PublicKey, Signature, - SignatureScheme, Signer, SuiKeyPair, + SignatureScheme, SuiKeyPair, }; use crate::key_derive::{derive_key_pair_from_path, generate_new_key}; @@ -31,12 +31,12 @@ pub enum Keystore { } #[enum_dispatch] pub trait AccountKeystore: Send + Sync { - #[warn(deprecated)] - fn sign(&self, address: &SuiAddress, msg: &[u8]) -> Result; fn add_key(&mut self, keypair: SuiKeyPair) -> Result<(), anyhow::Error>; fn keys(&self) -> Vec; fn get_key(&self, address: &SuiAddress) -> Result<&SuiKeyPair, anyhow::Error>; + fn sign_hashed(&self, address: &SuiAddress, msg: &[u8]) -> Result; + fn sign_secure( &self, address: &SuiAddress, @@ -128,17 +128,14 @@ impl<'de> Deserialize<'de> for FileBasedKeystore { } impl AccountKeystore for FileBasedKeystore { - #[warn(deprecated)] - fn sign(&self, address: &SuiAddress, msg: &[u8]) -> Result { - Ok(self - .keys - .get(address) - .ok_or_else(|| { + fn sign_hashed(&self, address: &SuiAddress, msg: &[u8]) -> Result { + Ok(Signature::new_hashed( + msg, + self.keys.get(address).ok_or_else(|| { signature::Error::from_source(format!("Cannot find key for address: [{address}]")) - })? - .sign(msg)) + })?, + )) } - fn sign_secure( &self, address: &SuiAddress, @@ -228,17 +225,14 @@ pub struct InMemKeystore { } impl AccountKeystore for InMemKeystore { - #[warn(deprecated)] - fn sign(&self, address: &SuiAddress, msg: &[u8]) -> Result { - Ok(self - .keys - .get(address) - .ok_or_else(|| { + fn sign_hashed(&self, address: &SuiAddress, msg: &[u8]) -> Result { + Ok(Signature::new_hashed( + msg, + self.keys.get(address).ok_or_else(|| { signature::Error::from_source(format!("Cannot find key for address: [{address}]")) - })? - .sign(msg)) + })?, + )) } - fn sign_secure( &self, address: &SuiAddress, diff --git a/crates/sui-open-rpc/spec/openrpc.json b/crates/sui-open-rpc/spec/openrpc.json index 2d45d467a370c..3fddf8aa4b4bf 100644 --- a/crates/sui-open-rpc/spec/openrpc.json +++ b/crates/sui-open-rpc/spec/openrpc.json @@ -152,7 +152,7 @@ { "name": "signatures", "value": [ - "ALhGcUtEV3kT9ukC4ItNjlv5/gATBGmWEQan7ri2YKHzbtI2rSMpWvxUg7CN+Jap2zPyhfXIes+xt9Q60H2Ylw1Dij1TvBYKLcfLNo8fq6GASb9yfo6uvuwNUBGkTf54wQ==" + "ALGzUorJ5eSfoPZSblxKUjNuNnB9R06PFVBmbopMmzcmHgZZWAfIpxifHMP9ogXFMxb0weTzRgVbUf241OrvvA1Dij1TvBYKLcfLNo8fq6GASb9yfo6uvuwNUBGkTf54wQ==" ] }, { @@ -224,10 +224,10 @@ } }, "txSignatures": [ - "ALhGcUtEV3kT9ukC4ItNjlv5/gATBGmWEQan7ri2YKHzbtI2rSMpWvxUg7CN+Jap2zPyhfXIes+xt9Q60H2Ylw1Dij1TvBYKLcfLNo8fq6GASb9yfo6uvuwNUBGkTf54wQ==" + "ALGzUorJ5eSfoPZSblxKUjNuNnB9R06PFVBmbopMmzcmHgZZWAfIpxifHMP9ogXFMxb0weTzRgVbUf241OrvvA1Dij1TvBYKLcfLNo8fq6GASb9yfo6uvuwNUBGkTf54wQ==" ] }, - "rawTransaction": "AQAAAAAAAgAge6kd3H5xfPcIyTcGDwQEhzbsM/sXRtmZpeWM1cZ37YABAE+C8chYe5jWTAC/tGw4Q72L9sz6fGWoYThpjNH9ysPcAgAAAAAAAAAgsQwARuKTwIzsD0B4PZrEv0q7TX+CBkf9hdGRg97nx/8BAQEBAQABAABh+7W080KkC9v4f+SpRrnjjRjPj/x7AAC5dRdce2qVdgHo2MfOhj8xPaPb2SqD7ybRKLiP5mvybg4NCc2vcn0dhAIAAAAAAAAAIMzKqXoCzDqYBBZw4WCdtxTphYyW6eRphV8c87/fl+hlYfu1tPNCpAvb+H/kqUa5440Yz4/8ewAAuXUXXHtqlXYBAAAAAAAAAOgDAAAAAAAAAAFhALhGcUtEV3kT9ukC4ItNjlv5/gATBGmWEQan7ri2YKHzbtI2rSMpWvxUg7CN+Jap2zPyhfXIes+xt9Q60H2Ylw1Dij1TvBYKLcfLNo8fq6GASb9yfo6uvuwNUBGkTf54wQ==", + "rawTransaction": "AQAAAAAAAgAge6kd3H5xfPcIyTcGDwQEhzbsM/sXRtmZpeWM1cZ37YABAE+C8chYe5jWTAC/tGw4Q72L9sz6fGWoYThpjNH9ysPcAgAAAAAAAAAgsQwARuKTwIzsD0B4PZrEv0q7TX+CBkf9hdGRg97nx/8BAQEBAQABAABh+7W080KkC9v4f+SpRrnjjRjPj/x7AAC5dRdce2qVdgHo2MfOhj8xPaPb2SqD7ybRKLiP5mvybg4NCc2vcn0dhAIAAAAAAAAAIMzKqXoCzDqYBBZw4WCdtxTphYyW6eRphV8c87/fl+hlYfu1tPNCpAvb+H/kqUa5440Yz4/8ewAAuXUXXHtqlXYBAAAAAAAAAOgDAAAAAAAAAAFhALGzUorJ5eSfoPZSblxKUjNuNnB9R06PFVBmbopMmzcmHgZZWAfIpxifHMP9ogXFMxb0weTzRgVbUf241OrvvA1Dij1TvBYKLcfLNo8fq6GASb9yfo6uvuwNUBGkTf54wQ==", "effects": { "messageVersion": "v1", "status": { @@ -1421,10 +1421,10 @@ } }, "txSignatures": [ - "ALwMcYis9u/ynkzVhCb8UNtgS8kWOk6MaMWXlWOXONq5KDbfEv49Z0cEvz2tObRkDjaS9kxNSc+Cjfe1F0PtEwptCxWrqsEEHAdxoMDwblU5hyWJ8H3zFvk20E2fO5bzHA==" + "AJY4zVI51I4JcmR04FFv9e5pShBckeC0Tu7Jsejgt84ZqDwX+yZXN4Tlo/wmFUDaZh34SS2BmXW+J4HVC8+xFwptCxWrqsEEHAdxoMDwblU5hyWJ8H3zFvk20E2fO5bzHA==" ] }, - "rawTransaction": "AQAAAAAAAgAggZbQSLem0EyO3IlXnYb9P8kMUvmhTGuBK5T+YTxbzrsBAF7rHUSeJRYWbVfXH96xVNDcns23swBX0KkyaEysNSzcAgAAAAAAAAAg43+UGkUe+CCaD7+/G1SbK7Jrjq7giJUUbfJ7w88mEMEBAQEBAQABAACCF5xX1Ylbq/tlXNYujohqUzNLXnvpvmWOt1nMNeP8ZgEaPomAKdAk7sHUTGr14vrN7YTQO1NzUU8W49ZuAAgQUQIAAAAAAAAAIGS7c6HtWLLBiwy/N3eS4gbmuA1NXupk4ucFY7FYkCbEghecV9WJW6v7ZVzWLo6IalMzS1576b5ljrdZzDXj/GYBAAAAAAAAAOgDAAAAAAAAAAFhALwMcYis9u/ynkzVhCb8UNtgS8kWOk6MaMWXlWOXONq5KDbfEv49Z0cEvz2tObRkDjaS9kxNSc+Cjfe1F0PtEwptCxWrqsEEHAdxoMDwblU5hyWJ8H3zFvk20E2fO5bzHA==", + "rawTransaction": "AQAAAAAAAgAggZbQSLem0EyO3IlXnYb9P8kMUvmhTGuBK5T+YTxbzrsBAF7rHUSeJRYWbVfXH96xVNDcns23swBX0KkyaEysNSzcAgAAAAAAAAAg43+UGkUe+CCaD7+/G1SbK7Jrjq7giJUUbfJ7w88mEMEBAQEBAQABAACCF5xX1Ylbq/tlXNYujohqUzNLXnvpvmWOt1nMNeP8ZgEaPomAKdAk7sHUTGr14vrN7YTQO1NzUU8W49ZuAAgQUQIAAAAAAAAAIGS7c6HtWLLBiwy/N3eS4gbmuA1NXupk4ucFY7FYkCbEghecV9WJW6v7ZVzWLo6IalMzS1576b5ljrdZzDXj/GYBAAAAAAAAAOgDAAAAAAAAAAFhAJY4zVI51I4JcmR04FFv9e5pShBckeC0Tu7Jsejgt84ZqDwX+yZXN4Tlo/wmFUDaZh34SS2BmXW+J4HVC8+xFwptCxWrqsEEHAdxoMDwblU5hyWJ8H3zFvk20E2fO5bzHA==", "effects": { "messageVersion": "v1", "status": { diff --git a/crates/sui-rosetta/src/construction.rs b/crates/sui-rosetta/src/construction.rs index 6ea21a181a0e6..78d9e0fafb705 100644 --- a/crates/sui-rosetta/src/construction.rs +++ b/crates/sui-rosetta/src/construction.rs @@ -5,6 +5,7 @@ use axum::extract::State; use axum::{Extension, Json}; use axum_extra::extract::WithRejection; use fastcrypto::encoding::{Encoding, Hex}; +use fastcrypto::hash::HashFunction; use futures::StreamExt; use shared_crypto::intent::{Intent, IntentMessage}; @@ -13,7 +14,7 @@ use sui_json_rpc_types::{ }; use sui_sdk::rpc_types::SuiExecutionStatus; use sui_types::base_types::SuiAddress; -use sui_types::crypto::{SignatureScheme, ToFromBytes}; +use sui_types::crypto::{DefaultHash, SignatureScheme, ToFromBytes}; use sui_types::error::SuiError; use sui_types::messages::{Transaction, TransactionData, TransactionDataAPI}; use sui_types::signature::GenericSignature; @@ -66,11 +67,15 @@ pub async fn payloads( let intent_msg = IntentMessage::new(Intent::default(), data); let intent_msg_bytes = bcs::to_bytes(&intent_msg)?; + let mut hasher = DefaultHash::default(); + hasher.update(&bcs::to_bytes(&intent_msg).expect("Message serialization should not fail")); + let digest = hasher.finalize().digest; + Ok(ConstructionPayloadsResponse { unsigned_transaction: Hex::from_bytes(&intent_msg_bytes), payloads: vec![SigningPayload { account_identifier: address.into(), - hex_bytes: Hex::encode(bcs::to_bytes(&intent_msg)?), + hex_bytes: Hex::encode(digest), signature_type: Some(SignatureType::Ed25519), }], }) diff --git a/crates/sui-rosetta/tests/rosetta_client.rs b/crates/sui-rosetta/tests/rosetta_client.rs index 02907c2666bfe..b977a65b076d2 100644 --- a/crates/sui-rosetta/tests/rosetta_client.rs +++ b/crates/sui-rosetta/tests/rosetta_client.rs @@ -143,10 +143,8 @@ impl RosettaClient { let signing_payload = payloads.payloads.first().unwrap(); let bytes = Hex::decode(&signing_payload.hex_bytes).unwrap(); let signer = signing_payload.account_identifier.address; - let signature = AccountKeystore::sign(keystore, &signer, &bytes).unwrap(); - let public_key = AccountKeystore::get_key(keystore, &signer) - .unwrap() - .public(); + let signature = keystore.sign_hashed(&signer, &bytes).unwrap(); + let public_key = keystore.get_key(&signer).unwrap().public(); let combine: ConstructionCombineResponse = self .call( RosettaEndpoint::Combine, diff --git a/crates/sui-types/src/crypto.rs b/crates/sui-types/src/crypto.rs index 3d3fa9f2f9a93..a937e4a693e19 100644 --- a/crates/sui-types/src/crypto.rs +++ b/crates/sui-types/src/crypto.rs @@ -667,14 +667,18 @@ impl<'de> Deserialize<'de> for Signature { } impl Signature { + /// The messaged passed in is already hashed form. + pub fn new_hashed(hashed_msg: &[u8], secret: &dyn Signer) -> Self { + Signer::sign(secret, hashed_msg) + } + pub fn new_secure(value: &IntentMessage, secret: &dyn Signer) -> Self where T: Serialize, { - Signer::sign( - secret, - &bcs::to_bytes(&value).expect("Message serialization should not fail"), - ) + let mut hasher = DefaultHash::default(); + hasher.update(&bcs::to_bytes(&value).expect("Message serialization should not fail")); + Signer::sign(secret, &hasher.finalize().digest) } /// Parse [enum CompressedSignature] from trait SuiSignature `flag || sig || pk`. @@ -1004,10 +1008,6 @@ pub trait SuiSignature: Sized + ToFromBytes { fn public_key_bytes(&self) -> &[u8]; fn scheme(&self) -> SignatureScheme; - fn verify(&self, value: &T, author: SuiAddress) -> SuiResult<()> - where - T: Signable>; - fn verify_secure(&self, value: &IntentMessage, author: SuiAddress) -> SuiResult<()> where T: Serialize; @@ -1030,27 +1030,16 @@ impl SuiSignature for S { S::PubKey::SIGNATURE_SCHEME } - fn verify(&self, value: &T, author: SuiAddress) -> SuiResult<()> - where - T: Signable>, - { - // Currently done twice - can we improve on this?; - let (sig, pk) = &self.get_verification_inputs(author)?; - let mut message = Vec::new(); - value.write(&mut message); - pk.verify(&message[..], sig) - .map_err(|e| SuiError::InvalidSignature { - error: format!("{}", e), - }) - } - fn verify_secure(&self, value: &IntentMessage, author: SuiAddress) -> Result<(), SuiError> where T: Serialize, { - let message = bcs::to_bytes(&value).expect("Message serialization should not fail"); + let mut hasher = DefaultHash::default(); + hasher.update(&bcs::to_bytes(&value).expect("Message serialization should not fail")); + let digest = hasher.finalize().digest; + let (sig, pk) = &self.get_verification_inputs(author)?; - pk.verify(&message[..], sig) + pk.verify(&digest, sig) .map_err(|e| SuiError::InvalidSignature { error: format!("{}", e), }) diff --git a/crates/sui-types/src/multisig.rs b/crates/sui-types/src/multisig.rs index 40a93fdfaaa81..00dfe7b422163 100644 --- a/crates/sui-types/src/multisig.rs +++ b/crates/sui-types/src/multisig.rs @@ -2,7 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 use crate::{ - crypto::{CompressedSignature, SignatureScheme}, + crypto::{CompressedSignature, DefaultHash, SignatureScheme}, signature::AuthenticatorTrait, sui_serde::SuiBitmap, }; @@ -11,6 +11,7 @@ use fastcrypto::{ ed25519::Ed25519PublicKey, encoding::Base64, error::FastCryptoError, + hash::HashFunction, secp256k1::Secp256k1PublicKey, secp256r1::Secp256r1PublicKey, traits::{ToFromBytes, VerifyingKey}, @@ -115,6 +116,9 @@ impl AuthenticatorTrait for MultiSig { } let mut weight_sum: u16 = 0; let message = bcs::to_bytes(&value).expect("Message serialization should not fail"); + let mut hasher = DefaultHash::default(); + hasher.update(message); + let digest = hasher.finalize().digest; // Verify each signature against its corresponding signature scheme and public key. // TODO: further optimization can be done because multiple Ed25519 signatures can be batch verified. @@ -133,7 +137,7 @@ impl AuthenticatorTrait for MultiSig { error: "Invalid public key".to_string(), } })?; - pk.verify(&message, &s.try_into()?) + pk.verify(&digest, &s.try_into()?) } CompressedSignature::Secp256k1(s) => { let pk = Secp256k1PublicKey::from_bytes(pk_map.0.as_ref()).map_err(|_| { @@ -141,7 +145,7 @@ impl AuthenticatorTrait for MultiSig { error: "Invalid public key".to_string(), } })?; - pk.verify(&message, &s.try_into()?) + pk.verify(&digest, &s.try_into()?) } CompressedSignature::Secp256r1(s) => { let pk = Secp256r1PublicKey::from_bytes(pk_map.0.as_ref()).map_err(|_| { @@ -149,7 +153,7 @@ impl AuthenticatorTrait for MultiSig { error: "Invalid public key".to_string(), } })?; - pk.verify(&message, &s.try_into()?) + pk.verify(&digest, &s.try_into()?) } }; if res.is_ok() { diff --git a/doc/src/learn/exchange-integration-guide.md b/doc/src/learn/exchange-integration-guide.md index d802431d42bd7..5934315f07478 100644 --- a/doc/src/learn/exchange-integration-guide.md +++ b/doc/src/learn/exchange-integration-guide.md @@ -92,7 +92,7 @@ The following code sample demonstrates how to derive a Sui address in Rust: ```rust let flag = 0x00; // 0x00 = ED25519, 0x01 = Secp256k1, 0x02 = Secp256r1, 0x03 = Multisig // Hash the [flag, public key] bytearray using Blake2b -let mut hasher = UserHash::default(); +let mut hasher = DefaultHash::default(); hasher.update([flag]); hasher.update(pk); let arr = hasher.finalize(); diff --git a/sdk/typescript/src/signers/raw-signer.ts b/sdk/typescript/src/signers/raw-signer.ts index 366cb297c19c5..3135d60b5030e 100644 --- a/sdk/typescript/src/signers/raw-signer.ts +++ b/sdk/typescript/src/signers/raw-signer.ts @@ -1,6 +1,7 @@ // Copyright (c) Mysten Labs, Inc. // SPDX-License-Identifier: Apache-2.0 +import { blake2b } from '@noble/hashes/blake2b'; import { Keypair } from '../cryptography/keypair'; import { SerializedSignature, @@ -24,7 +25,8 @@ export class RawSigner extends SignerWithProvider { async signData(data: Uint8Array): Promise { const pubkey = this.keypair.getPublicKey(); - const signature = this.keypair.signData(data, false); + const digest = blake2b(data, { dkLen: 32 }); + const signature = this.keypair.signData(digest, false); const signatureScheme = this.keypair.getKeyScheme(); return toSerializedSignature({ diff --git a/sdk/typescript/src/utils/verify.ts b/sdk/typescript/src/utils/verify.ts index ffcfa44f31072..bd73f18198181 100644 --- a/sdk/typescript/src/utils/verify.ts +++ b/sdk/typescript/src/utils/verify.ts @@ -9,6 +9,7 @@ import { fromSerializedSignature, SerializedSignature, } from '../cryptography/signature'; +import { blake2b } from '@noble/hashes/blake2b'; // TODO: This might actually make sense to eventually move to the `Keypair` instances themselves, as // it could allow the Sui.js to be tree-shaken a little better, possibly allowing keypairs that are @@ -24,18 +25,18 @@ export async function verifyMessage( IntentScope.PersonalMessage, typeof message === 'string' ? fromB64(message) : message, ); - + const digest = blake2b(messageBytes, { dkLen: 32 }); switch (signature.signatureScheme) { case 'ED25519': return nacl.sign.detached.verify( - messageBytes, + digest, signature.signature, signature.pubKey.toBytes(), ); case 'Secp256k1': return secp.verify( secp.Signature.fromCompact(signature.signature), - await secp.utils.sha256(messageBytes), + await secp.utils.sha256(digest), signature.pubKey.toBytes(), ); default: diff --git a/sdk/typescript/test/e2e/raw-signer.test.ts b/sdk/typescript/test/e2e/raw-signer.test.ts index a1e7f44b8cd1b..b994f8f9f48fa 100644 --- a/sdk/typescript/test/e2e/raw-signer.test.ts +++ b/sdk/typescript/test/e2e/raw-signer.test.ts @@ -13,6 +13,7 @@ import { import * as secp from '@noble/secp256k1'; import { Signature } from '@noble/secp256k1'; import { setup, TestToolbox } from './utils/setup'; +import { blake2b } from '@noble/hashes/blake2b'; describe('RawSigner', () => { let toolbox: TestToolbox; @@ -24,11 +25,12 @@ describe('RawSigner', () => { it('Ed25519 keypair signData', async () => { const keypair = new Ed25519Keypair(); const signData = new TextEncoder().encode('hello world'); + const digest = blake2b(signData, { dkLen: 32 }); const signer = new RawSigner(keypair, toolbox.provider); const serializedSignature = await signer.signData(signData); const { signature, pubKey } = fromSerializedSignature(serializedSignature); const isValid = nacl.sign.detached.verify( - signData, + digest, signature, pubKey.toBytes(), ); @@ -59,7 +61,8 @@ describe('RawSigner', () => { it('Secp256k1 keypair signData', async () => { const keypair = new Secp256k1Keypair(); const signData = new TextEncoder().encode('hello world'); - const msgHash = await secp.utils.sha256(signData); + const digest = blake2b(signData, { dkLen: 32 }); + const msgHash = await secp.utils.sha256(digest); const signer = new RawSigner(keypair, toolbox.provider); const serializedSignature = await signer.signData(signData); const { signature, pubKey } = fromSerializedSignature(serializedSignature);