Test of malicious behavior during zero check protocol #1205
Conversation
| impl<F: Fn(&MaliciousHelperContext, &mut Vec<u8>) + Send + Sync> MaliciousHelper<F> { | ||
| impl<F> MaliciousHelper<F> | ||
| where | ||
| F: Fn(MaliciousHelperContext, &mut Vec<u8>) -> Pin<Box<dyn Future<Output = ()> + Send>>, |
There was a problem hiding this comment.
Do we need this given that it is only used in non-async context? it may be better to convert non-async code to async inside this helper - non-async code is much easier to deal with
| ); | ||
|
|
||
| tracing::info!("{:?}", &rv); |
There was a problem hiding this comment.
| tracing::info!("{:?}", &rv); |
| Ok(rv == F::ZERO) | ||
| } | ||
|
|
||
| #[cfg(test)] | ||
| pub async fn check_zero_fp32bitprime<C>( |
There was a problem hiding this comment.
do you have plans to use it outside of this module or we can move it into tests?
There was a problem hiding this comment.
the same argument applies to SH1 as well
| @@ -124,4 +187,122 @@ mod tests { | |||
|
|
|||
| Ok(()) | |||
| } | |||
|
|
|||
| pub struct NotifyOnceCell<T: Clone + Send + Sync> { | |||
There was a problem hiding this comment.
how is it different from https://docs.rs/tokio/latest/tokio/sync/struct.OnceCell.html? Maybe you could elaborate that in the documentation to this struct
| } | ||
| } | ||
|
|
||
| struct MaliciousCheckZeroInterceptor { |
There was a problem hiding this comment.
lets explain what is the goal for this interceptor because it does seem very specific to check zero use case
| tracing::info!("got shares: {sh1:?} {sh2:?}"); | ||
| let adjusted_share = -sh1 - sh2; | ||
| tracing::info!("adjusted share {adjusted_share:?}"); | ||
| adjusted_share.serialize( |
There was a problem hiding this comment.
I've added serialize_to_slice and deserialize_from_slice methods to Serializable trait (only for tests). I think you may want to use them here
| }) | ||
| .await; | ||
|
|
||
| assert_eq!(res0, false, "zero check failed on H1"); |
There was a problem hiding this comment.
do you prefer this syntax over assert!(!res0, "...")?
| config.stream_interceptor = Arc::new(MaliciousCheckZeroInterceptor::new()); | ||
| let world = TestWorld::new_with(&config); | ||
| let mut rng = thread_rng(); | ||
| let v = rng.gen::<Fp32BitPrime>(); |
There was a problem hiding this comment.
Does this test benefit from using a random value or we can use a constant here to avoid false negatives?
| ) | ||
| .await?; | ||
|
|
||
| tracing::info!("reveal ({:?}): left {left:?} right {right:?} from left {share_from_left:?} from right {share_from_right:?}", ctx.role()); |
There was a problem hiding this comment.
that will spam the output I think similarly to another info! trace above. Lets change it to trace! or remove
|
What I haven't realized is that this test demonstrates that a malicious helper can break the security by making other helpers accept the result of check zero protocol even when H1 is malicious, H2, H3 are honest helpers executing
At this point, H1 can craft the response for multiplication to H2 that is still waiting and make the resulting share |
Test case related to #1204 -- will post more detail to the issue.