feat: secure p2p #354
Merged
feat: secure p2p #354
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Terraform Feature Environment (dev-354)Terraform Initialization ⚙️
|
ChaoticTempest
force-pushed
the
phuong/feat/secure-p2p
branch
from
0f6bbbc
to
4ca4fe6
Compare
itegulov
reviewed
Nov 16, 2023
keys/src/hpke.rs
Outdated
|
|
||
| /// This can be used to customize the generated key. This will be used as a sort of | ||
| /// versioning mechanism for the key. | ||
| const INFO_ENTROPY: &[u8] = b"session-key-v1"; |
There was a problem hiding this comment.
Hmm, can you give an example of a situation when we would want "bump" this version?
There was a problem hiding this comment.
this is just extra info for generating the derived key fro encryption. So it's more for contextual info. It's useful when we end up using the same key as encryption and signing, but we don't se it can just empty or static like this. I just arbitrarily made it into a versioning scheme
| @@ -34,6 +42,7 @@ pub struct InitializingContractState { | |||
| #[derive(BorshDeserialize, BorshSerialize, Serialize, Deserialize, Debug)] | |||
| pub struct RunningContractState { | |||
| pub epoch: u64, | |||
| // TODO: why is this account id for participants instead of participant id? | |||
There was a problem hiding this comment.
This is kind of like a preparation for #330
Ideally nodes should be identified by the account id they use for interacting with the contract and the participant id should be given to them at runtime by the contract
| @@ -0,0 +1,156 @@ | |||
| use borsh::{self, BorshDeserialize, BorshSerialize}; | |||
There was a problem hiding this comment.
FYI I want Michel to take a look at this to make sure we are not misusing anything here, will let you know how it goes
There was a problem hiding this comment.
@itegulov Are we good to go?
volovyks
approved these changes
Nov 22, 2023
| @@ -22,6 +26,10 @@ pub struct ParticipantInfo { | |||
| pub id: ParticipantId, | |||
| pub account_id: AccountId, | |||
| pub url: String, | |||
| /// The public key used for encrypting messages. | |||
| pub cipher_pk: hpke::PublicKey, | |||
| /// The public key used for verifying messages. | |||
There was a problem hiding this comment.
I would rather have longer names here and in other places, like msg_encryption_pk, msg_signature_pk. People and other developers can confuse these with key shares.
| @@ -0,0 +1,156 @@ | |||
| use borsh::{self, BorshDeserialize, BorshSerialize}; | |||
There was a problem hiding this comment.
@itegulov Are we good to go?
| client: &Client, | ||
| url: U, | ||
| message: MpcMessage, | ||
| ) -> Result<(), SendError> { | ||
| let encrypted = SignedMessage::encrypt(message, participant, sign_sk, cipher_pk) |
There was a problem hiding this comment.
This is encryption and signing in one function, right?
| #[error("sync failed: {0}")] | ||
| SyncError(String), | ||
| #[error(transparent)] | ||
| DataConversion(#[from] serde_json::Error), |
|
Ok, I am merging this to avoid conflicts and we can address comments in a follow up PR |
Terraform Feature Environment Destroy (dev-354)Terraform Initialization ⚙️
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This implements secure p2p communication between two nodes.
Adds/msg-privateendpoint for secure messaging on protocolSendPrivateactions.msgendpoint is now encrypted when sending messages