feat(zcash): sign checked PCZTs with a single parse and in-memory postflight
What changed, and why it matters
This commit adds a new, more efficient way to sign Zcash shielded transactions on Keystone hardware wallets. Instead of parsing and serializing the transaction multiple times, it parses once, signs in memory, and verifies signatures before returning the result. The change also preserves the checked/verified transaction data through the signing process rather than discarding it. There is no clear security bug being fixed; it appears to be a hardening and performance improvement for the Zcash cypherpunk feature.
Treat as a routine hardening/refactoring change. Reviewers should verify that the new `sign_checked_pczt` path is used by the firmware's UI/transaction approval flow, that the in-memory verification correctly covers all supported shielded action types, and that no legacy multi-parse signing path remains exposed to users. No urgent security response is indicated by the diff alone.
Security signals we found
New in-memory post-sign verification path reduces opportunities for serialization/deserialization attacks or state mismatches.
Previously discarded verifier output is now retained and propagated, eliminating a potential check-then-drop pattern.
Single-parse path reduces attack surface by avoiding repeated PCZT parsing.
Foreign seed and unsupported Sapling rejection tests confirm existing access-control boundaries are preserved.
No explicit vulnerability, CVE, or security advisory is mentioned in the commit or supplied references.
Evidence from the diff
The commit refactors Zcash PCZT (Partially Created Zcash Transaction) signing in the Keystone 3 firmware’s Rust code. Key changes: (1) signable_shielded_actions now returns both the list of signable actions and the verified Pczt object by calling verifier.finish() instead of dropping the verifier. (2) ensure_shielded_actions_are_signed now returns the verified Pczt. (3) A new public function sign_checked_pczt (and batch variant) parses the PCZT once, extracts signable actions, signs in memory via a new sign_pczt_to_pczt helper, and then verifies signatures in memory before serializing. (4) sign_pczt_to_pczt is a non-serializing variant of sign_pczt to avoid a byte round-trip. Tests are added for owned Orchard actions, foreign seed rejection, and batch policy rejection of unsupported Sapling transactions.
Changed components
rust/apps/zcash/src/lib.rsrust/apps/zcash/src/pczt/sign.rsZcash PCZT signing flow (cypherpunk feature)Orchard shielded action signingBatch and single-transaction signing policiesInspect captured patch +199 / −15
diff --git a/rust/apps/zcash/src/lib.rs b/rust/apps/zcash/src/lib.rs
index 4eca1a3..79e0847 100644
--- a/rust/apps/zcash/src/lib.rs
+++ b/rust/apps/zcash/src/lib.rs
@@ -556,7 +556,7 @@ fn signable_shielded_actions<P: consensus::Parameters>(
seed_fingerprint: &[u8; 32],
account_index: zip32::AccountId,
policy: ShieldedActionPolicy,
-) -> Result<Vec<SignableShieldedAction>> {
+) -> Result<(Vec<SignableShieldedAction>, Pczt)> {
use zcash_vendor::pczt::roles::verifier::Verifier;
if policy == ShieldedActionPolicy::Batch {
@@ -598,16 +598,16 @@ fn signable_shielded_actions<P: consensus::Parameters>(
} else {
verifier
};
- drop(verifier);
+ let pczt = verifier.finish();
- Ok(actions)
+ Ok((actions, pczt))
}
#[cfg(feature = "cypherpunk")]
fn ensure_shielded_actions_are_signed(
signed_pczt: Pczt,
signable_actions: &[SignableShieldedAction],
-) -> Result<()> {
+) -> Result<Pczt> {
use zcash_vendor::pczt::roles::verifier::Verifier;
#[cfg(zcash_unstable = "nu6.3")]
@@ -628,9 +628,8 @@ fn ensure_shielded_actions_are_signed(
} else {
verifier
};
- drop(verifier);
- Ok(())
+ Ok(verifier.finish())
}
/// Checks whether the PCZT contains at least one non-dummy supported shielded
@@ -651,15 +650,14 @@ pub fn ensure_pczt_has_signable_shielded_action<P: consensus::Parameters>(
let account_index = zip32::AccountId::try_from(account_index)
.map_err(|_e| ZcashError::InvalidDataError("invalid account index".to_string()))?;
- if signable_shielded_actions(
+ let (signable_actions, _pczt) = signable_shielded_actions(
params,
pczt,
seed_fingerprint,
account_index,
ShieldedActionPolicy::Batch,
- )?
- .is_empty()
- {
+ )?;
+ if signable_actions.is_empty() {
Err(ZcashError::PcztNoMyInputs)
} else {
Ok(())
@@ -679,7 +677,7 @@ pub fn ensure_signable_shielded_actions_are_signed<P: consensus::Parameters>(
let unsigned_pczt = pczt::parse_pczt(unsigned_pczt)?;
let account_index = zip32::AccountId::try_from(account_index)
.map_err(|_e| ZcashError::InvalidDataError("invalid account index".to_string()))?;
- let signable_actions = signable_shielded_actions(
+ let (signable_actions, _pczt) = signable_shielded_actions(
params,
unsigned_pczt,
seed_fingerprint,
@@ -691,7 +689,8 @@ pub fn ensure_signable_shielded_actions_are_signed<P: consensus::Parameters>(
} else {
let signed_pczt = pczt::parse_pczt(signed_pczt)
.map_err(|_| ZcashError::InvalidPczt("invalid signed pczt data".to_string()))?;
- ensure_shielded_actions_are_signed(signed_pczt, &signable_actions)
+ ensure_shielded_actions_are_signed(signed_pczt, &signable_actions)?;
+ Ok(())
}
}
@@ -708,7 +707,7 @@ pub fn ensure_owned_supported_shielded_actions_are_signed<P: consensus::Paramete
let unsigned_pczt = pczt::parse_pczt(unsigned_pczt)?;
let account_index = zip32::AccountId::try_from(account_index)
.map_err(|_e| ZcashError::InvalidDataError("invalid account index".to_string()))?;
- let signable_actions = signable_shielded_actions(
+ let (signable_actions, _pczt) = signable_shielded_actions(
params,
unsigned_pczt,
seed_fingerprint,
@@ -720,8 +719,83 @@ pub fn ensure_owned_supported_shielded_actions_are_signed<P: consensus::Paramete
} else {
let signed_pczt = pczt::parse_pczt(signed_pczt)
.map_err(|_| ZcashError::InvalidPczt("invalid signed pczt data".to_string()))?;
- ensure_shielded_actions_are_signed(signed_pczt, &signable_actions)
+ ensure_shielded_actions_are_signed(signed_pczt, &signable_actions)?;
+ Ok(())
+ }
+}
+
+/// Signs a preflight-checked, normalized PCZT and confirms in memory that every
+/// supported shielded action owned by (`seed_fingerprint`, `account_index`)
+/// received a spend authorization signature. Single-transaction policy: a PCZT
+/// with no owned shielded action still signs if any action matched the seed.
+/// Parses `checked_pczt` exactly once and returns the redacted, version-stamped
+/// response bytes.
+#[cfg(feature = "cypherpunk")]
+pub fn sign_checked_pczt<P: consensus::Parameters>(
+ params: &P,
+ checked_pczt: &[u8],
+ seed: &[u8],
+ seed_fingerprint: &[u8; 32],
+ account_index: u32,
+) -> Result<Vec<u8>> {
+ sign_checked_pczt_with_policy(
+ params,
+ checked_pczt,
+ seed,
+ seed_fingerprint,
+ account_index,
+ ShieldedActionPolicy::Single,
+ )
+}
+
+/// Signs a preflight-checked, normalized PCZT and confirms in memory that every
+/// supported shielded action owned by (`seed_fingerprint`, `account_index`)
+/// received a spend authorization signature. Batch policy: additionally rejects
+/// PCZT shapes the batch flow does not support and requires at least one owned
+/// signable shielded action. Parses `checked_pczt` exactly once and returns the
+/// redacted, version-stamped response bytes.
+#[cfg(feature = "cypherpunk")]
+pub fn sign_checked_batch_pczt<P: consensus::Parameters>(
+ params: &P,
+ checked_pczt: &[u8],
+ seed: &[u8],
+ seed_fingerprint: &[u8; 32],
+ account_index: u32,
+) -> Result<Vec<u8>> {
+ sign_checked_pczt_with_policy(
+ params,
+ checked_pczt,
+ seed,
+ seed_fingerprint,
+ account_index,
+ ShieldedActionPolicy::Batch,
+ )
+}
+
+#[cfg(feature = "cypherpunk")]
+fn sign_checked_pczt_with_policy<P: consensus::Parameters>(
+ params: &P,
+ checked_pczt: &[u8],
+ seed: &[u8],
+ seed_fingerprint: &[u8; 32],
+ account_index: u32,
+ policy: ShieldedActionPolicy,
+) -> Result<Vec<u8>> {
+ let pczt = pczt::parse_pczt(checked_pczt)?;
+ let account_index = zip32::AccountId::try_from(account_index)
+ .map_err(|_e| ZcashError::InvalidDataError("invalid account index".to_string()))?;
+ let (signable_actions, pczt) =
+ signable_shielded_actions(params, pczt, seed_fingerprint, account_index, policy)?;
+ if policy == ShieldedActionPolicy::Batch && signable_actions.is_empty() {
+ return Err(ZcashError::PcztNoMyInputs);
}
+ let signed = pczt::sign::sign_pczt_to_pczt(pczt, seed)?;
+ let signed = if signable_actions.is_empty() {
+ signed
+ } else {
+ ensure_shielded_actions_are_signed(signed, &signable_actions)?
+ };
+ Ok(signed.serialize())
}
#[cfg(feature = "cypherpunk")]
@@ -1444,4 +1518,103 @@ mod tests {
)
.unwrap();
}
+
+ #[cfg(zcash_unstable = "nu6.3")]
+ #[test]
+ fn test_sign_checked_pczt_signs_owned_orchard_actions() {
+ let sample = pczt::test_support::sample_orchard_change_pczt();
+ let normalized = preflight_pczt_cypherpunk(
+ &pczt::test_support::Nu6_3Network,
+ &sample.bytes,
+ &sample.ufvk_text,
+ &sample.seed_fingerprint,
+ 0,
+ )
+ .unwrap();
+
+ let signed = sign_checked_pczt(
+ &pczt::test_support::Nu6_3Network,
+ &normalized,
+ &sample.seed,
+ &sample.seed_fingerprint,
+ 0,
+ )
+ .unwrap();
+
+ let parsed = Pczt::parse(&signed).expect("signed PCZT must parse");
+ let signed_actions = parsed
+ .orchard()
+ .actions()
+ .iter()
+ .filter(|action| action.spend().spend_auth_sig().is_some())
+ .count();
+ assert_eq!(signed_actions, 2);
+ }
+
+ #[cfg(zcash_unstable = "nu6.3")]
+ #[test]
+ fn test_sign_checked_pczt_rejects_foreign_seed() {
+ let sample = pczt::test_support::sample_orchard_change_pczt();
+ let normalized = preflight_pczt_cypherpunk(
+ &pczt::test_support::Nu6_3Network,
+ &sample.bytes,
+ &sample.ufvk_text,
+ &sample.seed_fingerprint,
+ 0,
+ )
+ .unwrap();
+ let foreign_seed = [9u8; 32];
+ let foreign_fingerprint = calculate_seed_fingerprint(&foreign_seed).unwrap();
+
+ let result = sign_checked_pczt(
+ &pczt::test_support::Nu6_3Network,
+ &normalized,
+ &foreign_seed,
+ &foreign_fingerprint,
+ 0,
+ );
+ assert!(matches!(result, Err(ZcashError::PcztNoMyInputs)));
+ }
+
+ #[cfg(zcash_unstable = "nu6.3")]
+ #[test]
+ fn test_sign_checked_batch_pczt_signs_and_rejects_sapling() {
+ let sample = pczt::test_support::sample_orchard_change_pczt();
+ let signed = sign_checked_batch_pczt(
+ &pczt::test_support::Nu6_3Network,
+ &sample.bytes,
+ &sample.seed,
+ &sample.seed_fingerprint,
+ 0,
+ )
+ .unwrap();
+ assert!(Pczt::parse(&signed)
+ .unwrap()
+ .orchard()
+ .actions()
+ .iter()
+ .any(|action| action.spend().spend_auth_sig().is_some()));
+
+ // Batch policy: account 1 owns nothing in this PCZT.
+ assert_eq!(
+ sign_checked_batch_pczt(
+ &pczt::test_support::Nu6_3Network,
+ &sample.bytes,
+ &sample.seed,
+ &sample.seed_fingerprint,
+ 1,
+ )
+ .unwrap_err(),
+ ZcashError::PcztNoMyInputs
+ );
+
+ let sapling_sample = pczt_with_sapling_output();
+ assert_batch_unsupported_sapling_error(sign_checked_batch_pczt(
+ &pczt::test_support::Nu6_3Network,
+ &sapling_sample.bytes,
+ &sapling_sample.seed,
+ &sapling_sample.seed_fingerprint,
+ 0,
+ ));
+ }
}
diff --git a/rust/apps/zcash/src/pczt/sign.rs b/rust/apps/zcash/src/pczt/sign.rs
index 61ddf7a..bdaa8c1 100644
--- a/rust/apps/zcash/src/pczt/sign.rs
+++ b/rust/apps/zcash/src/pczt/sign.rs
@@ -238,8 +238,19 @@ impl PcztSigner for SeedSigner<'_> {
}
}
+/// Signs `pczt` and serializes the stamped, redacted response.
+///
+/// Thin wrapper over `sign_pczt_to_pczt`; see it for the full contract.
#[cfg(feature = "cypherpunk")]
pub fn sign_pczt(pczt: Pczt, seed: &[u8]) -> crate::Result<Vec<u8>> {
+ Ok(sign_pczt_to_pczt(pczt, seed)?.serialize())
+}
+
+/// `sign_pczt`, but returns the stamped, redacted PCZT without serializing it,
+/// so callers that still need the parsed value (in-memory post-sign
+/// verification) avoid a byte round trip.
+#[cfg(feature = "cypherpunk")]
+pub fn sign_pczt_to_pczt(pczt: Pczt, seed: &[u8]) -> crate::Result<Pczt> {
super::validate_supported_pczt(&pczt)?;
let seed_fingerprint =
@@ -277,7 +288,7 @@ pub fn sign_pczt(pczt: Pczt, seed: &[u8]) -> crate::Result<Vec<u8>> {
return Err(ZcashError::PcztNoMyInputs);
}
- Ok(stamp_and_redact(signer.finish()).serialize())
+ Ok(stamp_and_redact(signer.finish()))
}
fn stamp_and_redact(pczt: Pczt) -> Pczt {
Why this scored 29/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.