Sapling note encryption by str4d · Pull Request #69 · zcash/librustzcash

str4d · 2019-03-07T22:00:02Z

Implements full trial decryption (per the protocol spec) as well as compact trial decryption (per ZIP 307).

str4d · 2019-03-23T10:38:11Z

Addressed @ebfull's comments.

Part of ZIP 307.

str4d · 2019-04-05T20:08:54Z

Rebased on master to address merge conflicts.

This crate exposes both the ChaCha20Poly1305 IETF construction, and the underlying ChaCha20 IETF primitive, removing the need for depending on our own fork of the previous chacha20-poly1305-aead crate.

The crypto_api_chachapoly uses two new features introduced in 1.32: - Self struct constructors - u64::to_le_bytes()

str4d · 2019-04-11T13:47:06Z

In the last month, a new crate has been published that exposes both the ChaCha20Poly1305 IETF construction and the underlying ChaCha20 IETF primitive. I've pushed a commit that switches us from our own fork of an implementation (which would inhibit us publishing crates) to this new crate. The previous test vectors all pass.

Note that this requires raising our minimum Rust version to 1.32. Incidentally, by doing this we should be able to replace our own usage of the byteorder crate with conversion functions on the inbuilt primitives.

Make util module public

Eirik0

Looks good, just had a couple of minor nits. I requested changes.

daira · 2019-05-28T23:35:29Z

Please note that merging this also blocks on reviewing the crypto_api and crypto_api_chachapoly crates. We shouldn't switch to them without verifying that they have identical behaviour on all inputs.

str4d · 2019-05-29T15:59:47Z

@daira we don't yet depend on any Chacha20Poly1305 implementation in the Rust codebase. When I said "switch", I meant that this PR originally added a dependency on (a fork of) one implementation, and it now adds a dependency on a different implementation. I agree that we should still be happy that the dependency is solid for our usage.

Eirik0

utACK modulo @daira's comment

daira · 2019-05-30T21:13:38Z

@defuse, @str4d and I reviewed the crypto_api_chachapoly AEAD implementation. I initially thought that there was a problem with this implementation when the AAD (which we don't use) or the plaintext is not a multiple of 16 bytes — but I was mistaken. The necessary padding is implemented correctly in poly1305_update here:
https://github.com/KizzyCode/crypto_api_chachapoly/blob/933944e01d83064907613f7964044f333828e9de/src/poly1305.rs#L41-L57
and the is_last flag is used correctly by the caller here:
https://github.com/KizzyCode/crypto_api_chachapoly/blob/933944e01d83064907613f7964044f333828e9de/src/poly1305.rs#L134-L147

I have not yet reviewed the PR itself.

str4d · 2019-06-04T09:42:35Z

Two issues were opened on crypto_api_chachapoly as a result of the review, which do not affect this PR:

CHACHAPOLY_MAX is too large by 1 block KizzyCode/crypto_api_chachapoly#2
- We only encrypt notes, which are fixed-length.
User of new_sec_key might assume it fills the entire buffer KizzyCode/crypto_api_chachapoly#3
- We do not use the new_sec_key function.

daira

Some changes needed.

daira · 2019-06-04T12:27:30Z

+
+    let pk_d = edwards::Point::<Bls12, _>::read(&op[0..32], &JUBJUB)
+        .ok()?
+        .as_prime_order(&JUBJUB)?;


as_prime_order does not check that the point is of prime order (in KA^Sapling.PublicPrimeOrder which is J^(r)∗), as required by section 4.17.3. It only checks that it is in the prime-order subgroup J^(r).

Misleading method name as_prime_order noted at zcash/sapling-crypto#97 (comment) .

daira · 2019-06-04T18:50:20Z

+            assert_eq!(&ne.encrypt_outgoing_plaintext(&cv, &cmu)[..], &tv.c_out[..]);
+        }
+    }
+}


Despite my nitpicking, these are excellent tests! 👍

daira · 2019-06-04T18:51:19Z

+}
+
+pub(crate) fn make_test_vectors() -> Vec<TestVector> {
+    // From https://github.com/zcash-hackworks/zcash-test-vectors/blob/master/sapling_note_encryption.py


I have not checked that these match the source.

This matches how sapling_ka_agree and kdf_sapling are defined in the protocol spec. kdf_sapling also now takes ownership of dhsecret to discourage use of the sapling_ka_agree output elsewhere.

str4d · 2019-06-06T12:58:06Z

Addressed @daira's comments. I also replaced all uses of Vec with fixed-length arrays.

daira

ut(ACK+cov) modulo one unaddressed comment (the missing spec reference).

daira · 2019-06-06T18:06:20Z

 "checksum constant_time_eq 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)" = "8ff012e225ce166d4422e0e78419d901719760f62ae2b7969ca6b564d1b54a9e"
 "checksum crossbeam 0.3.2 (registry+https://github.com/rust-lang/crates.io-index)" = "24ce9782d4d5c53674646a6a4c1863a21a8fc0cb649b3c94dfc16e45071dea19"
+"checksum crypto_api 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)" = "2f855e87e75a4799e18b8529178adcde6fd4f97c1449ff4821e747ff728bb102"
+"checksum crypto_api_chachapoly 0.1.8 (registry+https://github.com/rust-lang/crates.io-index)" = "2f9ee35dbace0831b5fe7cb9b43eb029aa14a10f594a115025d4628a2baa63ab"


I have not checked these hashes.

daira · 2019-06-06T19:11:07Z

+        NOTE_PLAINTEXT_SIZE
+    );
+
+    // Check note plaintext version


str4d · 2019-06-06T19:29:25Z

@daira the comment was added to the top of the file in 23aa869.

…cash#69) * Ballot scaling in ZKP #1: convert zatoshi to ballot count Replace the minimum-weight check (condition 8) with ballot scaling that floor-divides v_total by 12,500,000 to produce num_ballots. Condition 7 now hashes num_ballots into the VAN commitment instead of the raw v_total. Circuit constraints for condition 8: - num_ballots * BALLOT_DIVISOR + remainder == v_total - remainder < 2^24 (via shift-by-2^6 into 30-bit lookup check) - 0 < num_ballots <= 2^30 (via nb_minus_one 30-bit range check) Adds MulChip (c = a * b gate) used for the reconstruction constraint and the remainder bit-shift. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Apply ballot scaling to VAN hashes in ZKP #2 and librustvoting ZKP #1 now hashes num_ballots (not raw zatoshi) into VAN commitments. Update all downstream VAN hash callers to match: - vote_proof/builder.rs: convert total_note_value to num_ballots before VAN integrity hashing and share splitting - governance.rs: construct_van now divides total_weight by BALLOT_DIVISOR - Update test values to use weights >= 12,500,000 (one ballot minimum) - Freeze new known-answer VAN test vector Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>

str4d requested a review from ebfull March 7, 2019 22:35

str4d mentioned this pull request Mar 7, 2019

Block header parsing and transaction ID computation #66

Merged

ebfull requested changes Mar 15, 2019

View reviewed changes

str4d and others added 13 commits April 5, 2019 21:05

Move Sapling key structs from zip32 to zcash_primitives

9b455a1

Implement Sapling note encryption

65bbe7d

Sapling note encryption test vectors

70caa7d

Test prf_ock

757316d

Trial Sapling note decryption

6996853

Trial Sapling output recovery

8e098d4

Trial Sapling compact note decryption

484330e

Part of ZIP 307.

Impl traits and functions for Memo

247f3fb

Deduplicate Sapling key agreement logic

2b1583d

Use a slice instead of a vector in prf_expand()

566db65

Inline empty nonces

899d852

Enforce consistent plaintext and ciphertext lengths

9086dd9

Test invalid decryption edge cases

e17e4b1

str4d force-pushed the sapling-note-encryption branch from d4e6a66 to e17e4b1 Compare April 5, 2019 20:08

str4d requested a review from ebfull April 5, 2019 20:08

str4d added 2 commits April 11, 2019 06:33

Switch to crypto_api_chachapoly crate

6dcb404

This crate exposes both the ChaCha20Poly1305 IETF construction, and the underlying ChaCha20 IETF primitive, removing the need for depending on our own fork of the previous chacha20-poly1305-aead crate.

Raise minimum Rust version to 1.32

34658c4

The crypto_api_chachapoly uses two new features introduced in 1.32: - Self struct constructors - u64::to_le_bytes()

Document note_encryption module

edf7bc1

str4d force-pushed the sapling-note-encryption branch from 9207b35 to edf7bc1 Compare April 11, 2019 23:13

str4d mentioned this pull request Apr 16, 2019

Sapling commitment tree #68

Merged

bitcartel pushed a commit to bitcartel/librustzcash that referenced this pull request May 16, 2019

Merge pull request zcash#69 from ebfull/make-util-pub

7beeb52

Make util module public

Eirik0 suggested changes May 28, 2019

View reviewed changes

Comment thread zcash_primitives/src/note_encryption.rs Outdated

Comment thread zcash_primitives/src/note_encryption.rs

Comment thread zcash_primitives/src/note_encryption.rs

Comment thread zcash_primitives/src/note_encryption.rs

Eirik0 approved these changes May 29, 2019

View reviewed changes

mms710 requested a review from daira May 30, 2019 17:08

ebfull approved these changes May 30, 2019

View reviewed changes

daira requested changes Jun 4, 2019

View reviewed changes

str4d mentioned this pull request Jun 5, 2019

AeadCipher::seal_to implementation copies plaintext into output buffer before error checks KizzyCode/crypto_api_chachapoly#4

Closed

str4d added 9 commits June 5, 2019 13:55

Replace AeadCipher::seal with AeadCipher::seal_to

6d03b5c

Add comments with specification references

23aa869

Tweaks to debug output and function names

d4fce58

Use fixed-length arrays instead of Vec

75bede4

Require that ak in FullViewingKey is prime order

6846ac5

Simplify Memo::to_utf8 implementation

9d80be6

Return edwards::Point from sapling_ka_agree

060977f

This matches how sapling_ka_agree and kdf_sapling are defined in the protocol spec. kdf_sapling also now takes ownership of dhsecret to discourage use of the sapling_ka_agree output elsewhere.

Check note plaintext version byte when decrypting

fdb6e20

Test both invalid and incorrect diversifiers

b65aae9

str4d requested a review from daira June 6, 2019 12:58

ebfull approved these changes Jun 6, 2019

View reviewed changes

daira approved these changes Jun 6, 2019

View reviewed changes

str4d merged commit 3b6f5e3 into zcash:master Jun 6, 2019

str4d deleted the sapling-note-encryption branch June 6, 2019 19:50

KizzyCode added a commit to KizzyCode/crypto_api_chachapoly that referenced this pull request Jun 6, 2019

Made ChaCha20 public as requested in zcash/librustzcash#69 (comment)

4fe55c5

str4d mentioned this pull request Jun 10, 2019

Migrate to blake2b_simd and blake2s_simd crates #83

Merged

str4d mentioned this pull request Aug 5, 2019

Upgrade dependencies #94

Merged

str4d added this to the v0.1.0 milestone Aug 22, 2019

str4d mentioned this pull request Jan 26, 2022

Add auto-shielding to the data access API #341

Merged

Conversation

str4d commented Mar 7, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

str4d commented Mar 23, 2019

Uh oh!

str4d commented Apr 5, 2019

Uh oh!

str4d commented Apr 11, 2019

Uh oh!

Eirik0 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

daira commented May 28, 2019

Uh oh!

str4d commented May 29, 2019

Uh oh!

Eirik0 left a comment

Choose a reason for hiding this comment

Uh oh!

daira commented May 30, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

str4d commented Jun 4, 2019

Uh oh!

daira left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

daira Jun 4, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

daira Jun 4, 2019

Choose a reason for hiding this comment

Uh oh!

daira Jun 4, 2019

Choose a reason for hiding this comment

Uh oh!

str4d commented Jun 6, 2019

Uh oh!

daira left a comment

Choose a reason for hiding this comment

Uh oh!

daira Jun 6, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

daira Jun 6, 2019

Choose a reason for hiding this comment

Uh oh!

str4d commented Jun 6, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

daira commented May 30, 2019 •

edited

Loading