Skip to content

Derive Hash for a bunch of types #663

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/modular/boxed_monty_form.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use zeroize::Zeroize;

/// Parameters to efficiently go to/from the Montgomery form for an odd modulus whose size and value
/// are both chosen at runtime.
#[derive(Clone, Debug, Eq, PartialEq)]
#[derive(Clone, Debug, Eq, PartialEq, Hash)]
pub struct BoxedMontyParams {
/// The constant modulus
modulus: Odd<BoxedUint>,
Expand Down Expand Up @@ -115,7 +115,7 @@ impl BoxedMontyParams {
}

/// An integer in Montgomery form represented using heap-allocated limbs.
#[derive(Clone, Debug, Eq, PartialEq)]
#[derive(Clone, Debug, Eq, PartialEq, Hash)]
pub struct BoxedMontyForm {
/// Value in the Montgomery form.
montgomery_form: BoxedUint,
Expand Down
2 changes: 1 addition & 1 deletion src/modular/boxed_monty_form/inv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ impl PrecomputeInverter for BoxedMontyParams {
}
}

/// Bernstein-Yang inverter which inverts [`DynResidue`] types.
/// Bernstein-Yang inverter which inverts [`MontyForm`] types.
pub struct BoxedMontyFormInverter {
/// Precomputed Bernstein-Yang inverter.
inverter: BoxedSafeGcdInverter,
Expand Down
2 changes: 1 addition & 1 deletion src/modular/const_monty_form.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ pub trait ConstMontyParams<const LIMBS: usize>:
/// The modulus is constant, so it cannot be set at runtime.
///
/// Internally, the value is stored in Montgomery form (multiplied by MOD::ONE) until it is retrieved.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
Copy link
Member

@tarcieri tarcieri Aug 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a little concerned about deriving Hash for potentially secret-containing types. Though the default Hasher is SipHash which is fine when properly keyed, other hashers do not necessarily prevent preimage attacks

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think documenting the caveats properly is enough? A bigint library that blocks all potential security misuses risks crippling itself a fair bit and it’s not obvious where to draw the line. I’d argue that in this case it’s expected that users know about the security hazards.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would definitely need documentation. This library is documented as being constant-time by default and we are careful to treat each integer as potentially containing a secret, with separate *_vartime methods which act in variable-time so as to make those sort of secret accesses easier to audit.

I would also want to know what a use case is for keying a HashMap with ConstMontyForm.

pub struct ConstMontyForm<MOD: ConstMontyParams<LIMBS>, const LIMBS: usize> {
montgomery_form: Uint<LIMBS>,
phantom: PhantomData<MOD>,
Expand Down
4 changes: 2 additions & 2 deletions src/modular/monty_form.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use crate::{Concat, Limb, Monty, NonZero, Odd, Split, Uint, Word};
use subtle::{Choice, ConditionallySelectable, ConstantTimeEq};

/// Parameters to efficiently go to/from the Montgomery form for an odd modulus provided at runtime.
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
pub struct MontyParams<const LIMBS: usize> {
/// The constant modulus
modulus: Odd<Uint<LIMBS>>,
Expand Down Expand Up @@ -145,7 +145,7 @@ impl<const LIMBS: usize> ConstantTimeEq for MontyParams<LIMBS> {

/// An integer in Montgomery form represented using `LIMBS` limbs.
/// The odd modulus is set at runtime.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto here re: potentially secret-containing types

pub struct MontyForm<const LIMBS: usize> {
montgomery_form: Uint<LIMBS>,
params: MontyParams<LIMBS>,
Expand Down
2 changes: 1 addition & 1 deletion src/uint/div_limb.rs
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ pub(crate) const fn div3by2(
}

/// A pre-calculated reciprocal for division by a single limb.
#[derive(Copy, Clone, Debug, PartialEq, Eq)]
#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)]
pub struct Reciprocal {
divisor_normalized: Word,
shift: u32,
Expand Down
2 changes: 1 addition & 1 deletion src/wrapping.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use serdect::serde::{Deserialize, Deserializer, Serialize, Serializer};
///
/// This is analogous to [`core::num::Wrapping`] but allows this crate to
/// define trait impls for this type.
#[derive(Copy, Clone, Debug, Default, Eq, PartialEq, PartialOrd, Ord)]
#[derive(Copy, Clone, Debug, Default, Eq, PartialEq, PartialOrd, Ord, Hash)]
pub struct Wrapping<T>(pub T);

impl<T: WrappingAdd> Add<Self> for Wrapping<T> {
Expand Down