From 1e2bc7f65ca68674d522d6220d41b7369b3fbedf Mon Sep 17 00:00:00 2001
From: Jack Grigg <jack@z.cash>
Date: Thu, 22 Aug 2019 00:57:04 +0100
Subject: [PATCH] Test nullifiers in constant time

Checking for spent notes in a block is still not completely constant
time, due to filtering out negative results of the constant-time
comparison.

Part of #84.
---
 Cargo.lock                              |  7 ++++
 zcash_client_backend/Cargo.toml         |  1 +
 zcash_client_backend/src/welding_rig.rs | 46 +++++++++++++------------
 3 files changed, 32 insertions(+), 22 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index 75ff772..fe20d7a 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -588,6 +588,11 @@ dependencies = [
  "opaque-debug 0.2.3 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
+[[package]]
+name = "subtle"
+version = "2.2.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+
 [[package]]
 name = "syn"
 version = "1.0.5"
@@ -645,6 +650,7 @@ dependencies = [
  "rand_core 0.5.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "rand_os 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)",
  "rand_xorshift 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "subtle 2.2.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "zcash_primitives 0.1.0",
 ]
 
@@ -751,6 +757,7 @@ dependencies = [
 "checksum semver 0.9.0 (registry+https://github.com/rust-lang/crates.io-index)" = "1d7eb9ef2c18661902cc47e535f9bc51b78acd254da71d375c2f6720d9a40403"
 "checksum semver-parser 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)" = "388a1df253eca08550bef6c72392cfe7c30914bf41df5269b68cbd6ff8f570a3"
 "checksum sha2 0.8.0 (registry+https://github.com/rust-lang/crates.io-index)" = "7b4d8bfd0e469f417657573d8451fb33d16cfe0989359b93baf3a1ffc639543d"
+"checksum subtle 2.2.1 (registry+https://github.com/rust-lang/crates.io-index)" = "ab3af2eb31c42e8f0ccf43548232556c42737e01a96db6e1777b0be108e79799"
 "checksum syn 1.0.5 (registry+https://github.com/rust-lang/crates.io-index)" = "66850e97125af79138385e9b88339cbcd037e3f28ceab8c5ad98e64f0f1f80bf"
 "checksum typenum 1.11.2 (registry+https://github.com/rust-lang/crates.io-index)" = "6d2783fe2d6b8c1101136184eb41be8b1ad379e4657050b8aaff0c79ee7575f9"
 "checksum unicode-xid 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)" = "826e7639553986605ec5979c7dd957c7895e93eabed50ab2ffa7f6128a75097c"
diff --git a/zcash_client_backend/Cargo.toml b/zcash_client_backend/Cargo.toml
index e639efd..3c8a014 100644
--- a/zcash_client_backend/Cargo.toml
+++ b/zcash_client_backend/Cargo.toml
@@ -17,6 +17,7 @@ ff = { version = "0.5.0", path = "../ff" }
 hex = "0.3"
 pairing = { version = "0.15.0", path = "../pairing" }
 protobuf = "2"
+subtle = "2"
 zcash_primitives = { version = "0.1.0", path = "../zcash_primitives" }
 
 [build-dependencies]
diff --git a/zcash_client_backend/src/welding_rig.rs b/zcash_client_backend/src/welding_rig.rs
index e3f9d8b..307dcea 100644
--- a/zcash_client_backend/src/welding_rig.rs
+++ b/zcash_client_backend/src/welding_rig.rs
@@ -3,6 +3,7 @@
 use ff::{PrimeField, PrimeFieldRepr};
 use pairing::bls12_381::{Bls12, Fr, FrRepr};
 use std::collections::HashSet;
+use subtle::{ConditionallySelectable, ConstantTimeEq, CtOption};
 use zcash_primitives::{
     jubjub::{edwards, fs::Fs},
     merkle_tree::{CommitmentTree, IncrementalWitness},
@@ -116,28 +117,29 @@ pub fn scan_block(
         let num_outputs = tx.outputs.len();
 
         // Check for spent notes
-        let shielded_spends: Vec<_> =
-            tx.spends
-                .into_iter()
-                .enumerate()
-                .filter_map(|(index, spend)| {
-                    if let Some(account) = nullifiers.iter().find_map(|&(nf, acc)| {
-                        if nf == &spend.nf[..] {
-                            Some(acc)
-                        } else {
-                            None
-                        }
-                    }) {
-                        Some(WalletShieldedSpend {
-                            index,
-                            nf: spend.nf,
-                            account,
-                        })
-                    } else {
-                        None
-                    }
-                })
-                .collect();
+        // The only step that is not constant-time is the filter() at the end.
+        let shielded_spends: Vec<_> = tx
+            .spends
+            .into_iter()
+            .enumerate()
+            .map(|(index, spend)| {
+                // Find the first tracked nullifier that matches this spend, and produce
+                // a WalletShieldedSpend if there is a match, in constant time.
+                nullifiers
+                    .iter()
+                    .map(|&(nf, account)| CtOption::new(account as u64, nf.ct_eq(&spend.nf[..])))
+                    .fold(CtOption::new(0, 0.into()), |first, next| {
+                        CtOption::conditional_select(&next, &first, first.is_some())
+                    })
+                    .map(|account| WalletShieldedSpend {
+                        index,
+                        nf: spend.nf,
+                        account: account as usize,
+                    })
+            })
+            .filter(|spend| spend.is_some().into())
+            .map(|spend| spend.unwrap())
+            .collect();
 
         // Collect the set of accounts that were spent from in this transaction
         let spent_from_accounts: HashSet<_> =