Protocol context is a trait #253

akoshelev · 2022-11-18T07:02:09Z

This change makes ProtocolContext a trait and implements it for two newly created structs that represent semi-honest and malicious contexts.

Each protocol can decide whether it supports semi-honest/malicious security model or both by injecting SemiHonestProtocolContext, MaliciousProtocolContext or generic type parameter C: ProtocolContext<F> accordingly.

There are a couple of important changes to the context API:

narrow no longer enforces "don't narrow to the same step twice" rule. There were two reasons for having this rule.
- Don't use the same shared randomness more than once. Validate indices used in PRSS #208 protects against it
- Make sure every communication is uniquely addressable by record_id + step. If someone narrows to the same step more than once and uses the same record id to communicate, Infra is in trouble. It won't be able to resolve these two communications and likely apply last-write-wins which is obviously not correct here. It is also mitigated because of acceptable record ranges introduced in Range of accepted records inside the send buffer #183. Now if protocol attempts to send a record outside of valid range, it will be caught. If it attempts to send the same record twice within the acceptable range, fsv will panic.
bind is gone. No need to explicitly bind context to a record because of the above

Need to clean up the following * Remove `bind` * Relax `narrow` * Introduce a small context struct that clone will refer to

What is left: * Introduce a small context struct that clone will refer to

akoshelev · 2022-11-18T07:02:58Z

src/protocol/context/malicious.rs

@@ -0,0 +1,102 @@
+use crate::ff::Field;


moved context implementations to a separate folder

akoshelev · 2022-11-18T07:23:28Z

src/protocol/context/malicious.rs

+/// in 3 party MPC ring.
+#[derive(Clone, Debug)]
+pub struct MaliciousProtocolContext<'a, F: Field> {
+    inner: ContextInner<'a>,


@martinthomson I remember we were talking about using references here, but I couldn't find a good way to do it. there must be something that holds onto this inner struct for the duration of protocol execution. It didn't seem very ergonomic but I could be missing something

I think the idea was to collect the static components of the context (role, prss, gateway) into GlobalContext so that a single &GlobalContext needs to be duplicated each time the context is refined, rather than multiple items.

This is something that I was planning to look at, but I haven't done anything with it yet. Hopefully it can be done in a way that doesn't change the context API.

With #261 I think we can make it ergonomic to use because we can just stitch global contexts inside test world and create contexts on demand. For real world implementation I am not sure if we can make it better than just holding onto both global context and protocol context

akoshelev · 2022-11-18T07:31:24Z

src/protocol/context/mod.rs

+    fn mesh(&self) -> Mesh<'_, '_>;
+
+    /// Generates a new share of one
+    fn share_of_one(&self) -> <Self as ProtocolContext<F>>::Share;


share_of_one is now defined inside protocol context trait. That allows malicious context to generate it without exposing the internal state (r_share)

akoshelev · 2022-11-18T07:33:27Z

src/protocol/mod.rs

@@ -115,18 +83,10 @@ impl Step {
        {
            let s = String::from(step.as_ref());
            assert!(!s.contains('/'), "The string for a step cannot contain '/'");
-            assert!(


step validation is gone

akoshelev · 2022-11-18T07:33:47Z

src/protocol/modulus_conversion/specialized_mul.rs

@@ -215,19 +216,19 @@ pub mod tests {

            let result_shares = try_join_all([
                multiply_two_shares_mostly_zeroes(
-                    context[0].bind(record_id),
+                    context[0].clone(),


no more bind calls

akoshelev · 2022-11-18T07:35:55Z

src/protocol/sort/bit_permutation.rs

@@ -33,13 +32,15 @@ use futures::future::try_join_all;
 ///
 /// ## Errors
 /// It will propagate errors from multiplication protocol.
-pub async fn bit_permutation<'a, F: Field, S: SecretSharing<F>>(
-    ctx: ProtocolContext<'a, S, F>,
+pub async fn bit_permutation<


this is how the protocols would look like if they can work in semi-honest and malicious setting.

Note that there is no more SecureMul constraint because it is a supertrait for ProtocolContext.

That's really nice. Thanks for eliminating those constraints!

akoshelev · 2022-11-18T07:36:26Z

src/protocol/sort/reshare.rs

@@ -36,7 +37,7 @@ impl<F: Field> Reshare<F> {
    ///    `to_helper.right` = (`rand_right`, part1 + part2) = (r0, part1 + part2)
    pub async fn execute(
        self,
-        ctx: &ProtocolContext<'_, Replicated<F>, F>,
+        ctx: &SemiHonestProtocolContext<'_, F>,


this probably needs to be fixed later

You mean the fact that it's a reference and not an actual object? @richajaindce

I changed it in #257 . Will rebase on this PR once we are happy with it

akoshelev · 2022-11-18T07:37:17Z

src/test_fixture/mod.rs

@@ -67,10 +70,10 @@ pub fn make_malicious_contexts<F: Field>(test_world: &TestWorld) -> [MaliciousCo
 /// # Panics
 /// Never, but then Rust doesn't know that; this is only needed because we don't have `each_ref()`.
 #[must_use]
-pub fn narrow_contexts<'a, F: Field, S: SecretSharing<F>>(
-    contexts: &[ProtocolContext<'a, S, F>; 3],
+pub fn narrow_contexts<C: Debug + ProtocolContext<F, Share = S>, F: Field, S: SecretSharing<F>>(


Debug because there is a call to unwrap inside this function

It's good that ProtocolContext is not Debug in the normal case. It's OK to be Debug for tests, but we shouldn't need it for real code.

akoshelev · 2022-11-18T07:41:25Z

src/protocol/context/mod.rs

+
+/// Context used by each helper to perform secure computation. Provides access to shared randomness
+/// generator and communication channel.
+pub trait ProtocolContext<F: Field>:


no more bind

At the start of the meeting yesterday, @martinthomson and I were discussing checking for bound contexts using the type system. main...andyleiserson:ipa:context2 has some changes I was working on in that direction. I was thinking it might make sense to get rid of the separate record_id parameter and just pass it via the context.

I had also worked on some changes to enforce one-time use restrictions. Those are in andyleiserson/ipa@context2...andyleiserson:ipa:context3.

Hmm, with this change we don't need record_id inside the context, unless we want to change the way how we multiply shares. Currently it is ctx.multiply(record_id, a, b), do we want ctx.multiply(a, b)?

I kinda like what Andy has done here. The ctx.multiply(a, b) would be ctx.bind(record).multiply(a, b), which seems like a solid approach, though it looks like ctx.multiply(record, a, b) also works fine.

into_parts() taking a reference seems unfortunate; it would be good to understand what parts of that don't work; is it that these functions also narrow()? It seems like you might want something on which you can narrow only in addition to the usable bits if that is the case.

The ergonomics of into_parts leaves a little to be desired. It's not awful, but it loses some of the simplicity.

into_parts() taking a reference seems unfortunate

Agreed, this was temporary so that I didn't have to update all references to the context all at once. I would want to fix it before merging anything.

It seems like you might want something on which you can narrow only in addition to the usable bits if that is the case.

Also agreed, this was on my to do list.

I am a little hesitant because ctx.multiply(a, b) syntax invites us to solve the problem "how to bind once", while ctx.multiply(record, a, b) does not.

By "how to bind once", do you mean keeping track of whether the context is already bound and issuing a warning if it is, similar to the "context narrowed more than once" warning that is removed by this PR?

My proposal is to keep track of that with the type system. So if it compiles, the context is not bound more than once. The benefit of this is there's less chance of a bug due to passing a wrong or inconsistent record_id somewhere.

I guess I am debating whether we need compile-time check vs the runtime check that already exists. There are some really nice things we can do with bounded contexts - for example implement multiplication and reveal for contexts that carry record_id , so nobody will be able to call multiply on Context<.., AnyRecord>. On the other hand, it requires some machinery and makes code a bit harder to understand? Let me add this topic to the agenda for the Thursday sync-up - there are some really nice things we can do with the model you proposed.

akoshelev · 2022-11-18T08:21:55Z

I think tests from #241 now can be enabled - I would expect Infra to panic if someone tries to send the same message twice. I can also add a test for that (Friday PT).

I also didn't update the panic message inside fsv to make it clear what went wrong for protocol authors.

martinthomson

This is definitely an improvement, but I think that we need to iterate on the structure of the Context implementations.

martinthomson · 2022-11-20T22:03:52Z

src/protocol/context/mod.rs

+/// Context used by each helper to perform secure computation. Provides access to shared randomness
+/// generator and communication channel.
+pub trait ProtocolContext<F: Field>:
+    Clone + SecureMul<F, Share = <Self as ProtocolContext<F>>::Share> + Reveal


Suggested change

Clone + SecureMul<F, Share = <Self as ProtocolContext<F>>::Share> + Reveal

Clone + SecureMul<F, Share = <Self as ProtocolContext<F>>::Share> + Reveal<Share = <Self as ProtocolContext<F>>::Share>

Reveal and SecureMul are different in that one takes F and the other does not. Can we work out which one we prefer? If we can avoid multiple type parameters, maybe that is better.

There was a need to make Reveal work with any field as Context<F> was used to reveal Fp2 values. It is no longer the case, so I am going to change Reveal and make it look the same as SecureMul

src/protocol/context/mod.rs

martinthomson · 2022-11-20T22:18:00Z

src/protocol/context/mod.rs

+#[derive(Clone, Debug)]
+struct ContextInner<'a> {
+    role: Role,
+    step: Step,


My thinking (see #139) had Step outside of the shared part. That is, the shared components are kept in the inner piece, but the part that needs to change is kept with each instance.

As long as the inner object is Sync, then we should be able to refer to it with Arc. Arc is just a pointer, so cloning it is as cheap as copying that and doing a relaxed atomic increment.

I can take a stab at moving step away from inner.

all right, I've opened a can of worms here. I tried to avoid Arc but I don't seem that I can do it. The most annoying thing is that we really need Arc for tests only. Arc causes double-indirection every time we need to dereference Gateway or Participant :(

Is using Arc ok for now or is there a better way to handle this? Also I think if we migrate all tests to use #261, we don't have this problem so should we push these first?

graph TD; TestWorld-->|owns|Gateway; ContextInner-->|borrows|Gateway; subgraph ephemeral Context-->|borrows|ContextInner; end Test-->|owns|Context; Test-->|owns|TestWorld;

Loading

For now, let's go with Arc, but we can mark it as needing attention later.

The extra indirection doesn't bother me too much. It might be something we can fix in production with another generic attribute, maybe with conditional compilation (Arc for tests, ~~ownership~~simple reference to Sync in production).

it is done and both contexts now use Arc. I've wasted another couple of hours trying to make MaliciousContext look like SemiHonest or, more precisely Context<F, Share = Replicated<F>>. So sad it didn't work (rust-lang/rust#20400)

src/protocol/context/mod.rs

src/protocol/context/semi_honest.rs

src/protocol/context/malicious.rs

martinthomson · 2022-11-20T22:30:44Z

src/test_fixture/mod.rs

@@ -67,10 +70,10 @@ pub fn make_malicious_contexts<F: Field>(test_world: &TestWorld) -> [MaliciousCo
 /// # Panics
 /// Never, but then Rust doesn't know that; this is only needed because we don't have `each_ref()`.
 #[must_use]
-pub fn narrow_contexts<'a, F: Field, S: SecretSharing<F>>(
-    contexts: &[ProtocolContext<'a, S, F>; 3],
+pub fn narrow_contexts<C: Debug + ProtocolContext<F, Share = S>, F: Field, S: SecretSharing<F>>(


It's good that ProtocolContext is not Debug in the normal case. It's OK to be Debug for tests, but we shouldn't need it for real code.

src/helpers/messaging.rs

benjaminsavage

This is a huge win. I can't wait to land this. Thanks @akoshelev!

src/helpers/messaging.rs

src/protocol/context/malicious.rs

benjaminsavage · 2022-11-21T02:40:04Z

src/protocol/mod.rs

@@ -3,7 +3,6 @@ mod batch;
 mod boolean;
 mod check_zero;
 pub mod context;
-pub mod context_traits;


benjaminsavage · 2022-11-21T02:41:01Z

src/protocol/mod.rs

-    /// This tracks the different values that have been provided to `narrow()`.
-    #[cfg(debug_assertions)]
-    used: Arc<Mutex<HashSet<String>>>,


benjaminsavage · 2022-11-21T02:41:29Z

src/protocol/mod.rs

@@ -115,18 +83,10 @@ impl Step {
        {
            let s = String::from(step.as_ref());
            assert!(!s.contains('/'), "The string for a step cannot contain '/'");
-            assert!(


benjaminsavage · 2022-11-21T02:46:02Z

src/protocol/sort/bit_permutation.rs

@@ -33,13 +32,15 @@ use futures::future::try_join_all;
 ///
 /// ## Errors
 /// It will propagate errors from multiplication protocol.
-pub async fn bit_permutation<'a, F: Field, S: SecretSharing<F>>(
-    ctx: ProtocolContext<'a, S, F>,
+pub async fn bit_permutation<


That's really nice. Thanks for eliminating those constraints!

benjaminsavage · 2022-11-21T02:47:32Z

src/protocol/sort/reshare.rs

@@ -36,7 +37,7 @@ impl<F: Field> Reshare<F> {
    ///    `to_helper.right` = (`rand_right`, part1 + part2) = (r0, part1 + part2)
    pub async fn execute(
        self,
-        ctx: &ProtocolContext<'_, Replicated<F>, F>,
+        ctx: &SemiHonestProtocolContext<'_, F>,


You mean the fact that it's a reference and not an actual object? @richajaindce

and make it look the same as SecureMul. We don't need this advanced feature anymore

martinthomson · 2022-11-22T23:02:05Z

Progress!

akoshelev added 6 commits November 17, 2022 22:19

Context trait

afae54f

Need to clean up the following * Remove `bind` * Relax `narrow` * Introduce a small context struct that clone will refer to

Remove bind and checks from narrow

423d80d

What is left: * Introduce a small context struct that clone will refer to

ContextInner

593da70

Formatting and Clippy

fda058a

Create folder for protocol contexts

0fdc2a7

Bring back step string validation in debug mode

7b7d3d4

akoshelev requested review from martinthomson and benjaminsavage November 18, 2022 07:02

akoshelev added 2 commits November 17, 2022 23:18

Get rid of Cow inside protocol contexts

d97a580

Delete share_of_one module

e086292

akoshelev commented Nov 18, 2022

View reviewed changes

Remove commented lines (can someone create a clippy rule for that?)

5ed71ed

akoshelev commented Nov 18, 2022

View reviewed changes

akoshelev marked this pull request as ready for review November 18, 2022 07:50

martinthomson reviewed Nov 20, 2022

View reviewed changes

src/helpers/messaging.rs Outdated Show resolved Hide resolved

benjaminsavage reviewed Nov 21, 2022

View reviewed changes

martinthomson mentioned this pull request Nov 21, 2022

Simplify test #261

Merged

akoshelev mentioned this pull request Nov 21, 2022

Reveal_permutation can now work with SecretSharing #248

Closed

akoshelev added 5 commits November 21, 2022 13:52

Remove GAT from Reveal

15cb5f1

and make it look the same as SecureMul. We don't need this advanced feature anymore

s/ProtocolContext/Context

8e57309

Fix formatting

e07835c

Specialize ContextInner for SemiHonest/Malicious impls

f5a73d8

Merge from main

84e41bf

akoshelev requested a review from martinthomson November 22, 2022 18:13

Merge from main

83f8701

martinthomson approved these changes Nov 22, 2022

View reviewed changes

martinthomson merged commit 219e283 into private-attribution:main Nov 22, 2022

andyleiserson mentioned this pull request Nov 23, 2022

tests for #135 #241

Closed

andyleiserson mentioned this pull request Nov 30, 2022

New step model rejects attempts to use the same step on multiple records #135

Closed

	Clone + SecureMul<F, Share = <Self as ProtocolContext<F>>::Share> + Reveal
	Clone + SecureMul<F, Share = <Self as ProtocolContext<F>>::Share> + Reveal<Share = <Self as ProtocolContext<F>>::Share>

Protocol context is a trait #253

Protocol context is a trait #253

Conversation

akoshelev commented Nov 18, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

akoshelev commented Nov 18, 2022

martinthomson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

martinthomson Nov 22, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

benjaminsavage left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

martinthomson commented Nov 22, 2022

martinthomson Nov 22, 2022 •

edited

Loading