Skip to content

Commit 857f6de

Browse files
authored
elliptic-curve: remove insecure BatchInvert API (#1865)
Implementing the idea I had in #1864 (comment), which in hindsight I actually think is better. The new implementation actually *should* take ownership instead of just cloning *for* the user. See [C-CALLER-CONTROL API guidelines](https://rust-lang.github.io/api-guidelines/flexibility.html?highlight=clone#caller-decides-where-to-copy-and-place-data-c-caller-control). This has the advantage of getting rid of the insecure `batch_invert_mut` API where users have to make sure to actually check the returned `Choice` and discard the input slice. I could also quickly add the following for even more control if desirable: ```rust #[cfg(feature = "alloc")] impl<'this, T> BatchInvert<&'this mut [Self]> for T where T: Field, { type Output = &'this mut [Self]; fn batch_invert(field_elements: &'this mut [Self]) -> CtOption<&'this mut [Self]> { let mut field_elements_pad: Vec<Self> = vec![Self::default(); field_elements.len()]; let inversion_succeeded = invert_batch_internal(field_elements, &mut field_elements_pad); CtOption::new(field_elements, inversion_succeeded) } } ```
1 parent b01c581 commit 857f6de

File tree

1 file changed

+36
-24
lines changed

1 file changed

+36
-24
lines changed

elliptic-curve/src/ops.rs

Lines changed: 36 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -19,18 +19,8 @@ pub trait BatchInvert<FieldElements: ?Sized>: Field + Sized {
1919

2020
/// Invert a batch of field elements.
2121
fn batch_invert(
22-
field_elements: &FieldElements,
22+
field_elements: FieldElements,
2323
) -> CtOption<<Self as BatchInvert<FieldElements>>::Output>;
24-
25-
/// Invert a batch of field elements in-place.
26-
///
27-
/// # ⚠️ Warning
28-
///
29-
/// Even though `field_elements` is modified regardless of success, on failure it does not
30-
/// contain correctly inverted scalars and should be discarded instead.
31-
///
32-
/// Consider using [`Self::batch_invert()`] instead.
33-
fn batch_invert_mut(field_elements: &mut FieldElements) -> Choice;
3424
}
3525

3626
impl<const N: usize, T> BatchInvert<[T; N]> for T
@@ -39,38 +29,60 @@ where
3929
{
4030
type Output = [Self; N];
4131

42-
fn batch_invert(field_elements: &[Self; N]) -> CtOption<[Self; N]> {
43-
let mut field_elements_inverses = *field_elements;
44-
let inversion_succeeded = Self::batch_invert_mut(&mut field_elements_inverses);
32+
fn batch_invert(mut field_elements: [Self; N]) -> CtOption<[Self; N]> {
33+
let mut field_elements_pad = [Self::default(); N];
34+
let inversion_succeeded =
35+
invert_batch_internal(&mut field_elements, &mut field_elements_pad);
4536

46-
CtOption::new(field_elements_inverses, inversion_succeeded)
37+
CtOption::new(field_elements, inversion_succeeded)
4738
}
39+
}
4840

49-
fn batch_invert_mut(field_elements: &mut [T; N]) -> Choice {
50-
let mut field_elements_pad = [Self::default(); N];
41+
#[cfg(feature = "alloc")]
42+
impl<'this, T> BatchInvert<&'this mut [Self]> for T
43+
where
44+
T: Field,
45+
{
46+
type Output = &'this mut [Self];
47+
48+
fn batch_invert(field_elements: &'this mut [Self]) -> CtOption<&'this mut [Self]> {
49+
let mut field_elements_pad: Vec<Self> = vec![Self::default(); field_elements.len()];
50+
let inversion_succeeded = invert_batch_internal(field_elements, &mut field_elements_pad);
5151

52-
invert_batch_internal(field_elements, &mut field_elements_pad)
52+
CtOption::new(field_elements, inversion_succeeded)
5353
}
5454
}
5555

5656
#[cfg(feature = "alloc")]
57-
impl<T> BatchInvert<[T]> for T
57+
impl<T> BatchInvert<&[Self]> for T
5858
where
5959
T: Field,
6060
{
6161
type Output = Vec<Self>;
6262

6363
fn batch_invert(field_elements: &[Self]) -> CtOption<Vec<Self>> {
64-
let mut field_elements_inverses: Vec<Self> = field_elements.to_owned();
65-
let inversion_succeeded = Self::batch_invert_mut(field_elements_inverses.as_mut_slice());
64+
let mut field_elements: Vec<Self> = field_elements.to_owned();
65+
let mut field_elements_pad: Vec<Self> = vec![Self::default(); field_elements.len()];
66+
let inversion_succeeded =
67+
invert_batch_internal(&mut field_elements, &mut field_elements_pad);
6668

67-
CtOption::new(field_elements_inverses, inversion_succeeded)
69+
CtOption::new(field_elements, inversion_succeeded)
6870
}
71+
}
72+
73+
#[cfg(feature = "alloc")]
74+
impl<T> BatchInvert<Vec<Self>> for T
75+
where
76+
T: Field,
77+
{
78+
type Output = Vec<Self>;
6979

70-
fn batch_invert_mut(field_elements: &mut [T]) -> Choice {
80+
fn batch_invert(mut field_elements: Vec<Self>) -> CtOption<Vec<Self>> {
7181
let mut field_elements_pad: Vec<Self> = vec![Self::default(); field_elements.len()];
82+
let inversion_succeeded =
83+
invert_batch_internal(&mut field_elements, &mut field_elements_pad);
7284

73-
invert_batch_internal(field_elements, field_elements_pad.as_mut())
85+
CtOption::new(field_elements, inversion_succeeded)
7486
}
7587
}
7688

0 commit comments

Comments
 (0)