diff --git a/.github/workflows/pull_request.yml b/.github/workflows/pull_request.yml index 328418e..95ec339 100644 --- a/.github/workflows/pull_request.yml +++ b/.github/workflows/pull_request.yml @@ -62,7 +62,7 @@ jobs: - name: Check formatting run: cargo fmt --all --check - name: Check Clippy - run: cargo clippy --workspace --all-features --examples -- -D warnings + run: cargo clippy --workspace --all-features --examples --lib -- -D warnings docs: diff --git a/.gitignore b/.gitignore index 6fc6951..1e83165 100644 --- a/.gitignore +++ b/.gitignore @@ -1,5 +1,5 @@ target -profile.json* +profile*.json* perf.data* .flyio diff --git a/Cargo.toml b/Cargo.toml index 0dc6290..d68f55a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -57,3 +57,6 @@ inherits = "release" codegen-units = 1 inherits = "release" lto = "fat" + +[workspace.lints.clippy] +unwrap_used = "warn" diff --git a/cryprot-codes/Cargo.toml b/cryprot-codes/Cargo.toml index 12bcbb8..6a42df0 100644 --- a/cryprot-codes/Cargo.toml +++ b/cryprot-codes/Cargo.toml @@ -13,6 +13,9 @@ repository.workspace = true bench-libote = ["dep:libote-codes"] libote-compat = ["dep:libote-codes"] +[lints] +workspace = true + [lib] bench = false diff --git a/cryprot-codes/src/ex_conv.rs b/cryprot-codes/src/ex_conv.rs index 823f8ad..d64e918 100644 --- a/cryprot-codes/src/ex_conv.rs +++ b/cryprot-codes/src/ex_conv.rs @@ -269,7 +269,7 @@ impl ExConvCode { #[cfg(debug_assertions)] for (i, bb) in bb.iter().enumerate() { let exp = if ((b >> i) & 1) != 0 { xi } else { Block::ZERO }; - debug_assert_eq!(exp, bytemuck::cast(*bb)) + debug_assert_eq!(exp, bytemuck::cast(*bb)); } seq!(N in 0..8 { diff --git a/cryprot-core/Cargo.toml b/cryprot-core/Cargo.toml index e375f84..182c09d 100644 --- a/cryprot-core/Cargo.toml +++ b/cryprot-core/Cargo.toml @@ -9,6 +9,9 @@ version = "0.2.0" authors.workspace = true repository.workspace = true +[lints] +workspace = true + [lib] bench = false diff --git a/cryprot-core/src/aes_hash.rs b/cryprot-core/src/aes_hash.rs index 698b0f1..45f5693 100644 --- a/cryprot-core/src/aes_hash.rs +++ b/cryprot-core/src/aes_hash.rs @@ -85,7 +85,7 @@ impl AesHash { for chunk in x.chunks_mut(AES_PAR_BLOCKS) { self.aes .encrypt_blocks_b2b(bytemuck::cast_slice(chunk), &mut tmp[..chunk.len()]) - .unwrap(); + .expect("in and out always have same length"); chunk .iter_mut() .zip(tmp) @@ -101,7 +101,7 @@ impl AesHash { for (chunk_idx, chunk) in x.chunks_mut(AES_PAR_BLOCKS).enumerate() { self.aes .encrypt_blocks_b2b(bytemuck::cast_slice(chunk), &mut tmp[..chunk.len()]) - .unwrap(); + .expect("in and out always have same length"); chunk .iter_mut() .zip(&mut tmp) diff --git a/cryprot-core/src/block/gf128.rs b/cryprot-core/src/block/gf128.rs index 6c10aa5..0f0d1eb 100644 --- a/cryprot-core/src/block/gf128.rs +++ b/cryprot-core/src/block/gf128.rs @@ -117,6 +117,11 @@ mod clmul { #[target_feature(enable = "pclmulqdq")] #[inline] pub unsafe fn clmul128(a: __m128i, b: __m128i) -> (__m128i, __m128i) { + // This is currently needed because we run the nightly version of + // clippy where this is an unused unsafe because the used + // intrinsincs have been marked safe on nightly but not yet on + // stable. + #[allow(unused_unsafe)] unsafe { // NOTE: I tried using karatsuba but it was slightly slower than the naive // multiplication diff --git a/cryprot-core/src/transpose/avx2.rs b/cryprot-core/src/transpose/avx2.rs index 53c8a82..5a32716 100644 --- a/cryprot-core/src/transpose/avx2.rs +++ b/cryprot-core/src/transpose/avx2.rs @@ -1,8 +1,14 @@ +//! Implementation of AVX2 BitMatrix transpose based on libOTe. use std::{arch::x86_64::*, hint::unreachable_unchecked}; #[inline] #[target_feature(enable = "avx2")] +/// Must be called with `matches!(shift, 2 | 4 | 8 | 16 | 32)` unsafe fn _mm256_slli_epi64_var_shift(a: __m256i, shift: usize) -> __m256i { + debug_assert!( + matches!(shift, 2 | 4 | 8 | 16 | 32), + "Must be called with correct shift" + ); unsafe { match shift { 2 => _mm256_slli_epi64::<2>(a), @@ -17,7 +23,12 @@ unsafe fn _mm256_slli_epi64_var_shift(a: __m256i, shift: usize) -> __m256i { #[inline] #[target_feature(enable = "avx2")] +/// Must be called with `matches!(shift, 2 | 4 | 8 | 16 | 32)` unsafe fn _mm256_srli_epi64_var_shift(a: __m256i, shift: usize) -> __m256i { + debug_assert!( + matches!(shift, 2 | 4 | 8 | 16 | 32), + "Must be called with correct shift" + ); unsafe { match shift { 2 => _mm256_srli_epi64::<2>(a), @@ -43,8 +54,8 @@ unsafe fn avx_transpose_block_iter1( ) { if j < (1 << block_size_shift) && block_size_shift == 6 { unsafe { - let x = &mut *in_out.add(j / 2); - let y = &mut *in_out.add(j / 2 + 32); + let x = in_out.add(j / 2); + let y = in_out.add(j / 2 + 32); let out_x = _mm256_unpacklo_epi64(*x, *y); let out_y = _mm256_unpackhi_epi64(*x, *y); @@ -65,8 +76,8 @@ unsafe fn avx_transpose_block_iter1( } unsafe { - let x = &mut *in_out.add(j / 2); - let y = &mut *in_out.add(j / 2 + (1 << (block_size_shift - 1))); + let x = in_out.add(j / 2); + let y = in_out.add(j / 2 + (1 << (block_size_shift - 1))); // Special case for 2x2 blocks (block_size_shift == 1) if block_size_shift == 1 { @@ -141,9 +152,12 @@ unsafe fn avx_transpose_block( const AVX_BLOCK_SHIFT: usize = 4; const AVX_BLOCK_SIZE: usize = 1 << AVX_BLOCK_SHIFT; -// Main entry point for matrix transpose +/// Transpose 128x128 bit matrix using AVX2. +/// +/// # Safety +/// AVX2 needs to be enabled. #[target_feature(enable = "avx2")] -pub unsafe fn avx_transpose128x128(in_out: &mut [__m256i; 64]) { +pub fn avx_transpose128x128(in_out: &mut [__m256i; 64]) { const MAT_SIZE_SHIFT: usize = 7; unsafe { let in_out = in_out.as_mut_ptr(); @@ -188,6 +202,7 @@ pub unsafe fn transpose_bitmatrix(input: &[u8], output: &mut [u8], rows: usize) let cols = input.len() * 8 / rows; assert_eq!(0, cols % 128); assert_eq!(0, rows % 128); + #[allow(unused_unsafe)] let mut buf = [unsafe { _mm256_setzero_si256() }; 64]; let in_stride = cols / 8; let out_stride = rows / 8; @@ -210,11 +225,8 @@ pub unsafe fn transpose_bitmatrix(input: &[u8], output: &mut [u8], rows: usize) std::ptr::copy_nonoverlapping(src_row, buf_u8_ptr.add(k * 16), 16); } } - // SAFETY: avx2 is enabled - unsafe { - // Transpose the 128x128 bit square - avx_transpose128x128(&mut buf); - } + // Transpose the 128x128 bit square + avx_transpose128x128(&mut buf); unsafe { // needs to be recreated because prev &mut borrow invalidates ptr diff --git a/cryprot-net/Cargo.toml b/cryprot-net/Cargo.toml index ae14a7d..9a91e36 100644 --- a/cryprot-net/Cargo.toml +++ b/cryprot-net/Cargo.toml @@ -13,6 +13,9 @@ repository.workspace = true __testing = ["dep:anyhow", "metrics"] metrics = ["dep:tracing-subscriber", "serde/derive"] +[lints] +workspace = true + [dependencies] anyhow = { workspace = true, optional = true } bincode.workspace = true diff --git a/cryprot-ot/Cargo.toml b/cryprot-ot/Cargo.toml index 9a30729..b074d13 100644 --- a/cryprot-ot/Cargo.toml +++ b/cryprot-ot/Cargo.toml @@ -9,6 +9,9 @@ version = "0.2.0" authors.workspace = true repository.workspace = true +[lints] +workspace = true + [lib] # https://bheisler.github.io/criterion.rs/book/faq.html#cargo-bench-gives-unrecognized-option-errors-for-valid-command-line-options bench = false diff --git a/cryprot-ot/src/adapter.rs b/cryprot-ot/src/adapter.rs index 55570c2..ba77f4d 100644 --- a/cryprot-ot/src/adapter.rs +++ b/cryprot-ot/src/adapter.rs @@ -1,10 +1,13 @@ //! Adapters for OT types. +use std::io; + use bitvec::{order::Lsb0, vec::BitVec}; use cryprot_core::{Block, buf::Buf}; use cryprot_net::ConnectionError; use futures::{SinkExt, StreamExt}; use subtle::ConditionallySelectable; +use thiserror::Error; use tokio::io::{AsyncReadExt, AsyncWriteExt}; use crate::{ @@ -38,8 +41,20 @@ impl SemiHonest for ChosenChoice

{} // TODO is there something I can cite that this holds? impl Malicious for ChosenChoice

{} +#[derive(Error, Debug)] +pub enum Error { + #[error("unable to perform R-OTs")] + Rot(E), + #[error("error in sending correction values for C-OT")] + Correction(io::Error), + #[error("expected correction values but receiver is closed")] + MissingCorrection, + #[error("connection error to peer")] + Connecion(#[from] ConnectionError), +} + impl RotReceiver for ChosenChoice { - type Error = R::Error; + type Error = Error; async fn receive_into( &mut self, @@ -50,30 +65,33 @@ impl RotReceiver for ChosenChoice { .0 .rand_choice_receive_into(ots) .await - .map_err(|_| ()) - .unwrap(); + .map_err(Error::Rot)?; for (c1, c2) in rand_choices.iter_mut().zip(choices) { *c1 ^= *c2; } let mut bv: BitVec = BitVec::with_capacity(choices.len()); bv.extend(rand_choices.iter().map(|c| c.unwrap_u8() != 0)); - let (mut tx, _) = self.connection().stream().await.unwrap(); - tx.send(bv).await.unwrap(); + let (mut tx, _) = self.connection().stream().await?; + tx.send(bv).await.map_err(Error::Correction)?; Ok(()) } } impl RotSender for ChosenChoice { - type Error = S::Error; + type Error = Error; async fn send_into( &mut self, ots: &mut impl cryprot_core::buf::Buf<[cryprot_core::Block; 2]>, ) -> Result<(), Self::Error> { - self.0.send_into(ots).await.map_err(|_| ()).unwrap(); - let (_, mut rx) = self.connection().stream().await.unwrap(); - let correction: BitVec = rx.next().await.unwrap().unwrap(); + self.0.send_into(ots).await.map_err(Error::Rot)?; + let (_, mut rx) = self.connection().stream().await?; + let correction: BitVec = rx + .next() + .await + .ok_or(Error::MissingCorrection)? + .map_err(Error::Correction)?; for (ots, c_bit) in ots.iter_mut().zip(correction) { let tmp = *ots; diff --git a/cryprot-ot/src/extension.rs b/cryprot-ot/src/extension.rs index 8910ed3..951ab38 100644 --- a/cryprot-ot/src/extension.rs +++ b/cryprot-ot/src/extension.rs @@ -94,6 +94,9 @@ pub enum Error { #[error("Commitment does not match seed")] WrongCommitment, /// Only possible for malicious variant. + #[error("sender did not receiver x value in KOS check")] + MissingXValue, + /// Only possible for malicious variant. #[error("malicious check failed")] MaliciousCheck, #[doc(hidden)] @@ -195,8 +198,7 @@ impl RotSender for OtExtensionSender { "count % batch_size must be multiple of 128" ); - let batch_sizes = iter::repeat(batch_size) - .take(batches) + let batch_sizes = iter::repeat_n(batch_size, batches) .chain((batch_size_remainder != 0).then_some(batch_size_remainder)); if !self.has_base_ots() { @@ -344,7 +346,7 @@ impl RotSender for OtExtensionSender { }); for ((v, s), q_idx) in owned_v_mat_ref.iter().zip(challenge_iter).zip(q_idx_iter) { - let (qi, qi2) = v.clmul(&s); + let (qi, qi2) = v.clmul(s); q1[q_idx] ^= qi; q2[q_idx] ^= qi2; } @@ -352,8 +354,10 @@ impl RotSender for OtExtensionSender { for (q1i, q2i) in q1.iter_mut().zip(&q2) { *q1i = Block::gf_reduce(q1i, q2i); } - let mut u = kos_ch_r.recv().unwrap(); - let received_x = u.pop().unwrap(); + let mut u = kos_ch_r.recv()?; + let Some(received_x) = u.pop() else { + return Err(Error::MissingXValue); + }; for ((received_t, base_choice), q1i) in u.iter().zip(&base_choices).zip(&q1) { let tt = Block::conditional_select(&Block::ZERO, &received_x, *base_choice) ^ *q1i; @@ -392,8 +396,10 @@ impl RotSender for OtExtensionSender { { let mut kos_recv = kos_recv.as_stream(); - let u = kos_recv.next().await.unwrap().unwrap(); - kos_ch_s_task.send(u).unwrap(); + let u = kos_recv.next().await.ok_or(Error::UnexcpectedClose)??; + if kos_ch_s_task.send(u).is_err() { + break 'success false; + } } true @@ -403,7 +409,10 @@ impl RotSender for OtExtensionSender { } } - let (owned_ots, base_rngs, base_choices) = jh.await.expect("panic in worker thread")?; + let (owned_ots, base_rngs, base_choices) = match jh.await { + Ok(res) => res?, + Err(panicked) => resume_unwind(panicked), + }; self.base_rngs = base_rngs; self.base_choices = base_choices; *ots = owned_ots; @@ -606,8 +615,7 @@ impl RotReceiver for OtExtensionReceiver { let t_mat_ref = &t_mat; let batches = count / batch_size; - let batch_sizes = iter::repeat(batch_size) - .take(batches) + let batch_sizes = iter::repeat_n(batch_size, batches) .chain((batch_size_remainder != 0).then_some(batch_size_remainder)); let choice_blocks: Vec<_> = choice_vec @@ -646,7 +654,7 @@ impl RotReceiver for OtExtensionReceiver { }); for ((t, s), t_idx) in t_mat_ref.iter().zip(challenge_iter).zip(t_idx_iter) { - let (ti, ti2) = t.clmul(&s); + let (ti, ti2) = t.clmul(s); t1[t_idx] ^= ti; t2[t_idx] ^= ti2; } @@ -655,7 +663,7 @@ impl RotReceiver for OtExtensionReceiver { *t1i = Block::gf_reduce(t1i, t2i); } t1.push(Block::gf_reduce(&x1, &x2)); - kos_ch_s.send(t1).unwrap(); + kos_ch_s.send(t1)?; } Ok::<_, Error>((ots, base_rngs)) }); @@ -702,7 +710,10 @@ impl RotReceiver for OtExtensionReceiver { } } - let (owned_ots, base_rngs) = jh.await.expect("panic in worker thread")?; + let (owned_ots, base_rngs) = match jh.await { + Ok(res) => res?, + Err(panicked) => resume_unwind(panicked), + }; self.base_rngs = base_rngs; *ots = owned_ots; diff --git a/cryprot-ot/src/lib.rs b/cryprot-ot/src/lib.rs index 8efd5f9..8766c99 100644 --- a/cryprot-ot/src/lib.rs +++ b/cryprot-ot/src/lib.rs @@ -1,3 +1,4 @@ +#![warn(clippy::unwrap_used)] //! CryProt-OT implements several [oblivious transfer](https://en.wikipedia.org/wiki/Oblivious_transfer) protocols. //! //! - base OT: "Simplest OT" [[CO15](https://eprint.iacr.org/2015/267)] diff --git a/cryprot-pprf/Cargo.toml b/cryprot-pprf/Cargo.toml index e43d4d9..d31f655 100644 --- a/cryprot-pprf/Cargo.toml +++ b/cryprot-pprf/Cargo.toml @@ -9,6 +9,9 @@ version = "0.2.0" authors.workspace = true repository.workspace = true +[lints] +workspace = true + [dependencies] aes.workspace = true bytemuck.workspace = true diff --git a/cryprot-pprf/src/lib.rs b/cryprot-pprf/src/lib.rs index 15a21d1..5fe6894 100644 --- a/cryprot-pprf/src/lib.rs +++ b/cryprot-pprf/src/lib.rs @@ -362,7 +362,7 @@ impl RegularPprfReceiver { ) -> Result<(), Error> { let size = self.conf.size(); let mut output = mem::take(out); - let (_, mut rx) = self.conn.stream().await.unwrap(); + let (_, mut rx) = self.conn.stream().await?; let (send, recv) = std::sync::mpsc::channel(); let jh = spawn_compute(move || { if output.len() < size { @@ -386,7 +386,10 @@ impl RegularPprfReceiver { allocate_zeroed_vec(2_usize.pow(dd as u32)); for g in (0..pnt_count).step_by(PARALLEL_TREES) { - let tree_grp: TreeGrp = recv.recv().unwrap(); + let Ok(tree_grp): Result = recv.recv() else { + // Async task is dropped, so we simply return the output which will be dropped + return output; + }; assert_eq!(g, tree_grp.g); if depth > 1 {