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

SecretStore: versioned keys #6910

Merged
merged 43 commits into from
Nov 2, 2017
Merged

SecretStore: versioned keys #6910

merged 43 commits into from
Nov 2, 2017

Conversation

svyatonik
Copy link
Collaborator

@svyatonik svyatonik commented Oct 27, 2017

Changes:

  1. instead of rewriting share (this could lead to share loss as described in TODO3 from SecretStore: administrative sessions prototypes #6605 ), ShareAdd session would append new version to existing key
  2. before decryption/signing session can start, we must choose version we want to use in this session => new KeyVersionNegotiationSession
  3. ShareRemove && ShareMove are obsoleted && removed, since we do not want to lose and change any key share versions
  4. I have no plans to make an API for starting ShareAdd session right now, because this change have removed its main use case. So it will be used only as nested session in ServersSetChange session
  5. changed ShareAdd implementation so now it works even if some owners of key shares are offline/removed. Thought it would be an easy task, but spent several days working on it && had to switch the paper, this session is based on.
  6. ServersSetChange session now uses KeyVersionNegotiationSession + ShareAdd session to add shares to all new nodes && after completion just clears databases of nodes, that are removed from cluster

This PR fixes most of #6605 TODOs (except for 1 && 5.1).

Behind the scenes: initial idea (#6605 (comment)) was built on assumption that node is static, when is isolated from cluster. But this isn't true in general - we can make cluster1, then split it to cluster2 + cluster3 && then merge back to cluster4. So failed ShareAdd session is not the only reason of different key versions. And there's no easy way to protect key shares from tree-like versioning (had some exotic ideas of how to achieve this, though) . So let's just live with it (I felt depressed before accepting this fact).

@svyatonik svyatonik added A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. M4-core ⛓ Core client code / Rust. labels Oct 27, 2017
@5chdn 5chdn added this to the 1.9 milestone Oct 29, 2017
@svyatonik
Copy link
Collaborator Author

Latest commit have fixed the test for me, but looks like now there are some ci problems.

@svyatonik svyatonik added A0-pleasereview 🤓 Pull request needs code review. and removed A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. labels Oct 30, 2017
@arkpar arkpar added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Nov 1, 2017
@arkpar arkpar merged commit 7703cd2 into master Nov 2, 2017
@arkpar arkpar deleted the secretstore_key_version branch November 2, 2017 14:33
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.

3 participants