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

SecretStore: secretstore_signRawHash method #7336

Merged
merged 1 commit into from Dec 29, 2017

Conversation

svyatonik
Copy link
Collaborator

closes #7248

Overview:

  1. added secretstore_signRawHash method
  2. renamed secretstore_signServersSet (semantics is changed also)
  3. marked secretstore API set as unsafe, because it now allows to sign any passed raw hash (without prepending with the prefix && without applying hash function)

secretstore_signRawHash can now be used to sign the server key id, which was initially intended to be a keccak256(document_contents). It also should be used to sign the servers set hash after calling the secretstore_serversSetHash method.

@svyatonik svyatonik added A0-pleasereview 🤓 Pull request needs code review. M6-rpcapi 📣 RPC API. labels Dec 19, 2017
Copy link
Collaborator

@tomusdrw tomusdrw left a comment

Choose a reason for hiding this comment

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

lgtm

@@ -100,7 +101,29 @@ fn rpc_secretstore_shadow_decrypt() {
}

#[test]
fn rpc_secretstore_sign_servers_set() {
fn rpc_secretstore_servers_set_hash() {
let deps = Dependencies::new();
Copy link
Collaborator

Choose a reason for hiding this comment

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

@tomusdrw tomusdrw added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Dec 21, 2017
@@ -627,6 +626,7 @@ impl ApiSet {
public_list.insert(Api::ParityAccounts);
public_list.insert(Api::ParitySet);
public_list.insert(Api::Signer);
public_list.insert(Api::SecretStore);
Copy link
Collaborator

Choose a reason for hiding this comment

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

just a question. Above you've added a comment SecretStore (UNSAFE: arbitrary hash signing), but here SecretStore is added to a SafeContext. Isn't it contradictory?

Copy link
Collaborator Author

@svyatonik svyatonik Dec 29, 2017

Choose a reason for hiding this comment

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

I was also confused a bit :) If I understood correctly, there are:

  1. safe/unsafe APIs
  2. safe/unsafe API Contexts

So: SecretStore API is unsafe => it is only enabled when API context is safe. Here's couple of proofs I've found:

  1. https://github.com/paritytech/parity/blob/master/parity/rpc_apis.rs#L148 (reveres proof)
  2. ParityAccounts API, which is also unsafe, is included to the SafeContext API set

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, indeed that the reasoning, although It's extremely confusing (sorry about that:() Feel free to rename it.

@debris debris merged commit 483480d into master Dec 29, 2017
@debris debris deleted the secretstore_rpc_sign_raw_hash branch December 29, 2017 10:10
@5chdn 5chdn added this to the 1.9 milestone Jan 2, 2018
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. M6-rpcapi 📣 RPC API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RPC method to generate signatures for Secretstore
4 participants