diff --git a/Cargo.lock b/Cargo.lock index f40fc072..0a779193 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3407,9 +3407,9 @@ dependencies = [ [[package]] name = "subtle" -version = "2.3.0" +version = "2.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "343f3f510c2915908f155e94f17220b19ccfacf2a64a2a5d8004f2c3e311e7fd" +checksum = "1e81da0851ada1f3e9d4312c704aa4f8806f0f9d69faaf8df2f3464b4a9437c2" [[package]] name = "syn" @@ -4442,6 +4442,7 @@ dependencies = [ "serde-big-array", "sha2", "spandoc", + "subtle", "thiserror", "tracing", "x25519-dalek", diff --git a/zebra-chain/Cargo.toml b/zebra-chain/Cargo.toml index 6ed29f2c..271405d8 100644 --- a/zebra-chain/Cargo.toml +++ b/zebra-chain/Cargo.toml @@ -41,6 +41,7 @@ secp256k1 = { version = "0.20.2", features = ["serde"] } serde = { version = "1", features = ["serde_derive", "rc"] } serde-big-array = "0.3.2" sha2 = { version = "0.9.5", features=["compress"] } +subtle = "2.4" thiserror = "1" x25519-dalek = { version = "1.1", features = ["serde"] } diff --git a/zebra-chain/src/orchard/keys.rs b/zebra-chain/src/orchard/keys.rs index c873e5d4..6fa917d0 100644 --- a/zebra-chain/src/orchard/keys.rs +++ b/zebra-chain/src/orchard/keys.rs @@ -23,6 +23,7 @@ use halo2::{ pasta::pallas, }; use rand_core::{CryptoRng, RngCore}; +use subtle::{Choice, ConstantTimeEq}; use crate::{ parameters::Network, @@ -133,7 +134,7 @@ mod sk_hrp { /// key types derive from the [`SpendingKey`] value. /// /// [ps]: https://zips.z.cash/protocol/nu5.pdf#orchardkeycomponents -#[derive(Copy, Clone, Debug, Eq, PartialEq)] +#[derive(Copy, Clone, Debug)] #[cfg_attr( any(test, feature = "proptest-impl"), derive(proptest_derive::Arbitrary) @@ -143,9 +144,20 @@ pub struct SpendingKey { bytes: [u8; 32], } -impl From for [u8; 32] { - fn from(sk: SpendingKey) -> Self { - sk.bytes +impl ConstantTimeEq for SpendingKey { + /// Check whether two `SpendingKey`s are equal, runtime independent of the + /// value of the secret. + /// + /// # Note + /// + /// This function short-circuits if the networks of the keys are different. + /// Otherwise, it should execute in time independent of the `bytes` value. + fn ct_eq(&self, other: &Self) -> Choice { + if self.network != other.network { + return Choice::from(0); + } + + self.bytes.ct_eq(&other.bytes) } } @@ -160,6 +172,20 @@ impl fmt::Display for SpendingKey { } } +impl From for [u8; 32] { + fn from(sk: SpendingKey) -> Self { + sk.bytes + } +} + +impl Eq for SpendingKey {} + +impl PartialEq for SpendingKey { + fn eq(&self, other: &Self) -> bool { + self.ct_eq(other).unwrap_u8() == 1u8 + } +} + impl SpendingKey { /// Generate a new `SpendingKey`. /// @@ -195,12 +221,20 @@ impl SpendingKey { /// A Spend authorizing key (_ask_), as described in [protocol specification /// §4.2.3][orchardkeycomponents]. /// -/// Used to generate _spend authorization randomizers_ to sign each _Spend -/// Description_, proving ownership of notes. +/// Used to generate _spend authorization randomizers_ to sign each _Action +/// Description_ that spends notes, proving ownership of notes. /// /// [orchardkeycomponents]: https://zips.z.cash/protocol/nu5.pdf#orchardkeycomponents -#[derive(Copy, Clone, Eq, PartialEq)] -pub struct SpendAuthorizingKey(pub pallas::Scalar); +#[derive(Copy, Clone)] +pub struct SpendAuthorizingKey(pub(crate) pallas::Scalar); + +impl ConstantTimeEq for SpendAuthorizingKey { + /// Check whether two `SpendAuthorizingKey`s are equal, runtime independent + /// of the value of the secret. + fn ct_eq(&self, other: &Self) -> Choice { + self.0.to_bytes().ct_eq(&other.0.to_bytes()) + } +} impl fmt::Debug for SpendAuthorizingKey { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { @@ -232,11 +266,19 @@ impl From for SpendAuthorizingKey { } } +impl Eq for SpendAuthorizingKey {} + +impl PartialEq for SpendAuthorizingKey { + fn eq(&self, other: &Self) -> bool { + self.ct_eq(other).unwrap_u8() == 1u8 + } +} + impl PartialEq<[u8; 32]> for SpendAuthorizingKey { // TODO: do we want to use constant-time comparison here? #[allow(clippy::cmp_owned)] fn eq(&self, other: &[u8; 32]) -> bool { - <[u8; 32]>::from(*self) == *other + self.0.to_bytes().ct_eq(other).unwrap_u8() == 1u8 } } @@ -248,7 +290,7 @@ impl PartialEq<[u8; 32]> for SpendAuthorizingKey { /// /// [orchardkeycomponents]: https://zips.z.cash/protocol/nu5.pdf#orchardkeycomponents #[derive(Copy, Clone, Debug)] -pub struct SpendValidatingKey(pub redpallas::VerificationKey); +pub struct SpendValidatingKey(pub(crate) redpallas::VerificationKey); impl Eq for SpendValidatingKey {} @@ -275,14 +317,16 @@ impl From for SpendValidatingKey { impl PartialEq for SpendValidatingKey { #[allow(clippy::cmp_owned)] fn eq(&self, other: &Self) -> bool { - <[u8; 32]>::from(self.0) == <[u8; 32]>::from(other.0) + // XXX: These redpallas::VerificationKey(Bytes) fields are pub(crate) + self.0.bytes.bytes == other.0.bytes.bytes } } impl PartialEq<[u8; 32]> for SpendValidatingKey { #[allow(clippy::cmp_owned)] fn eq(&self, other: &[u8; 32]) -> bool { - <[u8; 32]>::from(self.0) == *other + // XXX: These redpallas::VerificationKey(Bytes) fields are pub(crate) + self.0.bytes.bytes == *other } } @@ -292,8 +336,16 @@ impl PartialEq<[u8; 32]> for SpendValidatingKey { /// Used to create a _Nullifier_ per note. /// /// [orchardkeycomponents]: https://zips.z.cash/protocol/nu5.pdf#orchardkeycomponents -#[derive(Copy, Clone, PartialEq)] -pub struct NullifierDerivingKey(pub pallas::Base); +#[derive(Copy, Clone)] +pub struct NullifierDerivingKey(pub(crate) pallas::Base); + +impl ConstantTimeEq for NullifierDerivingKey { + /// Check whether two `NullifierDerivingKey`s are equal, runtime independent + /// of the value of the secret. + fn ct_eq(&self, other: &Self) -> Choice { + self.0.to_bytes().ct_eq(&other.0.to_bytes()) + } +} impl fmt::Debug for NullifierDerivingKey { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { @@ -303,6 +355,8 @@ impl fmt::Debug for NullifierDerivingKey { } } +impl Eq for NullifierDerivingKey {} + impl From for [u8; 32] { fn from(nk: NullifierDerivingKey) -> [u8; 32] { nk.0.to_bytes() @@ -339,12 +393,16 @@ impl From for NullifierDerivingKey { } } -impl Eq for NullifierDerivingKey {} +impl PartialEq for NullifierDerivingKey { + fn eq(&self, other: &Self) -> bool { + self.ct_eq(other).unwrap_u8() == 1u8 + } +} impl PartialEq<[u8; 32]> for NullifierDerivingKey { #[allow(clippy::cmp_owned)] fn eq(&self, other: &[u8; 32]) -> bool { - <[u8; 32]>::from(*self) == *other + self.0.to_bytes().ct_eq(other).unwrap_u8() == 1u8 } } @@ -352,8 +410,16 @@ impl PartialEq<[u8; 32]> for NullifierDerivingKey { /// /// https://zips.z.cash/protocol/nu5.pdf#orchardkeycomponents // XXX: Should this be replaced by commitment::CommitmentRandomness? -#[derive(Copy, Clone, Eq, PartialEq)] -pub struct IvkCommitRandomness(pallas::Scalar); +#[derive(Copy, Clone)] +pub struct IvkCommitRandomness(pub(crate) pallas::Scalar); + +impl ConstantTimeEq for IvkCommitRandomness { + /// Check whether two `IvkCommitRandomness`s are equal, runtime independent + /// of the value of the secret. + fn ct_eq(&self, other: &Self) -> Choice { + self.0.to_bytes().ct_eq(&other.0.to_bytes()) + } +} impl fmt::Debug for IvkCommitRandomness { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { @@ -363,6 +429,8 @@ impl fmt::Debug for IvkCommitRandomness { } } +impl Eq for IvkCommitRandomness {} + impl From for IvkCommitRandomness { /// rivk = ToScalar^Orchard(PRF^expand_sk ([8])) /// @@ -386,6 +454,18 @@ impl From for pallas::Scalar { } } +impl PartialEq for IvkCommitRandomness { + fn eq(&self, other: &Self) -> bool { + self.ct_eq(other).unwrap_u8() == 1u8 + } +} + +impl PartialEq<[u8; 32]> for IvkCommitRandomness { + fn eq(&self, other: &[u8; 32]) -> bool { + self.0.to_bytes().ct_eq(other).unwrap_u8() == 1u8 + } +} + impl TryFrom<[u8; 32]> for IvkCommitRandomness { type Error = &'static str; @@ -415,12 +495,29 @@ mod ivk_hrp { /// Used to decrypt incoming notes without spending them. /// /// [ps]: https://zips.z.cash/protocol/nu5.pdf#orchardkeycomponents -#[derive(Copy, Clone, Eq, PartialEq)] +#[derive(Copy, Clone)] pub struct IncomingViewingKey { network: Network, scalar: pallas::Scalar, } +impl ConstantTimeEq for IncomingViewingKey { + /// Check whether two `IncomingViewingKey`s are equal, runtime independent + /// of the value of the secret. + /// + /// # Note + /// + /// This function short-circuits if the networks of the keys are different. + /// Otherwise, it should execute in time independent of the `bytes` value. + fn ct_eq(&self, other: &Self) -> Choice { + if self.network != other.network { + return Choice::from(0); + } + + self.scalar.to_bytes().ct_eq(&other.scalar.to_bytes()) + } +} + impl fmt::Debug for IncomingViewingKey { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { f.debug_tuple("IncomingViewingKey") @@ -440,6 +537,8 @@ impl fmt::Display for IncomingViewingKey { } } +impl Eq for IncomingViewingKey {} + impl From for IncomingViewingKey { /// Commit^ivk_rivk(ak, nk) := /// SinsemillaShortCommit_rcm (︁"z.cash:Orchard-CommitIvk", I2LEBSP_l(ak) || I2LEBSP_l(nk)︁) mod r_P @@ -472,9 +571,15 @@ impl From for IncomingViewingKey { } } +impl PartialEq for IncomingViewingKey { + fn eq(&self, other: &Self) -> bool { + self.ct_eq(other).unwrap_u8() == 1u8 + } +} + impl PartialEq<[u8; 32]> for IncomingViewingKey { fn eq(&self, other: &[u8; 32]) -> bool { - self.scalar.to_bytes() == *other + self.scalar.to_bytes().ct_eq(other).unwrap_u8() == 1u8 } } @@ -515,34 +620,6 @@ pub struct FullViewingKey { ivk_commit_randomness: IvkCommitRandomness, } -impl fmt::Debug for FullViewingKey { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - f.debug_struct("FullViewingKey") - .field("network", &self.network) - .field("spend_validating_key", &self.spend_validating_key) - .field("nullifier_deriving_key", &self.nullifier_deriving_key) - .field("ivk_commit_randomness", &self.ivk_commit_randomness) - .finish() - } -} - -impl fmt::Display for FullViewingKey { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - let mut bytes = io::Cursor::new(Vec::new()); - - let _ = bytes.write_all(&<[u8; 32]>::from(self.spend_validating_key)); - let _ = bytes.write_all(&<[u8; 32]>::from(self.nullifier_deriving_key)); - let _ = bytes.write_all(&<[u8; 32]>::from(self.ivk_commit_randomness)); - - let hrp = match self.network { - Network::Mainnet => fvk_hrp::MAINNET, - Network::Testnet => fvk_hrp::TESTNET, - }; - - bech32::encode_to_fmt(f, hrp, bytes.get_ref().to_base32(), Variant::Bech32).unwrap() - } -} - impl FullViewingKey { /// [4.2.3]: https://zips.z.cash/protocol/nu5.pdf#orchardkeycomponents #[allow(non_snake_case)] @@ -578,14 +655,50 @@ impl FullViewingKey { } } +impl fmt::Debug for FullViewingKey { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + f.debug_struct("FullViewingKey") + .field("network", &self.network) + .field("spend_validating_key", &self.spend_validating_key) + .field("nullifier_deriving_key", &self.nullifier_deriving_key) + .field("ivk_commit_randomness", &self.ivk_commit_randomness) + .finish() + } +} + +impl fmt::Display for FullViewingKey { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + let mut bytes = io::Cursor::new(Vec::new()); + + let _ = bytes.write_all(&<[u8; 32]>::from(self.spend_validating_key)); + let _ = bytes.write_all(&<[u8; 32]>::from(self.nullifier_deriving_key)); + let _ = bytes.write_all(&<[u8; 32]>::from(self.ivk_commit_randomness)); + + let hrp = match self.network { + Network::Mainnet => fvk_hrp::MAINNET, + Network::Testnet => fvk_hrp::TESTNET, + }; + + bech32::encode_to_fmt(f, hrp, bytes.get_ref().to_base32(), Variant::Bech32).unwrap() + } +} + /// An outgoing viewing key, as described in [protocol specification /// §4.2.3][ps]. /// /// Used to decrypt outgoing notes without spending them. /// /// [ps]: https://zips.z.cash/protocol/nu5.pdf#orchardkeycomponents -#[derive(Copy, Clone, Eq, PartialEq)] -pub struct OutgoingViewingKey(pub [u8; 32]); +#[derive(Copy, Clone)] +pub struct OutgoingViewingKey(pub(crate) [u8; 32]); + +impl ConstantTimeEq for OutgoingViewingKey { + /// Check whether two `OutgoingViewingKey`s are equal, runtime independent + /// of the value of the secret. + fn ct_eq(&self, other: &Self) -> Choice { + self.0.ct_eq(&other.0) + } +} impl fmt::Debug for OutgoingViewingKey { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { @@ -595,6 +708,8 @@ impl fmt::Debug for OutgoingViewingKey { } } +impl Eq for OutgoingViewingKey {} + impl From<[u8; 32]> for OutgoingViewingKey { /// Generate an `OutgoingViewingKey` from existing bytes. fn from(bytes: [u8; 32]) -> Self { @@ -623,9 +738,15 @@ impl From for OutgoingViewingKey { } } +impl PartialEq for OutgoingViewingKey { + fn eq(&self, other: &Self) -> bool { + self.ct_eq(other).unwrap_u8() == 1u8 + } +} + impl PartialEq<[u8; 32]> for OutgoingViewingKey { fn eq(&self, other: &[u8; 32]) -> bool { - self.0 == *other + self.0.ct_eq(other).unwrap_u8() == 1u8 } } @@ -685,7 +806,7 @@ impl From for [u8; 32] { any(test, feature = "proptest-impl"), derive(proptest_derive::Arbitrary) )] -pub struct Diversifier(pub [u8; 11]); +pub struct Diversifier(pub(crate) [u8; 11]); impl fmt::Debug for Diversifier { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { @@ -776,7 +897,7 @@ impl Diversifier { /// [concretediversifyhash]: https://zips.z.cash/protocol/nu5.pdf#concretediversifyhash /// https://zips.z.cash/protocol/nu5.pdf#orchardkeycomponents #[derive(Copy, Clone, PartialEq)] -pub struct TransmissionKey(pub pallas::Affine); +pub struct TransmissionKey(pub(crate) pallas::Affine); impl fmt::Debug for TransmissionKey { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { @@ -833,7 +954,7 @@ impl From<(IncomingViewingKey, Diversifier)> for TransmissionKey { impl PartialEq<[u8; 32]> for TransmissionKey { #[allow(clippy::cmp_owned)] fn eq(&self, other: &[u8; 32]) -> bool { - <[u8; 32]>::from(*self) == *other + &self.0.to_bytes() == other } } @@ -842,7 +963,7 @@ impl PartialEq<[u8; 32]> for TransmissionKey { /// https://zips.z.cash/protocol/nu5.pdf#concreteorchardkeyagreement /// https://zips.z.cash/protocol/nu5.pdf#saplingandorchardencrypt #[derive(Copy, Clone, Deserialize, PartialEq, Serialize)] -pub struct EphemeralPublicKey(#[serde(with = "serde_helpers::Affine")] pub pallas::Affine); +pub struct EphemeralPublicKey(#[serde(with = "serde_helpers::Affine")] pub(crate) pallas::Affine); impl fmt::Debug for EphemeralPublicKey { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { @@ -874,7 +995,7 @@ impl From<&EphemeralPublicKey> for [u8; 32] { impl PartialEq<[u8; 32]> for EphemeralPublicKey { #[allow(clippy::cmp_owned)] fn eq(&self, other: &[u8; 32]) -> bool { - <[u8; 32]>::from(self) == *other + &self.0.to_bytes() == other } } diff --git a/zebra-chain/src/sapling/keys.rs b/zebra-chain/src/sapling/keys.rs index 7aa226d5..5e00b04c 100644 --- a/zebra-chain/src/sapling/keys.rs +++ b/zebra-chain/src/sapling/keys.rs @@ -24,6 +24,7 @@ use std::{ use bech32::{self, FromBase32, ToBase32, Variant}; use rand_core::{CryptoRng, RngCore}; +use subtle::{Choice, ConstantTimeEq}; use crate::{ parameters::Network, @@ -194,10 +195,10 @@ mod sk_hrp { /// §4.2.2][ps]. /// /// Our root secret key of the Sapling key derivation tree. All other -/// Sapling key types derive from the SpendingKey value. +/// Sapling key types derive from the `SpendingKey` value. /// /// [ps]: https://zips.z.cash/protocol/protocol.pdf#saplingkeycomponents -#[derive(Copy, Clone, Debug, Eq, PartialEq)] +#[derive(Copy, Clone, Debug)] #[cfg_attr( any(test, feature = "proptest-impl"), derive(proptest_derive::Arbitrary) @@ -207,10 +208,42 @@ pub struct SpendingKey { bytes: [u8; 32], } +impl SpendingKey { + /// Generate a new _SpendingKey_. + pub fn new(csprng: &mut T) -> Self + where + T: RngCore + CryptoRng, + { + let mut bytes = [0u8; 32]; + csprng.fill_bytes(&mut bytes); + + Self::from(bytes) + } +} + +impl ConstantTimeEq for SpendingKey { + /// Check whether two `SpendingKey`s are equal, runtime independent of the + /// value of the secret. + /// + /// # Note + /// + /// This function short-circuits if the networks of the keys are different. + /// Otherwise, it should execute in time independent of the `bytes` value. + fn ct_eq(&self, other: &Self) -> Choice { + if self.network != other.network { + return Choice::from(0); + } + + self.bytes.ct_eq(&other.bytes) + } +} + +impl Eq for SpendingKey {} + // TODO: impl a From that accepts a Network? impl From<[u8; 32]> for SpendingKey { - /// Generate a _SpendingKey_ from existing bytes. + /// Generate a `SpendingKey` from existing bytes. fn from(bytes: [u8; 32]) -> Self { Self { network: Network::default(), @@ -254,16 +287,9 @@ impl FromStr for SpendingKey { } } -impl SpendingKey { - /// Generate a new _SpendingKey_. - pub fn new(csprng: &mut T) -> Self - where - T: RngCore + CryptoRng, - { - let mut bytes = [0u8; 32]; - csprng.fill_bytes(&mut bytes); - - Self::from(bytes) +impl PartialEq for SpendingKey { + fn eq(&self, other: &Self) -> bool { + self.ct_eq(other).unwrap_u8() == 1u8 } } @@ -274,8 +300,16 @@ impl SpendingKey { /// _Spend Description_, proving ownership of notes. /// /// [ps]: https://zips.z.cash/protocol/protocol.pdf#saplingkeycomponents -#[derive(Copy, Clone, Eq, PartialEq)] -pub struct SpendAuthorizingKey(pub Scalar); +#[derive(Copy, Clone)] +pub struct SpendAuthorizingKey(pub(crate) Scalar); + +impl ConstantTimeEq for SpendAuthorizingKey { + /// Check whether two `SpendAuthorizingKey`s are equal, runtime independent + /// of the value of the secret. + fn ct_eq(&self, other: &Self) -> Choice { + self.0.to_bytes().ct_eq(&other.0.to_bytes()) + } +} impl fmt::Debug for SpendAuthorizingKey { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { @@ -285,6 +319,8 @@ impl fmt::Debug for SpendAuthorizingKey { } } +impl Eq for SpendAuthorizingKey {} + impl From for [u8; 32] { fn from(sk: SpendAuthorizingKey) -> Self { sk.0.to_bytes() @@ -304,11 +340,15 @@ impl From for SpendAuthorizingKey { } } +impl PartialEq for SpendAuthorizingKey { + fn eq(&self, other: &Self) -> bool { + self.ct_eq(other).unwrap_u8() == 1u8 + } +} + impl PartialEq<[u8; 32]> for SpendAuthorizingKey { - // TODO: do we want to use constant-time comparison here? - #[allow(clippy::cmp_owned)] fn eq(&self, other: &[u8; 32]) -> bool { - <[u8; 32]>::from(*self) == *other + self.0.to_bytes().ct_eq(other).unwrap_u8() == 1u8 } } @@ -318,8 +358,16 @@ impl PartialEq<[u8; 32]> for SpendAuthorizingKey { /// Used in the _Spend Statement_ to prove nullifier integrity. /// /// [ps]: https://zips.z.cash/protocol/protocol.pdf#saplingkeycomponents -#[derive(Copy, Clone, Eq, PartialEq)] -pub struct ProofAuthorizingKey(pub Scalar); +#[derive(Copy, Clone)] +pub struct ProofAuthorizingKey(pub(crate) Scalar); + +impl ConstantTimeEq for ProofAuthorizingKey { + /// Check whether two `ProofAuthorizingKey`s are equal, runtime independent + /// of the value of the secret. + fn ct_eq(&self, other: &Self) -> Choice { + self.0.to_bytes().ct_eq(&other.0.to_bytes()) + } +} impl fmt::Debug for ProofAuthorizingKey { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { @@ -329,6 +377,8 @@ impl fmt::Debug for ProofAuthorizingKey { } } +impl Eq for ProofAuthorizingKey {} + impl From for [u8; 32] { fn from(nsk: ProofAuthorizingKey) -> Self { nsk.0.to_bytes() @@ -347,11 +397,15 @@ impl From for ProofAuthorizingKey { } } +impl PartialEq for ProofAuthorizingKey { + fn eq(&self, other: &Self) -> bool { + self.ct_eq(other).unwrap_u8() == 1u8 + } +} + impl PartialEq<[u8; 32]> for ProofAuthorizingKey { - // TODO: do we want to use constant-time comparison here? - #[allow(clippy::cmp_owned)] fn eq(&self, other: &[u8; 32]) -> bool { - <[u8; 32]>::from(*self) == *other + self.0.to_bytes().ct_eq(other).unwrap_u8() == 1u8 } } @@ -362,7 +416,7 @@ impl PartialEq<[u8; 32]> for ProofAuthorizingKey { /// /// [ps]: https://zips.z.cash/protocol/protocol.pdf#saplingkeycomponents #[derive(Copy, Clone, Eq, PartialEq)] -pub struct OutgoingViewingKey(pub [u8; 32]); +pub struct OutgoingViewingKey(pub(crate) [u8; 32]); impl fmt::Debug for OutgoingViewingKey { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { @@ -414,7 +468,7 @@ impl PartialEq<[u8; 32]> for OutgoingViewingKey { /// /// [ps]: https://zips.z.cash/protocol/protocol.pdf#saplingkeycomponents #[derive(Copy, Clone, Debug)] -pub struct AuthorizingKey(pub redjubjub::VerificationKey); +pub struct AuthorizingKey(pub(crate) redjubjub::VerificationKey); impl Eq for AuthorizingKey {} @@ -439,24 +493,32 @@ impl From for AuthorizingKey { impl PartialEq for AuthorizingKey { fn eq(&self, other: &Self) -> bool { - <[u8; 32]>::from(self.0) == <[u8; 32]>::from(other.0) + self == &<[u8; 32]>::from(*other) } } impl PartialEq<[u8; 32]> for AuthorizingKey { fn eq(&self, other: &[u8; 32]) -> bool { - <[u8; 32]>::from(self.0) == *other + &<[u8; 32]>::from(self.0) == other } } /// A _Nullifier Deriving Key_, as described in [protocol /// specification §4.2.2][ps]. /// -/// Used to create a _Nullifier_ per note. +/// Used to create a `Nullifier` per note. /// /// [ps]: https://zips.z.cash/protocol/protocol.pdf#saplingkeycomponents -#[derive(Copy, Clone, PartialEq)] -pub struct NullifierDerivingKey(pub jubjub::AffinePoint); +#[derive(Copy, Clone)] +pub struct NullifierDerivingKey(pub(crate) jubjub::AffinePoint); + +impl ConstantTimeEq for NullifierDerivingKey { + /// Check whether two `NullifierDerivingKey`s are equal, runtime independent + /// of the value of the secret. + fn ct_eq(&self, other: &Self) -> Choice { + self.0.to_bytes().ct_eq(&other.0.to_bytes()) + } +} impl fmt::Debug for NullifierDerivingKey { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { @@ -508,11 +570,15 @@ impl From for NullifierDerivingKey { } } +impl PartialEq for NullifierDerivingKey { + fn eq(&self, other: &Self) -> bool { + self.ct_eq(other).unwrap_u8() == 1u8 + } +} + impl PartialEq<[u8; 32]> for NullifierDerivingKey { - // TODO: do we want to use constant-time comparison here? - #[allow(clippy::cmp_owned)] fn eq(&self, other: &[u8; 32]) -> bool { - <[u8; 32]>::from(*self) == *other + self.0.to_bytes().ct_eq(other).unwrap_u8() == 1u8 } } @@ -638,7 +704,7 @@ impl PartialEq<[u8; 32]> for IncomingViewingKey { any(test, feature = "proptest-impl"), derive(proptest_derive::Arbitrary) )] -pub struct Diversifier(pub [u8; 11]); +pub struct Diversifier(pub(crate) [u8; 11]); impl fmt::Debug for Diversifier { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { @@ -758,7 +824,7 @@ impl Diversifier { /// /// [ps]: https://zips.z.cash/protocol/protocol.pdf#concretediversifyhash #[derive(Copy, Clone, PartialEq)] -pub struct TransmissionKey(pub jubjub::AffinePoint); +pub struct TransmissionKey(pub(crate) jubjub::AffinePoint); impl fmt::Debug for TransmissionKey { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { @@ -802,10 +868,8 @@ impl From<(IncomingViewingKey, Diversifier)> for TransmissionKey { } impl PartialEq<[u8; 32]> for TransmissionKey { - // TODO: do we want to use constant-time comparison here? - #[allow(clippy::cmp_owned)] fn eq(&self, other: &[u8; 32]) -> bool { - <[u8; 32]>::from(*self) == *other + &self.0.to_bytes() == other } } @@ -899,7 +963,7 @@ impl FromStr for FullViewingKey { /// https://zips.z.cash/protocol/protocol.pdf#concretesaplingkeyagreement #[derive(Copy, Clone, Deserialize, PartialEq, Serialize)] pub struct EphemeralPublicKey( - #[serde(with = "serde_helpers::AffinePoint")] pub jubjub::AffinePoint, + #[serde(with = "serde_helpers::AffinePoint")] pub(crate) jubjub::AffinePoint, ); impl fmt::Debug for EphemeralPublicKey { @@ -926,10 +990,8 @@ impl From<&EphemeralPublicKey> for [u8; 32] { } impl PartialEq<[u8; 32]> for EphemeralPublicKey { - // TODO: do we want to use constant-time comparison here? - #[allow(clippy::cmp_owned)] fn eq(&self, other: &[u8; 32]) -> bool { - <[u8; 32]>::from(self) == *other + &self.0.to_bytes() == other } }