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

SecretStore: ignore removed authorities when running auto-migration #7674

Merged
merged 1 commit into from
Feb 5, 2018
Merged

SecretStore: ignore removed authorities when running auto-migration #7674

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
27 changes: 11 additions & 16 deletions secret_store/src/key_server_cluster/connection_trigger.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,8 @@ pub struct SimpleServersSetChangeSessionCreatorConnector {
pub enum ConnectionsAction {
/// Connect to nodes from old set only.
ConnectToCurrentSet,
/// Connect to nodes from both old and migration sets.
ConnectToCurrentAndMigrationSet,
/// Connect to nodes from migration set.
ConnectToMigrationSet,
}

/// Trigger connections.
Expand Down Expand Up @@ -151,14 +151,9 @@ impl TriggerConnections {
ConnectionsAction::ConnectToCurrentSet => {
adjust_connections(self.self_key_pair.public(), data, &server_set.current_set);
},
ConnectionsAction::ConnectToCurrentAndMigrationSet => {
let mut old_and_migration_set = BTreeMap::new();
if let Some(migration) = server_set.migration.as_ref() {
old_and_migration_set.extend(migration.set.iter().map(|(node_id, node_addr)| (node_id.clone(), node_addr.clone())));
}
old_and_migration_set.extend(server_set.current_set.iter().map(|(node_id, node_addr)| (node_id.clone(), node_addr.clone())));

adjust_connections(self.self_key_pair.public(), data, &old_and_migration_set);
ConnectionsAction::ConnectToMigrationSet => {
let migration_set = server_set.migration.as_ref().map(|s| s.set.clone()).unwrap_or_default();
adjust_connections(self.self_key_pair.public(), data, &migration_set);
},
}
}
Expand Down Expand Up @@ -335,25 +330,25 @@ mod tests {
}

#[test]
fn maintain_connects_to_current_and_migration_set_works() {
fn maintain_connects_to_migration_set_works() {
let connections = create_connections();
let self_node_id = connections.self_key_pair.public().clone();
let current_node_id = Random.generate().unwrap().public().clone();
let migration_node_id = Random.generate().unwrap().public().clone();
let new_node_id = Random.generate().unwrap().public().clone();

let mut connections_data: ClusterConnectionsData = Default::default();
connections.maintain(ConnectionsAction::ConnectToCurrentAndMigrationSet, &mut connections_data, &KeyServerSetSnapshot {
current_set: vec![(self_node_id.clone(), "127.0.0.1:8081".parse().unwrap()),
(current_node_id.clone(), "127.0.0.1:8082".parse().unwrap())].into_iter().collect(),
connections.maintain(ConnectionsAction::ConnectToMigrationSet, &mut connections_data, &KeyServerSetSnapshot {
current_set: vec![(current_node_id.clone(), "127.0.0.1:8082".parse().unwrap())].into_iter().collect(),
new_set: vec![(new_node_id.clone(), "127.0.0.1:8083".parse().unwrap())].into_iter().collect(),
migration: Some(KeyServerSetMigration {
set: vec![(migration_node_id.clone(), "127.0.0.1:8084".parse().unwrap())].into_iter().collect(),
set: vec![(self_node_id.clone(), "127.0.0.1:8081".parse().unwrap()),
(migration_node_id.clone(), "127.0.0.1:8084".parse().unwrap())].into_iter().collect(),
..Default::default()
}),
});

assert_eq!(vec![current_node_id, migration_node_id].into_iter().collect::<BTreeSet<_>>(),
assert_eq!(vec![migration_node_id].into_iter().collect::<BTreeSet<_>>(),
connections_data.nodes.keys().cloned().collect::<BTreeSet<_>>());
}

Expand Down
43 changes: 9 additions & 34 deletions secret_store/src/key_server_cluster/connection_trigger_with_migration.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -260,12 +260,11 @@ impl TriggerSession {
let migration = server_set.migration.as_ref()
.expect("action is Start only when migration is started (see maintain_session); qed");

let old_set: BTreeSet<_> = server_set.current_set.keys()
.chain(migration.set.keys())
.cloned().collect();
let new_set: BTreeSet<_> = migration.set.keys()
.cloned()
.collect();
// we assume that authorities that are removed from the servers set are either offline, or malicious
// => they're not involved in ServersSetChangeSession
// => both sets are the same
let old_set: BTreeSet<_> = migration.set.keys().cloned().collect();
let new_set = old_set.clone();

let signatures = self.self_key_pair.sign(&ordered_nodes_hash(&old_set))
.and_then(|old_set_signature| self.self_key_pair.sign(&ordered_nodes_hash(&new_set))
Expand Down Expand Up @@ -336,8 +335,7 @@ fn maintain_session(self_node_id: &NodeId, connected: &BTreeSet<NodeId>, snapsho
},
// migration is active && there's no active session => start it
(MigrationState::Started, SessionState::Idle) => {
match is_connected_to_all_nodes(self_node_id, &snapshot.current_set, connected) &&
is_connected_to_all_nodes(self_node_id, &snapshot.migration.as_ref().expect(migration_data_proof).set, connected) &&
match is_connected_to_all_nodes(self_node_id, &snapshot.migration.as_ref().expect(migration_data_proof).set, connected) &&
select_master_node(snapshot) == self_node_id {
true => Some(SessionAction::Start),
// we are not connected to all required nodes yet or we are not on master node => wait for it
Expand Down Expand Up @@ -406,7 +404,7 @@ fn maintain_connections(migration_state: MigrationState, session_state: SessionS
// but it participates in new key generation session
// it is ok, since 'officialy' here means that this node is a owner of all old shares
(MigrationState::Required, _) |
(MigrationState::Started, _) => Some(ConnectionsAction::ConnectToCurrentAndMigrationSet),
(MigrationState::Started, _) => Some(ConnectionsAction::ConnectToMigrationSet),
}
}

Expand All @@ -430,15 +428,6 @@ fn select_master_node(snapshot: &KeyServerSetSnapshot) -> &NodeId {
when Started: migration.is_some() && we return migration.master; qed;\
when Required: current_set != new_set; this means that at least one set is non-empty; we try to take node from each set; qed"))
}
/*server_set_state.migration.as_ref()
.map(|m| &m.master)
.unwrap_or_else(|| server_set_state.current_set.keys()
.filter(|n| server_set_state.new_set.contains_key(n))
.nth(0)
.or_else(|| server_set_state.new_set.keys().nth(0)))
.expect("select_master_node is only called when migration is Required or Started;"
"when Started: migration.is_some() && we have migration.master; qed"
"when Required: current_set != migration_set; this means that at least one set is non-empty; we select")*/
}

#[cfg(test)]
Expand Down Expand Up @@ -558,13 +547,13 @@ mod tests {
#[test]
fn maintain_connections_connects_to_current_and_old_set_when_migration_is_required() {
assert_eq!(maintain_connections(MigrationState::Required,
SessionState::Idle), Some(ConnectionsAction::ConnectToCurrentAndMigrationSet));
SessionState::Idle), Some(ConnectionsAction::ConnectToMigrationSet));
}

#[test]
fn maintain_connections_connects_to_current_and_old_set_when_migration_is_started() {
assert_eq!(maintain_connections(MigrationState::Started,
SessionState::Idle), Some(ConnectionsAction::ConnectToCurrentAndMigrationSet));
SessionState::Idle), Some(ConnectionsAction::ConnectToMigrationSet));
}

#[test]
Expand Down Expand Up @@ -597,20 +586,6 @@ mod tests {
}, MigrationState::Started, SessionState::Idle), None);
}

#[test]
fn maintain_session_does_nothing_when_migration_started_on_master_node_and_no_session_and_not_connected_to_current_nodes() {
assert_eq!(maintain_session(&1.into(), &Default::default(), &KeyServerSetSnapshot {
current_set: vec![(1.into(), "127.0.0.1:8181".parse().unwrap()),
(2.into(), "127.0.0.1:8181".parse().unwrap())].into_iter().collect(),
new_set: Default::default(),
migration: Some(KeyServerSetMigration {
master: 1.into(),
set: vec![(1.into(), "127.0.0.1:8181".parse().unwrap())].into_iter().collect(),
..Default::default()
}),
}, MigrationState::Started, SessionState::Idle), None);
}

#[test]
fn maintain_session_does_nothing_when_migration_started_on_master_node_and_no_session_and_not_connected_to_migration_nodes() {
assert_eq!(maintain_session(&1.into(), &Default::default(), &KeyServerSetSnapshot {
Expand Down