Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Encryption support for the statement store #14440

Merged
merged 8 commits into from
Jul 17, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions bin/node/cli/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,7 @@ pub fn new_partial(
&config.data_path,
Default::default(),
client.clone(),
keystore_container.keystore(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
keystore_container.keystore(),
keystore_container.local_keystore(),

config.prometheus_registry(),
&task_manager.spawn_handle(),
)
Expand Down
15 changes: 15 additions & 0 deletions client/keystore/src/local.rs
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,21 @@ impl Keystore for LocalKeystore {
self.sign::<ed25519::Pair>(key_type, public, msg)
}

fn with_ed25519_key(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already provide key_pair in the local key store. You can directly use this?

&self,
key_type: KeyTypeId,
public: &ed25519::Public,
f: &mut dyn FnMut(&ed25519::Pair),
) -> std::result::Result<bool, TraitError> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I remember doing a similar function in a branch.

One possible issue is that this is maybe not appropriate for a future hardware keystore, so a specialized function may be more suitable, but at this point I don't know if it is worth doing (given this would mean putting eceis in keystore and I think it was a point that was not suitable from previous PR).

Since it is explicitely a Statement store key that would not be possible to run on HSM, it may not be such an issue.

let pair = self.0.read().key_pair_by_type::<ed25519::Pair>(public, key_type)?;
Ok(if let Some(pair) = pair {
f(&pair);
true
} else {
false
})
}

fn ecdsa_public_keys(&self, key_type: KeyTypeId) -> Vec<ecdsa::Public> {
self.public_keys::<ecdsa::Pair>(key_type)
}
Expand Down
1 change: 1 addition & 0 deletions client/statement-store/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ prometheus-endpoint = { package = "substrate-prometheus-endpoint", version = "0.
sp-api = { version = "4.0.0-dev", path = "../../primitives/api" }
sp-blockchain = { version = "4.0.0-dev", path = "../../primitives/blockchain" }
sp-core = { version = "21.0.0", path = "../../primitives/core" }
sp-keystore = { version = "0.27.0", path = "../../primitives/keystore" }
sp-runtime = { version = "24.0.0", path = "../../primitives/runtime" }
sp-tracing = { version = "10.0.0", path = "../../primitives/tracing" }
sc-client-api = { version = "4.0.0-dev", path = "../api" }
Expand Down
67 changes: 60 additions & 7 deletions client/statement-store/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@ use parking_lot::RwLock;
use prometheus_endpoint::Registry as PrometheusRegistry;
use sp_api::ProvideRuntimeApi;
use sp_blockchain::HeaderBackend;
use sp_core::{hexdisplay::HexDisplay, traits::SpawnNamed, Decode, Encode};
use sp_core::{crypto::UncheckedFrom, hexdisplay::HexDisplay, traits::SpawnNamed, Decode, Encode};
use sp_keystore::Keystore;
use sp_runtime::traits::Block as BlockT;
use sp_statement_store::{
runtime_api::{InvalidStatement, StatementSource, ValidStatement, ValidateStatement},
Expand Down Expand Up @@ -199,6 +200,7 @@ pub struct Store {
+ Send
+ Sync,
>,
keystore: Arc<dyn Keystore>,
// Used for testing
time_override: Option<u64>,
metrics: PrometheusMetrics,
Expand Down Expand Up @@ -477,6 +479,7 @@ impl Store {
path: &std::path::Path,
options: Options,
client: Arc<Client>,
keystore: Arc<dyn Keystore>,
prometheus: Option<&PrometheusRegistry>,
task_spawner: &dyn SpawnNamed,
) -> Result<Arc<Store>>
Expand All @@ -491,7 +494,7 @@ impl Store {
+ 'static,
Client::Api: ValidateStatement<Block>,
{
let store = Arc::new(Self::new(path, options, client.clone(), prometheus)?);
let store = Arc::new(Self::new(path, options, client.clone(), keystore, prometheus)?);
client.execution_extensions().register_statement_store(store.clone());

// Perform periodic statement store maintenance
Expand All @@ -517,6 +520,7 @@ impl Store {
path: &std::path::Path,
options: Options,
client: Arc<Client>,
keystore: Arc<dyn Keystore>,
prometheus: Option<&PrometheusRegistry>,
) -> Result<Store>
where
Expand Down Expand Up @@ -565,6 +569,7 @@ impl Store {
db,
index: RwLock::new(Index::new(options)),
validate_fn,
keystore,
time_override: None,
metrics: PrometheusMetrics::new(prometheus),
};
Expand Down Expand Up @@ -762,7 +767,35 @@ impl StatementStore for Store {
/// Return the decrypted data of all known statements whose decryption key is identified as
/// `dest`. The key must be available to the client.
fn posted_clear(&self, match_all_topics: &[Topic], dest: [u8; 32]) -> Result<Vec<Vec<u8>>> {
self.collect_statements(Some(dest), match_all_topics, |statement| statement.into_data())
self.collect_statements(Some(dest), match_all_topics, |statement| {
if let (Some(key), Some(_)) = (statement.decryption_key(), statement.data()) {
let public = UncheckedFrom::unchecked_from(key);
let mut out = None;
if let Err(e) = self.keystore.with_ed25519_key(
sp_core::crypto::key_types::STATEMENT,
&public,
&mut |pair| match statement.decrypt_private(pair) {
Ok(r) => out = r,
Err(e) => log::debug!(
target: LOG_TARGET,
"Decryption error: {:?}, for statement {:?}",
e,
HexDisplay::from(&statement.hash())
),
},
) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can have with_ed25519_key use a function that return an error as param, and potentially have posted clear report this in its list for results.

log::debug!(
target: LOG_TARGET,
"Keystore error error: {:?}, for statement {:?}",
e,
HexDisplay::from(&statement.hash())
)
}
out
} else {
None
}
})
}

/// Submit a statement to the store. Validates the statement and returns validation result.
Expand Down Expand Up @@ -910,6 +943,7 @@ mod tests {
RuntimeApi { _inner: self.clone() }.into()
}
}

sp_api::mock_impl_runtime_apis! {
impl ValidateStatement<Block> for RuntimeApi {
fn validate_statement(
Expand Down Expand Up @@ -977,7 +1011,8 @@ mod tests {
let client = std::sync::Arc::new(TestClient);
let mut path: std::path::PathBuf = temp_dir.path().into();
path.push("db");
let store = Store::new(&path, Default::default(), client, None).unwrap();
let keystore = std::sync::Arc::new(sp_keystore::testing::MemoryKeystore::new());
let store = Store::new(&path, Default::default(), client, keystore, None).unwrap();
(store, temp_dir) // return order is important. Store must be dropped before TempDir
}

Expand Down Expand Up @@ -1080,12 +1115,13 @@ mod tests {
assert_eq!(store.statements().unwrap().len(), 3);
assert_eq!(store.broadcasts(&[]).unwrap().len(), 3);
assert_eq!(store.statement(&statement1.hash()).unwrap(), Some(statement1.clone()));
let keystore = store.keystore.clone();
drop(store);

let client = std::sync::Arc::new(TestClient);
let mut path: std::path::PathBuf = temp.path().into();
path.push("db");
let store = Store::new(&path, Default::default(), client, None).unwrap();
let store = Store::new(&path, Default::default(), client, keystore, None).unwrap();
assert_eq!(store.statements().unwrap().len(), 3);
assert_eq!(store.broadcasts(&[]).unwrap().len(), 3);
assert_eq!(store.statement(&statement1.hash()).unwrap(), Some(statement1));
Expand Down Expand Up @@ -1190,7 +1226,6 @@ mod tests {
statement(2, 4, None, 1000).hash(),
statement(3, 4, Some(3), 300).hash(),
statement(3, 5, None, 500).hash(),
//statement(4, 6, None, 100).hash(),
];
expected_statements.sort();
let mut statements: Vec<_> =
Expand All @@ -1214,13 +1249,31 @@ mod tests {
store.set_time(DEFAULT_PURGE_AFTER_SEC + 1);
store.maintain();
assert_eq!(store.index.read().expired.len(), 0);
let keystore = store.keystore.clone();
drop(store);

let client = std::sync::Arc::new(TestClient);
let mut path: std::path::PathBuf = temp.path().into();
path.push("db");
let store = Store::new(&path, Default::default(), client, None).unwrap();
let store = Store::new(&path, Default::default(), client, keystore, None).unwrap();
assert_eq!(store.statements().unwrap().len(), 0);
assert_eq!(store.index.read().expired.len(), 0);
}

#[test]
fn posted_clear_decrypts() {
let (store, _temp) = test_store();
let public = store
.keystore
.ed25519_generate_new(sp_core::crypto::key_types::STATEMENT, None)
.unwrap();
let statement1 = statement(1, 1, None, 100);
let mut statement2 = statement(1, 2, None, 0);
let plain = b"The most valuable secret".to_vec();
statement2.encrypt(&plain, &public).unwrap();
store.submit(statement1, StatementSource::Network);
store.submit(statement2, StatementSource::Network);
let posted_clear = store.posted_clear(&[], public.into()).unwrap();
assert_eq!(posted_clear, vec![plain]);
}
}
1 change: 0 additions & 1 deletion primitives/core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ sp-runtime-interface = { version = "17.0.0", default-features = false, path = ".
w3f-bls = { version = "0.1.3", default-features = false, optional = true}

[dev-dependencies]
rand = "0.8.5"
criterion = "0.4.0"
serde_json = "1.0"
sp-core-hashing-proc-macro = { version = "9.0.0", path = "./hashing/proc-macro" }
Expand Down
11 changes: 11 additions & 0 deletions primitives/keystore/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,17 @@ pub trait Keystore: Send + Sync {
};
Ok(signature)
}

/// Call a custom function with an ed25519 private key that matches given public key.
///
/// Returns `Ok(true)` if the key is found and `f` was called. `Ok(false)` if the key is missing
/// and an error when something has failed.
fn with_ed25519_key(
&self,
key_type: KeyTypeId,
public: &ed25519::Public,
f: &mut dyn FnMut(&ed25519::Pair),
) -> std::result::Result<bool, Error>;
}

/// A shared pointer to a keystore implementation.
Expand Down
14 changes: 14 additions & 0 deletions primitives/keystore/src/testing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,20 @@ impl Keystore for MemoryKeystore {
.iter()
.all(|(k, t)| self.keys.read().get(t).and_then(|s| s.get(k)).is_some())
}

fn with_ed25519_key(
&self,
key_type: KeyTypeId,
public: &ed25519::Public,
f: &mut dyn FnMut(&ed25519::Pair),
) -> std::result::Result<bool, Error> {
Ok(if let Some(pair) = self.pair::<ed25519::Pair>(key_type, public) {
f(&pair);
true
} else {
false
})
}
}

impl Into<KeystorePtr> for MemoryKeystore {
Expand Down
17 changes: 17 additions & 0 deletions primitives/statement-store/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,15 @@ sp-externalities = { version = "0.19.0", default-features = false, path = "../ex
thiserror = { version = "1.0", optional = true }
log = { version = "0.4.17", optional = true }

# ECIES dependencies
ed25519-dalek = { version = "1.0", optional = true }
x25519-dalek = { version = "2.0.0-pre.1", optional = true }
curve25519-dalek = { version = "3.2", optional = true }
aes-gcm = { version = "0.10", optional = true }
hkdf = { version = "0.12.0", optional = true }
sha2 = { version = "0.10.0", optional = true }
rand = { version = "0.8.5", features = ["small_rng"], optional = true }

[features]
default = ["std"]
std = [
Expand All @@ -38,6 +47,14 @@ std = [
"sp-application-crypto/std",
"thiserror",
"log",

"ed25519-dalek",
"x25519-dalek",
"curve25519-dalek",
"aes-gcm",
"hkdf",
"sha2",
"rand",
]
serde = [
"scale-info/serde",
Expand Down
Loading