SecretStore: return error 404 when there's no key shares for given key on all nodes

This commit is contained in:
Svyatoslav Nikolsky 2017-12-19 11:02:13 +03:00
parent 4971171e8a
commit 861aa1fa4b
3 changed files with 31 additions and 10 deletions

View File

@ -227,7 +227,7 @@ impl<T> SessionImpl<T> where T: SessionTransport {
// try to complete session // try to complete session
Self::try_complete(&self.core, &mut *data); Self::try_complete(&self.core, &mut *data);
if no_confirmations_required && data.state != SessionState::Finished { if no_confirmations_required && data.state != SessionState::Finished {
return Err(Error::ConsensusUnreachable); return Err(Error::MissingKeyShare);
} else if data.state == SessionState::Finished { } else if data.state == SessionState::Finished {
return Ok(()); return Ok(());
} }
@ -454,6 +454,8 @@ impl FastestResultComputer {
impl SessionResultComputer for FastestResultComputer { impl SessionResultComputer for FastestResultComputer {
fn compute_result(&self, threshold: Option<usize>, confirmations: &BTreeSet<NodeId>, versions: &BTreeMap<H256, BTreeSet<NodeId>>) -> Option<Result<(H256, NodeId), Error>> { fn compute_result(&self, threshold: Option<usize>, confirmations: &BTreeSet<NodeId>, versions: &BTreeMap<H256, BTreeSet<NodeId>>) -> Option<Result<(H256, NodeId), Error>> {
match self.threshold.or(threshold) { match self.threshold.or(threshold) {
// if there's no versions at all && we're not waiting for confirmations anymore
_ if confirmations.is_empty() && versions.is_empty() => Some(Err(Error::MissingKeyShare)),
// if we have key share on this node // if we have key share on this node
Some(threshold) => { Some(threshold) => {
// select version this node have, with enough participants // select version this node have, with enough participants
@ -489,6 +491,9 @@ impl SessionResultComputer for LargestSupportResultComputer {
if !confirmations.is_empty() { if !confirmations.is_empty() {
return None; return None;
} }
if versions.is_empty() {
return Some(Err(Error::MissingKeyShare));
}
versions.iter() versions.iter()
.max_by_key(|&(_, ref n)| n.len()) .max_by_key(|&(_, ref n)| n.len())
@ -507,7 +512,8 @@ mod tests {
use key_server_cluster::cluster::tests::DummyCluster; use key_server_cluster::cluster::tests::DummyCluster;
use key_server_cluster::admin_sessions::ShareChangeSessionMeta; use key_server_cluster::admin_sessions::ShareChangeSessionMeta;
use key_server_cluster::message::{Message, KeyVersionNegotiationMessage, RequestKeyVersions, KeyVersions}; use key_server_cluster::message::{Message, KeyVersionNegotiationMessage, RequestKeyVersions, KeyVersions};
use super::{SessionImpl, SessionTransport, SessionParams, FastestResultComputer, SessionState}; use super::{SessionImpl, SessionTransport, SessionParams, FastestResultComputer, LargestSupportResultComputer,
SessionResultComputer, SessionState};
struct DummyTransport { struct DummyTransport {
cluster: Arc<DummyCluster>, cluster: Arc<DummyCluster>,
@ -722,4 +728,19 @@ mod tests {
// we can't be sure that node has given key version because previous ShareAdd session could fail // we can't be sure that node has given key version because previous ShareAdd session could fail
assert!(ml.session(0).data.lock().state != SessionState::Finished); assert!(ml.session(0).data.lock().state != SessionState::Finished);
} }
#[test]
fn fastest_computer_returns_missing_share_if_no_versions_returned() {
let computer = FastestResultComputer {
self_node_id: Default::default(),
threshold: None,
};
assert_eq!(computer.compute_result(Some(10), &Default::default(), &Default::default()), Some(Err(Error::MissingKeyShare)));
}
#[test]
fn largest_computer_returns_missing_share_if_no_versions_returned() {
let computer = LargestSupportResultComputer;
assert_eq!(computer.compute_result(Some(10), &Default::default(), &Default::default()), Some(Err(Error::MissingKeyShare)));
}
} }

View File

@ -452,8 +452,8 @@ impl ClusterCore {
let is_master_node = meta.self_node_id == meta.master_node_id; let is_master_node = meta.self_node_id == meta.master_node_id;
if is_master_node && session.is_finished() { if is_master_node && session.is_finished() {
data.sessions.negotiation_sessions.remove(&session.id()); data.sessions.negotiation_sessions.remove(&session.id());
if let Ok((version, master)) = session.wait() { match session.wait() {
match session.continue_action() { Ok((version, master)) => match session.continue_action() {
Some(ContinueAction::Decrypt(session, is_shadow_decryption)) => { Some(ContinueAction::Decrypt(session, is_shadow_decryption)) => {
let initialization_error = if data.self_key_pair.public() == &master { let initialization_error = if data.self_key_pair.public() == &master {
session.initialize(version, is_shadow_decryption) session.initialize(version, is_shadow_decryption)
@ -479,19 +479,18 @@ impl ClusterCore {
} }
}, },
None => (), None => (),
} },
} else { Err(error) => match session.continue_action() {
match session.continue_action() {
Some(ContinueAction::Decrypt(session, _)) => { Some(ContinueAction::Decrypt(session, _)) => {
data.sessions.decryption_sessions.remove(&session.id()); data.sessions.decryption_sessions.remove(&session.id());
session.on_session_error(&meta.self_node_id, Error::ConsensusUnreachable); session.on_session_error(&meta.self_node_id, error);
}, },
Some(ContinueAction::Sign(session, _)) => { Some(ContinueAction::Sign(session, _)) => {
data.sessions.signing_sessions.remove(&session.id()); data.sessions.signing_sessions.remove(&session.id());
session.on_session_error(&meta.self_node_id, Error::ConsensusUnreachable); session.on_session_error(&meta.self_node_id, error);
}, },
None => (), None => (),
} },
} }
} }
} }

View File

@ -136,6 +136,7 @@ impl From<key_server_cluster::Error> for Error {
fn from(err: key_server_cluster::Error) -> Self { fn from(err: key_server_cluster::Error) -> Self {
match err { match err {
key_server_cluster::Error::AccessDenied => Error::AccessDenied, key_server_cluster::Error::AccessDenied => Error::AccessDenied,
key_server_cluster::Error::MissingKeyShare => Error::DocumentNotFound,
_ => Error::Internal(err.into()), _ => Error::Internal(err.into()),
} }
} }