2PC Private Channels implementation #5765
Conversation
|
The PR refactoring so that the 2PC works on referrals has been pushed. Should we remove the draft state to check if the CI passes? @NejcZdovc Thanks! Edit: I set the PR as ready for review. |
components/private_channel/rust/ffi/private-channel/repsys-crypto/Cargo.toml
Outdated
Show resolved
Hide resolved
components/private_channel/rust/ffi/private-channel/repsys-crypto/src/lib.rs
Outdated
Show resolved
Hide resolved
components/private_channel/rust/ffi/private-channel/repsys-crypto/src/lib.rs
Outdated
Show resolved
Hide resolved
components/private_channel/rust/ffi/private-channel/repsys-crypto/src/lib.rs
Outdated
Show resolved
Hide resolved
components/private_channel/rust/ffi/private-channel/repsys-crypto/src/lib.rs
Show resolved
Hide resolved
components/private_channel/rust/ffi/private-channel/repsys-crypto/src/lib.rs
Outdated
Show resolved
Hide resolved
iquerejeta
force-pushed
the
private_channels
branch
from
5a7a088
to
1e5d67b
Compare
@isislovecruft great, thanks! Where can we find your comments on the |
|
We've changed slightly the protocol to require one key pair per signal sent over by the client -- instead of one key pair per round, regardless of the number of encrypted signals. The goal with this change is to decrease the attack surface for a possible brute-force attack from the server. In the previous design (one keypair per round), the server could compare 1 signal against N different plain texts (N being the number of signals sent over in one round), since all signals were encrypted using the same key. The user has no visibility if the server is misbehaving With the "multikey" approach, it is impossible for the server to compare one signal against a plaintext more than once since each encrypted signal is "locked" by one keypair. These changes have been added in between review rounds, so @isislovecruft feel free to reach out for questions or comments on these changes. Thanks! |
gpestana
force-pushed
the
private_channels
branch
from
cfa0d75
to
0e5b564
Compare
| let mut proofs_correct_decryption: Vec<CompactProof> = Vec::new(); | ||
|
|
||
| for (index, value) in partial_decryption.iter().enumerate() { | ||
| proofs_correct_decryption | ||
| .push(sks[index].prove_correct_decryption(&randomized_vector[index], &value.points.1)); | ||
| } |
There was a problem hiding this comment.
3-way zips are not very convenient unfortunately, but I would recommend a map here at least
components/private_channel/rust/ffi/private-channel/repsys-crypto/src/lib.rs
Outdated
Show resolved
Hide resolved
| let parsed_str = raw_str.replace(&['[', ']'][..], ""); | ||
| let v_enc: Vec<u8> = parsed_str | ||
| .split(", ") | ||
| .map(|s| s.parse::<u8>().unwrap()) |
There was a problem hiding this comment.
Probably best to return ResultSecondRound::default() rather than panic on malformed data here.
Result implements FromIterator, which means you can actually collect the iterated Result<u8, _>s into a Result<Vec<u8>, _> and it will stop as soon as the first parse error is encountered.
|
|
||
| #include "base/logging.h" | ||
| #include "base/task/post_task.h" | ||
| #include "chrome/browser/browser_process.h" |
There was a problem hiding this comment.
chrome/browser deps are not allowed in components. See "Recipes for Breaking //chrome Dependencies" https://www.chromium.org/developers/design-documents/cookbook
| "https://repsys.rewards.brave.software"; | ||
| extern const char PRIVATE_CHANNEL_PRODUCTION_SERVER[] = | ||
| "https://repsys.rewards.brave.com"; | ||
| extern const char PRIVATE_CHANNEL_DEVELOPMENT_SERVER[] = |
There was a problem hiding this comment.
these should set from gn, not hard-coded
gpestana
force-pushed
the
private_channels
branch
from
f1521a4
to
e7dde25
Compare
gpestana
force-pushed
the
private_channels
branch
from
b292d2f
to
1f46ff6
Compare
| const char kPrivateChannelVersion[] = | ||
| "30ee124d76368c52339c8d965d3a07c1db66a555"; | ||
|
|
||
| extern const char PRIVATE_CHANNEL_STAGING_SERVER[] = |
There was a problem hiding this comment.
this needs to move to gn config - see https://github.com/brave/brave-core/pull/7067/files
| SecondRoundArtifacts::~SecondRoundArtifacts() {} | ||
|
|
||
| ChallengeArtifacts ChallengeFirstRound(std::string server_pk_str) { | ||
| // TODO(@gpestana): finish |
| const std::string api_version) { | ||
| std::string url; | ||
|
|
||
| // @gpestana(TODO: refactor to static values, based on env flag) |
There was a problem hiding this comment.
|
|
||
| #include <string> | ||
|
|
||
| namespace request_utils { |
There was a problem hiding this comment.
namespaces should match the component name
|
|
||
| #include <string> | ||
|
|
||
| std::string convert_to_str_array(const uint8_t* ptr, int size); |
There was a problem hiding this comment.
this should go in a namespace
| @@ -202,6 +207,13 @@ bool BraveReferralsService::GetMatchingReferralHeaders( | |||
| } | |||
|
|
|||
| void BraveReferralsService::OnFinalizationChecksTimerFired() { | |||
There was a problem hiding this comment.
can this be called more than once? I don't think so, just want to double-check
There was a problem hiding this comment.
either way we may want to protect against calling PerformReferralAttestation more than once inside PrivateChannel unless you're sure it won't cause a problem if it gets called during or after a previous call
gpestana
force-pushed
the
private_channels
branch
from
4ec731a
to
216375d
Compare
gpestana
force-pushed
the
private_channels
branch
from
216375d
to
e824023
Compare
|
this one looks stale, mb we could close @gpestana ? |
|
@iefremov let's close it for now and re-open in the future if needed. thanks! |
Implements the client-side of the two-party computation protocol for privacy-preserving attestation.
This PR replaces #5161
Submitter Checklist:
npm run lint)git rebase master(if needed).git rebase -ito squash commits (if needed).Test Plan:
Reviewer Checklist:
After-merge Checklist:
changes has landed on.