-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Conversation
// if session is completed => stop | ||
let session = session.clone().expect("session.method() call finished with success; session exists; qed"); | ||
if session.is_finished() { | ||
info!(target: "secretstore_net", "{}: signing session completed", data.self_key_pair.public()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure there is a need to notify console of each session (info!
) ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is convenient for me right now :) In production environment it could be a little annoying, as long as SecretStore is run along with synchronization. But one day (I hope :)) it will became a standalone IPC-module && creating/completion sessions would be its main 'side effect' => it would be great to see these notifications in console.
I could remove this for now, if you think it is annoying.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nah, if it is intentional, nevermind
secret_store/src/http_listener.rs
Outdated
return_bytes::<i32>(req, res, empty.map(|_| None)) | ||
} | ||
|
||
fn return_server_pubic_key(req: HttpRequest, res: HttpResponse, server_public: Result<Public, Error>) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo, should be server_public
secret_store/src/key_server.rs
Outdated
let message_signature = signing_session.wait()?; | ||
|
||
// compose two message signature components into single one | ||
let mut combined_signature: Vec<u8> = Vec::with_capacity(64); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could be just an array
// => do not use these in encryption session | ||
let mut encrypted_data = self.key_storage.get(&session_id).map_err(|e| Error::KeyStorage(e.into()))?; | ||
let disconnected_nodes: BTreeSet<_> = encrypted_data.id_numbers.keys().cloned().collect(); | ||
let disconnected_nodes: BTreeSet<_> = disconnected_nodes.difference(&cluster.nodes()).cloned().collect(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to collect
here since all you do with the set is iterate it. Also applies to a few places like this below.
Requirements:
a set of N KeyServers, sharing KeyPair (public is known to every node, secret could be restored by t KeyServers) must be able to sign message (message hash).
Options:
Of these options, 3rd one has been choosen for now.
Paper, on which current implementation is based:
https://www.researchgate.net/profile/A_Chronopoulos/publication/224386852_Efficient_multi-party_digital_signature_using_adaptive_secret_sharing_for_low-power_devices_in_wireless_networksPLEASE_REFERENCE_IN_YOUR_PAPERS/links/00b7d52c2a3fb5bf8e000000/Efficient-multi-party-digital-signature-using-adaptive-secret-sharing-for-low-power-devices-in-wireless-networksPLEASE-REFERENCE-IN-YOUR-PAPERS.pdf
also (important): http://www.wu.ece.ufl.edu/mypapers/notesMulti-partyDigitalSignature.pdf
PR outline:
1.1) there was same issue as with earlier ECDKG implementation - signing only worked for t-of-N groups, where t was even. This issue has been fixed.
1.2) I have slightly changed signature calculation, as it was not compatible with some non-distributed EC-Schnorr implementation, I've found on github. Now it is compatible. And non-distributed code is also in math.rs (local_compute_signature())
1.3)
check_signature_share()
from math.rs is currently not implemented, as suggested implementation from paper seems wrong.3.1) client asks KeyServer to generate server key. KeyServer returns JointServerPublic to the client
3.2) client generates and encrypts DocumentSecret (which is point on EC in this case) using following procedure:
3.2.1) client generates random EC-field scalar k
3.2.2) client computes CommonPoint = k * ECGenerationPoint
3.2.3) client computes EncryptedDocumentSecret = DocumentSecret + k * JointServerPublic
3.3) client posts pair { CommonPoint, EncryptedDocumentSecret } to KeyServer.
3.4) in combination with 'shadow key retrieval', which was introduced before, this guarantees that KeyServer will never see unencrypted DocumentSecret. Previously DocumentSecret was generated on KeyServer.
4.1) upgrade is implemented as a single im-place transaction => all db should fit into memory. I assume it is not an issue, because SecretStore is only starting its first deployment => db should be small.
4.2) I have tested upgrade on my own database && everything seems to work (also have a test for it). But it would be better to make backup of SecretStore db befrore upgrading production nodes, as it could potentially lead to data loss.
consensus_session.rs
.API changes:
Previous KeyServer API remains active - you still can simultaneously generate ServerKey && DocumentKey, retrieve DocumentKey && DocumentKey shadow.
In addition, following HTTP requests are now supported:
result is JointServerPublic:
result is signature of message_hash, encrypted with caller' public key:
Next PRs (TODOs for me):