Harden CVC authentication and preserve APDU errors

cktap-ffi/src/error.rs

@@ -27,6 +27,36 @@ impl From<rust_cktap::FromSliceError> for KeyError {
     }
 }
 
+/// Errors returned when a CVC does not satisfy its local constraints
+#[derive(Debug, Copy, Clone, PartialEq, Eq, thiserror::Error, uniffi::Error)]
+pub enum CvcError {
+    /// The CVC contains fewer than six bytes
+    #[error("CVC is too short: {length} bytes; minimum is 6")]
+    TooShort { length: u32 },
+    /// The CVC contains more than 32 bytes
+    #[error("CVC is too long: {length} bytes; maximum is 32")]
+    TooLong { length: u32 },
+    /// The CVC contains a byte that is not an ASCII digit
+    #[error("CVC contains a byte that is not an ASCII digit at byte index {index}")]
+    NonAsciiDigit { index: u32 },
+}
+
+impl From<rust_cktap::CvcError> for CvcError {
+    fn from(value: rust_cktap::CvcError) -> Self {
+        match value {
+            rust_cktap::CvcError::TooShort { length } => Self::TooShort {
+                length: u32::try_from(length).unwrap_or(u32::MAX),
+            },
+            rust_cktap::CvcError::TooLong { length } => Self::TooLong {
+                length: u32::try_from(length).unwrap_or(u32::MAX),
+            },
+            rust_cktap::CvcError::NonAsciiDigit { index } => Self::NonAsciiDigit {
+                index: u32::try_from(index).unwrap_or(u32::MAX),
+            },
+        }
+    }
+}
+
 /// Errors returned by the CkTap card.
 #[derive(Debug, Copy, Clone, PartialEq, Eq, thiserror::Error, uniffi::Error)]
 pub enum CardError {
@@ -72,6 +102,24 @@ impl From<rust_cktap::CardError> for CardError {
     }
 }
 
+impl From<CardError> for rust_cktap::CardError {
+    fn from(value: CardError) -> Self {
+        match value {
+            CardError::UnluckyNumber => Self::UnluckyNumber,
+            CardError::BadArguments => Self::BadArguments,
+            CardError::BadAuth => Self::BadAuth,
+            CardError::NeedsAuth => Self::NeedsAuth,
+            CardError::UnknownCommand => Self::UnknownCommand,
+            CardError::InvalidCommand => Self::InvalidCommand,
+            CardError::InvalidState => Self::InvalidState,
+            CardError::WeakNonce => Self::WeakNonce,
+            CardError::BadCBOR => Self::BadCBOR,
+            CardError::BackupFirst => Self::BackupFirst,
+            CardError::RateLimited => Self::RateLimited,
+        }
+    }
+}
+
 /// Errors returned by the card, CBOR deserialization or value encoding, or the APDU transport.
 #[derive(Debug, Clone, PartialEq, Eq, thiserror::Error, uniffi::Error)]
 pub enum CkTapError {
@@ -83,6 +131,8 @@ pub enum CkTapError {
     CborValue { msg: String },
     #[error("APDU transport error: {msg}")]
     Transport { msg: String },
+    #[error("Unknown card error code ({code}): {message}")]
+    UnknownErrorCode { code: u16, message: String },
     #[error("Unknown card type")]
     UnknownCardType,
 }
@@ -94,11 +144,29 @@ impl From<rust_cktap::CkTapError> for CkTapError {
             rust_cktap::CkTapError::CborDe(msg) => CkTapError::CborDe { msg },
             rust_cktap::CkTapError::CborValue(msg) => CkTapError::CborValue { msg },
             rust_cktap::CkTapError::Transport(msg) => CkTapError::Transport { msg },
+            rust_cktap::CkTapError::UnknownErrorCode { code, message } => {
+                CkTapError::UnknownErrorCode { code, message }
+            }
             rust_cktap::CkTapError::UnknownCardType => CkTapError::UnknownCardType,
         }
     }
 }
 
+impl From<CkTapError> for rust_cktap::CkTapError {
+    fn from(value: CkTapError) -> Self {
+        match value {
+            CkTapError::Card { err } => Self::Card(err.into()),
+            CkTapError::CborDe { msg } => Self::CborDe(msg),
+            CkTapError::CborValue { msg } => Self::CborValue(msg),
+            CkTapError::Transport { msg } => Self::Transport(msg),
+            CkTapError::UnknownErrorCode { code, message } => {
+                Self::UnknownErrorCode { code, message }
+            }
+            CkTapError::UnknownCardType => Self::UnknownCardType,
+        }
+    }
+}
+
 /// Errors returned by the `status` command.
 #[derive(Debug, Clone, PartialEq, Eq, thiserror::Error, uniffi::Error)]
 pub enum StatusError {
@@ -114,6 +182,23 @@ pub enum StatusError {
     },
 }
 
+/// Errors returned by the `init` command
+#[derive(Debug, Clone, PartialEq, Eq, thiserror::Error, uniffi::Error)]
+pub enum InitError {
+    /// The card or transport rejected the command
+    #[error(transparent)]
+    CkTap {
+        #[from]
+        err: CkTapError,
+    },
+    /// The CVC failed local validation
+    #[error(transparent)]
+    Cvc {
+        #[from]
+        err: CvcError,
+    },
+}
+
 impl From<rust_cktap::StatusError> for StatusError {
     fn from(value: rust_cktap::StatusError) -> Self {
         match value {
@@ -136,6 +221,12 @@ pub enum ReadError {
         #[from]
         err: KeyError,
     },
+    /// The CVC failed local validation
+    #[error(transparent)]
+    Cvc {
+        #[from]
+        err: CvcError,
+    },
 }
 
 impl From<rust_cktap::ReadError> for ReadError {
@@ -191,6 +282,12 @@ pub enum DeriveError {
     },
     #[error("Invalid chain code: {msg}")]
     InvalidChainCode { msg: String },
+    /// The CVC failed local validation
+    #[error(transparent)]
+    Cvc {
+        #[from]
+        err: CvcError,
+    },
 }
 
 impl From<rust_cktap::DeriveError> for DeriveError {
@@ -217,6 +314,12 @@ pub enum UnsealError {
         #[from]
         err: KeyError,
     },
+    /// The CVC failed local validation
+    #[error(transparent)]
+    Cvc {
+        #[from]
+        err: CvcError,
+    },
 }
 
 impl From<rust_cktap::UnsealError> for UnsealError {
@@ -251,6 +354,12 @@ pub enum DumpError {
     /// successful `unseal` command.
     #[error("Slot was unsealed improperly: {slot}")]
     SlotTampered { slot: u8 },
+    /// The CVC failed local validation
+    #[error(transparent)]
+    Cvc {
+        #[from]
+        err: CvcError,
+    },
 }
 
 impl From<rust_cktap::DumpError> for DumpError {
@@ -295,6 +404,12 @@ pub enum SignPsbtError {
     PsbtEncoding { msg: String },
     #[error("Error in PSBT Base64 encoding: {msg}")]
     Base64Encoding { msg: String },
+    /// The CVC failed local validation
+    #[error(transparent)]
+    Cvc {
+        #[from]
+        err: CvcError,
+    },
 }
 
 impl From<rust_cktap::SignPsbtError> for SignPsbtError {
@@ -344,20 +459,32 @@ pub enum ChangeError {
         #[from]
         err: CkTapError,
     },
-    #[error("new cvc is too short, must be at least 6 bytes, was only {len} bytes")]
-    TooShort { len: u32 },
-    #[error("new cvc is too long, must be at most 32 bytes, was {len} bytes")]
-    TooLong { len: u32 },
     #[error("new cvc is the same as the old one")]
     SameAsOld,
+    /// The current CVC failed local validation
+    #[error("invalid current CVC: {err}")]
+    CurrentCvc { err: CvcError },
+    /// The new CVC failed local validation
+    #[error("invalid new CVC: {err}")]
+    NewCvc { err: CvcError },
+}
+
+impl ChangeError {
+    /// Wrap a CVC validation failure for the current CVC
+    pub(crate) fn current_cvc(err: impl Into<CvcError>) -> Self {
+        Self::CurrentCvc { err: err.into() }
+    }
+
+    /// Wrap a CVC validation failure for the new CVC
+    pub(crate) fn new_cvc(err: impl Into<CvcError>) -> Self {
+        Self::NewCvc { err: err.into() }
+    }
 }
 
 impl From<rust_cktap::ChangeError> for ChangeError {
     fn from(value: rust_cktap::ChangeError) -> Self {
         match value {
             rust_cktap::ChangeError::CkTap(err) => ChangeError::CkTap { err: err.into() },
-            rust_cktap::ChangeError::TooShort(len) => ChangeError::TooShort { len },
-            rust_cktap::ChangeError::TooLong(len) => ChangeError::TooLong { len },
             rust_cktap::ChangeError::SameAsOld => ChangeError::SameAsOld,
         }
     }
@@ -373,6 +500,12 @@ pub enum XpubError {
     },
     #[error("BIP32 error: {msg}")]
     Bip32 { msg: String },
+    /// The CVC failed local validation
+    #[error(transparent)]
+    Cvc {
+        #[from]
+        err: CvcError,
+    },
 }
 
 impl From<rust_cktap::XpubError> for XpubError {

cktap-ffi/src/lib.rs

@@ -8,11 +8,12 @@ mod tap_signer;
 
 uniffi::setup_scaffolding!();
 
-use crate::error::{CertsError, CkTapError, ReadError, StatusError};
+use crate::error::{CertsError, CkTapError, CvcError, ReadError, StatusError};
 use crate::sats_card::SatsCard;
 use crate::sats_chip::SatsChip;
 use crate::tap_signer::TapSigner;
 use futures::lock::Mutex;
+use rust_cktap::Cvc;
 use rust_cktap::shared::{Certificate, Read};
 use std::fmt::Debug;
 use std::sync::Arc;
@@ -34,7 +35,7 @@ impl rust_cktap::CkTransport for CkTransportWrapper {
         self.0
             .transmit_apdu(command_apdu)
             .await
-            .map_err(|e| rust_cktap::CkTapError::Transport(e.to_string()))
+            .map_err(rust_cktap::CkTapError::from)
     }
 }
 
@@ -65,10 +66,16 @@ pub async fn to_cktap(transport: Box<dyn CkTransport>) -> Result<CkTapCard, Stat
 
 // command helpers
 
+fn parse_optional_cvc(cvc: Option<String>) -> Result<Option<Cvc>, CvcError> {
+    cvc.map(Cvc::try_from).transpose().map_err(CvcError::from)
+}
+
 async fn read(
     card: &mut (impl Read + Send + Sync),
     cvc: Option<String>,
 ) -> Result<String, ReadError> {
+    let cvc = parse_optional_cvc(cvc)?;
+
     card.read(cvc)
         .await
         .map(|pk| pk.to_string())

cktap-ffi/src/sats_card.rs

@@ -3,12 +3,13 @@
 
 use crate::check_cert;
 use crate::error::{
-    CertsError, CkTapError, DeriveError, DumpError, ReadError, SignPsbtError, UnsealError,
+    CertsError, CkTapError, CvcError, DeriveError, DumpError, ReadError, SignPsbtError, UnsealError,
 };
+use crate::parse_optional_cvc;
 use futures::lock::Mutex;
 use rust_cktap::descriptor::Wpkh;
 use rust_cktap::shared::{Authentication, Nfc, Read, Wait};
-use rust_cktap::{Psbt, rand_chaincode};
+use rust_cktap::{Cvc, Psbt, rand_chaincode};
 use std::str::FromStr;
 
 #[derive(uniffi::Object)]
@@ -81,6 +82,7 @@ impl SatsCard {
 
     /// Open a new slot, it will be the current active but must be unused (no address)
     pub async fn new_slot(&self, cvc: String) -> Result<u8, DeriveError> {
+        let cvc = Cvc::try_from(cvc).map_err(CvcError::from)?;
         let mut card = self.0.lock().await;
         let (active_slot, _) = card.slots;
         let new_slot_chain_code = rand_chaincode();
@@ -99,6 +101,7 @@ impl SatsCard {
 
     /// Unseal currently active slot
     pub async fn unseal(&self, cvc: String) -> Result<SlotDetails, UnsealError> {
+        let cvc = Cvc::try_from(cvc).map_err(CvcError::from)?;
         let mut card = self.0.lock().await;
         let active_slot = card.slots.0;
         let (privkey, pubkey) = card.unseal(active_slot, &cvc).await?;
@@ -112,6 +115,7 @@ impl SatsCard {
     /// This is only needed for debugging, use `sign_psbt` for signing
     /// If no CVC given only pubkey and pubkey descriptor returned.
     pub async fn dump(&self, slot: u8, cvc: Option<String>) -> Result<SlotDetails, DumpError> {
+        let cvc = parse_optional_cvc(cvc)?;
         let mut card = self.0.lock().await;
         let (privkey, pubkey) = card.dump(slot, cvc).await?;
         Ok(SlotDetails {
@@ -128,6 +132,7 @@ impl SatsCard {
         psbt: String,
         cvc: String,
     ) -> Result<String, SignPsbtError> {
+        let cvc = Cvc::try_from(cvc).map_err(CvcError::from)?;
         let mut card = self.0.lock().await;
         let psbt = Psbt::from_str(&psbt)?;
         let signed_psbt = card.sign_psbt(slot, psbt, &cvc).await?;

cktap-ffi/src/sats_chip.rs

@@ -2,11 +2,13 @@
 // SPDX-License-Identifier: MIT OR Apache-2.0
 
 use crate::error::{
-    CertsError, ChangeError, CkTapError, DeriveError, ReadError, SignPsbtError, XpubError,
+    CertsError, ChangeError, CkTapError, CvcError, DeriveError, InitError, ReadError,
+    SignPsbtError, XpubError,
 };
 use crate::tap_signer::{change, derive, init, sign_psbt};
 use crate::{check_cert, read};
 use futures::lock::Mutex;
+use rust_cktap::Cvc;
 use rust_cktap::shared::{Authentication, Nfc, Wait};
 use rust_cktap::tap_signer::TapSignerShared;
 
@@ -54,24 +56,30 @@ impl SatsChip {
         check_cert(&mut *card).await
     }
 
-    pub async fn init(&self, cvc: String) -> Result<(), CkTapError> {
+    pub async fn init(&self, cvc: String) -> Result<(), InitError> {
+        let cvc = Cvc::try_from(cvc).map_err(CvcError::from)?;
         let mut card = self.0.lock().await;
-        init(&mut *card, cvc).await
+        init(&mut *card, cvc).await?;
+        Ok(())
     }
 
     pub async fn sign_psbt(&self, psbt: String, cvc: String) -> Result<String, SignPsbtError> {
+        let cvc = Cvc::try_from(cvc).map_err(CvcError::from)?;
         let mut card = self.0.lock().await;
         let psbt = sign_psbt(&mut *card, psbt, cvc).await?;
         Ok(psbt)
     }
 
     pub async fn derive(&self, path: Vec<u32>, cvc: String) -> Result<String, DeriveError> {
+        let cvc = Cvc::try_from(cvc).map_err(CvcError::from)?;
         let mut card = self.0.lock().await;
         let pubkey = derive(&mut *card, path, cvc).await?;
         Ok(pubkey)
     }
 
     pub async fn change(&self, new_cvc: String, cvc: String) -> Result<(), ChangeError> {
+        let new_cvc = Cvc::try_from(new_cvc).map_err(ChangeError::new_cvc)?;
+        let cvc = Cvc::try_from(cvc).map_err(ChangeError::current_cvc)?;
         let mut card = self.0.lock().await;
         change(&mut *card, new_cvc, cvc).await?;
         Ok(())
@@ -84,6 +92,7 @@ impl SatsChip {
     }
 
     pub async fn xpub(&self, master: bool, cvc: String) -> Result<String, XpubError> {
+        let cvc = Cvc::try_from(cvc).map_err(CvcError::from)?;
         let mut card = self.0.lock().await;
         let xpub = card.xpub(master, &cvc).await?;
         Ok(xpub.to_string())