Skip to content

Commit

Permalink
SecretStore: service pack 1 (#8435)
Browse files Browse the repository at this point in the history 
* SecretStore: error unify initial commit

SecretStore: pass real error in error messages

SecretStore: is_internal_error -> Error::is_non_fatal

warnings

SecretStore: ConsensusTemporaryUnreachable

fix after merge

removed comments

removed comments

SecretStore: updated HTTP error responses

SecretStore: more ConsensusTemporaryUnreachable tests

fix after rebase

* SecretStore: unified SS contract config options && read

* SecretStore: service pack

SecretStore: service pack (continue)

* fixed grumbles
  • Loading branch information
@svyatonik @tavakyan
svyatonik authored and tavakyan committed Jun 16, 2018
1 parent cc59965 commit 5443f99
Show file tree
Hide file tree
Showing 24 changed files with 855 additions and 436 deletions.
51 changes: 29 additions & 22 deletions parity/cli/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -563,38 +563,42 @@ usage! {
"--no-secretstore-http",
"Disable Secret Store HTTP API.",

FLAG flag_no_secretstore_acl_check: (bool) = false, or |c: &Config| c.secretstore.as_ref()?.disable_acl_check.clone(),
"--no-acl-check",
"Disable ACL check (useful for test environments).",

FLAG flag_no_secretstore_auto_migrate: (bool) = false, or |c: &Config| c.secretstore.as_ref()?.disable_auto_migrate.clone(),
"--no-secretstore-auto-migrate",
"Do not run servers set change session automatically when servers set changes. This option has no effect when servers set is read from configuration file.",

ARG arg_secretstore_contract: (String) = "none", or |c: &Config| c.secretstore.as_ref()?.service_contract.clone(),
ARG arg_secretstore_acl_contract: (Option<String>) = Some("registry".into()), or |c: &Config| c.secretstore.as_ref()?.acl_contract.clone(),
"--secretstore-acl-contract=[SOURCE]",
"Secret Store permissioning contract address source: none, registry (contract address is read from 'secretstore_acl_checker' entry in registry) or address.",

ARG arg_secretstore_contract: (Option<String>) = None, or |c: &Config| c.secretstore.as_ref()?.service_contract.clone(),
"--secretstore-contract=[SOURCE]",
"Secret Store Service contract address source: none, registry (contract address is read from secretstore_service entry in registry) or address.",
"Secret Store Service contract address source: none, registry (contract address is read from 'secretstore_service' entry in registry) or address.",

ARG arg_secretstore_srv_gen_contract: (String) = "none", or |c: &Config| c.secretstore.as_ref()?.service_contract_srv_gen.clone(),
ARG arg_secretstore_srv_gen_contract: (Option<String>) = None, or |c: &Config| c.secretstore.as_ref()?.service_contract_srv_gen.clone(),
"--secretstore-srv-gen-contract=[SOURCE]",
"Secret Store Service server key generation contract address source: none, registry (contract address is read from secretstore_service_srv_gen entry in registry) or address.",
"Secret Store Service server key generation contract address source: none, registry (contract address is read from 'secretstore_service_srv_gen' entry in registry) or address.",

ARG arg_secretstore_srv_retr_contract: (String) = "none", or |c: &Config| c.secretstore.as_ref()?.service_contract_srv_retr.clone(),
ARG arg_secretstore_srv_retr_contract: (Option<String>) = None, or |c: &Config| c.secretstore.as_ref()?.service_contract_srv_retr.clone(),
"--secretstore-srv-retr-contract=[SOURCE]",
"Secret Store Service server key retrieval contract address source: none, registry (contract address is read from secretstore_service_srv_retr entry in registry) or address.",
"Secret Store Service server key retrieval contract address source: none, registry (contract address is read from 'secretstore_service_srv_retr' entry in registry) or address.",

ARG arg_secretstore_doc_store_contract: (String) = "none", or |c: &Config| c.secretstore.as_ref()?.service_contract_doc_store.clone(),
ARG arg_secretstore_doc_store_contract: (Option<String>) = None, or |c: &Config| c.secretstore.as_ref()?.service_contract_doc_store.clone(),
"--secretstore-doc-store-contract=[SOURCE]",
"Secret Store Service document key store contract address source: none, registry (contract address is read from secretstore_service_doc_store entry in registry) or address.",
"Secret Store Service document key store contract address source: none, registry (contract address is read from 'secretstore_service_doc_store' entry in registry) or address.",

ARG arg_secretstore_doc_sretr_contract: (String) = "none", or |c: &Config| c.secretstore.as_ref()?.service_contract_doc_sretr.clone(),
ARG arg_secretstore_doc_sretr_contract: (Option<String>) = None, or |c: &Config| c.secretstore.as_ref()?.service_contract_doc_sretr.clone(),
"--secretstore-doc-sretr-contract=[SOURCE]",
"Secret Store Service document key shadow retrieval contract address source: none, registry (contract address is read from secretstore_service_doc_sretr entry in registry) or address.",
"Secret Store Service document key shadow retrieval contract address source: none, registry (contract address is read from 'secretstore_service_doc_sretr' entry in registry) or address.",

ARG arg_secretstore_nodes: (String) = "", or |c: &Config| c.secretstore.as_ref()?.nodes.as_ref().map(|vec| vec.join(",")),
"--secretstore-nodes=[NODES]",
"Comma-separated list of other secret store cluster nodes in form NODE_PUBLIC_KEY_IN_HEX@NODE_IP_ADDR:NODE_PORT.",

ARG arg_secretstore_server_set_contract: (Option<String>) = Some("registry".into()), or |c: &Config| c.secretstore.as_ref()?.server_set_contract.clone(),
"--secretstore-server-set-contract=[SOURCE]",
"Secret Store server set contract address source: none, registry (contract address is read from 'secretstore_server_set' entry in registry) or address.",

ARG arg_secretstore_interface: (String) = "local", or |c: &Config| c.secretstore.as_ref()?.interface.clone(),
"--secretstore-interface=[IP]",
"Specify the hostname portion for listening to Secret Store Key Server internal requests, IP should be an interface's IP address, or local.",
Expand Down Expand Up @@ -1193,8 +1197,8 @@ struct Dapps {
struct SecretStore {
disable: Option<bool>,
disable_http: Option<bool>,
disable_acl_check: Option<bool>,
disable_auto_migrate: Option<bool>,
acl_contract: Option<String>,
service_contract: Option<String>,
service_contract_srv_gen: Option<String>,
service_contract_srv_retr: Option<String>,
Expand All @@ -1203,6 +1207,7 @@ struct SecretStore {
self_secret: Option<String>,
admin_public: Option<String>,
nodes: Option<Vec<String>>,
server_set_contract: Option<String>,
interface: Option<String>,
port: Option<u16>,
http_interface: Option<String>,
Expand Down Expand Up @@ -1620,16 +1625,17 @@ mod tests {
// SECRETSTORE
flag_no_secretstore: false,
flag_no_secretstore_http: false,
flag_no_secretstore_acl_check: false,
flag_no_secretstore_auto_migrate: false,
arg_secretstore_contract: "none".into(),
arg_secretstore_srv_gen_contract: "none".into(),
arg_secretstore_srv_retr_contract: "none".into(),
arg_secretstore_doc_store_contract: "none".into(),
arg_secretstore_doc_sretr_contract: "none".into(),
arg_secretstore_acl_contract: Some("registry".into()),
arg_secretstore_contract: Some("none".into()),
arg_secretstore_srv_gen_contract: Some("none".into()),
arg_secretstore_srv_retr_contract: Some("none".into()),
arg_secretstore_doc_store_contract: Some("none".into()),
arg_secretstore_doc_sretr_contract: Some("none".into()),
arg_secretstore_secret: None,
arg_secretstore_admin_public: None,
arg_secretstore_nodes: "".into(),
arg_secretstore_server_set_contract: Some("registry".into()),
arg_secretstore_interface: "local".into(),
arg_secretstore_port: 8083u16,
arg_secretstore_http_interface: "local".into(),
Expand Down Expand Up @@ -1881,8 +1887,8 @@ mod tests {
secretstore: Some(SecretStore {
disable: None,
disable_http: None,
disable_acl_check: None,
disable_auto_migrate: None,
acl_contract: None,
service_contract: None,
service_contract_srv_gen: None,
service_contract_srv_retr: None,
Expand All @@ -1891,6 +1897,7 @@ mod tests {
self_secret: None,
admin_public: None,
nodes: None,
server_set_contract: None,
interface: None,
port: Some(8083),
http_interface: None,
Expand Down
3 changes: 2 additions & 1 deletion parity/cli/tests/config.full.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,12 +91,13 @@ pass = "test_pass"
[secretstore]
disable = false
disable_http = false
disable_acl_check = false
acl_contract = "registry"
service_contract = "none"
service_contract_srv_gen = "none"
service_contract_srv_retr = "none"
service_contract_doc_store = "none"
service_contract_doc_sretr = "none"
server_set_contract = "registry"
nodes = []
http_interface = "local"
http_port = 8082
Expand Down
25 changes: 15 additions & 10 deletions parity/configuration.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -604,15 +604,16 @@ impl Configuration {
Ok(SecretStoreConfiguration {
enabled: self.secretstore_enabled(),
http_enabled: self.secretstore_http_enabled(),
acl_check_enabled: self.secretstore_acl_check_enabled(),
auto_migrate_enabled: self.secretstore_auto_migrate_enabled(),
acl_check_contract_address: self.secretstore_acl_check_contract_address()?,
service_contract_address: self.secretstore_service_contract_address()?,
service_contract_srv_gen_address: self.secretstore_service_contract_srv_gen_address()?,
service_contract_srv_retr_address: self.secretstore_service_contract_srv_retr_address()?,
service_contract_doc_store_address: self.secretstore_service_contract_doc_store_address()?,
service_contract_doc_sretr_address: self.secretstore_service_contract_doc_sretr_address()?,
self_secret: self.secretstore_self_secret()?,
nodes: self.secretstore_nodes()?,
key_server_set_contract_address: self.secretstore_key_server_set_contract_address()?,
interface: self.secretstore_interface(),
port: self.args.arg_ports_shift + self.args.arg_secretstore_port,
http_interface: self.secretstore_http_interface(),
Expand Down Expand Up @@ -1099,14 +1100,14 @@ impl Configuration {
!self.args.flag_no_secretstore_http && cfg!(feature = "secretstore")
}

fn secretstore_acl_check_enabled(&self) -> bool {
!self.args.flag_no_secretstore_acl_check
}

fn secretstore_auto_migrate_enabled(&self) -> bool {
!self.args.flag_no_secretstore_auto_migrate
}

fn secretstore_acl_check_contract_address(&self) -> Result<Option<SecretStoreContractAddress>, String> {
into_secretstore_service_contract_address(self.args.arg_secretstore_acl_contract.as_ref())
}

fn secretstore_service_contract_address(&self) -> Result<Option<SecretStoreContractAddress>, String> {
into_secretstore_service_contract_address(self.args.arg_secretstore_contract.as_ref())
}
Expand All @@ -1127,6 +1128,10 @@ impl Configuration {
into_secretstore_service_contract_address(self.args.arg_secretstore_doc_sretr_contract.as_ref())
}

fn secretstore_key_server_set_contract_address(&self) -> Result<Option<SecretStoreContractAddress>, String> {
into_secretstore_service_contract_address(self.args.arg_secretstore_server_set_contract.as_ref())
}

fn verifier_settings(&self) -> VerifierSettings {
let mut settings = VerifierSettings::default();
settings.scale_verifiers = self.args.flag_scale_verifiers;
Expand All @@ -1145,11 +1150,11 @@ impl Configuration {
}
}

fn into_secretstore_service_contract_address(s: &str) -> Result<Option<SecretStoreContractAddress>, String> {
match s {
"none" => Ok(None),
"registry" => Ok(Some(SecretStoreContractAddress::Registry)),
a => Ok(Some(SecretStoreContractAddress::Address(a.parse().map_err(|e| format!("{}", e))?))),
fn into_secretstore_service_contract_address(s: Option<&String>) -> Result<Option<SecretStoreContractAddress>, String> {
match s.map(String::as_str) {
None | Some("none") => Ok(None),
Some("registry") => Ok(Some(SecretStoreContractAddress::Registry)),
Some(a) => Ok(Some(SecretStoreContractAddress::Address(a.parse().map_err(|e| format!("{}", e))?))),
}
}

Expand Down
14 changes: 9 additions & 5 deletions parity/secretstore.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,10 @@ pub struct Configuration {
pub enabled: bool,
/// Is HTTP API enabled?
pub http_enabled: bool,
/// Is ACL check enabled.
pub acl_check_enabled: bool,
/// Is auto migrate enabled.
pub auto_migrate_enabled: bool,
/// ACL check contract address.
pub acl_check_contract_address: Option<ContractAddress>,
/// Service contract address.
pub service_contract_address: Option<ContractAddress>,
/// Server key generation service contract address.
Expand All @@ -68,6 +68,8 @@ pub struct Configuration {
pub self_secret: Option<NodeSecretKey>,
/// Other nodes IDs + addresses.
pub nodes: BTreeMap<Public, (String, u16)>,
/// Key Server Set contract address. If None, 'nodes' map is used.
pub key_server_set_contract_address: Option<ContractAddress>,
/// Interface to listen to
pub interface: String,
/// Port to listen to
Expand Down Expand Up @@ -135,7 +137,7 @@ mod server {
impl KeyServer {
/// Create new key server
pub fn new(mut conf: Configuration, deps: Dependencies) -> Result<Self, String> {
if !conf.acl_check_enabled {
if conf.acl_check_contract_address.is_none() {
warn!("Running SecretStore with disabled ACL check: {}", Red.bold().paint("everyone has access to stored keys"));
}

Expand Down Expand Up @@ -174,7 +176,7 @@ mod server {
service_contract_srv_retr_address: conf.service_contract_srv_retr_address.map(into_service_contract_address),
service_contract_doc_store_address: conf.service_contract_doc_store_address.map(into_service_contract_address),
service_contract_doc_sretr_address: conf.service_contract_doc_sretr_address.map(into_service_contract_address),
acl_check_enabled: conf.acl_check_enabled,
acl_check_contract_address: conf.acl_check_contract_address.map(into_service_contract_address),
cluster_config: ethcore_secretstore::ClusterConfiguration {
threads: 4,
listener_address: ethcore_secretstore::NodeAddress {
Expand All @@ -185,6 +187,7 @@ mod server {
address: ip,
port: port,
})).collect(),
key_server_set_contract_address: conf.key_server_set_contract_address.map(into_service_contract_address),
allow_connecting_to_higher_nodes: true,
admin_public: conf.admin_public,
auto_migrate_enabled: conf.auto_migrate_enabled,
Expand Down Expand Up @@ -212,8 +215,8 @@ impl Default for Configuration {
Configuration {
enabled: true,
http_enabled: true,
acl_check_enabled: true,
auto_migrate_enabled: true,
acl_check_contract_address: Some(ContractAddress::Registry),
service_contract_address: None,
service_contract_srv_gen_address: None,
service_contract_srv_retr_address: None,
Expand All @@ -222,6 +225,7 @@ impl Default for Configuration {
self_secret: None,
admin_public: None,
nodes: BTreeMap::new(),
key_server_set_contract_address: Some(ContractAddress::Registry),
interface: "127.0.0.1".to_owned(),
port: 8083,
http_interface: "127.0.0.1".to_owned(),
Expand Down
47 changes: 25 additions & 22 deletions secret_store/src/acl_storage.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@ use std::sync::Arc;
use std::collections::{HashMap, HashSet};
use std::time::Duration;
use parking_lot::{Mutex, RwLock};
use ethcore::client::{BlockId, ChainNotify, ChainRoute, CallContract, RegistryInfo};
use ethcore::client::{BlockId, ChainNotify, ChainRoute, CallContract};
use ethereum_types::{H256, Address};
use bytes::Bytes;
use trusted_client::TrustedClient;
use types::{Error, ServerKeyId};
use types::{Error, ServerKeyId, ContractAddress};

use_contract!(acl_storage, "AclStorage", "res/acl_storage.json");

Expand All @@ -44,8 +44,10 @@ pub struct OnChainAclStorage {
struct CachedContract {
/// Blockchain client.
client: TrustedClient,
/// Contract address.
contract_addr: Option<Address>,
/// Contract address source.
address_source: ContractAddress,
/// Current contract address.
contract_address: Option<Address>,
/// Contract at given address.
contract: acl_storage::AclStorage,
}
Expand All @@ -57,10 +59,10 @@ pub struct DummyAclStorage {
}

impl OnChainAclStorage {
pub fn new(trusted_client: TrustedClient) -> Result<Arc<Self>, Error> {
pub fn new(trusted_client: TrustedClient, address_source: ContractAddress) -> Result<Arc<Self>, Error> {
let client = trusted_client.get_untrusted();
let acl_storage = Arc::new(OnChainAclStorage {
contract: Mutex::new(CachedContract::new(trusted_client)),
contract: Mutex::new(CachedContract::new(trusted_client, address_source)),
});
client
.ok_or_else(|| Error::Internal("Constructing OnChainAclStorage without active Client".into()))?
Expand All @@ -78,36 +80,37 @@ impl AclStorage for OnChainAclStorage {
impl ChainNotify for OnChainAclStorage {
fn new_blocks(&self, _imported: Vec<H256>, _invalid: Vec<H256>, route: ChainRoute, _sealed: Vec<H256>, _proposed: Vec<Bytes>, _duration: Duration) {
if !route.enacted().is_empty() || !route.retracted().is_empty() {
self.contract.lock().update()
self.contract.lock().update_contract_address()
}
}
}

impl CachedContract {
pub fn new(client: TrustedClient) -> Self {
CachedContract {
pub fn new(client: TrustedClient, address_source: ContractAddress) -> Self {
let mut contract = CachedContract {
client,
contract_addr: None,
address_source,
contract_address: None,
contract: acl_storage::AclStorage::default(),
}
};
contract.update_contract_address();
contract
}

pub fn update(&mut self) {
if let Some(client) = self.client.get() {
match client.registry_address(ACL_CHECKER_CONTRACT_REGISTRY_NAME.to_owned(), BlockId::Latest) {
Some(new_contract_addr) if Some(new_contract_addr).as_ref() != self.contract_addr.as_ref() => {
trace!(target: "secretstore", "Configuring for ACL checker contract from {}", new_contract_addr);
self.contract_addr = Some(new_contract_addr);
},
Some(_) | None => ()
}
pub fn update_contract_address(&mut self) {
let contract_address = self.client.read_contract_address(ACL_CHECKER_CONTRACT_REGISTRY_NAME.into(), &self.address_source);
if contract_address != self.contract_address {
trace!(target: "secretstore", "Configuring for ACL checker contract from address {:?}",
contract_address);

self.contract_address = contract_address;
}
}

pub fn check(&mut self, requester: Address, document: &ServerKeyId) -> Result<bool, Error> {
if let Some(client) = self.client.get() {
// call contract to check access
match self.contract_addr {
// call contract to check accesss
match self.contract_address {
Some(contract_address) => {
let do_call = |data| client.call_contract(BlockId::Latest, contract_address, data);
self.contract.functions()
Expand Down
3 changes: 2 additions & 1 deletion secret_store/src/key_server.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,7 @@ pub mod tests {
address: "127.0.0.1".into(),
port: start_port + (j as u16),
})).collect(),
key_server_set_contract_address: None,
allow_connecting_to_higher_nodes: false,
admin_public: None,
auto_migrate_enabled: false,
Expand All @@ -312,7 +313,7 @@ pub mod tests {
.collect();
let key_storages = (0..num_nodes).map(|_| Arc::new(DummyKeyStorage::default())).collect::<Vec<_>>();
let key_servers: Vec<_> = configs.into_iter().enumerate().map(|(i, cfg)|
KeyServerImpl::new(&cfg, Arc::new(MapKeyServerSet::new(key_servers_set.clone())),
KeyServerImpl::new(&cfg, Arc::new(MapKeyServerSet::new(false, key_servers_set.clone())),
Arc::new(PlainNodeKeyPair::new(key_pairs[i].clone())),
Arc::new(DummyAclStorage::default()),
key_storages[i].clone()).unwrap()
Expand Down
Loading

0 comments on commit 5443f99

Please sign in to comment.