diff --git a/Cargo.lock b/Cargo.lock index a8935a23..8365228a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -144,6 +144,7 @@ dependencies = [ "bs58 0.5.0", "fluence-keypair", "serde", + "thiserror", ] [[package]] @@ -179,6 +180,7 @@ dependencies = [ "air-interpreter-cid", "air-interpreter-data", "air-interpreter-interface", + "air-interpreter-signatures", "aquavm-air", "avm-interface", "avm-server", @@ -202,6 +204,7 @@ dependencies = [ "air-interpreter-signatures", "air-test-utils", "aquavm-air-parser", + "fluence-keypair", "itertools", "maplit", "nom", @@ -341,6 +344,7 @@ dependencies = [ "air-utils", "aquavm-air-parser", "borsh 0.10.3", + "bs58 0.5.0", "concat-idents", "criterion 0.3.6", "csv", diff --git a/air/Cargo.toml b/air/Cargo.toml index 11912c8c..0ec49045 100644 --- a/air/Cargo.toml +++ b/air/Cargo.toml @@ -52,6 +52,7 @@ fluence-app-service = "0.29.0" marine-rs-sdk = { version = "0.10.0", features = ["logger"] } borsh = "0.10.3" +bs58 = "0.5.0" # the feature just silence a warning in the criterion 0.3.x. criterion = { version = "0.3.3", features = ["html_reports"] } csv = "1.1.5" diff --git a/air/src/farewell_step/outcome.rs b/air/src/farewell_step/outcome.rs index 14369319..49fae0df 100644 --- a/air/src/farewell_step/outcome.rs +++ b/air/src/farewell_step/outcome.rs @@ -24,9 +24,9 @@ use crate::INTERPRETER_SUCCESS; use air_interpreter_data::InterpreterData; use air_interpreter_interface::CallRequests; +use air_interpreter_signatures::KeyPair; use air_utils::measure; use fluence_keypair::error::SigningError; -use fluence_keypair::KeyPair; use std::fmt::Debug; use std::hash::Hash; @@ -138,7 +138,7 @@ fn sign_result(exec_ctx: &mut ExecutionCtx<'_>, keypair: &KeyPair) -> Result<(), .map_err(signing_error_into_outcome)?; let current_pubkey = keypair.public(); - exec_ctx.signature_store.put(current_pubkey.into(), current_signature); + exec_ctx.signature_store.put(current_pubkey, current_signature); Ok(()) } diff --git a/air/src/preparation_step/errors.rs b/air/src/preparation_step/errors.rs index 23661b53..2c780482 100644 --- a/air/src/preparation_step/errors.rs +++ b/air/src/preparation_step/errors.rs @@ -81,11 +81,8 @@ pub enum PreparationError { }, /// Malformed keypair format data. - #[error("malformed keypair format: {error:?}")] - MalformedKeyPairData { - #[from] - error: fluence_keypair::error::DecodingError, - }, + #[error("malformed keypair format: {0}")] + MalformedKeyPairData(#[from] air_interpreter_signatures::KeyError), /// Failed to verify CidStore contents of the current data. #[error(transparent)] diff --git a/air/src/preparation_step/preparation.rs b/air/src/preparation_step/preparation.rs index c0709579..2eaea201 100644 --- a/air/src/preparation_step/preparation.rs +++ b/air/src/preparation_step/preparation.rs @@ -21,10 +21,11 @@ use crate::execution_step::TraceHandler; use air_interpreter_data::InterpreterData; use air_interpreter_interface::RunParameters; +use air_interpreter_signatures::KeyError; +use air_interpreter_signatures::KeyPair; use air_interpreter_signatures::SignatureStore; use air_parser::ast::Instruction; use fluence_keypair::KeyFormat; -use fluence_keypair::KeyPair; use std::convert::TryFrom; @@ -88,7 +89,7 @@ pub(crate) fn prepare<'i>( )?; let trace_handler = TraceHandler::from_trace(prev_data.trace, current_data.trace); - let key_format = KeyFormat::try_from(run_parameters.key_format)?; + let key_format = KeyFormat::try_from(run_parameters.key_format).map_err(KeyError::from)?; let keypair = KeyPair::from_secret_key(run_parameters.secret_key_bytes, key_format)?; let result = PreparationDescriptor { diff --git a/air/src/signing_step.rs b/air/src/signing_step.rs index 2804a0c8..a663755b 100644 --- a/air/src/signing_step.rs +++ b/air/src/signing_step.rs @@ -16,7 +16,9 @@ use crate::ExecutionError; -use air_interpreter_signatures::{PeerCidTracker, SignatureStore}; +use air_interpreter_signatures::KeyPair; +use air_interpreter_signatures::PeerCidTracker; +use air_interpreter_signatures::SignatureStore; #[cfg(feature = "gen_signatures")] #[tracing::instrument(skip_all)] @@ -24,7 +26,7 @@ pub(crate) fn sign_produced_cids( signature_tracker: &mut PeerCidTracker, signature_store: &mut SignatureStore, salt: &str, - keypair: &fluence_keypair::KeyPair, + keypair: &KeyPair, ) -> Result<(), ExecutionError> { use crate::UncatchableError; @@ -42,7 +44,7 @@ pub(crate) fn sign_produced_cids( _signature_tracker: &mut PeerCidTracker, _signature_store: &mut SignatureStore, _salt: &str, - _keypair: &fluence_keypair::KeyPair, + _keypair: &KeyPair, ) -> Result<(), ExecutionError> { Ok(()) } diff --git a/air/tests/test_module/features/signatures.rs b/air/tests/test_module/features/signatures.rs index 0192440a..4546f052 100644 --- a/air/tests/test_module/features/signatures.rs +++ b/air/tests/test_module/features/signatures.rs @@ -14,6 +14,8 @@ * limitations under the License. */ +#[cfg(feature = "check_signatures")] +mod algorithms; #[cfg(feature = "check_signatures")] mod attacks; #[cfg(feature = "check_signatures")] diff --git a/air/tests/test_module/features/signatures/algorithms.rs b/air/tests/test_module/features/signatures/algorithms.rs new file mode 100644 index 00000000..32286cef --- /dev/null +++ b/air/tests/test_module/features/signatures/algorithms.rs @@ -0,0 +1,95 @@ +/* + * Copyright 2023 Fluence Labs Limited + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +use air::{min_supported_version, PreparationError}; +use air_interpreter_data::{verification::DataVerifierError, InterpreterData}; +use air_interpreter_signatures::KeyError; +use air_test_utils::{ + assert_error_eq, + prelude::{request_sent_by, unit_call_service}, + test_runner::{create_avm, create_avm_with_key, NativeAirRunner, TestRunParameters}, +}; +use fluence_keypair::KeyFormat; +use serde_json::json; + +/// Checking that other peers' key algorithms are valid. +#[test] +fn test_banned_signature() { + let air_script = r#"(call "other_peer_id" ("" "") [])"#; + + let bad_algo_keypair = fluence_keypair::KeyPair::generate_secp256k1(); + let bad_algo_pk = bad_algo_keypair.public(); + let bad_algo_signature: air_interpreter_signatures::Signature = + air_interpreter_signatures::sign_cids(vec![], "particle_id", &bad_algo_keypair) + .unwrap() + .into(); + + let bad_algo_pk_ser = bs58::encode(bad_algo_pk.encode()).into_string(); + let bad_signature_store = json!({ + bad_algo_pk_ser: bad_algo_signature, + }); + let bad_peer_id = bad_algo_pk.to_peer_id().to_string(); + + let trace = vec![request_sent_by("init_peer_fake_id")]; + + let mut data = serde_json::to_value(InterpreterData::from_execution_result( + trace.into(), + <_>::default(), + <_>::default(), + <_>::default(), + min_supported_version().clone(), + )) + .unwrap(); + + data["signatures"] = bad_signature_store; + + let current_data = data.to_string(); + + let mut avm = create_avm(unit_call_service(), "other_peer_id"); + let res = avm + .call( + air_script, + "", + current_data, + TestRunParameters::from_init_peer_id("init_peer_fake_id"), + ) + .unwrap(); + + assert_error_eq!( + &res, + PreparationError::DataSignatureCheckError(DataVerifierError::MalformedKey { + error: KeyError::AlgorithmNotWhitelisted(KeyFormat::Secp256k1), + peer_id: bad_peer_id + }) + ); +} + +/// Checking that local key is valid. +#[test] +fn test_banned_signing_key() { + let air_script = "(null)"; + let bad_algo_keypair = fluence_keypair::KeyPair::generate_secp256k1(); + + let mut avm = create_avm_with_key::(bad_algo_keypair, unit_call_service()); + let res = avm + .call(air_script, "", "", TestRunParameters::from_init_peer_id("init_peer_id")) + .unwrap(); + + assert_error_eq!( + &res, + PreparationError::MalformedKeyPairData(KeyError::AlgorithmNotWhitelisted(KeyFormat::Secp256k1)) + ); +} diff --git a/air/tests/test_module/issues/issue_632.rs b/air/tests/test_module/issues/issue_632.rs index de656a21..aec91f98 100644 --- a/air/tests/test_module/issues/issue_632.rs +++ b/air/tests/test_module/issues/issue_632.rs @@ -46,7 +46,7 @@ fn issue_310() { 0, None, call_results, - &key_pair, + key_pair.as_inner(), particle_id.to_owned(), ) .unwrap() diff --git a/crates/air-lib/interpreter-data/src/interpreter_data/errors.rs b/crates/air-lib/interpreter-data/src/interpreter_data/errors.rs index 78335fff..f3896c6e 100644 --- a/crates/air-lib/interpreter-data/src/interpreter_data/errors.rs +++ b/crates/air-lib/interpreter-data/src/interpreter_data/errors.rs @@ -17,11 +17,12 @@ use std::rc::Rc; use air_interpreter_cid::CidRef; +use air_interpreter_signatures::KeyError; use thiserror::Error as ThisError; #[derive(Debug, ThisError)] pub enum DataVerifierError { - #[error(transparent)] - MalformedKey(fluence_keypair::error::DecodingError), + #[error("malformed key at peer: {peer_id:?}: {error}")] + MalformedKey { error: KeyError, peer_id: String }, #[error(transparent)] MalformedSignature(fluence_keypair::error::DecodingError), diff --git a/crates/air-lib/interpreter-data/src/interpreter_data/verification.rs b/crates/air-lib/interpreter-data/src/interpreter_data/verification.rs index 4d3e9c06..9ff53891 100644 --- a/crates/air-lib/interpreter-data/src/interpreter_data/verification.rs +++ b/crates/air-lib/interpreter-data/src/interpreter_data/verification.rs @@ -42,6 +42,16 @@ impl<'data> DataVerifier<'data> { // it can be further optimized if only required parts are passed // SignatureStore is not used elsewhere pub fn new(data: &'data InterpreterData, salt: &'data str) -> Result { + // validate key algoritms + for (public_key, _) in data.signatures.iter() { + public_key + .validate() + .map_err(|error| DataVerifierError::MalformedKey { + error, + peer_id: public_key.to_peer_id(), + })?; + } + // it contains signature too; if we try to add a value to a peer w/o signature, it is an immediate error let mut grouped_cids: HashMap, PeerInfo<'data>> = data .signatures diff --git a/crates/air-lib/interpreter-signatures/Cargo.toml b/crates/air-lib/interpreter-signatures/Cargo.toml index bbada958..b5588abe 100644 --- a/crates/air-lib/interpreter-signatures/Cargo.toml +++ b/crates/air-lib/interpreter-signatures/Cargo.toml @@ -18,3 +18,8 @@ bs58 = "0.5.0" borsh = { version = "0.10.3", features = ["rc"]} borsh-derive = "0.10.3" serde = { version = "1.0.190", features = ["derive"] } +thiserror = "1.0.49" + +[features] +default = ["rand"] +rand = [] diff --git a/crates/air-lib/interpreter-signatures/src/lib.rs b/crates/air-lib/interpreter-signatures/src/lib.rs index 3c80e75b..9d4874b2 100644 --- a/crates/air-lib/interpreter-signatures/src/lib.rs +++ b/crates/air-lib/interpreter-signatures/src/lib.rs @@ -33,14 +33,24 @@ mod trackers; pub use crate::stores::*; pub use crate::trackers::*; -pub use fluence_keypair::KeyPair; +pub use fluence_keypair::KeyFormat; use borsh::BorshSerialize; +use fluence_keypair::error::SigningError; use serde::{Deserialize, Serialize}; +use std::convert::TryFrom; use std::hash::Hash; use std::ops::Deref; +#[derive(thiserror::Error, Debug)] +pub enum KeyError { + #[error("signature algorithm {0:?} not whitelisted")] + AlgorithmNotWhitelisted(fluence_keypair::KeyFormat), + #[error("invalid key data: {0}")] + InvalidKeyData(#[from] fluence_keypair::error::DecodingError), +} + /// An opaque serializable representation of a public key. /// /// It can be a string or a binary, you shouldn't care about it unless you change serialization format. @@ -55,6 +65,10 @@ pub struct PublicKey( ); impl PublicKey { + pub fn new(inner: fluence_keypair::PublicKey) -> Self { + Self(inner) + } + pub fn verify( &self, value: &T, @@ -66,6 +80,15 @@ impl PublicKey { let serialized_value = SaltedData::new(&value, salt).serialize(); pk.verify(&serialized_value, signature) } + + pub fn to_peer_id(&self) -> String { + self.0.to_peer_id().to_string() + } + + pub fn validate(&self) -> Result<(), KeyError> { + let key_format = self.get_key_format(); + validate_with_key_format((), key_format) + } } impl Deref for PublicKey { @@ -82,9 +105,72 @@ impl Hash for PublicKey { } } -impl From for PublicKey { - fn from(value: fluence_keypair::PublicKey) -> Self { - Self(value) +#[derive(Clone)] +pub struct KeyPair(fluence_keypair::KeyPair); + +impl KeyPair { + pub fn new(inner: fluence_keypair::KeyPair) -> Result { + let key_format = inner.key_format(); + validate_with_key_format((), key_format)?; + + Ok(Self(inner)) + } + + pub fn from_secret_key(secret_key: Vec, key_format: KeyFormat) -> Result { + let inner = fluence_keypair::KeyPair::from_secret_key(secret_key, key_format)?; + Self::new(inner) + } + + pub fn public(&self) -> PublicKey { + PublicKey::new(self.0.public()) + } + + pub fn key_format(&self) -> KeyFormat { + self.0.key_format() + } + + pub fn sign(&self, msg: &[u8]) -> Result { + self.0.sign(msg).map(Signature::new) + } + + pub fn secret(&self) -> Vec { + self.0.secret().expect("cannot fail on supported formats") + } + + pub fn into_inner(self) -> fluence_keypair::KeyPair { + self.0 + } + + pub fn as_inner(&self) -> &fluence_keypair::KeyPair { + &self.0 + } + + #[cfg(feature = "rand")] + pub fn generate(key_format: KeyFormat) -> Result { + validate_with_key_format((), key_format)?; + + Ok(Self(fluence_keypair::KeyPair::generate(key_format))) + } +} + +impl TryFrom for KeyPair { + type Error = KeyError; + + fn try_from(value: fluence_keypair::KeyPair) -> Result { + Self::new(value) + } +} + +impl From for fluence_keypair::KeyPair { + fn from(value: KeyPair) -> Self { + value.0 + } +} + +pub(crate) fn validate_with_key_format(inner: V, key_format: KeyFormat) -> Result { + match key_format { + fluence_keypair::KeyFormat::Ed25519 => Ok(inner), + _ => Err(KeyError::AlgorithmNotWhitelisted(key_format)), } } diff --git a/crates/air-lib/interpreter-signatures/src/sede.rs b/crates/air-lib/interpreter-signatures/src/sede.rs index 08a07603..683e6def 100644 --- a/crates/air-lib/interpreter-signatures/src/sede.rs +++ b/crates/air-lib/interpreter-signatures/src/sede.rs @@ -34,7 +34,7 @@ impl serde::de::Visitor<'_> for PublicKeyVisitor { type Value = fluence_keypair::PublicKey; fn expecting(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - formatter.write_str("expecting a base58-encoded public key string") + formatter.write_str("a base58-encoded public key string") } fn visit_str(self, v: &str) -> Result @@ -43,9 +43,8 @@ impl serde::de::Visitor<'_> for PublicKeyVisitor { { use serde::de; - let public_key = fluence_keypair::PublicKey::from_base58(v) - .map_err(|_| de::Error::invalid_value(de::Unexpected::Str(v), &self))?; - Ok(public_key) + fluence_keypair::PublicKey::from_base58(v) + .map_err(|_| de::Error::invalid_value(de::Unexpected::Str(v), &self)) } } diff --git a/crates/air-lib/interpreter-signatures/src/trackers.rs b/crates/air-lib/interpreter-signatures/src/trackers.rs index d9c62093..ba1cc467 100644 --- a/crates/air-lib/interpreter-signatures/src/trackers.rs +++ b/crates/air-lib/interpreter-signatures/src/trackers.rs @@ -14,11 +14,11 @@ * limitations under the License. */ +use crate::KeyPair; use crate::SaltedData; use air_interpreter_cid::{CidRef, CID}; use fluence_keypair::error::SigningError; -use fluence_keypair::KeyPair; use std::rc::Rc; @@ -48,17 +48,17 @@ impl PeerCidTracker { salt: &str, keypair: &KeyPair, ) -> Result { - sign_cids(self.cids.clone(), salt, keypair) + sign_cids(self.cids.clone(), salt, &keypair.0).map(Into::into) } } -fn sign_cids( +pub fn sign_cids( mut cids: Vec>, salt: &str, - keypair: &KeyPair, -) -> Result { + keypair: &fluence_keypair::KeyPair, +) -> Result { cids.sort_unstable(); let serialized_cids = SaltedData::new(&cids, salt).serialize(); - keypair.sign(&serialized_cids).map(crate::Signature::new) + keypair.sign(&serialized_cids) } diff --git a/crates/air-lib/test-utils/Cargo.toml b/crates/air-lib/test-utils/Cargo.toml index 230b94f6..fc20aa57 100644 --- a/crates/air-lib/test-utils/Cargo.toml +++ b/crates/air-lib/test-utils/Cargo.toml @@ -19,6 +19,7 @@ aquavm-air = { version = "0.54.0", path = "../../../air" } air-interpreter-cid = { version = "0.6.0", path = "../interpreter-cid" } air-interpreter-data = { version = "0.14.0", path = "../interpreter-data" } air-interpreter-interface = { version = "0.15.1", path = "../interpreter-interface" } +air-interpreter-signatures = { version = "0.1.3", path = "../interpreter-signatures" } avm-interface = { version = "0.29.2", path = "../../../avm/interface" } avm-server = { version = "0.33.3", path = "../../../avm/server" } marine-rs-sdk = "0.10.0" diff --git a/crates/air-lib/test-utils/src/key_utils.rs b/crates/air-lib/test-utils/src/key_utils.rs index 08d2fb40..9c1600f2 100644 --- a/crates/air-lib/test-utils/src/key_utils.rs +++ b/crates/air-lib/test-utils/src/key_utils.rs @@ -14,7 +14,7 @@ * limitations under the License. */ -use fluence_keypair::KeyPair; +use air_interpreter_signatures::KeyPair; use rand_chacha::rand_core::SeedableRng; /// Derive fake keypair for testing proposes. @@ -25,6 +25,7 @@ use rand_chacha::rand_core::SeedableRng; // Should be moved to test lib when keypair interface PR is merged. pub fn derive_dummy_keypair(seed: &str) -> (KeyPair, String) { use sha2::{Digest as _, Sha256}; + use std::convert::TryFrom; let mut rng = { let mut hasher = Sha256::new(); @@ -33,7 +34,8 @@ pub fn derive_dummy_keypair(seed: &str) -> (KeyPair, String) { }; let keypair_ed25519 = ed25519_dalek::Keypair::generate(&mut rng); - let keypair: KeyPair = KeyPair::Ed25519(keypair_ed25519.into()); + let keypair = fluence_keypair::KeyPair::Ed25519(keypair_ed25519.into()); + let keypair = KeyPair::try_from(keypair).expect("cannot happen"); let peer_id = keypair.public().to_peer_id().to_string(); (keypair, peer_id) diff --git a/crates/air-lib/test-utils/src/test_runner.rs b/crates/air-lib/test-utils/src/test_runner.rs index 0916035d..ac49177a 100644 --- a/crates/air-lib/test-utils/src/test_runner.rs +++ b/crates/air-lib/test-utils/src/test_runner.rs @@ -177,14 +177,15 @@ pub fn create_custom_avm( TestRunner { runner, call_service, - keypair, + keypair: keypair.into_inner(), } } pub fn create_avm_with_key( - keypair: KeyPair, + keypair: impl Into, call_service: CallServiceClosure, ) -> TestRunner { + let keypair = keypair.into(); let current_peer_id = keypair.public().to_peer_id().to_string(); let runner = R::new(current_peer_id); diff --git a/crates/testing-framework/Cargo.toml b/crates/testing-framework/Cargo.toml index 8d3ff279..a21467ea 100644 --- a/crates/testing-framework/Cargo.toml +++ b/crates/testing-framework/Cargo.toml @@ -18,6 +18,7 @@ air-test-utils = { version = "0.12.1", path = "../air-lib/test-utils" } aquavm-air-parser = { version = "0.10.0", path = "../air-lib/air-parser" } itertools = "0.10.5" +fluence-keypair = "0.10.1" strum = { version="0.24.1", features=["derive"] } nom = "7.1.3" nom_locate = "4.1.0" diff --git a/crates/testing-framework/src/ephemeral/mod.rs b/crates/testing-framework/src/ephemeral/mod.rs index 5c3d779d..9b06ed5c 100644 --- a/crates/testing-framework/src/ephemeral/mod.rs +++ b/crates/testing-framework/src/ephemeral/mod.rs @@ -22,7 +22,6 @@ use crate::{ services::{services_to_call_service_closure, MarineServiceHandle, NetworkServices}, }; -use air_interpreter_signatures::KeyPair; use air_test_utils::{ key_utils::derive_dummy_keypair, test_runner::{ @@ -30,6 +29,7 @@ use air_test_utils::{ }, RawAVMOutcome, }; +use fluence_keypair::KeyPair; use std::{borrow::Borrow, cell::RefCell, collections::HashMap, hash::Hash, ops::Deref, rc::Rc}; @@ -86,7 +86,7 @@ pub struct Peer { } impl Peer { - pub fn new(keypair: KeyPair, services: Rc<[MarineServiceHandle]>) -> Self { + pub fn new(keypair: impl Into, services: Rc<[MarineServiceHandle]>) -> Self { let call_service = services_to_call_service_closure(services); let runner = create_avm_with_key::(keypair, call_service);