From 09cdf362ad4aa575d55cf341f4c01c6d5ee58de4 Mon Sep 17 00:00:00 2001 From: Jun Kimura Date: Wed, 6 Dec 2023 00:16:36 +0900 Subject: [PATCH 1/2] keymanager: fix to use mutex to access connection Signed-off-by: Jun Kimura --- modules/keymanager/src/errors.rs | 10 +++++- modules/keymanager/src/lib.rs | 57 +++++++++++++++++++++++--------- 2 files changed, 51 insertions(+), 16 deletions(-) diff --git a/modules/keymanager/src/errors.rs b/modules/keymanager/src/errors.rs index 90f31bb4..0dc3c414 100644 --- a/modules/keymanager/src/errors.rs +++ b/modules/keymanager/src/errors.rs @@ -36,7 +36,15 @@ define_error! { Rusqlite [TraceError] - |_| { "rusqlite error" } + |_| { "rusqlite error" }, + + MutexLock + { + descr: String + } + |e| { + format_args!("mutex lock error: descr={}", e.descr) + } } } diff --git a/modules/keymanager/src/lib.rs b/modules/keymanager/src/lib.rs index 159ad7f1..020c345b 100644 --- a/modules/keymanager/src/lib.rs +++ b/modules/keymanager/src/lib.rs @@ -7,22 +7,23 @@ use lcp_types::{Mrenclave, Time}; use log::*; use rusqlite::{params, types::Type, Connection}; use serde::{Deserialize, Serialize}; +use std::sync::Mutex; use std::{ops::Deref, path::Path, time::Duration}; pub static KEY_MANAGER_DB: &str = "km.sqlite"; pub struct EnclaveKeyManager { - conn: Connection, + conn: Mutex, } impl EnclaveKeyManager { pub fn new(home_dir: &Path) -> Result { let km_db = home_dir.join(KEY_MANAGER_DB); let db_exists = km_db.exists(); - let conn = Connection::open(&km_db)?; + let conn = Mutex::new(Connection::open(&km_db)?); let this = Self { conn }; if !db_exists { - this.setup()?; + this.init_db()?; info!("initialized Key Manager: {:?}", km_db); } Ok(this) @@ -30,14 +31,18 @@ impl EnclaveKeyManager { #[cfg(test)] pub fn new_in_memory() -> Result { - let conn = Connection::open_in_memory()?; + let conn = Mutex::new(Connection::open_in_memory()?); let this = Self { conn }; - this.setup()?; + this.init_db()?; Ok(this) } - fn setup(&self) -> Result<(), Error> { - self.conn.execute_batch( + fn init_db(&self) -> Result<(), Error> { + let conn = self + .conn + .lock() + .map_err(|e| Error::mutex_lock(e.to_string()))?; + conn.execute_batch( r#" BEGIN; CREATE TABLE enclave_keys ( @@ -61,7 +66,11 @@ impl EnclaveKeyManager { /// Load a sealed enclave key by address pub fn load(&self, address: Address) -> Result { - let mut stmt = self.conn.prepare( + let conn = self + .conn + .lock() + .map_err(|e| Error::mutex_lock(e.to_string()))?; + let mut stmt = conn.prepare( "SELECT ek_sealed, mrenclave, avr, signature, signing_cert FROM enclave_keys WHERE ek_address = ?1", )?; let key_info = stmt.query_row(params![address.to_hex_string()], |row| { @@ -98,7 +107,11 @@ impl EnclaveKeyManager { sealed_ek: SealedEnclaveKey, mrenclave: Mrenclave, ) -> Result<(), Error> { - let mut stmt = self.conn.prepare( + let conn = self + .conn + .lock() + .map_err(|e| Error::mutex_lock(e.to_string()))?; + let mut stmt = conn.prepare( "INSERT INTO enclave_keys (ek_address, ek_sealed, mrenclave) VALUES (?1, ?2, ?3)", )?; let _ = stmt.execute(params![ @@ -115,9 +128,13 @@ impl EnclaveKeyManager { address: Address, avr: EndorsedAttestationVerificationReport, ) -> Result<(), Error> { + let conn = self + .conn + .lock() + .map_err(|e| Error::mutex_lock(e.to_string()))?; let attested_at = avr.get_avr()?.attestation_time()?; // update avr and attested_at and signature and sigining_cert - let mut stmt = self.conn.prepare( + let mut stmt = conn.prepare( "UPDATE enclave_keys SET avr = ?1, attested_at = ?2, signature = ?3, signing_cert = ?4 WHERE ek_address = ?5", )?; stmt.execute(params![ @@ -132,7 +149,11 @@ impl EnclaveKeyManager { /// Returns a list of available enclave keys pub fn available_keys(&self, mrenclave: Mrenclave) -> Result, Error> { - let mut stmt = self.conn.prepare( + let conn = self + .conn + .lock() + .map_err(|e| Error::mutex_lock(e.to_string()))?; + let mut stmt = conn.prepare( r#" SELECT ek_address, ek_sealed, mrenclave, avr, signature, signing_cert FROM enclave_keys @@ -164,7 +185,11 @@ impl EnclaveKeyManager { /// Returns a list of all enclave keys pub fn all_keys(&self) -> Result, Error> { - let mut stmt = self.conn.prepare( + let conn = self + .conn + .lock() + .map_err(|e| Error::mutex_lock(e.to_string()))?; + let mut stmt = conn.prepare( "SELECT ek_address, ek_sealed, mrenclave, avr, signature, signing_cert FROM enclave_keys ORDER BY updated_at DESC", )?; let key_infos = stmt @@ -200,10 +225,12 @@ impl EnclaveKeyManager { /// Prune keys after the expiration time(secs) from the attestation time. pub fn prune(&self, expiration_time: u64) -> Result { - let expired = (Time::now() - Duration::from_secs(expiration_time))?; - let mut stmt = self + let conn = self .conn - .prepare("DELETE FROM enclave_keys WHERE attested_at <= ?1")?; + .lock() + .map_err(|e| Error::mutex_lock(e.to_string()))?; + let expired = (Time::now() - Duration::from_secs(expiration_time))?; + let mut stmt = conn.prepare("DELETE FROM enclave_keys WHERE attested_at <= ?1")?; let count = stmt.execute(params![expired.as_unix_timestamp_secs()])?; Ok(count) } From 98326e3581d25a007587667d37c93afc16179839 Mon Sep 17 00:00:00 2001 From: Jun Kimura Date: Wed, 6 Dec 2023 00:19:33 +0900 Subject: [PATCH 2/2] service: remove explict Sync and Send markers from `AppService` Signed-off-by: Jun Kimura --- modules/enclave-api/src/enclave.rs | 8 ++-- modules/service/src/service.rs | 13 ------ modules/store/src/memory.rs | 63 ++++++++++++++++++++++++++++-- modules/store/src/transaction.rs | 2 +- 4 files changed, 64 insertions(+), 22 deletions(-) diff --git a/modules/enclave-api/src/enclave.rs b/modules/enclave-api/src/enclave.rs index 9d7c18c3..07cf73d6 100644 --- a/modules/enclave-api/src/enclave.rs +++ b/modules/enclave-api/src/enclave.rs @@ -9,7 +9,7 @@ use store::host::{HostStore, IntoCommitStore}; use store::transaction::{CommitStore, CreatedTx, UpdateKey}; /// `Enclave` keeps an enclave id and reference to the host environement -pub struct Enclave { +pub struct Enclave { pub(crate) path: PathBuf, pub(crate) key_manager: EnclaveKeyManager, pub(crate) store: Arc>, @@ -17,7 +17,7 @@ pub struct Enclave { _marker: PhantomData, } -impl Enclave { +impl Enclave { pub fn new( path: impl Into, key_manager: EnclaveKeyManager, @@ -49,7 +49,7 @@ impl Enclave { } /// `EnclaveInfo` is an accessor to enclave information -pub trait EnclaveInfo { +pub trait EnclaveInfo: Sync + Send { /// `get_eid` returns the enclave id fn get_eid(&self) -> sgx_enclave_id_t; /// `metadata` returns the metadata of the enclave @@ -58,7 +58,7 @@ pub trait EnclaveInfo { fn get_key_manager(&self) -> &EnclaveKeyManager; } -impl EnclaveInfo for Enclave { +impl EnclaveInfo for Enclave { /// `get_eid` returns the enclave id fn get_eid(&self) -> sgx_enclave_id_t { self.sgx_enclave.geteid() diff --git a/modules/service/src/service.rs b/modules/service/src/service.rs index 9bba0c1e..5ad80fd1 100644 --- a/modules/service/src/service.rs +++ b/modules/service/src/service.rs @@ -33,19 +33,6 @@ where } } -unsafe impl Send for AppService -where - S: CommitStore + 'static, - E: EnclaveProtoAPI + 'static, -{ -} -unsafe impl Sync for AppService -where - S: CommitStore + 'static, - E: EnclaveProtoAPI + 'static, -{ -} - impl AppService where S: CommitStore + 'static, diff --git a/modules/store/src/memory.rs b/modules/store/src/memory.rs index 82dcaeeb..f6aee602 100644 --- a/modules/store/src/memory.rs +++ b/modules/store/src/memory.rs @@ -3,17 +3,72 @@ use crate::store::TxId; use crate::transaction::{CommitStore, CreatedTx, Tx, TxAccessor}; use crate::{KVStore, Result}; use std::collections::HashMap; +use std::sync::Mutex; // MemStore is only available for testing purposes #[derive(Default, Debug)] -pub struct MemStore { +pub struct MemStore(Mutex); + +impl KVStore for MemStore { + fn get(&self, key: &[u8]) -> Option> { + self.0.lock().unwrap().get(key) + } + + fn set(&mut self, key: Vec, value: Vec) { + self.0.lock().unwrap().set(key, value) + } + + fn remove(&mut self, key: &[u8]) { + self.0.lock().unwrap().remove(key) + } +} + +impl TxAccessor for MemStore { + fn run_in_tx(&self, tx_id: TxId, f: impl FnOnce(&dyn KVStore) -> T) -> Result { + self.0.lock().unwrap().run_in_tx(tx_id, f) + } + + fn run_in_mut_tx( + &mut self, + tx_id: TxId, + f: impl FnOnce(&mut dyn KVStore) -> T, + ) -> Result { + self.0.lock().unwrap().run_in_mut_tx(tx_id, f) + } +} + +impl CommitStore for MemStore { + type Tx = MemTx; + + fn create_transaction( + &mut self, + _update_key: Option, + ) -> Result { + self.0.lock().unwrap().create_transaction(_update_key) + } + + fn begin(&mut self, tx: &::PreparedTx) -> Result<()> { + self.0.lock().unwrap().begin(tx) + } + + fn commit(&mut self, tx: ::PreparedTx) -> Result<()> { + self.0.lock().unwrap().commit(tx) + } + + fn rollback(&mut self, tx: ::PreparedTx) { + self.0.lock().unwrap().rollback(tx) + } +} + +#[derive(Default, Debug)] +pub struct InnerMemStore { running_tx_exists: bool, latest_tx_id: TxId, uncommitted_data: HashMap, Option>>, committed_data: HashMap, Vec>, } -impl KVStore for MemStore { +impl KVStore for InnerMemStore { fn get(&self, key: &[u8]) -> Option> { if self.running_tx_exists { match self.uncommitted_data.get(key) { @@ -42,7 +97,7 @@ impl KVStore for MemStore { } } -impl TxAccessor for MemStore { +impl TxAccessor for InnerMemStore { fn run_in_tx(&self, _tx_id: TxId, f: impl FnOnce(&dyn KVStore) -> T) -> Result { Ok(f(self)) } @@ -56,7 +111,7 @@ impl TxAccessor for MemStore { } } -impl CommitStore for MemStore { +impl CommitStore for InnerMemStore { type Tx = MemTx; fn create_transaction( diff --git a/modules/store/src/transaction.rs b/modules/store/src/transaction.rs index fd50cefb..e882d620 100644 --- a/modules/store/src/transaction.rs +++ b/modules/store/src/transaction.rs @@ -19,7 +19,7 @@ pub trait CreatedTx: Tx { } /// `CommitStore` is a store that supports transactions -pub trait CommitStore { +pub trait CommitStore: Sync + Send { type Tx: CreatedTx; /// `create_transaction` creates a transaction with a given `update_key`