SecretStore: generating and retrieving decryption keys via service contract #8029

svyatonik · 2018-03-01T13:17:04Z

on top of #7887
Updated contract(s) PR: https://github.com/paritytech/contracts/pull/108

New service contract APIs

Server Key generation API has been changed

This is what already was a part of SecretStoreService.
But I've changed how it works to support wider range of usage cases:

server key generation request is added via generateServerKey(key_id, threshold) method
server key generation is reported via ServerKeyGenerated(key_id, key_public) event
(new) server key generation can now fail if key has been already generated and error will be reported via ServerKeyGenerationError(key_id) event (previousy we were returning previously generated server key if generation of the same key has been requested)
(new) every key server must confirm key generation (previously we required threshold+1 confirmations)
(new) author of generation request is now important (you can only store document key for this server key by sending tx from the same address)

Server Key retrieval API has been splitted from Server key generation API

Retrieval was possible previously just by calling generation with the same key id. Now it is a separate API:

retrieval request is added via retrieveServerKey(key_id) method
retrieval is reported via ServerKeyRetrieved(key_id, key_public) event
retrieval error is reported via ServerKeyRetrievalError(key_id) event
since we do not know key threshold in service contract AND we do not want to blindly trust any key server that is reporting possibly-fake key_public:
4.1) we're waiting for same-threshold confirmations from 50%+1 key servers
4.2) once we know the threshold, we're waiting for threshold+1 confirmations of the same key_public

Document Key store API has been added

After server key is generated, the author of server key entry (the one who has sent generateServerKey transaction), can generate document key shadow using secretstore_generateDocumentKey RPC (see #7864). After generating, he can use service contract API to store generated key in SS:

store request is added via storeDocumentKey(key_id, common_point, encrypted_point) method
store is confirmed via DocumentKeyStored(key_id) event
store error is reported via DocumentKeyStoreError(key_id) event
we're waiting for store confirmations from all 100% key servers

Document Key shadow retrieval API has been added

retrieval request is added via retrieveDocumentKeyShadow(key_id, requester_public) method (tx author must be the owner of requester_public key)
retrieval is reported in two steps:
2.1) we're waiting for same threshold and comon_point from 50%+1 key servers. Once there are enough confirmations these values are reported via DocumentKeyCommonRetrieved(key_id, requester_address, common_point, threshold)
2.2) once requester sees DocumentKeyCommonRetrieved event, it must start waiting for threshold+1 DocumentKeyPersonalRetrieved(key_id, requester_address, decrypted_secret, shadow). After all data is collected, he can use secretstore_shadowDecrypt method to decrypt document
at any time DocumentKeyShadowRetrievalError(key_id, requester_address) can be raised, signalling that retrieval has failed

Other changes to service contract

there are currently several service contracts, instead of one that has been there previously. The reason is that even uncompleted all-in-one service contract takes >4mln gas to deploy. So there are 1 option for configuring every API described above (service_contract_srv_gen, service_contract_srv_retr, service_contract_doc_store, service_contract_doc_sretr) and also 'backup' option for all-in-one contract (service_contract)
extracted important portions of every contract in an interface so that every contract now have 3 kinds of public methods/events: for 'clients' of these contract (ClientApi interfaces), for key servers (KeyServerApi) and administrative methods (they're here for test period and possbily be changed/removed later).
service contracts are now tightly connected to KeyServerSet contracts (i.e. it is incorrect to use config-file key server set along with service contract)
to make gas cost of calling ServiceContract/KeyServerSet methods limited to some finite value, we now support only 256 key servers at most (when configured via contracts)
to be able to deal with service contract changes, single important value is added as a characteristic of KeyServerSet' current set - the number of block when this set has been changed last. When response cames from KeyServer and this number differs from values of previous responses, all responses are reset && KeyServers mut resend these. Also every request in service contract is considered as non-responded request to make KeyServers able to retry it in next Retry interval.

Important TODOs left

all errors that can occur in SecretStore should be splitted in two sets - internal errors (for example - db error, connectivity error, ...) and external errors (access denied, no such key, ...). Error must be reprted to contract only if it is an external error. If it is an internal error - just try to process request later.

…tract

…_contract

…c_key_via_contract

svyatonik · 2018-03-12T07:40:40Z

Found some issue with shutdown => marking as gotissues

svyatonik · 2018-03-12T11:33:34Z

SS-related issue is fixed in previous commit, but there's still some issue, which results in failed to join thread: Resource deadlock avoided (os error 35) error. It seems related to io worker threads - I'll try to investigate this later.

…_contract

folsen

I have only reviewed this for style/readability and that I can understand roughly what's going on. Have not reviewed any of the logic in-depth. But at this point I think that's the best anyone can do since I don't think anyone understands this system in depth except the author ^^

I will be digging into this code more to try to understand it at this level but I will need some time and I'm fine with merging this in the meantime since it's behind a compiler flag anyway.

folsen · 2018-03-27T11:29:26Z

secret_store/src/listener/service_contract.rs

+		Box::new(request_logs.into_iter()
+			.filter_map(|log| {
+				let raw_log: RawLog = (log.entry.topics.into_iter().map(|t| t.0.into()).collect(), log.entry.data).into();
+				if raw_log.topics[0] == *SERVER_KEY_GENERATION_REQUESTED_EVENT_NAME_HASH {


Can we make this a match statement for better readability?

Not sure :) These are not actually a constant expressions, but Deref calls (it is lazy_static 'consts'). I could replace with match topics[0] { x if x == *SERVER_KEY... }, but that's definitely not better :)

folsen · 2018-03-27T11:36:15Z

secret_store/src/listener/service_contract_listener.rs

+		// ignore result - the only thing that we can do is to log the error
+		let session_id = session.id();
+		let server_key_id = session_id.id;
+		if let Some(requester) = session.requester().and_then(|r| r.address(&server_key_id).ok()) {


Couldn't you concatenate all the 3 lines below into 1 if let statement? Since they don't depend on the previous value. So like if let (Some(x), Some(y)) = (session.requester()[..], session.origin()) ?

Merged two lines into 1. Not sure this improves readability, though :) 3rd line has side effects => do not want to depend on evaluation order => left separate if let

niklasad1 · 2018-04-03T09:07:40Z

secret_store/src/key_server_cluster/client_sessions/generation_session.rs

@@ -1405,7 +1405,7 @@ pub mod tests {
 			// run session to completion
 			let session_id = SessionId::default();
 			let session = clusters[0].client().new_generation_session(session_id, Default::default(), Default::default(), threshold).unwrap();
-			loop_until(&mut core, time::Duration::from_millis(1000), || session.joint_public_and_secret().is_some());
+			loop_until(&mut core, Duration::from_millis(1000), || session.joint_public_and_secret().is_some());


change to const?

svyatonik added 30 commits February 5, 2018 14:04

SecretStore: started document keys generation via contract

ec72714

Merge branch 'master' into secretstore_doc_key_via_contract

7271a8a

fixed Cargo.lock

63617c6

SecretStore: doc key contract gen tests

44c34ae

SecretStore: fixed log parsing

011ecb7

SecretStore: flush

08da162

SecretStore: secretstore_generateDocumentKey RPC

2806865

SecretStore: return encrypted_key from secretstore_generateDocumentKey

a0bb417

Merge branch 'master' into secretstore_doc_key_via_contract

4061f32

prepare to GenerateDocKey -> StoreDocKey

8a1ae98

SecretStore: ability to identify requester via Public/Address

c779580

SecretStore: store author address instead of public in db

c7cae5c

flush

bc9802d

Merge branch 'secretstore_requester' into secretstore_doc_key_via_con…

2e8bd45

…tract

Merge branch 'secretstore_address_in_db' into secretstore_doc_key_via…

214a7df

…_contract

SecretStore: flush

49d93c4

Merge branch 'master' into secretstore_doc_key_via_contract

c87d94d

SecretStore: fixed test

6043dc2

SecretStore: flush

7705ea2

SecretStore: flush

1674ab7

SecretStore: flush

50531c1

SecretStore: flush

f611172

SecretStore: start async generation session

821967b

SecretStore: process StoreDocumentKey service tasks

5399bee

SecretStore: flush

0cf4dd2

SecretStore: update service contact ABI

b81abe6

SecretStore: flush

a633522

SecretStore: flush

2303340

Merge branch 'secretstore_generateDocumentKeyRPC' into secretstore_do…

f387a43

…c_key_via_contract

SecretStore: fixed event

b6558b7

svyatonik added 8 commits March 8, 2018 14:41

SecretStore: fixes for updated contract

c9c0497

SecretStore: service fixes after testing

ad5b402

fixed cli test compilation

b81af6a

SecretStore: decryption_session_origin_is_known_to_all_initialized_nodes

c7dce94

SecretStore: added new contract listener tests

31338be

SecretStore: session_listener_works

c54e88f

Merge branch 'master' into secretstore_doc_key_via_contract

51fda49

removed optional TODO

fde55fc

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 Mar 12, 2018

svyatonik changed the title ~~SecretStore: generating and retrieving decryption keys via service contract [WIP]~~ SecretStore: generating and retrieving decryption keys via service contract Mar 12, 2018

svyatonik added A4-gotissues 💥 Pull request is reviewed and has significant issues which must be addressed. and removed A0-pleasereview 🤓 Pull request needs code review. labels Mar 12, 2018

SecretStore: fixed KeyServer shutdown

07b3d88

svyatonik added A0-pleasereview 🤓 Pull request needs code review. and removed A4-gotissues 💥 Pull request is reviewed and has significant issues which must be addressed. labels Mar 12, 2018

svyatonik added 3 commits March 19, 2018 08:55

Merge branch 'master' into secretstore_address_in_db

f169477

Merge branch 'secretstore_address_in_db' into secretstore_doc_key_via…

2b9aefc

…_contract

Merge branch 'master' into secretstore_doc_key_via_contract

003766e

folsen approved these changes Mar 27, 2018

View reviewed changes

svyatonik added 2 commits April 3, 2018 11:23

Merge branch 'master' into secretstore_doc_key_via_contract

efd0484

fixed warn + grumble

b99b42b

niklasad1 reviewed Apr 3, 2018

View reviewed changes

const durations

0cd88f5

5chdn added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Apr 3, 2018

5chdn merged commit ec96091 into master Apr 3, 2018

5chdn deleted the secretstore_doc_key_via_contract branch April 3, 2018 14:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SecretStore: generating and retrieving decryption keys via service contract #8029

SecretStore: generating and retrieving decryption keys via service contract #8029

svyatonik commented Mar 1, 2018 •

edited

Loading

svyatonik commented Mar 12, 2018

svyatonik commented Mar 12, 2018

folsen left a comment

folsen Mar 27, 2018

svyatonik Apr 3, 2018

folsen Mar 27, 2018

svyatonik Apr 3, 2018

niklasad1 Apr 3, 2018

svyatonik Apr 3, 2018

SecretStore: generating and retrieving decryption keys via service contract #8029

SecretStore: generating and retrieving decryption keys via service contract #8029

Conversation

svyatonik commented Mar 1, 2018 • edited Loading

New service contract APIs

Server Key generation API has been changed

Server Key retrieval API has been splitted from Server key generation API

Document Key store API has been added

Document Key shadow retrieval API has been added

Other changes to service contract

Important TODOs left

svyatonik commented Mar 12, 2018

svyatonik commented Mar 12, 2018

folsen left a comment

Choose a reason for hiding this comment

folsen Mar 27, 2018

Choose a reason for hiding this comment

svyatonik Apr 3, 2018

Choose a reason for hiding this comment

folsen Mar 27, 2018

Choose a reason for hiding this comment

svyatonik Apr 3, 2018

Choose a reason for hiding this comment

niklasad1 Apr 3, 2018

Choose a reason for hiding this comment

svyatonik Apr 3, 2018

Choose a reason for hiding this comment

svyatonik commented Mar 1, 2018 •

edited

Loading