diff --git a/librustzcash/include/librustzcash.h b/librustzcash/include/librustzcash.h index 4efb544d3..a3ca5e8d3 100644 --- a/librustzcash/include/librustzcash.h +++ b/librustzcash/include/librustzcash.h @@ -112,8 +112,7 @@ extern "C" { bool librustzcash_sapling_output_proof( void *ctx, const unsigned char *esk, - const unsigned char *diversifier, - const unsigned char *pk_d, + const unsigned char *payment_address, const unsigned char *rcm, const uint64_t value, unsigned char *cv, diff --git a/librustzcash/src/rustzcash.rs b/librustzcash/src/rustzcash.rs index 717dd61c5..e3a85e4aa 100644 --- a/librustzcash/src/rustzcash.rs +++ b/librustzcash/src/rustzcash.rs @@ -927,8 +927,7 @@ pub extern "system" fn librustzcash_sprout_verify( pub extern "system" fn librustzcash_sapling_output_proof( ctx: *mut SaplingProvingContext, esk: *const [c_uchar; 32], - diversifier: *const [c_uchar; 11], - pk_d: *const [c_uchar; 32], + payment_address: *const [c_uchar; 43], rcm: *const [c_uchar; 32], value: u64, cv: *mut [c_uchar; 32], @@ -940,26 +939,12 @@ pub extern "system" fn librustzcash_sapling_output_proof( Err(_) => return false, }; - // Grab the diversifier from the caller. - let diversifier = Diversifier(unsafe { *diversifier }); - - // Grab pk_d from the caller. - let pk_d = match edwards::Point::::read(&(unsafe { &*pk_d })[..], &JUBJUB) { - Ok(p) => p, - Err(_) => return false, - }; - - // pk_d should be prime order. - let pk_d = match pk_d.as_prime_order(&JUBJUB) { - Some(p) => p, - None => return false, - }; - - // Construct a payment address - let payment_address = PaymentAddress { - pk_d: pk_d, - diversifier: diversifier, - }; + // Grab the payment address from the caller + let payment_address = + match PaymentAddress::::from_bytes(unsafe { &*payment_address }, &JUBJUB) { + Some(pa) => pa, + None => return false, + }; // The caller provides the commitment randomness for the output note let rcm = match Fs::from_repr(read_fs(&(unsafe { &*rcm })[..])) { @@ -1230,14 +1215,7 @@ pub extern "system" fn librustzcash_zip32_xfvk_address( let addr_ret = unsafe { &mut *addr_ret }; j_ret.copy_from_slice(&(addr.0).0); - addr_ret - .get_mut(..11) - .unwrap() - .copy_from_slice(&addr.1.diversifier.0); - addr.1 - .pk_d - .write(addr_ret.get_mut(11..).unwrap()) - .expect("should be able to serialize a PaymentAddress"); + addr_ret.copy_from_slice(&addr.1.to_bytes()); true } diff --git a/librustzcash/src/tests/key_agreement.rs b/librustzcash/src/tests/key_agreement.rs index 8f2c273ff..1250cc724 100644 --- a/librustzcash/src/tests/key_agreement.rs +++ b/librustzcash/src/tests/key_agreement.rs @@ -47,7 +47,7 @@ fn test_key_agreement() { // Serialize pk_d for the call to librustzcash_sapling_ka_agree let mut addr_pk_d = [0u8; 32]; - addr.pk_d.write(&mut addr_pk_d[..]).unwrap(); + addr.pk_d().write(&mut addr_pk_d[..]).unwrap(); assert!(librustzcash_sapling_ka_agree( &addr_pk_d, @@ -59,7 +59,7 @@ fn test_key_agreement() { // using the diversifier and esk. let mut epk = [0u8; 32]; assert!(librustzcash_sapling_ka_derivepublic( - &addr.diversifier.0, + &addr.diversifier().0, &esk, &mut epk )); diff --git a/librustzcash/src/tests/key_components.rs b/librustzcash/src/tests/key_components.rs index a15a40a98..5975e5eb1 100644 --- a/librustzcash/src/tests/key_components.rs +++ b/librustzcash/src/tests/key_components.rs @@ -707,7 +707,7 @@ fn key_components() { let addr = fvk.to_payment_address(diversifier, &JUBJUB).unwrap(); { let mut vec = Vec::new(); - addr.pk_d.write(&mut vec).unwrap(); + addr.pk_d().write(&mut vec).unwrap(); assert_eq!(&vec, &tv.default_pk_d); } { diff --git a/zcash_client_backend/src/encoding.rs b/zcash_client_backend/src/encoding.rs index d39973d34..4e38995c5 100644 --- a/zcash_client_backend/src/encoding.rs +++ b/zcash_client_backend/src/encoding.rs @@ -7,10 +7,7 @@ use bech32::{self, Error, FromBase32, ToBase32}; use pairing::bls12_381::Bls12; use std::io::{self, Write}; use zcash_primitives::{ - jubjub::edwards, - primitives::{Diversifier, PaymentAddress}, -}; -use zcash_primitives::{ + primitives::PaymentAddress, zip32::{ExtendedFullViewingKey, ExtendedSpendingKey}, JUBJUB, }; @@ -113,10 +110,11 @@ pub fn decode_extended_full_viewing_key( /// 0xbc, 0xe5, /// ]); /// -/// let pa = PaymentAddress { -/// diversifier: Diversifier([0u8; 11]), -/// pk_d: edwards::Point::::rand(rng, &JUBJUB).mul_by_cofactor(&JUBJUB), -/// }; +/// let pa = PaymentAddress::from_parts( +/// Diversifier([0u8; 11]), +/// edwards::Point::::rand(rng, &JUBJUB).mul_by_cofactor(&JUBJUB), +/// ) +/// .unwrap(); /// /// assert_eq!( /// encode_payment_address(HRP_SAPLING_PAYMENT_ADDRESS, &pa), @@ -124,10 +122,7 @@ pub fn decode_extended_full_viewing_key( /// ); /// ``` pub fn encode_payment_address(hrp: &str, addr: &PaymentAddress) -> String { - bech32_encode(hrp, |w| { - w.write_all(&addr.diversifier.0)?; - addr.pk_d.write(w) - }) + bech32_encode(hrp, |w| w.write_all(&addr.to_bytes())) } /// Decodes a [`PaymentAddress`] from a Bech32-encoded string. @@ -153,10 +148,11 @@ pub fn encode_payment_address(hrp: &str, addr: &PaymentAddress) -> String /// 0xbc, 0xe5, /// ]); /// -/// let pa = PaymentAddress { -/// diversifier: Diversifier([0u8; 11]), -/// pk_d: edwards::Point::::rand(rng, &JUBJUB).mul_by_cofactor(&JUBJUB), -/// }; +/// let pa = PaymentAddress::from_parts( +/// Diversifier([0u8; 11]), +/// edwards::Point::::rand(rng, &JUBJUB).mul_by_cofactor(&JUBJUB), +/// ) +/// .unwrap(); /// /// assert_eq!( /// decode_payment_address( @@ -168,17 +164,13 @@ pub fn encode_payment_address(hrp: &str, addr: &PaymentAddress) -> String /// ``` pub fn decode_payment_address(hrp: &str, s: &str) -> Result>, Error> { bech32_decode(hrp, s, |data| { - let mut diversifier = Diversifier([0; 11]); - diversifier.0.copy_from_slice(&data[0..11]); - // Check that the diversifier is valid - if diversifier.g_d::(&JUBJUB).is_none() { + if data.len() != 43 { return None; } - edwards::Point::::read(&data[11..], &JUBJUB) - .ok()? - .as_prime_order(&JUBJUB) - .map(|pk_d| PaymentAddress { pk_d, diversifier }) + let mut bytes = [0; 43]; + bytes.copy_from_slice(&data); + PaymentAddress::::from_bytes(&bytes, &JUBJUB) }) } @@ -203,10 +195,11 @@ mod tests { 0xbc, 0xe5, ]); - let addr = PaymentAddress { - diversifier: Diversifier([0u8; 11]), - pk_d: edwards::Point::::rand(rng, &JUBJUB).mul_by_cofactor(&JUBJUB), - }; + let addr = PaymentAddress::from_parts( + Diversifier([0u8; 11]), + edwards::Point::::rand(rng, &JUBJUB).mul_by_cofactor(&JUBJUB), + ) + .unwrap(); let encoded_main = "zs1qqqqqqqqqqqqqqqqqrjq05nyfku05msvu49mawhg6kr0wwljahypwyk2h88z6975u563j8nfaxd"; @@ -247,10 +240,11 @@ mod tests { 0xbc, 0xe5, ]); - let addr = PaymentAddress { - diversifier: Diversifier([1u8; 11]), - pk_d: edwards::Point::::rand(rng, &JUBJUB).mul_by_cofactor(&JUBJUB), - }; + let addr = PaymentAddress::from_parts( + Diversifier([1u8; 11]), + edwards::Point::::rand(rng, &JUBJUB).mul_by_cofactor(&JUBJUB), + ) + .unwrap(); let encoded_main = encode_payment_address(constants::mainnet::HRP_SAPLING_PAYMENT_ADDRESS, &addr); diff --git a/zcash_primitives/src/note_encryption.rs b/zcash_primitives/src/note_encryption.rs index 26440160a..e728c5e66 100644 --- a/zcash_primitives/src/note_encryption.rs +++ b/zcash_primitives/src/note_encryption.rs @@ -232,10 +232,7 @@ fn prf_ock( /// /// let diversifier = Diversifier([0; 11]); /// let pk_d = diversifier.g_d::(&JUBJUB).unwrap(); -/// let to = PaymentAddress { -/// pk_d, -/// diversifier, -/// }; +/// let to = PaymentAddress::from_parts(diversifier, pk_d).unwrap(); /// let ovk = OutgoingViewingKey([0; 32]); /// /// let value = 1000; @@ -294,14 +291,14 @@ impl SaplingNoteEncryption { /// Generates `encCiphertext` for this note. pub fn encrypt_note_plaintext(&self) -> [u8; ENC_CIPHERTEXT_SIZE] { - let shared_secret = sapling_ka_agree(&self.esk, &self.to.pk_d); + let shared_secret = sapling_ka_agree(&self.esk, self.to.pk_d()); let key = kdf_sapling(shared_secret, &self.epk); // Note plaintext encoding is defined in section 5.5 of the Zcash Protocol // Specification. let mut input = [0; NOTE_PLAINTEXT_SIZE]; input[0] = 1; - input[1..12].copy_from_slice(&self.to.diversifier.0); + input[1..12].copy_from_slice(&self.to.diversifier().0); (&mut input[12..20]) .write_u64::(self.note.value) .unwrap(); @@ -375,7 +372,7 @@ fn parse_note_plaintext_without_memo( .g_d::(&JUBJUB)? .mul(ivk.into_repr(), &JUBJUB); - let to = PaymentAddress { pk_d, diversifier }; + let to = PaymentAddress::from_parts(diversifier, pk_d)?; let note = to.create_note(v, rcm, &JUBJUB).unwrap(); if note.cm(&JUBJUB) != *cmu { @@ -535,7 +532,7 @@ pub fn try_sapling_output_recovery( return None; } - let to = PaymentAddress { pk_d, diversifier }; + let to = PaymentAddress::from_parts(diversifier, pk_d)?; let note = to.create_note(v, rcm, &JUBJUB).unwrap(); if note.cm(&JUBJUB) != *cmu { @@ -698,10 +695,47 @@ mod tests { [u8; ENC_CIPHERTEXT_SIZE], [u8; OUT_CIPHERTEXT_SIZE], ) { - let diversifier = Diversifier([0; 11]); let ivk = Fs::random(&mut rng); + + let (ovk, ivk, cv, cmu, epk, enc_ciphertext, out_ciphertext) = + random_enc_ciphertext_with(ivk, rng); + + assert!(try_sapling_note_decryption(&ivk, &epk, &cmu, &enc_ciphertext).is_some()); + assert!(try_sapling_compact_note_decryption( + &ivk, + &epk, + &cmu, + &enc_ciphertext[..COMPACT_NOTE_SIZE] + ) + .is_some()); + assert!(try_sapling_output_recovery( + &ovk, + &cv, + &cmu, + &epk, + &enc_ciphertext, + &out_ciphertext + ) + .is_some()); + + (ovk, ivk, cv, cmu, epk, enc_ciphertext, out_ciphertext) + } + + fn random_enc_ciphertext_with( + ivk: Fs, + mut rng: &mut R, + ) -> ( + OutgoingViewingKey, + Fs, + edwards::Point, + Fr, + edwards::Point, + [u8; ENC_CIPHERTEXT_SIZE], + [u8; OUT_CIPHERTEXT_SIZE], + ) { + let diversifier = Diversifier([0; 11]); let pk_d = diversifier.g_d::(&JUBJUB).unwrap().mul(ivk, &JUBJUB); - let pa = PaymentAddress { diversifier, pk_d }; + let pa = PaymentAddress::from_parts_unchecked(diversifier, pk_d); // Construct the value commitment for the proof instance let value = 100; @@ -722,24 +756,6 @@ mod tests { let enc_ciphertext = ne.encrypt_note_plaintext(); let out_ciphertext = ne.encrypt_outgoing_plaintext(&cv, &cmu); - assert!(try_sapling_note_decryption(&ivk, epk, &cmu, &enc_ciphertext).is_some()); - assert!(try_sapling_compact_note_decryption( - &ivk, - epk, - &cmu, - &enc_ciphertext[..COMPACT_NOTE_SIZE] - ) - .is_some()); - assert!(try_sapling_output_recovery( - &ovk, - &cv, - &cmu, - &epk, - &enc_ciphertext, - &out_ciphertext - ) - .is_some()); - ( ovk, ivk, @@ -1256,6 +1272,20 @@ mod tests { ); } + #[test] + fn recovery_with_invalid_pk_d() { + let mut rng = OsRng; + + let ivk = Fs::zero(); + let (ovk, _, cv, cmu, epk, enc_ciphertext, out_ciphertext) = + random_enc_ciphertext_with(ivk, &mut rng); + + assert_eq!( + try_sapling_output_recovery(&ovk, &cv, &cmu, &epk, &enc_ciphertext, &out_ciphertext), + None + ); + } + #[test] fn test_vectors() { let test_vectors = crate::test_vectors::note_encryption::make_test_vectors(); @@ -1317,10 +1347,7 @@ mod tests { let ock = prf_ock(&ovk, &cv, &cmu, &epk); assert_eq!(ock.as_bytes(), tv.ock); - let to = PaymentAddress { - pk_d, - diversifier: Diversifier(tv.default_d), - }; + let to = PaymentAddress::from_parts(Diversifier(tv.default_d), pk_d).unwrap(); let note = to.create_note(tv.v, rcm, &JUBJUB).unwrap(); assert_eq!(note.cm(&JUBJUB), cmu); diff --git a/zcash_primitives/src/primitives.rs b/zcash_primitives/src/primitives.rs index 727402dac..a336673a7 100644 --- a/zcash_primitives/src/primitives.rs +++ b/zcash_primitives/src/primitives.rs @@ -94,10 +94,10 @@ impl ViewingKey { diversifier: Diversifier, params: &E::Params, ) -> Option> { - diversifier.g_d(params).map(|g_d| { + diversifier.g_d(params).and_then(|g_d| { let pk_d = g_d.mul(self.ivk(), params); - PaymentAddress { pk_d, diversifier } + PaymentAddress::from_parts(diversifier, pk_d) }) } } @@ -118,10 +118,16 @@ impl Diversifier { } } +/// A Sapling payment address. +/// +/// # Invariants +/// +/// `pk_d` is guaranteed to be prime-order (i.e. in the prime-order subgroup of Jubjub, +/// and not the identity). #[derive(Clone, Debug)] pub struct PaymentAddress { - pub pk_d: edwards::Point, - pub diversifier: Diversifier, + pk_d: edwards::Point, + diversifier: Diversifier, } impl PartialEq for PaymentAddress { @@ -131,6 +137,67 @@ impl PartialEq for PaymentAddress { } impl PaymentAddress { + /// Constructs a PaymentAddress from a diversifier and a Jubjub point. + /// + /// Returns None if `pk_d` is the identity. + pub fn from_parts( + diversifier: Diversifier, + pk_d: edwards::Point, + ) -> Option { + if pk_d == edwards::Point::zero() { + None + } else { + Some(PaymentAddress { pk_d, diversifier }) + } + } + + /// Constructs a PaymentAddress from a diversifier and a Jubjub point. + /// + /// Only for test code, as this explicitly bypasses the invariant. + #[cfg(test)] + pub(crate) fn from_parts_unchecked( + diversifier: Diversifier, + pk_d: edwards::Point, + ) -> Self { + PaymentAddress { pk_d, diversifier } + } + + /// Parses a PaymentAddress from bytes. + pub fn from_bytes(bytes: &[u8; 43], params: &E::Params) -> Option { + let diversifier = { + let mut tmp = [0; 11]; + tmp.copy_from_slice(&bytes[0..11]); + Diversifier(tmp) + }; + // Check that the diversifier is valid + if diversifier.g_d::(params).is_none() { + return None; + } + + edwards::Point::::read(&bytes[11..43], params) + .ok()? + .as_prime_order(params) + .and_then(|pk_d| PaymentAddress::from_parts(diversifier, pk_d)) + } + + /// Returns the byte encoding of this `PaymentAddress`. + pub fn to_bytes(&self) -> [u8; 43] { + let mut bytes = [0; 43]; + bytes[0..11].copy_from_slice(&self.diversifier.0); + self.pk_d.write(&mut bytes[11..]).unwrap(); + bytes + } + + /// Returns the [`Diversifier`] for this `PaymentAddress`. + pub fn diversifier(&self) -> &Diversifier { + &self.diversifier + } + + /// Returns `pk_d` for this `PaymentAddress`. + pub fn pk_d(&self) -> &edwards::Point { + &self.pk_d + } + pub fn g_d(&self, params: &E::Params) -> Option> { self.diversifier.g_d(params) } diff --git a/zcash_primitives/src/transaction/builder.rs b/zcash_primitives/src/transaction/builder.rs index 281ccbf41..b100d9c08 100644 --- a/zcash_primitives/src/transaction/builder.rs +++ b/zcash_primitives/src/transaction/builder.rs @@ -77,7 +77,7 @@ impl SaplingOutput { let note = Note { g_d, - pk_d: to.pk_d.clone(), + pk_d: to.pk_d().clone(), value: value.into(), r: rcm, }; @@ -344,10 +344,11 @@ impl Builder { } else if !self.spends.is_empty() { ( self.spends[0].extsk.expsk.ovk, - PaymentAddress { - diversifier: self.spends[0].diversifier, - pk_d: self.spends[0].note.pk_d.clone(), - }, + PaymentAddress::from_parts( + self.spends[0].diversifier, + self.spends[0].note.pk_d.clone(), + ) + .ok_or(Error::InvalidAddress)?, ) } else { return Err(Error::NoChangeAddress); @@ -450,16 +451,16 @@ impl Builder { (diversifier, g_d) }; - let pk_d = { + let (pk_d, payment_address) = loop { let dummy_ivk = Fs::random(&mut self.rng); - g_d.mul(dummy_ivk, &JUBJUB) + let pk_d = g_d.mul(dummy_ivk, &JUBJUB); + if let Some(addr) = PaymentAddress::from_parts(diversifier, pk_d.clone()) { + break (pk_d, addr); + } }; ( - PaymentAddress { - diversifier, - pk_d: pk_d.clone(), - }, + payment_address, Note { g_d, pk_d, @@ -644,7 +645,7 @@ mod tests { builder .add_sapling_spend( extsk.clone(), - to.diversifier, + *to.diversifier(), note1.clone(), witness1.clone(), ) @@ -683,10 +684,10 @@ mod tests { { let mut builder = Builder::new(0); builder - .add_sapling_spend(extsk.clone(), to.diversifier, note1, witness1) + .add_sapling_spend(extsk.clone(), *to.diversifier(), note1, witness1) .unwrap(); builder - .add_sapling_spend(extsk, to.diversifier, note2, witness2) + .add_sapling_spend(extsk, *to.diversifier(), note2, witness2) .unwrap(); builder .add_sapling_output(ovk, to, Amount::from_u64(30000).unwrap(), None) diff --git a/zcash_primitives/src/zip32.rs b/zcash_primitives/src/zip32.rs index e287602aa..6cbc46275 100644 --- a/zcash_primitives/src/zip32.rs +++ b/zcash_primitives/src/zip32.rs @@ -548,7 +548,7 @@ mod tests { let (j_m, addr_m) = xsk_m.default_address().unwrap(); assert_eq!(j_m.0, [0; 11]); assert_eq!( - addr_m.diversifier.0, + addr_m.diversifier().0, // Computed using this Rust implementation [59, 246, 250, 31, 131, 191, 69, 99, 200, 167, 19] ); diff --git a/zcash_proofs/src/circuit/sapling.rs b/zcash_proofs/src/circuit/sapling.rs index de6887b44..08e55e604 100644 --- a/zcash_proofs/src/circuit/sapling.rs +++ b/zcash_proofs/src/circuit/sapling.rs @@ -464,7 +464,7 @@ impl<'a, E: JubjubEngine> Circuit for Output<'a, E> { // they would like. { // Just grab pk_d from the witness - let pk_d = self.payment_address.as_ref().map(|e| e.pk_d.to_xy()); + let pk_d = self.payment_address.as_ref().map(|e| e.pk_d().to_xy()); // Witness the y-coordinate, encoded as little // endian bits (to match the representation) @@ -584,7 +584,7 @@ fn test_input_circuit_with_bls12_381() { } } - let g_d = payment_address.diversifier.g_d(params).unwrap(); + let g_d = payment_address.diversifier().g_d(params).unwrap(); let commitment_randomness = fs::Fs::random(rng); let auth_path = vec![Some((Fr::random(rng), rng.next_u32() % 2 != 0)); tree_depth]; let ar = fs::Fs::random(rng); @@ -595,7 +595,7 @@ fn test_input_circuit_with_bls12_381() { let note = Note { value: value_commitment.value, g_d: g_d.clone(), - pk_d: payment_address.pk_d.clone(), + pk_d: payment_address.pk_d().clone(), r: commitment_randomness.clone(), }; diff --git a/zcash_proofs/src/sapling/prover.rs b/zcash_proofs/src/sapling/prover.rs index 06034245f..9b0f8717c 100644 --- a/zcash_proofs/src/sapling/prover.rs +++ b/zcash_proofs/src/sapling/prover.rs @@ -100,7 +100,7 @@ impl SaplingProvingContext { g_d: diversifier .g_d::(params) .expect("was a valid diversifier before"), - pk_d: payment_address.pk_d.clone(), + pk_d: payment_address.pk_d().clone(), r: rcm, };