diff --git a/zcash_primitives/CHANGELOG.md b/zcash_primitives/CHANGELOG.md index 0d6be86c4d..3eac233a94 100644 --- a/zcash_primitives/CHANGELOG.md +++ b/zcash_primitives/CHANGELOG.md @@ -45,6 +45,8 @@ and this library adheres to Rust's notion of - `circuit::{SpendVerifyingKey, PreparedSpendVerifyingKey}` - `circuit::{OutputVerifyingKey, PreparedOutputVerifyingKey}` - `constants` module. + - `keys::SpendAuthorizingKey` + - `keys::SpendValidatingKey` - `note_encryption::CompactOutputDescription` (moved from `zcash_primitives::transaction::components::sapling`). - `note_encryption::SaplingDomain::new` @@ -145,6 +147,9 @@ and this library adheres to Rust's notion of - `circuit::ValueCommitmentOpening::value` is now represented as a `NoteValue` instead of as a bare `u64`. - `keys::DecodingError` has a new variant `UnsupportedChildIndex`. + - `keys::ExpandedSpendingKey.ask` now has type `SpendAuthorizingKey`. + - `keys::ProofGenerationKey.ak` now has type `SpendValidatingKey`. + - `keys::ViewingKey.ak` now has type `SpendValidatingKey`. - `note_encryption`: - `SaplingDomain` no longer has a `P: consensus::Parameters` type parameter. - The following methods now take a `Zip212Enforcement` argument instead of a @@ -237,6 +242,12 @@ and this library adheres to Rust's notion of - `ChildIndex::NonHardened` - `sapling::ExtendedFullViewingKey::derive_child` +### Fixed +- `zcash_primitives::keys::ExpandedSpendingKey::from_spending_key` now panics if the + spending key expands to `ask = 0`. This has a negligible probability of occurring. +- `zcash_primitives::zip32::ExtendedSpendingKey::derive_child` now panics if the + child key has `ask = 0`. This has a negligible probability of occurring. + ## [0.13.0] - 2023-09-25 ### Added - `zcash_primitives::consensus::BlockHeight::saturating_sub` diff --git a/zcash_primitives/src/sapling/builder.rs b/zcash_primitives/src/sapling/builder.rs index 02bebbe45f..9a7cdab319 100644 --- a/zcash_primitives/src/sapling/builder.rs +++ b/zcash_primitives/src/sapling/builder.rs @@ -3,7 +3,7 @@ use core::fmt; use std::{marker::PhantomData, sync::mpsc::Sender}; -use group::{ff::Field, GroupEncoding}; +use group::ff::Field; use rand::{seq::SliceRandom, RngCore}; use rand_core::CryptoRng; use redjubjub::{Binding, SpendAuth}; @@ -15,7 +15,7 @@ use crate::{ Authorization, Authorized, Bundle, GrothProofBytes, MapAuth, OutputDescription, SpendDescription, }, - keys::OutgoingViewingKey, + keys::{OutgoingViewingKey, SpendAuthorizingKey, SpendValidatingKey}, note_encryption::{sapling_note_encryption, Zip212Enforcement}, prover::{OutputProver, SpendProver}, util::generate_random_rseed_internal, @@ -114,8 +114,7 @@ impl SpendDescriptionInfo { // Construct the value commitment. let cv = ValueCommitment::derive(self.note.value(), self.rcv.clone()); - let ak = redjubjub::VerificationKey::try_from(self.proof_generation_key.ak.to_bytes()) - .expect("valid points are valid verification keys"); + let ak = self.proof_generation_key.ak.clone(); // This is the result of the re-randomization, we compute it for the caller let rk = ak.randomize(&self.alpha); @@ -691,7 +690,7 @@ impl InProgressSignatures for Unsigned { pub struct SigningParts { /// The spend validating key for this spend description. Used to match spend /// authorizing keys to spend descriptions they can create signatures for. - ak: redjubjub::VerificationKey, + ak: SpendValidatingKey, /// The randomization needed to derive the actual signing key for this note. alpha: jubjub::Scalar, } @@ -760,7 +759,7 @@ impl Bundle, V> { self, mut rng: R, sighash: [u8; 32], - signing_keys: &[redjubjub::SigningKey], + signing_keys: &[SpendAuthorizingKey], ) -> Result, Error> { signing_keys .iter() @@ -775,20 +774,15 @@ impl Bundle, V> { /// Signs this bundle with the given [`redjubjub::SigningKey`]. /// /// This will apply signatures for all notes controlled by this spending key. - pub fn sign( - self, - mut rng: R, - ask: &redjubjub::SigningKey, - ) -> Self { - let expected_ak = redjubjub::VerificationKey::from(ask); + pub fn sign(self, mut rng: R, ask: &SpendAuthorizingKey) -> Self { + let expected_ak = ask.into(); let sighash = self.authorization().sigs.sighash; self.map_authorization(( |proof| proof, |proof| proof, |maybe| match maybe { MaybeSigned::SigningMetadata(parts) if parts.ak == expected_ak => { - let rsk = ask.randomize(&parts.alpha); - MaybeSigned::Signature(rsk.sign(&mut rng, &sighash)) + MaybeSigned::Signature(ask.randomize(&parts.alpha).sign(&mut rng, &sighash)) } s => s, }, @@ -920,12 +914,7 @@ pub mod testing { bundle.create_proofs(&MockSpendProver, &MockOutputProver, &mut rng, None); bundle - .apply_signatures( - &mut rng, - fake_sighash_bytes, - &[redjubjub::SigningKey::try_from(extsk.expsk.ask.to_bytes()) - .expect("valid scalars are valid signing keys")], - ) + .apply_signatures(&mut rng, fake_sighash_bytes, &[extsk.expsk.ask]) .unwrap() }, ) diff --git a/zcash_primitives/src/sapling/circuit.rs b/zcash_primitives/src/sapling/circuit.rs index 4683f052c2..cbdf0573e5 100644 --- a/zcash_primitives/src/sapling/circuit.rs +++ b/zcash_primitives/src/sapling/circuit.rs @@ -160,7 +160,7 @@ impl Circuit for Spend { // Prover witnesses ak (ensures that it's on the curve) let ak = ecc::EdwardsPoint::witness( cs.namespace(|| "ak"), - self.proof_generation_key.as_ref().map(|k| k.ak.into()), + self.proof_generation_key.as_ref().map(|k| (&k.ak).into()), )?; // There are no sensible attacks on small order points @@ -634,9 +634,12 @@ pub struct PreparedOutputVerifyingKey(pub(crate) groth16::PreparedVerifyingKey); + +impl PartialEq for SpendAuthorizingKey { + fn eq(&self, other: &Self) -> bool { + <[u8; 32]>::from(self.0) + .ct_eq(&<[u8; 32]>::from(other.0)) + .into() + } +} + +impl Eq for SpendAuthorizingKey {} + +impl From<&SpendValidatingKey> for jubjub::ExtendedPoint { + fn from(spend_validating_key: &SpendValidatingKey) -> jubjub::ExtendedPoint { + jubjub::ExtendedPoint::from_bytes(&spend_validating_key.to_bytes()).unwrap() + } +} + +impl SpendAuthorizingKey { + /// Derives ask from sk. Internal use only, does not enforce all constraints. + fn derive_inner(sk: &[u8]) -> jubjub::Scalar { + jubjub::Scalar::from_bytes_wide(prf_expand(sk, &[0x00]).as_array()) + } + + /// Constructs a `SpendAuthorizingKey` from a raw scalar. + pub(crate) fn from_scalar(ask: jubjub::Scalar) -> Option { + if ask.is_zero().into() { + None + } else { + Some(SpendAuthorizingKey(ask.to_bytes().try_into().unwrap())) + } + } + + /// Derives a `SpendAuthorizingKey` from a spending key. + fn from_spending_key(sk: &[u8]) -> Option { + Self::from_scalar(Self::derive_inner(sk)) + } + + /// Parses a `SpendAuthorizingKey` from its encoded form. + pub(crate) fn from_bytes(bytes: &[u8]) -> Option { + <[u8; 32]>::try_from(bytes) + .ok() + .and_then(|b| { + // RedJubjub.Private permits the full set of Jubjub scalars including + // zero. However, a SpendAuthorizingKey is further restricted within the + // Sapling key tree to be a non-zero scalar. + jubjub::Scalar::from_repr(b) + .and_then(|s| { + CtOption::new( + redjubjub::SigningKey::try_from(b) + .expect("RedJubjub permits the set of valid SpendAuthorizingKeys"), + !s.is_zero(), + ) + }) + .into() + }) + .map(SpendAuthorizingKey) + } + + /// Converts this spend authorizing key to its serialized form. + pub(crate) fn to_bytes(&self) -> [u8; 32] { + <[u8; 32]>::from(self.0) + } + + /// Converts this spend authorizing key to a raw scalar. + /// + /// Only used for ZIP 32 child derivation. + pub(crate) fn to_scalar(&self) -> jubjub::Scalar { + jubjub::Scalar::from_repr(self.0.into()).unwrap() + } + + /// Randomizes this spend authorizing key with the given `randomizer`. + /// + /// The resulting key can be used to actually sign a spend. + pub fn randomize(&self, randomizer: &jubjub::Scalar) -> redjubjub::SigningKey { + self.0.randomize(randomizer) + } +} + +/// A key used to validate spend authorization signatures. +/// +/// Defined in [Zcash Protocol Spec § 4.2.2: Sapling Key Components][saplingkeycomponents]. +/// +/// [saplingkeycomponents]: https://zips.z.cash/protocol/protocol.pdf#saplingkeycomponents +#[derive(Clone, Debug)] +pub struct SpendValidatingKey(redjubjub::VerificationKey); + +impl From<&SpendAuthorizingKey> for SpendValidatingKey { + fn from(ask: &SpendAuthorizingKey) -> Self { + SpendValidatingKey((&ask.0).into()) + } +} + +impl PartialEq for SpendValidatingKey { + fn eq(&self, other: &Self) -> bool { + <[u8; 32]>::from(self.0) + .ct_eq(&<[u8; 32]>::from(other.0)) + .into() + } +} + +impl Eq for SpendValidatingKey {} + +impl SpendValidatingKey { + /// For circuit tests only. + #[cfg(test)] + pub(crate) fn fake_random(mut rng: R) -> Self { + loop { + if let Some(k) = Self::from_bytes(&jubjub::SubgroupPoint::random(&mut rng).to_bytes()) { + break k; + } + } + } + + /// Parses a `SpendValidatingKey` from its encoded form. + pub(crate) fn from_bytes(bytes: &[u8]) -> Option { + <[u8; 32]>::try_from(bytes) + .ok() + .and_then(|b| { + // RedJubjub.Public permits the full set of Jubjub points including the + // identity and cofactors; this is the type used for `rk` in Spend + // descriptions. However, a SpendValidatingKey is further restricted + // within the Sapling key tree to be a non-identity element of the + // prime-order subgroup. + jubjub::SubgroupPoint::from_bytes(&b) + .and_then(|p| { + CtOption::new( + redjubjub::VerificationKey::try_from(b) + .expect("RedJubjub permits the set of valid SpendValidatingKeys"), + !p.is_identity(), + ) + }) + .into() + }) + .map(SpendValidatingKey) + } + + /// Converts this spend validating key to its serialized form, + /// `LEBS2OSP_256(repr_J(ak))`. + pub(crate) fn to_bytes(&self) -> [u8; 32] { + <[u8; 32]>::from(self.0) + } + + /// Randomizes this spend validating key with the given `randomizer`. + pub fn randomize(&self, randomizer: &jubjub::Scalar) -> redjubjub::VerificationKey { + self.0.randomize(randomizer) + } +} + /// An outgoing viewing key #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub struct OutgoingViewingKey(pub [u8; 32]); @@ -45,7 +204,7 @@ pub struct OutgoingViewingKey(pub [u8; 32]); /// A Sapling expanded spending key #[derive(Clone)] pub struct ExpandedSpendingKey { - pub ask: jubjub::Fr, + pub ask: SpendAuthorizingKey, pub nsk: jubjub::Fr, pub ovk: OutgoingViewingKey, } @@ -58,8 +217,15 @@ impl fmt::Debug for ExpandedSpendingKey { } impl ExpandedSpendingKey { + /// Expands a spending key into its components. + /// + /// # Panics + /// + /// Panics if this spending key expands to `ask = 0`. This has a negligible + /// probability of occurring. pub fn from_spending_key(sk: &[u8]) -> Self { - let ask = jubjub::Fr::from_bytes_wide(prf_expand(sk, &[0x00]).as_array()); + let ask = + SpendAuthorizingKey::from_spending_key(sk).expect("negligible chance of ask == 0"); let nsk = jubjub::Fr::from_bytes_wide(prf_expand(sk, &[0x01]).as_array()); let mut ovk = OutgoingViewingKey([0u8; 32]); ovk.0 @@ -69,7 +235,7 @@ impl ExpandedSpendingKey { pub fn proof_generation_key(&self) -> ProofGenerationKey { ProofGenerationKey { - ak: SPENDING_KEY_GENERATOR * self.ask, + ak: (&self.ask).into(), nsk: self.nsk, } } @@ -85,8 +251,7 @@ impl ExpandedSpendingKey { }); } - let ask = Option::from(jubjub::Fr::from_repr(b[0..32].try_into().unwrap())) - .ok_or(DecodingError::InvalidAsk)?; + let ask = SpendAuthorizingKey::from_bytes(&b[0..32]).ok_or(DecodingError::InvalidAsk)?; let nsk = Option::from(jubjub::Fr::from_repr(b[32..64].try_into().unwrap())) .ok_or(DecodingError::InvalidNsk)?; let ovk = OutgoingViewingKey(b[64..96].try_into().unwrap()); @@ -119,7 +284,7 @@ impl ExpandedSpendingKey { /// [ZIP 32](https://zips.z.cash/zip-0032) pub fn to_bytes(&self) -> [u8; 96] { let mut result = [0u8; 96]; - result[0..32].copy_from_slice(&self.ask.to_repr()); + result[0..32].copy_from_slice(&self.ask.to_bytes()); result[32..64].copy_from_slice(&self.nsk.to_repr()); result[64..96].copy_from_slice(&self.ovk.0); result @@ -128,7 +293,7 @@ impl ExpandedSpendingKey { #[derive(Clone)] pub struct ProofGenerationKey { - pub ak: jubjub::SubgroupPoint, + pub ak: SpendValidatingKey, pub nsk: jubjub::Fr, } @@ -143,7 +308,7 @@ impl fmt::Debug for ProofGenerationKey { impl ProofGenerationKey { pub fn to_viewing_key(&self) -> ViewingKey { ViewingKey { - ak: self.ak, + ak: self.ak.clone(), nk: NullifierDerivingKey(constants::PROOF_GENERATION_KEY_GENERATOR * self.nsk), } } @@ -155,13 +320,13 @@ pub struct NullifierDerivingKey(pub jubjub::SubgroupPoint); #[derive(Debug, Clone)] pub struct ViewingKey { - pub ak: jubjub::SubgroupPoint, + pub ak: SpendValidatingKey, pub nk: NullifierDerivingKey, } impl ViewingKey { - pub fn rk(&self, ar: jubjub::Fr) -> jubjub::SubgroupPoint { - self.ak + constants::SPENDING_KEY_GENERATOR * ar + pub fn rk(&self, ar: jubjub::Fr) -> redjubjub::VerificationKey { + self.ak.randomize(&ar) } pub fn ivk(&self) -> SaplingIvk { @@ -184,7 +349,7 @@ impl Clone for FullViewingKey { fn clone(&self) -> Self { FullViewingKey { vk: ViewingKey { - ak: self.vk.ak, + ak: self.vk.ak.clone(), nk: self.vk.nk, }, ovk: self.ovk, @@ -196,7 +361,7 @@ impl FullViewingKey { pub fn from_expanded_spending_key(expsk: &ExpandedSpendingKey) -> Self { FullViewingKey { vk: ViewingKey { - ak: SPENDING_KEY_GENERATOR * expsk.ask, + ak: (&expsk.ask).into(), nk: NullifierDerivingKey(PROOF_GENERATION_KEY_GENERATOR * expsk.nsk), }, ovk: expsk.ovk, @@ -207,14 +372,14 @@ impl FullViewingKey { let ak = { let mut buf = [0u8; 32]; reader.read_exact(&mut buf)?; - jubjub::SubgroupPoint::from_bytes(&buf).and_then(|p| CtOption::new(p, !p.is_identity())) + SpendValidatingKey::from_bytes(&buf) }; let nk = { let mut buf = [0u8; 32]; reader.read_exact(&mut buf)?; jubjub::SubgroupPoint::from_bytes(&buf) }; - if ak.is_none().into() { + if ak.is_none() { return Err(io::Error::new( io::ErrorKind::InvalidInput, "ak not of prime order", diff --git a/zcash_primitives/src/sapling/zip32.rs b/zcash_primitives/src/sapling/zip32.rs index 4971ef0df4..edcb25678b 100644 --- a/zcash_primitives/src/sapling/zip32.rs +++ b/zcash_primitives/src/sapling/zip32.rs @@ -16,7 +16,10 @@ use crate::{ keys::{prf_expand, prf_expand_vec}, sapling::{ constants::PROOF_GENERATION_KEY_GENERATOR, - keys::{DecodingError, ExpandedSpendingKey, FullViewingKey, OutgoingViewingKey}, + keys::{ + DecodingError, ExpandedSpendingKey, FullViewingKey, OutgoingViewingKey, + SpendAuthorizingKey, + }, SaplingIvk, }, zip32::{ChainCode, ChildIndex, DiversifierIndex, Scope}, @@ -98,7 +101,7 @@ pub fn sapling_derive_internal_fvk( ( FullViewingKey { vk: ViewingKey { - ak: fvk.vk.ak, + ak: fvk.vk.ak.clone(), nk: nk_internal, }, ovk: ovk_internal, @@ -408,6 +411,12 @@ impl ExtendedSpendingKey { xsk } + /// Derives the child key at the given (hardened) index. + /// + /// # Panics + /// + /// Panics if the child key has `ask = 0`. This has a negligible probability of + /// occurring. #[must_use] pub fn derive_child(&self, i: ChildIndex) -> Self { let fvk = FullViewingKey::from_expanded_spending_key(&self.expsk); @@ -431,10 +440,15 @@ impl ExtendedSpendingKey { expsk: { let mut ask = jubjub::Fr::from_bytes_wide(prf_expand(i_l, &[0x13]).as_array()); let mut nsk = jubjub::Fr::from_bytes_wide(prf_expand(i_l, &[0x14]).as_array()); - ask.add_assign(&self.expsk.ask); + ask.add_assign(self.expsk.ask.to_scalar()); nsk.add_assign(&self.expsk.nsk); let ovk = derive_child_ovk(&self.expsk.ovk, i_l); - ExpandedSpendingKey { ask, nsk, ovk } + ExpandedSpendingKey { + ask: SpendAuthorizingKey::from_scalar(ask) + .expect("negligible chance of ask == 0"), + nsk, + ovk, + } }, dk: self.dk.derive_child(i_l), } @@ -474,7 +488,7 @@ impl ExtendedSpendingKey { child_index: self.child_index, chain_code: self.chain_code, expsk: ExpandedSpendingKey { - ask: self.expsk.ask, + ask: self.expsk.ask.clone(), nsk: nsk_internal, ovk: ovk_internal, }, @@ -1611,7 +1625,7 @@ mod tests { let xsks = [m, m_1h, m_1h_2h, m_1h_2h_3h]; for (xsk, tv) in xsks.iter().zip(test_vectors.iter()) { - assert_eq!(xsk.expsk.ask.to_repr().as_ref(), tv.ask.unwrap()); + assert_eq!(xsk.expsk.ask.to_bytes(), tv.ask.unwrap()); assert_eq!(xsk.expsk.nsk.to_repr().as_ref(), tv.nsk.unwrap()); assert_eq!(xsk.expsk.ovk.0, tv.ovk); @@ -1623,7 +1637,7 @@ mod tests { assert_eq!(&ser[..], &tv.xsk.unwrap()[..]); let internal_xsk = xsk.derive_internal(); - assert_eq!(internal_xsk.expsk.ask.to_repr().as_ref(), tv.ask.unwrap()); + assert_eq!(internal_xsk.expsk.ask.to_bytes(), tv.ask.unwrap()); assert_eq!( internal_xsk.expsk.nsk.to_repr().as_ref(), tv.internal_nsk.unwrap() diff --git a/zcash_primitives/src/transaction/builder.rs b/zcash_primitives/src/transaction/builder.rs index 448e1c8322..504ee2fdea 100644 --- a/zcash_primitives/src/transaction/builder.rs +++ b/zcash_primitives/src/transaction/builder.rs @@ -6,7 +6,6 @@ use std::fmt; use std::sync::mpsc::Sender; use rand::{rngs::OsRng, CryptoRng, RngCore}; -use redjubjub::SpendAuth; use crate::{ consensus::{self, BlockHeight, BranchId, NetworkUpgrade}, @@ -163,7 +162,7 @@ pub struct Builder<'a, P, R> { // `add_sapling_spend` or `add_orchard_spend`, we will build an unauthorized, unproven // transaction, and then the caller will be responsible for using the spending keys or their // derivatives for proving and signing to complete transaction creation. - sapling_asks: Vec>, + sapling_asks: Vec, orchard_saks: Vec, #[cfg(feature = "zfuture")] tze_builder: TzeBuilder<'a, TransactionData>, @@ -335,11 +334,7 @@ impl<'a, P: consensus::Parameters, R: RngCore + CryptoRng> Builder<'a, P, R> { self.sapling_builder .add_spend(&mut self.rng, &extsk, note, merkle_path)?; - // TODO: store a `redjubjub::SigningKey` inside `extsk`. - self.sapling_asks.push( - redjubjub::SigningKey::try_from(extsk.expsk.ask.to_bytes()) - .expect("valid scalar is valid signing key"), - ); + self.sapling_asks.push(extsk.expsk.ask); Ok(()) }