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

SecretStore: add node to existing session poc + discussion #6480

Merged
merged 2 commits into from
Oct 2, 2017

Conversation

svyatonik
Copy link
Collaborator

The purpose of this PR is mostly to discuss possible SecretStore reactions to KeyServer addition/removal.
Code changes are proof-of-concepts of shares refreshment and adding new nodes to session, without changing shared secret.

case1: new node is added + session t-of-N exists

We do not risk to loose any secrets in this case, but might want to convert existing sessions: from t-of-N to t-of-N+1.
See full_generation_math_session_with_adding_new_node test for implementation details.

My thoughts (labeled to refer later/in comments):
case1-1) no automatical actions should be performed when new node is added to the set
case1-2) there must be a way to extend existing session with new node (i.e. convert from t-of-N to t-of-N+1)

case2: existing node is removed + session t-of-N exists + number of nodes left is >= t

There's no immediate risk of losing any secrets. Existing session becames t-of-N-1.

My thoughts:
case2-1) no automatical actions should be performed in this case
case2-2) there's 3 reasons (at least those, I can see) of why node is removed:
case2-2-1) node is turned off for maintenance => it will be added to the set later
case2-2-2) node is replaced by other node => it is better to run (case1) before (case2) in this case (i.e. convert session to t+N+1 first)
case2-2-3) node is removed permanently (due to failure or smth similar) => SS administrator must add new (empty) server asap && extend existing sessions back to t-of-N
case2-3) to have actual information in db (not required, but desirable), there must be a way to 'shrink' existing session from t-of-N to t-of-N-1 (i.e. mark that removed KS do not hold secret share anymore)

case3: existing node is removed + session t-of-N exists + number of nodes left is < t

There's no way to recover secret after this happens => this situation must be avoided with all means.

My thoughts:
case3-1) no automatical actions can be performed in this case
case3-2) except for case-2-2-3, all possible reasons can be avoided by administrative means
case3-3) case3 when node is removed permanently (case-2-2-3) could only occur when N-t was not enough to react properly

Administrative API

So there are 2 additional APIs for SS:

  1. extend session with new node
  2. remove node from existing session
    I would like to make these APIs atomic (i.e. 'add single node to single session' or 'remove single node from single session') => there must be also API to list sessions
    Another way is to add APIs like: 'add node(s) to all existing sessions' or 'remove node(s) from all existing sessions'.
    But these are too complex (both for implemetation and for resolving issues).

There's also bonus API, which could be added (see full_generation_math_session_with_refreshing_shares): refresh secret shares.
This could be used externally, or triggered from within the SS.
It is to ensure that even if some KeyServer gains t different shares within T interval and there was secret share refreshment within T, it won't be able to recover secret.

I would like to extend existing KS HTTP interface with these methods.

Bad ideas

bad-1) automatical reactions - will make SS protocols too complex && also gives false confidence that SS can itself solve all problems (it can not if nodes are keep failing or if t/N~1)
bad-2) check if t-of-t sessions exists before removing node and transfer secret share from removing-node to staying-node - t-of-t situations must be avoided, not 'resolved'

@svyatonik svyatonik added A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust. labels Sep 7, 2017
@svyatonik
Copy link
Collaborator Author

Discussed in private: we need the procedure to migrate from KeyServerSet1 to KeyServerSet2. These sets could intersect, or be completely different. So what's described above is just an edge case for this procedure when single server is added/removed to the set.
In addition to basic scenarios described above, additional scenario implies from migration case - when we're moving all secret shares from KeyServer1 to KeyServer2. This also must be implemented.
Finally, there will be some agent, who will coordinate the whole migration procedure. For now it should be one of KeyServers.

Regarding this PR: case1 is a part of bigger migration procedure => this PR is still actual. Changed case1 test to support multiple nodes addition in single call.

@arkpar arkpar added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Oct 2, 2017
@arkpar arkpar merged commit 3a60d72 into master Oct 2, 2017
@arkpar arkpar deleted the secretstore_addnode_poc branch October 2, 2017 09:07
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well. M4-core ⛓ Core client code / Rust.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants