feat(keycache): adds a key cache and removes exposure of secret key

examples/stress.rs

@@ -441,7 +441,9 @@ impl Network {
 
         // The message dst is unique so we use it also as its indentifier.
         let bytes = bincode::serialize(&dst)?;
-        let signature_share = node.sign_with_secret_key_share(&bytes).await?;
+        let signature_share = node
+            .sign_as_elder(&bytes, &public_key_set.public_key())
+            .await?;
 
         let index = node.our_index().await?;
 

src/routing/approved.rs

@@ -45,6 +45,7 @@ use xor_name::{Prefix, XorName};
 
 pub(crate) const RESOURCE_PROOF_DATA_SIZE: usize = 64;
 pub(crate) const RESOURCE_PROOF_DIFFICULTY: u8 = 2;
+const KEY_CACHE_SIZE: u8 = 5;
 
 // The approved stage - node is a full member of a section and is performing its duties according
 // to its persona (adult or elder).
@@ -78,7 +79,7 @@ impl Approved {
         section_key_share: Option<SectionKeyShare>,
         event_tx: mpsc::UnboundedSender<Event>,
     ) -> Self {
-        let section_keys_provider = SectionKeysProvider::new(section_key_share);
+        let section_keys_provider = SectionKeysProvider::new(KEY_CACHE_SIZE, section_key_share);
 
         Self {
             node,
@@ -108,6 +109,35 @@ impl Approved {
         &self.network
     }
 
+    /// Is this node an elder?
+    pub fn is_elder(&self) -> bool {
+        self.section.is_elder(&self.node.name())
+    }
+
+    /// Tries to sign with the secret corresponding to the provided BLS public key
+    pub fn sign_with_section_key_share(
+        &self,
+        data: &[u8],
+        public_key: &bls::PublicKey,
+    ) -> Result<bls::SignatureShare> {
+        self.section_keys_provider.sign_with(data, public_key)
+    }
+
+    /// Returns the current BLS public key set
+    pub fn public_key_set(&self) -> Result<bls::PublicKeySet> {
+        Ok(self
+            .section_keys_provider
+            .key_share()?
+            .public_key_set
+            .clone())
+    }
+
+    /// Returns our index in the current BLS group if this node is a member of one, or
+    /// `Error::MissingSecretKeyShare` otherwise.
+    pub fn our_index(&self) -> Result<usize> {
+        Ok(self.section_keys_provider.key_share()?.index.clone())
+    }
+
     pub fn send_event(&self, event: Event) {
         // Note: cloning the sender to avoid mutable access. Should have negligible cost.
         if self.event_tx.clone().send(event).is_err() {
@@ -385,16 +415,6 @@ impl Approved {
         }
     }
 
-    /// Is this node an elder?
-    pub fn is_elder(&self) -> bool {
-        self.section.is_elder(&self.node.name())
-    }
-
-    /// Returns the current BLS public key set
-    pub fn section_key_share(&self) -> Option<&SectionKeyShare> {
-        self.section_keys_provider.key_share().ok()
-    }
-
     ////////////////////////////////////////////////////////////////////////////
     // Message handling
     ////////////////////////////////////////////////////////////////////////////
@@ -1541,7 +1561,7 @@ impl Approved {
                 info!("Demoted");
                 self.section = self.section.trimmed(1);
                 self.network = Network::new();
-                self.section_keys_provider = SectionKeysProvider::new(None);
+                self.section_keys_provider = SectionKeysProvider::new(KEY_CACHE_SIZE, None);
                 NodeElderChange::Demoted
             } else {
                 NodeElderChange::None

src/routing/mod.rs

@@ -27,7 +27,7 @@ use self::{
 };
 use crate::{
     crypto,
-    error::{Error, Result},
+    error::Result,
     event::{Event, NodeElderChange},
     location::{DstLocation, SrcLocation},
     messages::{Message, PING},
@@ -157,11 +157,25 @@ impl Routing {
         self.stage.state.lock().await.node().keypair.public
     }
 
-    /// Signs any data with the ed25519 key of this node.
-    pub async fn sign(&self, data: &[u8]) -> Signature {
+    /// Signs `data` with the ed25519 key of this node.
+    pub async fn sign_as_node(&self, data: &[u8]) -> Signature {
         self.stage.state.lock().await.node().keypair.sign(data)
     }
 
+    /// Signs `data` with the BLS secret key share of this node, if it has any. Returns
+    /// `Error::MissingSecretKeyShare` otherwise.
+    pub async fn sign_as_elder(
+        &self,
+        data: &[u8],
+        public_key: &bls::PublicKey,
+    ) -> Result<bls::SignatureShare> {
+        self.stage
+            .state
+            .lock()
+            .await
+            .sign_with_section_key_share(data, public_key)
+    }
+
     /// Verifies `signature` on `data` with the ed25519 public key of this node.
     pub async fn verify(&self, data: &[u8], signature: &Signature) -> bool {
         self.stage
@@ -311,37 +325,7 @@ impl Routing {
     /// Returns the current BLS public key set if this node has one, or
     /// `Error::InvalidState` otherwise.
     pub async fn public_key_set(&self) -> Result<bls::PublicKeySet> {
-        self.stage
-            .state
-            .lock()
-            .await
-            .section_key_share()
-            .map(|share| share.public_key_set.clone())
-            .ok_or(Error::InvalidState)
-    }
-
-    /// Returns the current BLS secret key share or `Error::InvalidState` if we are not
-    /// elder.
-    pub async fn secret_key_share(&self) -> Result<bls::SecretKeyShare> {
-        self.stage
-            .state
-            .lock()
-            .await
-            .section_key_share()
-            .map(|share| share.secret_key_share.clone())
-            .ok_or(Error::MissingSecretKeyShare)
-    }
-
-    /// Signs `data` with the BLS secret key share of this node, if it has any. Returns
-    /// `Error::MissingSecretKeyShare` otherwise.
-    pub async fn sign_with_secret_key_share(&self, data: &[u8]) -> Result<bls::SignatureShare> {
-        self.stage
-            .state
-            .lock()
-            .await
-            .section_key_share()
-            .map(|share| share.secret_key_share.sign(data))
-            .ok_or(Error::MissingSecretKeyShare)
+        self.stage.state.lock().await.public_key_set()
     }
 
     /// Returns our section proof chain.
@@ -352,13 +336,7 @@ impl Routing {
     /// Returns our index in the current BLS group if this node is a member of one, or
     /// `Error::MissingSecretKeyShare` otherwise.
     pub async fn our_index(&self) -> Result<usize> {
-        self.stage
-            .state
-            .lock()
-            .await
-            .section_key_share()
-            .map(|share| share.index)
-            .ok_or(Error::MissingSecretKeyShare)
+        self.stage.state.lock().await.our_index()
     }
 }
 

src/section/section_keys.rs

@@ -1,4 +1,4 @@
-// Copyright 2020 MaidSafe.net limited.
+// Copyright 2021 MaidSafe.net limited.
 //
 // This SAFE Network Software is licensed to you under The General Public License (GPL), version 3.
 // Unless required by applicable law or agreed to in writing, the SAFE Network Software distributed
@@ -7,6 +7,7 @@
 // permissions and limitations relating to use of the SAFE Network Software.
 
 use crate::error::{Error, Result};
+use std::collections::{BTreeMap, VecDeque};
 
 /// All the key material needed to sign or combine signature for our section key.
 #[derive(Debug)]
@@ -22,26 +23,40 @@ pub struct SectionKeyShare {
 /// Struct that holds the current section keys and helps with new key generation.
 #[derive(Debug)]
 pub struct SectionKeysProvider {
-    /// Our current section BLS keys.
-    current: Option<SectionKeyShare>,
+    /// A cache for current and previous section BLS keys.
+    cache: KeyCache,
     /// The new keys to use when section update completes.
     pending: Option<SectionKeyShare>,
 }
 
 impl SectionKeysProvider {
-    pub fn new(current: Option<SectionKeyShare>) -> Self {
-        Self {
-            current,
+    pub fn new(cache_size: u8, current: Option<SectionKeyShare>) -> Self {
+        let mut provider = Self {
             pending: None,
+            cache: KeyCache::with_capacity(cache_size as usize),
+        };
+        if let Some(share) = current {
+            let public_key = share.public_key_set.public_key();
+            provider.insert_dkg_outcome(share);
+            provider.finalise_dkg(&public_key);
         }
+        provider
     }
 
     pub fn key_share(&self) -> Result<&SectionKeyShare> {
-        self.current.as_ref().ok_or(Error::MissingSecretKeyShare)
+        self.cache.get_most_recent()
+    }
+
+    pub fn sign_with(
+        &self,
+        data: &[u8],
+        public_key: &bls::PublicKey,
+    ) -> Result<bls::SignatureShare> {
+        self.cache.sign_with(data, public_key)
     }
 
     pub fn has_key_share(&self) -> bool {
-        self.current.is_some()
+        self.cache.has_key_share()
     }
 
     pub fn insert_dkg_outcome(&mut self, share: SectionKeyShare) {
@@ -50,12 +65,92 @@ impl SectionKeysProvider {
 
     pub fn finalise_dkg(&mut self, public_key: &bls::PublicKey) {
         if let Some(share) = &self.pending {
-            let pending_public_key = share.public_key_set.public_key();
+            if *public_key != share.public_key_set.public_key() {
+                return;
+            }
+        }
+        if let Some(share) = self.pending.take() {
+            if let Some(evicted) = self.cache.add(public_key, share) {
+                trace!("evicted old key from cache: {:?}", evicted);
+            }
+            trace!("finalised DKG: {:?}", public_key);
+        }
+    }
+}
+
+/// Implementation of simple cache.
+#[derive(Debug)]
+pub struct KeyCache {
+    map: BTreeMap<Vec<u8>, SectionKeyShare>,
+    list: VecDeque<bls::PublicKey>,
+    capacity: usize,
+}
+
+impl KeyCache {
+    /// Constructor for capacity based `KeyCache`.
+    pub fn with_capacity(capacity: usize) -> KeyCache {
+        KeyCache {
+            map: BTreeMap::new(),
+            list: VecDeque::with_capacity(capacity),
+            capacity,
+        }
+    }
 
-            if pending_public_key == *public_key {
-                trace!("finalise DKG: {:?}", pending_public_key);
-                self.current = self.pending.take();
+    ///
+    pub fn has_key_share(&self) -> bool {
+        !self.list.is_empty()
+    }
+
+    ///
+    pub fn get_most_recent(&self) -> Result<&SectionKeyShare> {
+        if !self.list.is_empty() {
+            if let Some(public_key) = self.list.get(self.list.len() - 1) {
+                if let Some(share) = self.map.get(&bincode::serialize(public_key)?) {
+                    return Ok(share);
+                }
+            }
+        }
+        Err(Error::MissingSecretKeyShare)
+    }
+
+    /// Uses the secret key from cache, corresponding to
+    /// the provided public key.
+    pub fn sign_with(
+        &self,
+        data: &[u8],
+        public_key: &bls::PublicKey,
+    ) -> Result<bls::SignatureShare> {
+        let key = bincode::serialize(public_key)?;
+        if let Some(section_key) = self.map.get(&key) {
+            Ok(section_key.secret_key_share.sign(data))
+        } else {
+            Err(Error::MissingSecretKeyShare)
+        }
+    }
+
+    /// Adds a new key to the cache, and removes + returns the oldest
+    /// key if cache size is exceeded.
+    pub fn add(
+        &mut self,
+        public_key: &bls::PublicKey,
+        section_key_share: SectionKeyShare,
+    ) -> Option<bls::PublicKey> {
+        let key = bincode::serialize(public_key).ok()?;
+        if self.map.contains_key(&key) {
+            return None;
+        }
+
+        let mut evicted = None;
+        if self.list.capacity() == self.list.len() {
+            evicted = self.list.pop_front();
+            if let Some(public_key) = evicted {
+                let _ = self.map.remove(&bincode::serialize(&public_key).ok()?);
             }
         }
+
+        self.list.push_back(*public_key);
+        let _ = self.map.insert(key, section_key_share);
+
+        evicted
     }
 }