Skip to content
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

PaymentAddress encapsulation #109

Merged
merged 4 commits into from Sep 6, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 1 addition & 2 deletions librustzcash/include/librustzcash.h
Expand Up @@ -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,
Copy link
Contributor

Choose a reason for hiding this comment

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

What does this end up looking like on the cpp side? Previously we were passing in:

output.note.d.data(),
output.note.pk_d.begin(),

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On the C++ side, those two fields are concatenated.

const unsigned char *rcm,
const uint64_t value,
unsigned char *cv,
Expand Down
38 changes: 8 additions & 30 deletions librustzcash/src/rustzcash.rs
Expand Up @@ -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],
Expand All @@ -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::<Bls12, Unknown>::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::<Bls12>::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 })[..])) {
Expand Down Expand Up @@ -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
}
4 changes: 2 additions & 2 deletions librustzcash/src/tests/key_agreement.rs
Expand Up @@ -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,
Expand All @@ -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
));
Expand Down
2 changes: 1 addition & 1 deletion librustzcash/src/tests/key_components.rs
Expand Up @@ -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);
}
{
Expand Down
58 changes: 26 additions & 32 deletions zcash_client_backend/src/encoding.rs
Expand Up @@ -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,
};
Expand Down Expand Up @@ -113,21 +110,19 @@ pub fn decode_extended_full_viewing_key(
/// 0xbc, 0xe5,
/// ]);
///
/// let pa = PaymentAddress {
/// diversifier: Diversifier([0u8; 11]),
/// pk_d: edwards::Point::<Bls12, _>::rand(rng, &JUBJUB).mul_by_cofactor(&JUBJUB),
/// };
/// let pa = PaymentAddress::from_parts(
/// Diversifier([0u8; 11]),
/// edwards::Point::<Bls12, _>::rand(rng, &JUBJUB).mul_by_cofactor(&JUBJUB),
/// )
/// .unwrap();
///
/// assert_eq!(
/// encode_payment_address(HRP_SAPLING_PAYMENT_ADDRESS, &pa),
/// "ztestsapling1qqqqqqqqqqqqqqqqqrjq05nyfku05msvu49mawhg6kr0wwljahypwyk2h88z6975u563j0ym7pe",
/// );
/// ```
pub fn encode_payment_address(hrp: &str, addr: &PaymentAddress<Bls12>) -> 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.
Expand All @@ -153,10 +148,11 @@ pub fn encode_payment_address(hrp: &str, addr: &PaymentAddress<Bls12>) -> String
/// 0xbc, 0xe5,
/// ]);
///
/// let pa = PaymentAddress {
/// diversifier: Diversifier([0u8; 11]),
/// pk_d: edwards::Point::<Bls12, _>::rand(rng, &JUBJUB).mul_by_cofactor(&JUBJUB),
/// };
/// let pa = PaymentAddress::from_parts(
/// Diversifier([0u8; 11]),
/// edwards::Point::<Bls12, _>::rand(rng, &JUBJUB).mul_by_cofactor(&JUBJUB),
/// )
/// .unwrap();
///
/// assert_eq!(
/// decode_payment_address(
Expand All @@ -168,17 +164,13 @@ pub fn encode_payment_address(hrp: &str, addr: &PaymentAddress<Bls12>) -> String
/// ```
pub fn decode_payment_address(hrp: &str, s: &str) -> Result<Option<PaymentAddress<Bls12>>, 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::<Bls12>(&JUBJUB).is_none() {
if data.len() != 43 {
return None;
}

edwards::Point::<Bls12, _>::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::<Bls12>::from_bytes(&bytes, &JUBJUB)
})
}

Expand All @@ -203,10 +195,11 @@ mod tests {
0xbc, 0xe5,
]);

let addr = PaymentAddress {
diversifier: Diversifier([0u8; 11]),
pk_d: edwards::Point::<Bls12, _>::rand(rng, &JUBJUB).mul_by_cofactor(&JUBJUB),
};
let addr = PaymentAddress::from_parts(
Diversifier([0u8; 11]),
edwards::Point::<Bls12, _>::rand(rng, &JUBJUB).mul_by_cofactor(&JUBJUB),
)
.unwrap();

let encoded_main =
"zs1qqqqqqqqqqqqqqqqqrjq05nyfku05msvu49mawhg6kr0wwljahypwyk2h88z6975u563j8nfaxd";
Expand Down Expand Up @@ -247,10 +240,11 @@ mod tests {
0xbc, 0xe5,
]);

let addr = PaymentAddress {
diversifier: Diversifier([1u8; 11]),
pk_d: edwards::Point::<Bls12, _>::rand(rng, &JUBJUB).mul_by_cofactor(&JUBJUB),
};
let addr = PaymentAddress::from_parts(
Diversifier([1u8; 11]),
edwards::Point::<Bls12, _>::rand(rng, &JUBJUB).mul_by_cofactor(&JUBJUB),
)
.unwrap();

let encoded_main =
encode_payment_address(constants::mainnet::HRP_SAPLING_PAYMENT_ADDRESS, &addr);
Expand Down
91 changes: 59 additions & 32 deletions zcash_primitives/src/note_encryption.rs
Expand Up @@ -232,10 +232,7 @@ fn prf_ock(
///
/// let diversifier = Diversifier([0; 11]);
/// let pk_d = diversifier.g_d::<Bls12>(&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;
Expand Down Expand Up @@ -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::<LittleEndian>(self.note.value)
.unwrap();
Expand Down Expand Up @@ -375,7 +372,7 @@ fn parse_note_plaintext_without_memo(
.g_d::<Bls12>(&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 {
Expand Down Expand Up @@ -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)?;
Copy link
Contributor

Choose a reason for hiding this comment

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

This can propagate the error by returning None, but that is correct and fixes a bug :-)

let note = to.create_note(v, rcm, &JUBJUB).unwrap();

if note.cm(&JUBJUB) != *cmu {
Expand Down Expand Up @@ -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<R: RngCore + CryptoRng>(
ivk: Fs,
mut rng: &mut R,
) -> (
OutgoingViewingKey,
Fs,
edwards::Point<Bls12, Unknown>,
Fr,
edwards::Point<Bls12, PrimeOrder>,
[u8; ENC_CIPHERTEXT_SIZE],
[u8; OUT_CIPHERTEXT_SIZE],
) {
let diversifier = Diversifier([0; 11]);
let pk_d = diversifier.g_d::<Bls12>(&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;
Expand All @@ -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,
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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);

Expand Down