PR471 review: Fix issuance key derivation to use dedicated ZIP-32 context (#229)

Address the following review comments:
r2542655700
r2542668676

This PR introduces a dedicated ZIP-227 issuance key derivation using
hardened-only ZIP-32, separate from Orchard spending keys.

Author: ConstanceBeguier

src/issuance/auth.rs

@@ -20,23 +20,83 @@
 
 use alloc::vec::Vec;
 use core::{
+    fmt,
     fmt::{Debug, Formatter},
     mem::size_of_val,
 };
 
 use rand_core::CryptoRngCore;
 use secp256k1::{schnorr, Keypair, Message, Secp256k1, SecretKey, XOnlyPublicKey};
 
-use crate::{
-    issuance::Error,
-    zip32::{self, ExtendedSpendingKey},
-};
+use crate::issuance::Error;
 
-// Preserve '::' which specifies the EXTERNAL 'zip32' crate
-#[rustfmt::skip]
-pub use ::zip32::{AccountId, ChildIndex, DiversifierIndex, Scope, hardened_only};
+pub use ::zip32::{
+    hardened_only, hardened_only::HardenedOnlyKey, AccountId, ChildIndex, DiversifierIndex, Scope,
+};
+use zcash_spec::{PrfExpand, VariableLengthSlice};
 
 const ZIP32_PURPOSE_FOR_ISSUANCE: u32 = 227;
+const ZIP32_ORCHARD_ISSUANCE_PERSONALIZATION: &[u8; 16] = b"ZcashSA_Issue_V1";
+
+/// Errors produced in derivation of extended issuance keys
+#[derive(Debug, PartialEq, Eq)]
+pub enum Zip32Error {
+    /// A seed resulted in an invalid issuance key
+    InvalidIssuanceKey,
+    /// A non zero account when deriving an Orchard-ZSA issuance key
+    NonZeroAccount,
+}
+
+impl fmt::Display for Zip32Error {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            Zip32Error::InvalidIssuanceKey => {
+                write!(f, "Seed produced an invalid issuance authorizing key.")
+            }
+            Zip32Error::NonZeroAccount => {
+                write!(
+                    f,
+                    "A non zero account when deriving an Orchard-ZSA issuance key"
+                )
+            }
+        }
+    }
+}
+
+#[derive(Clone, Copy, Debug)]
+struct Issuance;
+
+impl hardened_only::Context for Issuance {
+    const MKG_DOMAIN: [u8; 16] = *ZIP32_ORCHARD_ISSUANCE_PERSONALIZATION;
+    const CKD_DOMAIN: PrfExpand<([u8; 32], [u8; 4], [u8; 1], VariableLengthSlice)> =
+        PrfExpand::ORCHARD_ZIP32_CHILD;
+}
+
+/// An extended issuance key.
+///
+/// Defined in [ZIP227: Issuance of Zcash Shielded Assets][issuancekeyderivation].
+///
+/// [issuancekeyderivation]: https://zips.z.cash/zip-0227#issuance-key-derivation
+#[derive(Debug, Clone)]
+pub(crate) struct ExtendedIssuanceKey {
+    inner: HardenedOnlyKey<Issuance>,
+}
+
+impl ExtendedIssuanceKey {
+    /// Derives an issuance extended key from a ZIP-32 seed and a hardened derivation path.
+    pub fn from_path(seed: &[u8], path: &[ChildIndex]) -> Self {
+        let mut key = HardenedOnlyKey::<Issuance>::master(&[seed]);
+        for index in path {
+            key = key.derive_child(*index);
+        }
+        Self { inner: key }
+    }
+
+    /// Returns the raw 32-byte sk.
+    pub fn sk_bytes(&self) -> [u8; 32] {
+        *self.inner.parts().0
+    }
+}
 
 /// Trait that defines the common interface for issuance authorization signature schemes.
 pub trait IssueAuthSigScheme {
@@ -157,13 +217,9 @@ impl IssueAuthKey<ZSASchnorr> {
     }
 
     /// Derives the Orchard-ZSA issuance key for the given seed, coin type, and account.
-    pub fn from_zip32_seed(
-        seed: &[u8],
-        coin_type: u32,
-        account: u32,
-    ) -> Result<Self, zip32::Error> {
+    pub fn from_zip32_seed(seed: &[u8], coin_type: u32, account: u32) -> Result<Self, Zip32Error> {
         if account != 0 {
-            return Err(zip32::Error::NonZeroAccount);
+            return Err(Zip32Error::NonZeroAccount);
         }
 
         // Call zip32 logic
@@ -173,12 +229,9 @@ impl IssueAuthKey<ZSASchnorr> {
             ChildIndex::hardened(account),
         ];
 
-        // we are reusing zip32 logic for deriving the key, zip32 should be updated as discussed
-        let &isk_bytes = ExtendedSpendingKey::<zip32::Issuance>::from_path(seed, path)?
-            .sk()
-            .to_bytes();
+        let isk_bytes = ExtendedIssuanceKey::from_path(seed, path).sk_bytes();
 
-        Self::from_bytes(&isk_bytes).ok_or(zip32::Error::InvalidSpendingKey)
+        Self::from_bytes(&isk_bytes).ok_or(Zip32Error::InvalidIssuanceKey)
     }
 }
 

src/keys.rs

@@ -4,7 +4,7 @@ use alloc::vec::Vec;
 use core::fmt::Debug;
 use core2::io::{self, Read, Write};
 
-use ::zip32::{hardened_only, ChildIndex};
+use ::zip32::ChildIndex;
 use aes::Aes256;
 use blake2b_simd::{Hash as Blake2bHash, Params};
 use fpe::ff1::{BinaryNumeralString, FF1};
@@ -107,7 +107,7 @@ impl SpendingKey {
             ChildIndex::hardened(coin_type),
             ChildIndex::hardened(account.into()),
         ];
-        ExtendedSpendingKey::<zip32::Orchard>::from_path(seed, path).map(|esk| esk.sk())
+        ExtendedSpendingKey::from_path(seed, path).map(|esk| esk.sk())
     }
 }
 
@@ -322,8 +322,8 @@ impl From<&SpendingKey> for FullViewingKey {
     }
 }
 
-impl<C: hardened_only::Context> From<&ExtendedSpendingKey<C>> for FullViewingKey {
-    fn from(extsk: &ExtendedSpendingKey<C>) -> Self {
+impl From<&ExtendedSpendingKey> for FullViewingKey {
+    fn from(extsk: &ExtendedSpendingKey) -> Self {
         (&extsk.sk()).into()
     }
 }

src/zip32.rs

@@ -17,10 +17,7 @@ use crate::{
 
 pub use zip32::ChildIndex;
 
-/// Personalization for the master extended spending key
 const ZIP32_ORCHARD_PERSONALIZATION: &[u8; 16] = b"ZcashIP32Orchard";
-/// Personalization for the master extended issuance key
-const ZIP32_ORCHARD_ISSUANCE_PERSONALIZATION: &[u8; 16] = b"ZcashSA_Issue_V1";
 const ZIP32_ORCHARD_FVFP_PERSONALIZATION: &[u8; 16] = b"ZcashOrchardFVFP";
 
 /// Errors produced in derivation of extended spending keys
@@ -30,8 +27,6 @@ pub enum Error {
     InvalidSpendingKey,
     /// A child index in a derivation path exceeded 2^31
     InvalidChildIndex(u32),
-    /// A non zero account when deriving an Orchard-ZSA issuance key
-    NonZeroAccount,
 }
 
 impl fmt::Display for Error {
@@ -126,37 +121,28 @@ impl KeyIndex {
 }
 
 #[derive(Clone, Copy, Debug)]
-pub(crate) struct Orchard;
+struct Orchard;
 
 impl hardened_only::Context for Orchard {
     const MKG_DOMAIN: [u8; 16] = *ZIP32_ORCHARD_PERSONALIZATION;
     const CKD_DOMAIN: PrfExpand<([u8; 32], [u8; 4], [u8; 1], VariableLengthSlice)> =
         PrfExpand::ORCHARD_ZIP32_CHILD;
 }
 
-#[derive(Clone, Copy, Debug)]
-pub(crate) struct Issuance;
-
-impl hardened_only::Context for Issuance {
-    const MKG_DOMAIN: [u8; 16] = *ZIP32_ORCHARD_ISSUANCE_PERSONALIZATION;
-    const CKD_DOMAIN: PrfExpand<([u8; 32], [u8; 4], [u8; 1], VariableLengthSlice)> =
-        PrfExpand::ORCHARD_ZIP32_CHILD;
-}
-
 /// An Orchard extended spending key.
 ///
 /// Defined in [ZIP32: Orchard extended keys][orchardextendedkeys].
 ///
 /// [orchardextendedkeys]: https://zips.z.cash/zip-0032#orchard-extended-keys
 #[derive(Debug, Clone)]
-pub(crate) struct ExtendedSpendingKey<C: hardened_only::Context> {
+pub(crate) struct ExtendedSpendingKey {
     depth: u8,
     parent_fvk_tag: FvkTag,
     child_index: KeyIndex,
-    inner: HardenedOnlyKey<C>,
+    inner: HardenedOnlyKey<Orchard>,
 }
 
-impl<C: hardened_only::Context> ConstantTimeEq for ExtendedSpendingKey<C> {
+impl ConstantTimeEq for ExtendedSpendingKey {
     fn ct_eq(&self, rhs: &Self) -> Choice {
         self.depth.ct_eq(&rhs.depth)
             & self.parent_fvk_tag.0.ct_eq(&rhs.parent_fvk_tag.0)
@@ -166,7 +152,7 @@ impl<C: hardened_only::Context> ConstantTimeEq for ExtendedSpendingKey<C> {
 }
 
 #[allow(non_snake_case)]
-impl<C: hardened_only::Context> ExtendedSpendingKey<C> {
+impl ExtendedSpendingKey {
     /// Returns the spending key of the child key corresponding to
     /// the path derived from the master key
     ///
@@ -250,7 +236,7 @@ mod tests {
     #[test]
     fn derive_child() {
         let seed = [0; 32];
-        let xsk_m = ExtendedSpendingKey::<Orchard>::master(&seed).unwrap();
+        let xsk_m = ExtendedSpendingKey::master(&seed).unwrap();
 
         let i_5 = ChildIndex::hardened(5);
         let xsk_5 = xsk_m.derive_child(i_5);
@@ -261,18 +247,18 @@ mod tests {
     #[test]
     fn path() {
         let seed = [0; 32];
-        let xsk_m = ExtendedSpendingKey::<Orchard>::master(&seed).unwrap();
+        let xsk_m = ExtendedSpendingKey::master(&seed).unwrap();
 
         let xsk_5h = xsk_m.derive_child(ChildIndex::hardened(5)).unwrap();
         assert!(bool::from(
-            ExtendedSpendingKey::<Orchard>::from_path(&seed, &[ChildIndex::hardened(5)],)
+            ExtendedSpendingKey::from_path(&seed, &[ChildIndex::hardened(5)],)
                 .unwrap()
                 .ct_eq(&xsk_5h)
         ));
 
         let xsk_5h_7 = xsk_5h.derive_child(ChildIndex::hardened(7)).unwrap();
         assert!(bool::from(
-            ExtendedSpendingKey::<Orchard>::from_path(
+            ExtendedSpendingKey::from_path(
                 &seed,
                 &[ChildIndex::hardened(5), ChildIndex::hardened(7)],
             )
@@ -294,9 +280,9 @@ mod tests {
         let i2h = ChildIndex::hardened(2);
         let i3h = ChildIndex::hardened(3);
 
-        let m = ExtendedSpendingKey::<Orchard>::master(&seed).unwrap();
+        let m = ExtendedSpendingKey::master(&seed).unwrap();
         let m_1h = m.derive_child(i1h).unwrap();
-        let m_1h_2h = ExtendedSpendingKey::<Orchard>::from_path(&seed, &[i1h, i2h]).unwrap();
+        let m_1h_2h = ExtendedSpendingKey::from_path(&seed, &[i1h, i2h]).unwrap();
         let m_1h_2h_3h = m_1h_2h.derive_child(i3h).unwrap();
 
         let xsks = [m, m_1h, m_1h_2h, m_1h_2h_3h];