Skip to content

Commit

Permalink
crypto: pass in hashed message for sign and verify (MystenLabs#9561)
Browse files Browse the repository at this point in the history
## Description 

Pass in hashed message for sign and verify API for user signatures.
Closes MystenLabs#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
  • Loading branch information
joyqvq authored Mar 20, 2023
1 parent 9ce7836 commit 9b29bef
Show file tree
Hide file tree
Showing 11 changed files with 68 additions and 67 deletions.
5 changes: 5 additions & 0 deletions .changeset/orange-birds-tan.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@mysten/sui.js": minor
---

Pass blake2b hash to signer API
36 changes: 15 additions & 21 deletions crates/sui-keys/src/keystore.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -31,12 +31,12 @@ pub enum Keystore {
}
#[enum_dispatch]
pub trait AccountKeystore: Send + Sync {
#[warn(deprecated)]
fn sign(&self, address: &SuiAddress, msg: &[u8]) -> Result<Signature, signature::Error>;
fn add_key(&mut self, keypair: SuiKeyPair) -> Result<(), anyhow::Error>;
fn keys(&self) -> Vec<PublicKey>;
fn get_key(&self, address: &SuiAddress) -> Result<&SuiKeyPair, anyhow::Error>;

fn sign_hashed(&self, address: &SuiAddress, msg: &[u8]) -> Result<Signature, signature::Error>;

fn sign_secure<T>(
&self,
address: &SuiAddress,
Expand Down Expand Up @@ -128,17 +128,14 @@ impl<'de> Deserialize<'de> for FileBasedKeystore {
}

impl AccountKeystore for FileBasedKeystore {
#[warn(deprecated)]
fn sign(&self, address: &SuiAddress, msg: &[u8]) -> Result<Signature, signature::Error> {
Ok(self
.keys
.get(address)
.ok_or_else(|| {
fn sign_hashed(&self, address: &SuiAddress, msg: &[u8]) -> Result<Signature, signature::Error> {
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<T>(
&self,
address: &SuiAddress,
Expand Down Expand Up @@ -228,17 +225,14 @@ pub struct InMemKeystore {
}

impl AccountKeystore for InMemKeystore {
#[warn(deprecated)]
fn sign(&self, address: &SuiAddress, msg: &[u8]) -> Result<Signature, signature::Error> {
Ok(self
.keys
.get(address)
.ok_or_else(|| {
fn sign_hashed(&self, address: &SuiAddress, msg: &[u8]) -> Result<Signature, signature::Error> {
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<T>(
&self,
address: &SuiAddress,
Expand Down
10 changes: 5 additions & 5 deletions crates/sui-open-rpc/spec/openrpc.json
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@
{
"name": "signatures",
"value": [
"ALhGcUtEV3kT9ukC4ItNjlv5/gATBGmWEQan7ri2YKHzbtI2rSMpWvxUg7CN+Jap2zPyhfXIes+xt9Q60H2Ylw1Dij1TvBYKLcfLNo8fq6GASb9yfo6uvuwNUBGkTf54wQ=="
"ALGzUorJ5eSfoPZSblxKUjNuNnB9R06PFVBmbopMmzcmHgZZWAfIpxifHMP9ogXFMxb0weTzRgVbUf241OrvvA1Dij1TvBYKLcfLNo8fq6GASb9yfo6uvuwNUBGkTf54wQ=="
]
},
{
Expand Down Expand Up @@ -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": {
Expand Down Expand Up @@ -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": {
Expand Down
9 changes: 7 additions & 2 deletions crates/sui-rosetta/src/construction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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;
Expand Down Expand Up @@ -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),
}],
})
Expand Down
6 changes: 2 additions & 4 deletions crates/sui-rosetta/tests/rosetta_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
37 changes: 13 additions & 24 deletions crates/sui-types/src/crypto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Signature>) -> Self {
Signer::sign(secret, hashed_msg)
}

pub fn new_secure<T>(value: &IntentMessage<T>, secret: &dyn Signer<Signature>) -> 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`.
Expand Down Expand Up @@ -1004,10 +1008,6 @@ pub trait SuiSignature: Sized + ToFromBytes {
fn public_key_bytes(&self) -> &[u8];
fn scheme(&self) -> SignatureScheme;

fn verify<T>(&self, value: &T, author: SuiAddress) -> SuiResult<()>
where
T: Signable<Vec<u8>>;

fn verify_secure<T>(&self, value: &IntentMessage<T>, author: SuiAddress) -> SuiResult<()>
where
T: Serialize;
Expand All @@ -1030,27 +1030,16 @@ impl<S: SuiSignatureInner + Sized> SuiSignature for S {
S::PubKey::SIGNATURE_SCHEME
}

fn verify<T>(&self, value: &T, author: SuiAddress) -> SuiResult<()>
where
T: Signable<Vec<u8>>,
{
// 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<T>(&self, value: &IntentMessage<T>, 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),
})
Expand Down
12 changes: 8 additions & 4 deletions crates/sui-types/src/multisig.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// SPDX-License-Identifier: Apache-2.0

use crate::{
crypto::{CompressedSignature, SignatureScheme},
crypto::{CompressedSignature, DefaultHash, SignatureScheme},
signature::AuthenticatorTrait,
sui_serde::SuiBitmap,
};
Expand All @@ -11,6 +11,7 @@ use fastcrypto::{
ed25519::Ed25519PublicKey,
encoding::Base64,
error::FastCryptoError,
hash::HashFunction,
secp256k1::Secp256k1PublicKey,
secp256r1::Secp256r1PublicKey,
traits::{ToFromBytes, VerifyingKey},
Expand Down Expand Up @@ -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.
Expand All @@ -133,23 +137,23 @@ 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(|_| {
SuiError::InvalidSignature {
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(|_| {
SuiError::InvalidSignature {
error: "Invalid public key".to_string(),
}
})?;
pk.verify(&message, &s.try_into()?)
pk.verify(&digest, &s.try_into()?)
}
};
if res.is_ok() {
Expand Down
2 changes: 1 addition & 1 deletion doc/src/learn/exchange-integration-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
4 changes: 3 additions & 1 deletion sdk/typescript/src/signers/raw-signer.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -24,7 +25,8 @@ export class RawSigner extends SignerWithProvider {

async signData(data: Uint8Array): Promise<SerializedSignature> {
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({
Expand Down
7 changes: 4 additions & 3 deletions sdk/typescript/src/utils/verify.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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:
Expand Down
7 changes: 5 additions & 2 deletions sdk/typescript/test/e2e/raw-signer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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(),
);
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit 9b29bef

Please sign in to comment.