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

SecretStore: encrypt messages using private key from key store #6146

Merged
merged 21 commits into from
Aug 9, 2017

Conversation

svyatonik
Copy link
Collaborator

@svyatonik svyatonik commented Jul 25, 2017

on top of #6107

closes #5502

Previously secret key, used to encrypt communication between two nodes was stored as plain text in parity configuration file. Now you can use secret key of any account from key store (similar to --engine-signer as Peter suggested). Previous configuration is also compatible - if len(configuration_file.secretstore.self_secret) == 64, then it is parsed as plain-text secret, if len is equal to 40, then it is parsed as account' address && password for this account is required (configuration_file.account.password).

@svyatonik svyatonik added A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. M4-core ⛓ Core client code / Rust. labels Jul 25, 2017
@svyatonik
Copy link
Collaborator Author

inprogress because #6107 is also inprogress

@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 Aug 7, 2017
@svyatonik
Copy link
Collaborator Author

This PR is for last 6 commits only

@@ -35,6 +35,7 @@ extern crate ethcore_bigint as bigint;
extern crate ethcrypto as crypto;
extern crate ethkey as _ethkey;
extern crate parity_wordlist;
extern crate ethcore_util as util;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't see where it's used. Isn't it redundant? Besides that ethstore is also compiled for android and ios. Bringing util dependency here, breaks that.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Checked it again - ethstore compiles without it. Looks like an artifact of some experiment. Will update PR

fn agree(&self, account: &StoreAccountRef, password: &str, other: &Public) -> Result<Secret, Error> {
let accounts = self.get_matching(account, password)?;
for account in accounts {
return account.agree(password, other);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

imo, it would be more idiomatic to write

match accounts.first() {
  Some(ref account) => account.agree(password, other),
  None => Err(Error::InvalidPassword),
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also fixed couple of other functions above

@debris debris added A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. and removed A0-pleasereview 🤓 Pull request needs code review. labels Aug 8, 2017
@svyatonik svyatonik added A0-pleasereview 🤓 Pull request needs code review. and removed A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. labels Aug 8, 2017
@debris debris added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Aug 8, 2017
@arkpar arkpar merged commit 33ba5b6 into master Aug 9, 2017
@arkpar arkpar deleted the secretstore_private_from_keystore branch August 9, 2017 09:09
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.

SecretStore: dynamically generate KeyPairs
3 participants