Skip to content

Commit

Permalink
Don't have the Helioselene Ciphersuite use the provided read_G
Browse files Browse the repository at this point in the history
The provided read_G compares the point's serialization against its
reserialization to check if the encoding was malleated. helioselene already
checks the point's encoded x coordinate is reduced and the point's sign bit
isn't for the representation of identity, preventing malleation. We accordingly
don't need this additional check which is expensive.
  • Loading branch information
kayabaNerve committed Jan 6, 2025
1 parent 914b80b commit 7d972e2
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 2 deletions.
32 changes: 30 additions & 2 deletions crypto/ciphersuite/src/helioselene.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
#[cfg(any(feature = "alloc", feature = "std"))]
use std_shims::io::{self, Read};

use zeroize::Zeroize;

use blake2::{Digest, Blake2b512};

use group::Group;

use group::{Group, GroupEncoding};
use helioselene::{Field25519, HelioseleneField, HeliosPoint, SelenePoint};

use crate::Ciphersuite;
Expand Down Expand Up @@ -37,6 +39,19 @@ impl Ciphersuite for Helios {
uniform.zeroize();
res
}

// We override the provided impl, which compares against the reserialization, because
// Helios::G::from_bytes already enforces canonically encoded points
#[cfg(any(feature = "alloc", feature = "std"))]
#[allow(non_snake_case)]
fn read_G<R: Read>(reader: &mut R) -> io::Result<Self::G> {
let mut encoding = <Self::G as GroupEncoding>::Repr::default();
reader.read_exact(encoding.as_mut())?;

let point = Option::<Self::G>::from(Self::G::from_bytes(&encoding))
.ok_or_else(|| io::Error::new(io::ErrorKind::Other, "invalid point"))?;
Ok(point)
}
}

#[derive(Clone, Copy, PartialEq, Eq, Debug, Zeroize)]
Expand Down Expand Up @@ -68,6 +83,19 @@ impl Ciphersuite for Selene {
uniform.zeroize();
res
}

// We override the provided impl, which compares against the reserialization, because
// Selene::G::from_bytes already enforces canonically encoded points
#[cfg(any(feature = "alloc", feature = "std"))]
#[allow(non_snake_case)]
fn read_G<R: Read>(reader: &mut R) -> io::Result<Self::G> {
let mut encoding = <Self::G as GroupEncoding>::Repr::default();
reader.read_exact(encoding.as_mut())?;

let point = Option::<Self::G>::from(Self::G::from_bytes(&encoding))
.ok_or_else(|| io::Error::new(io::ErrorKind::Other, "invalid point"))?;
Ok(point)
}
}

#[test]
Expand Down
3 changes: 3 additions & 0 deletions crypto/ciphersuite/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,9 @@ pub trait Ciphersuite:
}

/// Read a canonical point from something implementing std::io::Read.
///
/// The provided implementation is safe so long as `GroupEncoding::to_bytes` always returns a
/// canonical serialization.
#[cfg(any(feature = "alloc", feature = "std"))]
#[allow(non_snake_case)]
fn read_G<R: Read>(reader: &mut R) -> io::Result<Self::G> {
Expand Down

0 comments on commit 7d972e2

Please sign in to comment.