From 653adf7f4e7edca329b1a50e9677a55a84a98151 Mon Sep 17 00:00:00 2001 From: Mikhail Zabaluev Date: Fri, 20 Dec 2024 18:41:59 +0200 Subject: [PATCH 1/7] feat(signing): byte array conversions for ed25519 Provide From conversion impls to construct PrivateKey and Signature from byte arrays. The conversions into PrivateKey are fallible, while a Signature is allowed to be constructed from any 64 bytes. --- .../interface/src/cryptography/ed25519.rs | 36 ++++++++++++++++++- .../signing/interface/src/cryptography/mod.rs | 7 ++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/util/signing/interface/src/cryptography/ed25519.rs b/util/signing/interface/src/cryptography/ed25519.rs index e90070cd2..a004a5be6 100644 --- a/util/signing/interface/src/cryptography/ed25519.rs +++ b/util/signing/interface/src/cryptography/ed25519.rs @@ -1,4 +1,4 @@ -use crate::cryptography::Curve; +use crate::cryptography::{CryptoMaterialError, Curve}; use crate::{Verify, VerifyError}; use anyhow::Context; use ed25519_dalek::Verifier as _; @@ -32,3 +32,37 @@ impl Verify for Ed25519 { Ok(verifying_key.verify(message, &signature).is_ok()) } } + +impl TryFrom<[u8; 32]> for PublicKey { + type Error = CryptoMaterialError; + + #[inline] + fn try_from(bytes: [u8; 32]) -> Result { + PublicKey::try_from(&bytes) + } +} + +impl TryFrom<&[u8; 32]> for PublicKey { + type Error = CryptoMaterialError; + + fn try_from(bytes: &[u8; 32]) -> Result { + let key = ed25519_dalek::VerifyingKey::from_bytes(bytes) + .map_err(|e| CryptoMaterialError(e.into()))?; + Ok(PublicKey(key.to_bytes())) + } +} + +// Following the example given by ed25519, we allow any 64 bytes to construct a +// (possibly invalid) signature. +impl From<[u8; 64]> for Signature { + #[inline] + fn from(bytes: [u8; 64]) -> Self { + Signature(bytes) + } +} +impl From<&[u8; 64]> for Signature { + #[inline] + fn from(bytes: &[u8; 64]) -> Self { + Signature(*bytes) + } +} diff --git a/util/signing/interface/src/cryptography/mod.rs b/util/signing/interface/src/cryptography/mod.rs index edb4aef10..14346f4d5 100644 --- a/util/signing/interface/src/cryptography/mod.rs +++ b/util/signing/interface/src/cryptography/mod.rs @@ -29,6 +29,8 @@ macro_rules! fixed_size { pub mod ed25519; pub mod secp256k1; +use std::error::Error; + pub trait TryFromBytes: Sized { fn try_from_bytes(bytes: &[u8]) -> Result; } @@ -41,3 +43,8 @@ pub trait Curve { type PublicKey: TryFromBytes; type Signature; } + +/// Errors that occur when parsing signature or key material from byte sequences. +#[derive(Debug, thiserror::Error)] +#[error(transparent)] +pub struct CryptoMaterialError(Box); From 53e08b7dd0e18c05f5d467e8cdf948f824324a12 Mon Sep 17 00:00:00 2001 From: Mikhail Zabaluev Date: Fri, 20 Dec 2024 18:44:16 +0200 Subject: [PATCH 2/7] feat(signing): movement-signer-test crate Add TestSigner providing an in-process Signing implementation for tests. --- Cargo.lock | 8 +++++++ Cargo.toml | 4 +++- util/signing/testing/Cargo.toml | 18 +++++++++++++++ util/signing/testing/src/ed25519.rs | 34 +++++++++++++++++++++++++++++ util/signing/testing/src/lib.rs | 1 + 5 files changed, 64 insertions(+), 1 deletion(-) create mode 100644 util/signing/testing/Cargo.toml create mode 100644 util/signing/testing/src/ed25519.rs create mode 100644 util/signing/testing/src/lib.rs diff --git a/Cargo.lock b/Cargo.lock index 43734f0c6..6d251aa9e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10541,6 +10541,14 @@ dependencies = [ "vaultrs", ] +[[package]] +name = "movement-signer-test" +version = "0.0.2" +dependencies = [ + "ed25519-dalek 2.1.1", + "movement-signer", +] + [[package]] name = "movement-signing-aptos" version = "0.0.2" diff --git a/Cargo.toml b/Cargo.toml index e044247fc..fc470a6f6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -3,6 +3,8 @@ resolver = "2" members = [ + # FIXME: do we still need the demo? + "demo/hsm", "protocol-units/bridge/config", "protocol-units/bridge/setup", "protocol-units/execution/maptos/dof", @@ -45,7 +47,7 @@ members = [ "util/signing/integrations/aptos", "util/signing/providers/aws-kms", "util/signing/providers/hashicorp-vault", - "demo/hsm" + "util/signing/testing", ] [workspace.package] diff --git a/util/signing/testing/Cargo.toml b/util/signing/testing/Cargo.toml new file mode 100644 index 000000000..4dcc04f9f --- /dev/null +++ b/util/signing/testing/Cargo.toml @@ -0,0 +1,18 @@ +[package] +name = "movement-signer-test" +description = "Test support for Movement signing API" +version.workspace = true +edition.workspace = true +license.workspace = true +authors.workspace = true +repository.workspace = true +homepage.workspace = true +publish.workspace = true +rust-version.workspace = true + +[dependencies] +movement-signer = { workspace = true } +ed25519-dalek = { workspace = true } + +[lints] +workspace = true diff --git a/util/signing/testing/src/ed25519.rs b/util/signing/testing/src/ed25519.rs new file mode 100644 index 000000000..35a9d310c --- /dev/null +++ b/util/signing/testing/src/ed25519.rs @@ -0,0 +1,34 @@ +use movement_signer::cryptography::ed25519::{self, Ed25519}; +use movement_signer::{SignerError, Signing}; + +use ed25519_dalek::{Signer as _, SigningKey}; + +/// In-process signer used for testing the signing API. +/// +/// This signer wraps an Ed25519 private key to provide a signing service with +/// the Ed25519 elliptic curve. Because the private key is kept in process +/// memory, this signer implementation should not be used in production. +pub struct TestSigner { + signing_key: SigningKey, +} + +impl TestSigner { + pub fn new(signing_key: SigningKey) -> Self { + Self { signing_key } + } +} + +impl Signing for TestSigner { + async fn sign(&self, message: &[u8]) -> Result { + let signature = + self.signing_key.try_sign(message).map_err(|e| SignerError::Sign(e.into()))?; + Ok(signature.to_bytes().into()) + } + + async fn public_key(&self) -> Result { + let key = self.signing_key.verifying_key(); + // The conversion should never fail because it's round-tripping + // a valid key. + Ok(key.to_bytes().try_into().unwrap()) + } +} diff --git a/util/signing/testing/src/lib.rs b/util/signing/testing/src/lib.rs new file mode 100644 index 000000000..58845d3fa --- /dev/null +++ b/util/signing/testing/src/lib.rs @@ -0,0 +1 @@ +pub mod ed25519; From 7929c218a5886017f657796e98b680bb2e01ebfb Mon Sep 17 00:00:00 2001 From: Mikhail Zabaluev Date: Mon, 23 Dec 2024 11:38:38 +0200 Subject: [PATCH 3/7] refactor(signing): no need for custom trait TryFromBytes is not needed, neither is the public key as a member of HashiCorpVault state. --- .../signing/interface/src/cryptography/mod.rs | 16 ++--- .../providers/hashicorp-vault/src/hsm/mod.rs | 71 ++++--------------- 2 files changed, 20 insertions(+), 67 deletions(-) diff --git a/util/signing/interface/src/cryptography/mod.rs b/util/signing/interface/src/cryptography/mod.rs index 14346f4d5..7f4667f20 100644 --- a/util/signing/interface/src/cryptography/mod.rs +++ b/util/signing/interface/src/cryptography/mod.rs @@ -11,10 +11,14 @@ macro_rules! fixed_size { } } - impl crate::cryptography::TryFromBytes for $Name { - fn try_from_bytes(bytes: &[u8]) -> Result { + impl TryFrom<&[u8]> for $Name { + type Error = crate::cryptography::CryptoMaterialError; + + fn try_from(bytes: &[u8]) -> Result { + use crate::cryptography::CryptoMaterialError; + if bytes.len() != Self::BYTES_LEN { - Err(anyhow::anyhow!("invalid length"))?; + Err(CryptoMaterialError("invalid length".into()))?; } let mut inner = [0u8; Self::BYTES_LEN]; @@ -31,16 +35,12 @@ pub mod secp256k1; use std::error::Error; -pub trait TryFromBytes: Sized { - fn try_from_bytes(bytes: &[u8]) -> Result; -} - /// A designator for an elliptic curve. /// /// This trait has no methods, but it binds the types of the public key and /// the signature used by the EC digital signing algorithm. pub trait Curve { - type PublicKey: TryFromBytes; + type PublicKey; type Signature; } diff --git a/util/signing/providers/hashicorp-vault/src/hsm/mod.rs b/util/signing/providers/hashicorp-vault/src/hsm/mod.rs index 17b4a1228..ba2f77c03 100644 --- a/util/signing/providers/hashicorp-vault/src/hsm/mod.rs +++ b/util/signing/providers/hashicorp-vault/src/hsm/mod.rs @@ -1,8 +1,10 @@ use crate::cryptography::HashiCorpVaultCryptography; use anyhow::Context; -use movement_signer::cryptography::TryFromBytes; use movement_signer::{ - cryptography::{ed25519::Ed25519, Curve}, + cryptography::{ + ed25519::{self, Ed25519}, + Curve, + }, SignerError, Signing, }; use vaultrs::api::transit::{requests::CreateKeyRequest, responses::ReadKeyData}; @@ -14,7 +16,6 @@ pub struct HashiCorpVault { client: VaultClient, key_name: String, mount_name: String, - pub public_key: ::PublicKey, _cryptography_marker: std::marker::PhantomData, } @@ -23,19 +24,8 @@ where C: Curve + HashiCorpVaultCryptography, { /// Creates a new HashiCorp Vault HSM - pub fn new( - client: VaultClient, - key_name: String, - mount_name: String, - public_key: C::PublicKey, - ) -> Self { - Self { - client, - key_name, - mount_name, - public_key, - _cryptography_marker: std::marker::PhantomData, - } + pub fn new(client: VaultClient, key_name: String, mount_name: String) -> Self { + Self { client, key_name, mount_name, _cryptography_marker: std::marker::PhantomData } } /// Tries to create a new HashiCorp Vault HSM from the environment @@ -53,14 +43,8 @@ where let key_name = std::env::var("VAULT_KEY_NAME").context("VAULT_KEY_NAME not set")?; let mount_name = std::env::var("VAULT_MOUNT_NAME").context("VAULT_MOUNT_NAME not set")?; - let public_key = std::env::var("VAULT_PUBLIC_KEY").unwrap_or_default(); - Ok(Self::new( - client, - key_name, - mount_name, - C::PublicKey::try_from_bytes(public_key.as_bytes())?, - )) + Ok(Self::new(client, key_name, mount_name)) } /// Creates a new key in the transit backend @@ -73,35 +57,8 @@ where ) .await .context("Failed to create key")?; - Ok(self) } - - /// Fills with a public key fetched from vault. - pub async fn fill_with_public_key(self) -> Result { - let res = key::read(&self.client, self.mount_name.as_str(), self.key_name.as_str()) - .await - .context("Failed to read key")?; - println!("Read key: {:?}", res); - - let public_key = match res.keys { - ReadKeyData::Symmetric(_) => { - return Err(anyhow::anyhow!("Symmetric keys are not supported")); - } - ReadKeyData::Asymmetric(keys) => { - let key = keys.values().next().context("No key found")?; - base64::decode(key.public_key.as_str()).context("Failed to decode public key")? - } - }; - - println!("Public key: {:?}", public_key); - Ok(Self::new( - self.client, - self.key_name, - self.mount_name, - C::PublicKey::try_from_bytes(public_key.as_slice())?, - )) - } } impl Signing for HashiCorpVault { @@ -121,18 +78,14 @@ impl Signing for HashiCorpVault { return Err(SignerError::Internal("Symmetric keys are not supported".to_string())); } ReadKeyData::Asymmetric(keys) => { - let key = keys - .values() - .next() - .context("No key found") - .map_err(|e| SignerError::KeyNotFound)?; + let key = keys.values().next().ok_or_else(|| SignerError::KeyNotFound)?; base64::decode(key.public_key.as_str()) - .context("Failed to decode public key") - .map_err(|e| SignerError::Internal(e.to_string()))? + .context("failed to decode public key") + .map_err(|e| SignerError::Decode(e.into()))? } }; - Ok(::PublicKey::try_from_bytes(public_key.as_slice()) - .map_err(|e| SignerError::Internal(e.to_string()))?) + Ok(ed25519::PublicKey::try_from(public_key.as_slice()) + .map_err(|e| SignerError::Decode(e.into()))?) } } From a50bf0e4d81d73b90738961815a00dd67b4df99f Mon Sep 17 00:00:00 2001 From: Mikhail Zabaluev Date: Mon, 23 Dec 2024 13:06:49 +0200 Subject: [PATCH 4/7] test(signing): test TestSigner --- Cargo.lock | 5 ++++- util/signing/testing/Cargo.toml | 7 +++++++ util/signing/testing/tests/signer.rs | 23 +++++++++++++++++++++++ 3 files changed, 34 insertions(+), 1 deletion(-) create mode 100644 util/signing/testing/tests/signer.rs diff --git a/Cargo.lock b/Cargo.lock index 6d251aa9e..458cd49a0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6204,6 +6204,7 @@ checksum = "4a3daa8e81a3963a60642bcc1f90a670680bd4a77535faa384e9d1c79d620871" dependencies = [ "curve25519-dalek 4.1.3", "ed25519 2.2.3", + "rand_core 0.6.4", "serde", "sha2 0.10.8", "subtle", @@ -9110,7 +9111,6 @@ dependencies = [ "poem", "poem-openapi", "rand 0.7.3", - "rand_core 0.5.1", "schemars", "serde", "serde_json", @@ -10545,8 +10545,11 @@ dependencies = [ name = "movement-signer-test" version = "0.0.2" dependencies = [ + "anyhow", "ed25519-dalek 2.1.1", "movement-signer", + "rand 0.8.5", + "tokio", ] [[package]] diff --git a/util/signing/testing/Cargo.toml b/util/signing/testing/Cargo.toml index 4dcc04f9f..7d7bf7f49 100644 --- a/util/signing/testing/Cargo.toml +++ b/util/signing/testing/Cargo.toml @@ -14,5 +14,12 @@ rust-version.workspace = true movement-signer = { workspace = true } ed25519-dalek = { workspace = true } +[dev-dependencies] +anyhow = { workspace = true } +ed25519-dalek = { workspace = true, features = ["rand_core"] } +# Workspace is on rand 0.7 due largely to aptos-core +rand = "0.8" +tokio = { workspace = true, features = ["macros"] } + [lints] workspace = true diff --git a/util/signing/testing/tests/signer.rs b/util/signing/testing/tests/signer.rs new file mode 100644 index 000000000..62e262f0a --- /dev/null +++ b/util/signing/testing/tests/signer.rs @@ -0,0 +1,23 @@ +mod ed25519 { + use movement_signer::cryptography::ed25519::{Ed25519, PublicKey}; + use movement_signer::{Signing, Verify}; + use movement_signer_test::ed25519::TestSigner; + + use ed25519_dalek::SigningKey; + use rand::rngs::OsRng; + + #[tokio::test] + async fn basic_signing() -> anyhow::Result<()> { + let message = b"Hello, world!"; + let mut rng = OsRng; + let signing_key = SigningKey::generate(&mut rng); + let verifying_key = signing_key.verifying_key(); + let public_key = PublicKey::try_from(verifying_key.as_bytes() as &[u8])?; + let signer = TestSigner::new(signing_key); + + let signature = signer.sign(message).await?; + assert!(Ed25519.verify(message, &signature, &public_key)?); + + Ok(()) + } +} From ca1ffac49343f1300fee66a5e00ef9825cfa07a0 Mon Sep 17 00:00:00 2001 From: Mikhail Zabaluev Date: Mon, 23 Dec 2024 13:07:19 +0200 Subject: [PATCH 5/7] chore: clean up unused rand_core dep --- Cargo.toml | 1 - protocol-units/execution/maptos/opt-executor/Cargo.toml | 1 - 2 files changed, 2 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index fc470a6f6..cb40a6c34 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -269,7 +269,6 @@ proptest = { version = "1.3.1", default-features = false, features = ["alloc"] } proptest-derive = "0.4" quote = "1.0" rand = "0.7.3" -rand_core = "0.5.1" rayon = "1.10.0" reqwest = "0.12.4" risc0-build = "0.20" diff --git a/protocol-units/execution/maptos/opt-executor/Cargo.toml b/protocol-units/execution/maptos/opt-executor/Cargo.toml index 849f3d43f..5df77d9ea 100644 --- a/protocol-units/execution/maptos/opt-executor/Cargo.toml +++ b/protocol-units/execution/maptos/opt-executor/Cargo.toml @@ -31,7 +31,6 @@ lazy_static = "1.4.0" tokio = { workspace = true } tracing = { workspace = true } rand = { workspace = true } -rand_core = { workspace = true } bcs = { workspace = true } futures = { workspace = true } From 82b7b683ceff42597354bfd3f53663441a90c8d3 Mon Sep 17 00:00:00 2001 From: Mikhail Zabaluev Date: Fri, 3 Jan 2025 18:05:10 +0200 Subject: [PATCH 6/7] test(signing): Add executor test for signed transactions --- Cargo.lock | 7 +++ Cargo.toml | 1 + util/signing/testing/Cargo.toml | 7 +++ util/signing/testing/tests/execute.rs | 63 +++++++++++++++++++++++++++ 4 files changed, 78 insertions(+) create mode 100644 util/signing/testing/tests/execute.rs diff --git a/Cargo.lock b/Cargo.lock index 458cd49a0..aae63c892 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10546,9 +10546,16 @@ name = "movement-signer-test" version = "0.0.2" dependencies = [ "anyhow", + "aptos-crypto", + "aptos-types", + "chrono", "ed25519-dalek 2.1.1", + "maptos-dof-execution", + "maptos-execution-util", "movement-signer", + "movement-signing-aptos", "rand 0.8.5", + "tempfile", "tokio", ] diff --git a/Cargo.toml b/Cargo.toml index cb40a6c34..c00c2ed95 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -124,6 +124,7 @@ aptos-account-whitelist = { path = "protocol-units/access-control/aptos/account- movement-signer = { path = "util/signing/interface" } movement-signer-aws-kms = { path = "util/signing/providers/aws-kms" } movement-signer-hashicorp-vault = { path = "util/signing/providers/hashicorp-vault" } +movement-signing-aptos = { path = "util/signing/integrations/aptos" } ## vault vaultrs = { version = "0.7.3" } diff --git a/util/signing/testing/Cargo.toml b/util/signing/testing/Cargo.toml index 7d7bf7f49..71d0ab7dc 100644 --- a/util/signing/testing/Cargo.toml +++ b/util/signing/testing/Cargo.toml @@ -15,10 +15,17 @@ movement-signer = { workspace = true } ed25519-dalek = { workspace = true } [dev-dependencies] +maptos-dof-execution = { workspace = true } +maptos-execution-util = { workspace = true } +movement-signing-aptos = { workspace = true } +aptos-crypto = { workspace = true } +aptos-types = { workspace = true } anyhow = { workspace = true } +chrono = { workspace = true } ed25519-dalek = { workspace = true, features = ["rand_core"] } # Workspace is on rand 0.7 due largely to aptos-core rand = "0.8" +tempfile = { workspace = true } tokio = { workspace = true, features = ["macros"] } [lints] diff --git a/util/signing/testing/tests/execute.rs b/util/signing/testing/tests/execute.rs new file mode 100644 index 000000000..300f9deaa --- /dev/null +++ b/util/signing/testing/tests/execute.rs @@ -0,0 +1,63 @@ +use maptos_dof_execution::{v1::Executor, DynOptFinExecutor}; +use maptos_dof_execution::{ExecutableBlock, ExecutableTransactions, SignatureVerifiedTransaction}; +use maptos_execution_util::config::Config; +use movement_signer_test::ed25519::TestSigner; +use movement_signing_aptos::TransactionSigner; + +use aptos_crypto::{ed25519::Ed25519PrivateKey, HashValue, Uniform}; +use aptos_types::account_address::AccountAddress; +use aptos_types::chain_id::ChainId; +use aptos_types::transaction::{ + RawTransaction, Script, SignedTransaction, Transaction, TransactionPayload, +}; + +use anyhow::Context; +use tempfile::TempDir; + +fn setup(mut maptos_config: Config) -> Result<(Executor, TempDir), anyhow::Error> { + let tempdir = tempfile::tempdir()?; + // replace the db path with the temporary directory + maptos_config.chain.maptos_db_path.replace(tempdir.path().to_path_buf()); + let executor = Executor::try_from_config(maptos_config)?; + Ok((executor, tempdir)) +} + +async fn create_signed_transaction( + signer: &impl TransactionSigner, +) -> Result { + let transaction_payload = TransactionPayload::Script(Script::new(vec![0], vec![], vec![])); + let raw_transaction = RawTransaction::new( + AccountAddress::random(), + 0, + transaction_payload, + 0, + 0, + 0, + ChainId::test(), // This is the value used in aptos testing code. + ); + signer.sign_transaction(raw_transaction).await.context("failed to sign") +} + +#[tokio::test] +async fn execute_signed_transaction() -> Result<(), anyhow::Error> { + let private_key = Ed25519PrivateKey::generate_for_testing(); + let mut config = Config::default(); + let signing_key = ed25519_dalek::SigningKey::from_bytes(&private_key.to_bytes()); + config.chain.maptos_private_key = private_key.clone(); + let signer = TestSigner::new(signing_key); + let (executor, _tempdir) = setup(config)?; + let transaction = create_signed_transaction(&signer).await?; + let block_id = HashValue::random(); + let block_metadata = executor + .build_block_metadata(block_id.clone(), chrono::Utc::now().timestamp_micros() as u64) + .unwrap(); + let txs = ExecutableTransactions::Unsharded( + [Transaction::BlockMetadata(block_metadata), Transaction::UserTransaction(transaction)] + .into_iter() + .map(SignatureVerifiedTransaction::Valid) + .collect(), + ); + let block = ExecutableBlock::new(block_id.clone(), txs); + executor.execute_block_opt(block).await?; + Ok(()) +} From 0018145fc2bca8c2680e69acdf81c2079db7ab42 Mon Sep 17 00:00:00 2001 From: Mikhail Zabaluev Date: Fri, 3 Jan 2025 19:10:57 +0200 Subject: [PATCH 7/7] test: add signer tests to the unit-tests workflow --- .github/workflows/checks-all.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/checks-all.yml b/.github/workflows/checks-all.yml index 8c041ea50..ccaeb185f 100755 --- a/.github/workflows/checks-all.yml +++ b/.github/workflows/checks-all.yml @@ -69,7 +69,8 @@ jobs: -p memseq \ -p move-rocks \ -p movement-types \ - -p movement-config + -p movement-config \ + -p movement-signer-test EOF movement-full-node-local: