From 8ab3a43d6e3d49ffc1fda7596485482d70fd6cde Mon Sep 17 00:00:00 2001 From: Heiko Schaefer Date: Tue, 22 Feb 2022 10:18:44 +0100 Subject: [PATCH] Use Error::InternalError less, introduce additional specific error variants. --- openpgp-card-sequoia/src/card.rs | 32 ++++---- openpgp-card-sequoia/src/decryptor.rs | 4 +- openpgp-card-sequoia/src/privkey.rs | 17 ++-- openpgp-card-sequoia/src/signer.rs | 4 +- openpgp-card-sequoia/src/util.rs | 38 +++++++-- openpgp-card/src/algorithm.rs | 10 ++- openpgp-card/src/apdu.rs | 4 +- openpgp-card/src/apdu/command.rs | 6 +- openpgp-card/src/card_do.rs | 42 ++++++---- openpgp-card/src/card_do/algo_attrs.rs | 4 +- openpgp-card/src/card_do/algo_info.rs | 4 +- openpgp-card/src/card_do/application_id.rs | 4 +- openpgp-card/src/card_do/cardholder.rs | 4 +- openpgp-card/src/card_do/extended_cap.rs | 11 +-- .../src/card_do/extended_length_info.rs | 2 +- openpgp-card/src/card_do/fingerprint.rs | 8 +- openpgp-card/src/card_do/historical.rs | 36 +++++---- .../src/card_do/key_generation_times.rs | 9 +-- openpgp-card/src/card_do/pw_status.rs | 6 +- openpgp-card/src/crypto_data.rs | 2 +- openpgp-card/src/errors.rs | 18 +++-- openpgp-card/src/keys.rs | 24 +++--- openpgp-card/src/lib.rs | 25 +++++- openpgp-card/src/openpgp.rs | 55 +++++++------ openpgp-card/src/tlv.rs | 2 +- openpgp-card/src/tlv/value.rs | 2 +- pcsc/src/lib.rs | 61 +++++++------- scdc/src/lib.rs | 79 +++++++++++++------ tools/src/bin/opgpcard/main.rs | 2 +- 29 files changed, 312 insertions(+), 203 deletions(-) diff --git a/openpgp-card-sequoia/src/card.rs b/openpgp-card-sequoia/src/card.rs index cc89f6c..7212b83 100644 --- a/openpgp-card-sequoia/src/card.rs +++ b/openpgp-card-sequoia/src/card.rs @@ -4,8 +4,6 @@ //! Perform operations on a card. Different states of a card are modeled by //! different types, such as `Open`, `User`, `Sign`, `Admin`. -use anyhow::{anyhow, Result}; - use sequoia_openpgp::cert::amalgamation::key::ValidErasedKeyAmalgamation; use sequoia_openpgp::packet::key::SecretParts; use sequoia_openpgp::Cert; @@ -188,7 +186,7 @@ impl<'a> Open<'a> { self.ard.historical_bytes() } - pub fn extended_length_information(&self) -> Result> { + pub fn extended_length_information(&self) -> Result, Error> { self.ard.extended_length_information() } @@ -206,12 +204,12 @@ impl<'a> Open<'a> { self.ard.extended_capabilities() } - pub fn algorithm_attributes(&self, key_type: KeyType) -> Result { + pub fn algorithm_attributes(&self, key_type: KeyType) -> Result { self.ard.algorithm_attributes(key_type) } /// PW status Bytes - pub fn pw_status_bytes(&self) -> Result { + pub fn pw_status_bytes(&self) -> Result { self.ard.pw_status_bytes() } @@ -259,22 +257,22 @@ impl<'a> Open<'a> { // --- URL (5f50) --- - pub fn url(&mut self) -> Result { + pub fn url(&mut self) -> Result { Ok(String::from_utf8_lossy(&self.opt.url()?).to_string()) } // --- cardholder related data (65) --- - pub fn cardholder_related_data(&mut self) -> Result { + pub fn cardholder_related_data(&mut self) -> Result { self.opt.cardholder_related_data() } // --- security support template (7a) --- - pub fn security_support_template(&mut self) -> Result { + pub fn security_support_template(&mut self) -> Result { self.opt.security_support_template() } // DO "Algorithm Information" (0xFA) - pub fn algorithm_information(&mut self) -> Result> { + pub fn algorithm_information(&mut self) -> Result, Error> { // The DO "Algorithm Information" (Tag FA) shall be present if // Algorithm attributes can be changed let ec = self.extended_capabilities()?; @@ -288,20 +286,20 @@ impl<'a> Open<'a> { } /// Firmware Version, YubiKey specific (?) - pub fn firmware_version(&mut self) -> Result> { + pub fn firmware_version(&mut self) -> Result, Error> { self.opt.firmware_version() } // ---------- - pub fn public_key(&mut self, key_type: KeyType) -> Result { + pub fn public_key(&mut self, key_type: KeyType) -> Result { self.opt.public_key(key_type).map_err(|e| e.into()) } // ---------- /// Delete all state on this OpenPGP card - pub fn factory_reset(&mut self) -> Result<()> { + pub fn factory_reset(&mut self) -> Result<(), Error> { self.opt.factory_reset() } } @@ -354,12 +352,12 @@ impl<'app, 'open> Admin<'app, 'open> { impl Admin<'_, '_> { pub fn set_name(&mut self, name: &str) -> Result<(), Error> { if name.len() >= 40 { - return Err(anyhow!("name too long").into()); + return Err(Error::InternalError("name too long".into())); } // All chars must be in ASCII7 if name.chars().any(|c| !c.is_ascii()) { - return Err(anyhow!("Invalid char in name").into()); + return Err(Error::InternalError("Invalid char in name".into())); }; self.oc.opt.set_name(name.as_bytes()) @@ -367,7 +365,7 @@ impl Admin<'_, '_> { pub fn set_lang(&mut self, lang: &[Lang]) -> Result<(), Error> { if lang.len() > 8 { - return Err(anyhow!("lang too long").into()); + return Err(Error::InternalError("lang too long".into())); } self.oc.opt.set_lang(lang) @@ -379,7 +377,7 @@ impl Admin<'_, '_> { pub fn set_url(&mut self, url: &str) -> Result<(), Error> { if url.chars().any(|c| !c.is_ascii()) { - return Err(anyhow!("Invalid char in url").into()); + return Err(Error::InternalError("Invalid char in url".into())); } // Check for max len @@ -393,7 +391,7 @@ impl Admin<'_, '_> { self.oc.opt.set_url(url.as_bytes()) } else { - Err(anyhow!("URL too long").into()) + Err(Error::InternalError("URL too long".into())) } } diff --git a/openpgp-card-sequoia/src/decryptor.rs b/openpgp-card-sequoia/src/decryptor.rs index ff1a48b..cff5e3b 100644 --- a/openpgp-card-sequoia/src/decryptor.rs +++ b/openpgp-card-sequoia/src/decryptor.rs @@ -48,13 +48,13 @@ impl<'a, 'app> CardDecryptor<'a, 'app> { let public = eka.key().clone(); Ok(Self { ca, public }) } else { - Err(Error::InternalError(anyhow!( + Err(Error::InternalError(format!( "Failed to find (sub)key {} in cert", fp ))) } } else { - Err(Error::InternalError(anyhow!( + Err(Error::InternalError(format!( "Failed to get the decryption key's Fingerprint from the card" ))) } diff --git a/openpgp-card-sequoia/src/privkey.rs b/openpgp-card-sequoia/src/privkey.rs index dc0a1fa..d5dbb0b 100644 --- a/openpgp-card-sequoia/src/privkey.rs +++ b/openpgp-card-sequoia/src/privkey.rs @@ -4,7 +4,6 @@ use std::convert::TryFrom; use std::convert::TryInto; -use anyhow::Result; use openpgp::cert::amalgamation::key::ValidErasedKeyAmalgamation; use openpgp::crypto::{mpi, mpi::ProtectedMPI, mpi::MPI}; use openpgp::packet::{ @@ -49,14 +48,15 @@ impl SequoiaKey { /// Implement the `CardUploadableKey` trait that openpgp-card uses to /// upload (sub)keys to a card. impl CardUploadableKey for SequoiaKey { - fn private_key(&self) -> Result { + fn private_key(&self) -> Result { // Decrypt key with password, if set let key = match &self.password { None => self.key.clone(), Some(pw) => self .key .clone() - .decrypt_secret(&openpgp::crypto::Password::from(pw.as_str()))?, + .decrypt_secret(&openpgp::crypto::Password::from(pw.as_str())) + .map_err(|e| Error::InternalError(format!("sequoia decrypt failed {:?}", e)))?, }; // Get private cryptographic material @@ -124,8 +124,15 @@ struct SqRSA { impl SqRSA { #[allow(clippy::many_single_char_names)] - fn new(e: MPI, d: ProtectedMPI, n: MPI, p: ProtectedMPI, q: ProtectedMPI) -> Result { - let nettle = nettle::rsa::PrivateKey::new(d.value(), p.value(), q.value(), None)?; + fn new( + e: MPI, + d: ProtectedMPI, + n: MPI, + p: ProtectedMPI, + q: ProtectedMPI, + ) -> Result { + let nettle = nettle::rsa::PrivateKey::new(d.value(), p.value(), q.value(), None) + .map_err(|e| Error::InternalError(format!("nettle error {:?}", e)))?; Ok(Self { e, n, p, q, nettle }) } diff --git a/openpgp-card-sequoia/src/signer.rs b/openpgp-card-sequoia/src/signer.rs index cd0854e..9ff1a27 100644 --- a/openpgp-card-sequoia/src/signer.rs +++ b/openpgp-card-sequoia/src/signer.rs @@ -46,13 +46,13 @@ impl<'a, 'app> CardSigner<'a, 'app> { let key = eka.key().clone(); Ok(Self::with_pubkey(ca, key)) } else { - Err(Error::InternalError(anyhow!( + Err(Error::InternalError(format!( "Failed to find (sub)key {} in cert", fp ))) } } else { - Err(Error::InternalError(anyhow!( + Err(Error::InternalError(format!( "Failed to get the signing key's Fingerprint from the card" ))) } diff --git a/openpgp-card-sequoia/src/util.rs b/openpgp-card-sequoia/src/util.rs index ab0f49a..e31abd3 100644 --- a/openpgp-card-sequoia/src/util.rs +++ b/openpgp-card-sequoia/src/util.rs @@ -177,12 +177,14 @@ pub fn public_key_material_to_key( pkm: &PublicKeyMaterial, key_type: KeyType, time: KeyGenerationTime, -) -> Result { +) -> Result { let time = Timestamp::from(time.get()).into(); match pkm { PublicKeyMaterial::R(rsa) => { - let k4 = Key4::import_public_rsa(rsa.v(), rsa.n(), Some(time))?; + let k4 = Key4::import_public_rsa(rsa.v(), rsa.n(), Some(time)).map_err(|e| { + Error::InternalError(format!("sequoia Key4::import_public_rsa failed: {:?}", e)) + })?; Ok(k4.into()) } @@ -202,7 +204,13 @@ pub fn public_key_material_to_key( KeyType::Authentication | KeyType::Signing => { if algo_ecc.curve() == Curve::Ed25519 { // EdDSA - let k4 = Key4::import_public_ed25519(ecc.data(), time)?; + let k4 = + Key4::import_public_ed25519(ecc.data(), time).map_err(|e| { + Error::InternalError(format!( + "sequoia Key4::import_public_ed25519 failed: {:?}", + e + )) + })?; Ok(Key::from(k4)) } else { @@ -214,7 +222,13 @@ pub fn public_key_material_to_key( curve, q: mpi::MPI::new(ecc.data()), }, - )?; + ) + .map_err(|e| { + Error::InternalError(format!( + "sequoia Key4::new for ECDSA failed: {:?}", + e + )) + })?; Ok(k4.into()) } @@ -225,7 +239,13 @@ pub fn public_key_material_to_key( // ok when a cert already exists // EdDSA - let k4 = Key4::import_public_cv25519(ecc.data(), None, None, time)?; + let k4 = Key4::import_public_cv25519(ecc.data(), None, None, time) + .map_err(|e| { + Error::InternalError(format!( + "sequoia Key4::import_public_cv25519 failed: {:?}", + e + )) + })?; Ok(k4.into()) } else { @@ -242,7 +262,13 @@ pub fn public_key_material_to_key( hash: Default::default(), sym: Default::default(), }, - )?; + ) + .map_err(|e| { + Error::InternalError(format!( + "sequoia Key4::new for ECDH failed: {:?}", + e + )) + })?; Ok(k4.into()) } diff --git a/openpgp-card/src/algorithm.rs b/openpgp-card/src/algorithm.rs index d4355a1..93fc260 100644 --- a/openpgp-card/src/algorithm.rs +++ b/openpgp-card/src/algorithm.rs @@ -86,7 +86,7 @@ impl AlgoSimple { key_type: KeyType, ard: &ApplicationRelatedData, algo_info: Option, - ) -> Result { + ) -> Result { let algo = match self { Self::RSA1k => Algo::Rsa(keys::determine_rsa_attrs(1024, key_type, ard, algo_info)?), Self::RSA2k => Algo::Rsa(keys::determine_rsa_attrs(2048, key_type, ard, algo_info)?), @@ -189,7 +189,9 @@ impl Algo { match self { Algo::Rsa(rsa) => Self::rsa_algo_attrs(rsa), Algo::Ecc(ecc) => Self::ecc_algo_attrs(ecc.oid(), ecc.ecc_type()), - _ => Err(anyhow!("Unexpected Algo {:?}", self).into()), + _ => Err(Error::UnsupportedAlgo( + format!("Unexpected Algo {:?}", self).into(), + )), } } @@ -327,7 +329,7 @@ impl Curve { } impl TryFrom<&[u8]> for Curve { - type Error = anyhow::Error; + type Error = crate::Error; fn try_from(oid: &[u8]) -> Result { use Curve::*; @@ -349,7 +351,7 @@ impl TryFrom<&[u8]> for Curve { [0x2b, 0x65, 0x71] => Ed448, [0x2b, 0x65, 0x6f] => X448, - _ => return Err(anyhow!("Unknown curve OID {:?}", oid)), + _ => return Err(Error::ParseError(format!("Unknown curve OID {:?}", oid))), }; Ok(curve) diff --git a/openpgp-card/src/apdu.rs b/openpgp-card/src/apdu.rs index 4bb71ab..1d8c3f6 100644 --- a/openpgp-card/src/apdu.rs +++ b/openpgp-card/src/apdu.rs @@ -153,9 +153,7 @@ where let cla = if last { 0x00 } else { 0x10 }; let partial = Command::new(cla, cmd.ins(), cmd.p1(), cmd.p2(), d.to_vec()); - let serialized = partial - .serialize(ext_len, expect_response) - .map_err(Error::InternalError)?; + let serialized = partial.serialize(ext_len, expect_response)?; log::debug!(" -> chained APDU command: {:x?}", &serialized); diff --git a/openpgp-card/src/apdu/command.rs b/openpgp-card/src/apdu/command.rs index d2ad053..81dc83c 100644 --- a/openpgp-card/src/apdu/command.rs +++ b/openpgp-card/src/apdu/command.rs @@ -69,7 +69,11 @@ impl Command { /// Serialize a Command (for sending to a card). /// /// See OpenPGP card spec, chapter 7 (pg 47) - pub(crate) fn serialize(&self, ext_len: bool, expect_response: Expect) -> Result> { + pub(crate) fn serialize( + &self, + ext_len: bool, + expect_response: Expect, + ) -> Result, crate::Error> { // FIXME? (from scd/apdu.c): // T=0 does not allow the use of Lc together with Le; // thus disable Le in this case. diff --git a/openpgp-card/src/card_do.rs b/openpgp-card/src/card_do.rs index 7fe633d..d389767 100644 --- a/openpgp-card/src/card_do.rs +++ b/openpgp-card/src/card_do.rs @@ -3,7 +3,6 @@ //! OpenPGP card data objects (DO) -use anyhow::{anyhow, Result}; use chrono::{DateTime, Utc}; use std::convert::TryFrom; use std::convert::TryInto; @@ -41,7 +40,7 @@ impl ApplicationRelatedData { if let Some(aid) = aid { Ok(ApplicationIdentifier::try_from(&aid.serialize()[..])?) } else { - Err(anyhow!("Couldn't get Application ID.").into()) + Err(Error::NotFound("Couldn't get Application ID.".to_string())) } } @@ -54,13 +53,15 @@ impl ApplicationRelatedData { log::debug!("Historical bytes: {:x?}", hist); (hist.serialize().as_slice()).try_into() } else { - Err(anyhow!("Failed to get historical bytes.").into()) + Err(Error::NotFound( + "Failed to get historical bytes.".to_string(), + )) } } /// Get extended length information (ISO 7816-4), which /// contains maximum number of bytes for command and response. - pub fn extended_length_information(&self) -> Result> { + pub fn extended_length_information(&self) -> Result, Error> { // get from cached "application related data" let eli = self.0.find(&[0x7f, 0x66].into()); @@ -100,27 +101,29 @@ impl ApplicationRelatedData { version, ))?) } else { - Err(anyhow!("Failed to get extended capabilities.").into()) + Err(Error::NotFound( + "Failed to get extended capabilities.".to_string(), + )) } } /// Get algorithm attributes (for each key type) - pub fn algorithm_attributes(&self, key_type: KeyType) -> Result { + pub fn algorithm_attributes(&self, key_type: KeyType) -> Result { // get from cached "application related data" let aa = self.0.find(&[key_type.algorithm_tag()].into()); if let Some(aa) = aa { Algo::try_from(&aa.serialize()[..]) } else { - Err(anyhow!( + Err(Error::NotFound(format!( "Failed to get algorithm attributes for {:?}.", key_type - )) + ))) } } /// Get PW status Bytes - pub fn pw_status_bytes(&self) -> Result { + pub fn pw_status_bytes(&self) -> Result { // get from cached "application related data" let psb = self.0.find(&[0xc4].into()); @@ -131,7 +134,9 @@ impl ApplicationRelatedData { Ok(pws) } else { - Err(anyhow!("Failed to get PW status Bytes.")) + Err(Error::NotFound( + "Failed to get PW status Bytes.".to_string(), + )) } } @@ -148,12 +153,12 @@ impl ApplicationRelatedData { Ok(fp) } else { - Err(anyhow!("Failed to get fingerprints.").into()) + Err(Error::NotFound("Failed to get fingerprints.".into())) } } /// Generation dates/times of key pairs - pub fn key_generation_times(&self) -> Result, Error> { + pub fn key_generation_times(&self) -> Result, crate::Error> { let kg = self.0.find(&[0xcd].into()); if let Some(kg) = kg { @@ -163,7 +168,9 @@ impl ApplicationRelatedData { Ok(kg) } else { - Err(anyhow!("Failed to get key generation times.").into()) + Err(Error::NotFound(format!( + "Failed to get key generation times." + ))) } } } @@ -446,11 +453,14 @@ impl Fingerprint { } /// Helper fn for nom parsing -pub(crate) fn complete(result: nom::IResult<&[u8], O>) -> Result { - let (rem, output) = result.map_err(|err| anyhow!("Parsing failed: {:?}", err))?; +pub(crate) fn complete(result: nom::IResult<&[u8], O>) -> Result { + let (rem, output) = result.map_err(|_err| Error::ParseError(format!("Parsing failed")))?; if rem.is_empty() { Ok(output) } else { - Err(anyhow!("Parsing incomplete -- trailing data: {:x?}", rem)) + Err(Error::ParseError(format!( + "Parsing incomplete, trailing data: {:x?}", + rem + ))) } } diff --git a/openpgp-card/src/card_do/algo_attrs.rs b/openpgp-card/src/card_do/algo_attrs.rs index 23462f8..d793743 100644 --- a/openpgp-card/src/card_do/algo_attrs.rs +++ b/openpgp-card/src/card_do/algo_attrs.rs @@ -141,9 +141,9 @@ pub(crate) fn parse(input: &[u8]) -> nom::IResult<&[u8], Algo> { } impl TryFrom<&[u8]> for Algo { - type Error = anyhow::Error; + type Error = crate::Error; - fn try_from(data: &[u8]) -> Result { + fn try_from(data: &[u8]) -> Result { complete(parse(data)) } } diff --git a/openpgp-card/src/card_do/algo_info.rs b/openpgp-card/src/card_do/algo_info.rs index 5327060..816687d 100644 --- a/openpgp-card/src/card_do/algo_info.rs +++ b/openpgp-card/src/card_do/algo_info.rs @@ -87,9 +87,9 @@ pub(self) fn parse(input: &[u8]) -> nom::IResult<&[u8], Vec<(KeyType, Algo)>> { } impl TryFrom<&[u8]> for AlgoInfo { - type Error = anyhow::Error; + type Error = crate::Error; - fn try_from(input: &[u8]) -> Result { + fn try_from(input: &[u8]) -> Result { Ok(AlgoInfo(complete(parse(input))?)) } } diff --git a/openpgp-card/src/card_do/application_id.rs b/openpgp-card/src/card_do/application_id.rs index b319751..cab10f1 100644 --- a/openpgp-card/src/card_do/application_id.rs +++ b/openpgp-card/src/card_do/application_id.rs @@ -31,9 +31,9 @@ fn parse(input: &[u8]) -> nom::IResult<&[u8], ApplicationIdentifier> { } impl TryFrom<&[u8]> for ApplicationIdentifier { - type Error = anyhow::Error; + type Error = crate::Error; - fn try_from(data: &[u8]) -> Result { + fn try_from(data: &[u8]) -> Result { complete(parse(data)) } } diff --git a/openpgp-card/src/card_do/cardholder.rs b/openpgp-card/src/card_do/cardholder.rs index 95a5f7a..221e0db 100644 --- a/openpgp-card/src/card_do/cardholder.rs +++ b/openpgp-card/src/card_do/cardholder.rs @@ -25,9 +25,9 @@ impl CardholderRelatedData { } impl TryFrom<&[u8]> for CardholderRelatedData { - type Error = anyhow::Error; + type Error = crate::Error; - fn try_from(data: &[u8]) -> Result { + fn try_from(data: &[u8]) -> Result { let value = Value::from(data, true)?; let tlv = Tlv::new([0x65], value); diff --git a/openpgp-card/src/card_do/extended_cap.rs b/openpgp-card/src/card_do/extended_cap.rs index b78020d..0e50503 100644 --- a/openpgp-card/src/card_do/extended_cap.rs +++ b/openpgp-card/src/card_do/extended_cap.rs @@ -3,7 +3,6 @@ //! 4.4.3.7 Extended Capabilities -use anyhow::{anyhow, Result}; use std::convert::TryFrom; use crate::card_do::ExtendedCapabilities; @@ -80,15 +79,17 @@ impl TryFrom<(&[u8], u16)> for ExtendedCapabilities { let i9 = input[9]; if i8 > 1 { - return Err( - anyhow!("Illegal value '{}' for pin_block_2_format_support", i8).into(), - ); + return Err(Error::ParseError( + format!("Illegal value '{}' for pin_block_2_format_support", i8).into(), + )); } pin_block_2_format_support = Some(i8 != 0); if i9 > 1 { - return Err(anyhow!("Illegal value '{}' for mse_command_support", i9).into()); + return Err(Error::ParseError( + format!("Illegal value '{}' for mse_command_support", i9).into(), + )); } mse_command_support = Some(i9 != 0); } diff --git a/openpgp-card/src/card_do/extended_length_info.rs b/openpgp-card/src/card_do/extended_length_info.rs index 9c17b9b..de7f443 100644 --- a/openpgp-card/src/card_do/extended_length_info.rs +++ b/openpgp-card/src/card_do/extended_length_info.rs @@ -32,7 +32,7 @@ impl ExtendedLengthInfo { } impl TryFrom<&[u8]> for ExtendedLengthInfo { - type Error = anyhow::Error; + type Error = crate::Error; fn try_from(input: &[u8]) -> Result { let eli = complete(parse(input))?; diff --git a/openpgp-card/src/card_do/fingerprint.rs b/openpgp-card/src/card_do/fingerprint.rs index f695738..8b546b9 100644 --- a/openpgp-card/src/card_do/fingerprint.rs +++ b/openpgp-card/src/card_do/fingerprint.rs @@ -3,7 +3,6 @@ //! Fingerprint for a single key slot -use anyhow::anyhow; use nom::{bytes::complete as bytes, combinator, sequence}; use std::convert::TryFrom; use std::convert::TryInto; @@ -28,7 +27,9 @@ impl TryFrom<&[u8]> for Fingerprint { let array: [u8; 20] = input.try_into().unwrap(); Ok(array.into()) } else { - Err(anyhow!("Unexpected fingerprint length {}", input.len()).into()) + Err(Error::ParseError( + format!("Unexpected fingerprint length {}", input.len()).into(), + )) } } } @@ -88,8 +89,7 @@ impl TryFrom<&[u8]> for KeySet { // been completely consumed. self::fingerprints(input) .map(|res| res.1) - .map_err(|err| anyhow!("Parsing failed: {:?}", err)) - .map_err(Error::InternalError) + .map_err(|_err| Error::ParseError("Parsing failed".into())) } } diff --git a/openpgp-card/src/card_do/historical.rs b/openpgp-card/src/card_do/historical.rs index 7e0daca..969ea42 100644 --- a/openpgp-card/src/card_do/historical.rs +++ b/openpgp-card/src/card_do/historical.rs @@ -5,7 +5,6 @@ use crate::card_do::{CardCapabilities, CardServiceData, HistoricalBytes}; use crate::Error; -use anyhow::{anyhow, Result}; use std::convert::TryFrom; impl CardCapabilities { @@ -77,7 +76,7 @@ impl HistoricalBytes { } impl TryFrom<&[u8]> for HistoricalBytes { - type Error = Error; + type Error = crate::Error; fn try_from(mut data: &[u8]) -> Result { // workaround-hack for "ledger" with zero-padded historical bytes @@ -95,18 +94,18 @@ impl TryFrom<&[u8]> for HistoricalBytes { if len < 4 { // historical bytes cannot be this short - return Err(anyhow!(format!( - "Historical bytes too short ({} bytes), must be >= 4", - len - )) - .into()); + return Err(Error::ParseError( + format!("Historical bytes too short ({} bytes), must be >= 4", len).into(), + )); } if data[0] != 0 { // The OpenPGP application assumes a category indicator byte // set to '00' (o-card 3.4.1, pg 44) - return Err(anyhow!("Unexpected category indicator in historical bytes").into()); + return Err(Error::ParseError( + "Unexpected category indicator in historical bytes".into(), + )); } // category indicator byte @@ -127,14 +126,15 @@ impl TryFrom<&[u8]> for HistoricalBytes { // (1 byte for the tl, plus `l` bytes of data for this ctlv) // (e.g. len = 4 -> tl + 3byte data) if ctlv.len() < (1 + l as usize) { - return Err(anyhow!( - "Illegal length value in Historical Bytes TL {} len {} \ - l {}", - ctlv[0], - ctlv.len(), - l - ) - .into()); + return Err(Error::ParseError( + format!( + "Illegal length value in Historical Bytes TL {} len {} l {}", + ctlv[0], + ctlv.len(), + l + ) + .into(), + )); } match (t, l) { @@ -177,7 +177,9 @@ impl TryFrom<&[u8]> for HistoricalBytes { 5 } _ => { - return Err(anyhow!("unexpected status indicator in historical bytes").into()); + return Err(Error::ParseError( + "unexpected status indicator in historical bytes".into(), + )); } }; diff --git a/openpgp-card/src/card_do/key_generation_times.rs b/openpgp-card/src/card_do/key_generation_times.rs index 09e7005..e990c7a 100644 --- a/openpgp-card/src/card_do/key_generation_times.rs +++ b/openpgp-card/src/card_do/key_generation_times.rs @@ -3,12 +3,10 @@ //! Generation date/time of key pair (see spec pg. 24) -use anyhow::anyhow; -use chrono::{DateTime, NaiveDateTime, Utc}; -use nom::{combinator, number::complete as number, sequence}; - use crate::card_do::{KeyGenerationTime, KeySet}; use crate::Error; +use chrono::{DateTime, NaiveDateTime, Utc}; +use nom::{combinator, number::complete as number, sequence}; use std::convert::TryFrom; impl From for DateTime { @@ -69,8 +67,7 @@ impl TryFrom<&[u8]> for KeySet { // hasn't been completely consumed. self::key_generation_set(input) .map(|res| res.1) - .map_err(|err| anyhow!("Parsing failed: {:?}", err)) - .map_err(Error::InternalError) + .map_err(|_err| Error::ParseError(format!("Parsing failed"))) } } diff --git a/openpgp-card/src/card_do/pw_status.rs b/openpgp-card/src/card_do/pw_status.rs index c1fa9da..ae16b5d 100644 --- a/openpgp-card/src/card_do/pw_status.rs +++ b/openpgp-card/src/card_do/pw_status.rs @@ -3,8 +3,6 @@ //! PW status Bytes (see spec page 23) -use anyhow::anyhow; - use crate::card_do::PWStatusBytes; use crate::Error; use std::convert::TryFrom; @@ -41,7 +39,7 @@ impl PWStatusBytes { } impl TryFrom<&[u8]> for PWStatusBytes { - type Error = Error; + type Error = crate::Error; fn try_from(input: &[u8]) -> Result { if input.len() == 7 { @@ -67,7 +65,7 @@ impl TryFrom<&[u8]> for PWStatusBytes { err_count_pw3, }) } else { - Err(Error::InternalError(anyhow!( + Err(Error::ParseError(format!( "Unexpected length of PW Status Bytes: {}", input.len() ))) diff --git a/openpgp-card/src/crypto_data.rs b/openpgp-card/src/crypto_data.rs index c27c467..587cf0b 100644 --- a/openpgp-card/src/crypto_data.rs +++ b/openpgp-card/src/crypto_data.rs @@ -61,7 +61,7 @@ pub enum Cryptogram<'a> { /// to an OpenPGP card pub trait CardUploadableKey { /// private key data - fn private_key(&self) -> Result; + fn private_key(&self) -> Result; /// timestamp of (sub)key creation fn timestamp(&self) -> KeyGenerationTime; diff --git a/openpgp-card/src/errors.rs b/openpgp-card/src/errors.rs index 8c92836..7dba3d0 100644 --- a/openpgp-card/src/errors.rs +++ b/openpgp-card/src/errors.rs @@ -26,8 +26,18 @@ pub enum Error { #[error("Unexpected response length: {0}")] ResponseLength(usize), + #[error("Data not found: {0}")] + NotFound(String), + + #[error("Couldn't parse data: {0}")] + ParseError(String), + + #[error("Unsupported algorithm: {0}")] + UnsupportedAlgo(String), + + // FIXME: placeholder, remove again later? #[error("Internal error: {0}")] - InternalError(anyhow::Error), + InternalError(String), } impl From for Error { @@ -36,12 +46,6 @@ impl From for Error { } } -impl From for Error { - fn from(ae: anyhow::Error) -> Self { - Error::InternalError(ae) - } -} - /// OpenPGP card "Status Bytes" (ok statuses and errors) #[derive(thiserror::Error, Debug, PartialEq, Copy, Clone)] #[non_exhaustive] diff --git a/openpgp-card/src/keys.rs b/openpgp-card/src/keys.rs index 855369d..686aa53 100644 --- a/openpgp-card/src/keys.rs +++ b/openpgp-card/src/keys.rs @@ -3,7 +3,6 @@ //! Generate and import keys -use anyhow::{anyhow, Result}; use std::convert::TryFrom; use std::time::{SystemTime, UNIX_EPOCH}; @@ -77,7 +76,7 @@ pub(crate) fn gen_key_with_metadata( // Store creation timestamp (unix time format, limited to u32) let ts = time .duration_since(UNIX_EPOCH) - .map_err(|e| Error::InternalError(anyhow!(e)))? + .map_err(|e| Error::InternalError(format!("This should never happen {}", e)))? .as_secs() as u32; let ts = ts.into(); @@ -92,7 +91,7 @@ pub(crate) fn gen_key_with_metadata( } /// Transform a public key Tlv from the card into PublicKeyMaterial -fn tlv_to_pubkey(tlv: &Tlv, algo: &Algo) -> Result { +fn tlv_to_pubkey(tlv: &Tlv, algo: &Algo) -> Result { let n = tlv.find(&[0x81].into()); let v = tlv.find(&[0x82].into()); @@ -111,10 +110,10 @@ fn tlv_to_pubkey(tlv: &Tlv, algo: &Algo) -> Result { Ok(PublicKeyMaterial::E(ecc)) } - (_, _, _) => Err(anyhow!( + (_, _, _) => Err(Error::UnsupportedAlgo(format!( "Unexpected public key material from card {:?}", tlv - )), + ))), } } @@ -229,7 +228,7 @@ pub(crate) fn determine_rsa_attrs( key_type: KeyType, ard: &ApplicationRelatedData, algo_info: Option, -) -> Result { +) -> Result { // Figure out suitable RSA algorithm parameters: // Does the card offer a list of algorithms? @@ -273,13 +272,16 @@ pub(crate) fn determine_ecc_attrs( ecc_type: EccType, key_type: KeyType, algo_info: Option, -) -> Result { +) -> Result { // If we have an algo_info, refuse upload if oid is not listed if let Some(algo_info) = algo_info { let algos = check_card_algo_ecc(algo_info, key_type, oid); if algos.is_empty() { // If oid is not in algo_info, return error. - return Err(anyhow!("Oid {:?} unsupported according to algo_info", oid)); + return Err(Error::UnsupportedAlgo(format!( + "Oid {:?} unsupported according to algo_info", + oid + ))); } // Note: Looking up ecc_type in the card's "Algorithm Information" @@ -332,7 +334,9 @@ fn card_algo_rsa(algo_info: AlgoInfo, key_type: KeyType, rsa_bits: u16) -> Resul Ok((**algo.last().unwrap()).clone()) } else { // RSA with this bit length is not in algo_info - return Err(anyhow!("RSA {} unsupported according to algo_info", rsa_bits).into()); + return Err(Error::UnsupportedAlgo( + format!("RSA {} unsupported according to algo_info", rsa_bits).into(), + )); } } @@ -518,7 +522,7 @@ fn control_reference_template(key_type: KeyType) -> Result { KeyType::Decryption => 0xB8, KeyType::Signing => 0xB6, KeyType::Authentication => 0xA4, - _ => return Err(Error::InternalError(anyhow!("Unexpected KeyType"))), + _ => return Err(Error::InternalError("Unexpected KeyType".to_string())), }; Ok(Tlv::new([tag], Value::S(vec![]))) } diff --git a/openpgp-card/src/lib.rs b/openpgp-card/src/lib.rs index d9cbc09..71806f0 100644 --- a/openpgp-card/src/lib.rs +++ b/openpgp-card/src/lib.rs @@ -94,10 +94,10 @@ pub trait CardTransaction { fn feature_pinpad_modify(&self) -> bool; /// Verify the PIN `id` via the reader pinpad - fn pinpad_verify(&mut self, id: u8) -> Result>; + fn pinpad_verify(&mut self, pin: PinType) -> Result, Error>; /// Modify the PIN `id` via the reader pinpad - fn pinpad_modify(&mut self, id: u8) -> Result>; + fn pinpad_modify(&mut self, pin: PinType) -> Result, Error>; /// Select the OpenPGP card application fn select(&mut self) -> Result, Error> { @@ -110,7 +110,7 @@ pub trait CardTransaction { /// (This data should probably be cached in a higher layer. Some parts of /// it are needed regularly, and it does not usually change during /// normal use of a card.) - fn application_related_data(&mut self) -> Result { + fn application_related_data(&mut self) -> Result { let ad = commands::application_related_data(); let resp = apdu::send_command(self, ad, true)?; let value = Value::from(resp.data()?, true)?; @@ -128,7 +128,7 @@ pub trait CardTransaction { /// This fn initializes the CardCaps by requesting /// application_related_data from the card, and setting the /// capabilities accordingly. - fn initialize(&mut self) -> Result<()> { + fn initialize(&mut self) -> Result<(), Error> { let ard = self.application_related_data()?; // Determine chaining/extended length support from card @@ -242,6 +242,23 @@ impl CardCaps { } } +#[derive(Debug, Clone, Copy, Eq, PartialEq)] +pub enum PinType { + Sign, + User, + Admin, +} + +impl PinType { + pub fn id(&self) -> u8 { + match self { + PinType::Sign => 0x81, + PinType::User => 0x82, + PinType::Admin => 0x83, + } + } +} + /// Identify a Key slot on an OpenPGP card #[derive(Debug, Clone, Copy, Eq, PartialEq)] #[non_exhaustive] diff --git a/openpgp-card/src/openpgp.rs b/openpgp-card/src/openpgp.rs index 188350a..6a88bf3 100644 --- a/openpgp-card/src/openpgp.rs +++ b/openpgp-card/src/openpgp.rs @@ -1,7 +1,6 @@ // SPDX-FileCopyrightText: 2021-2022 Heiko Schaefer // SPDX-License-Identifier: MIT OR Apache-2.0 -use anyhow::{anyhow, Result}; use std::convert::{TryFrom, TryInto}; use crate::algorithm::{Algo, AlgoInfo, AlgoSimple}; @@ -14,7 +13,7 @@ use crate::card_do::{ use crate::crypto_data::{CardUploadableKey, Cryptogram, Hash, PublicKeyMaterial}; use crate::tlv::{value::Value, Tlv}; use crate::{ - apdu, keys, CardBackend, CardTransaction, Error, KeyType, SmartcardError, StatusBytes, + apdu, keys, CardBackend, CardTransaction, Error, KeyType, PinType, SmartcardError, StatusBytes, }; /// An OpenPGP card access object, backed by a CardBackend implementation. @@ -80,7 +79,7 @@ impl<'a> OpenPgpTransaction<'a> { /// (This data should probably be cached in a higher layer. Some parts of /// it are needed regularly, and it does not usually change during /// normal use of a card.) - pub fn application_related_data(&mut self) -> Result { + pub fn application_related_data(&mut self) -> Result { self.tx.application_related_data() } @@ -117,14 +116,14 @@ impl<'a> OpenPgpTransaction<'a> { // --- login data (5e) --- /// Get URL (5f50) - pub fn url(&mut self) -> Result> { + pub fn url(&mut self) -> Result, Error> { let resp = apdu::send_command(self.tx(), commands::url(), true)?; Ok(resp.data()?.to_vec()) } /// Get cardholder related data (65) - pub fn cardholder_related_data(&mut self) -> Result { + pub fn cardholder_related_data(&mut self) -> Result { let crd = commands::cardholder_related_data(); let resp = apdu::send_command(self.tx(), crd, true)?; resp.check_ok()?; @@ -133,15 +132,15 @@ impl<'a> OpenPgpTransaction<'a> { } /// Get security support template (7a) - pub fn security_support_template(&mut self) -> Result { + pub fn security_support_template(&mut self) -> Result { let sst = commands::security_support_template(); let resp = apdu::send_command(self.tx(), sst, true)?; resp.check_ok()?; let tlv = Tlv::try_from(resp.data()?)?; - let res = tlv - .find(&[0x93].into()) - .ok_or_else(|| anyhow!("Couldn't get SecuritySupportTemplate DO"))?; + let res = tlv.find(&[0x93].into()).ok_or_else(|| { + Error::NotFound("Couldn't get SecuritySupportTemplate DO".to_string()) + })?; if let Value::S(data) = res { let mut data = data.to_vec(); @@ -153,7 +152,9 @@ impl<'a> OpenPgpTransaction<'a> { let dsc: u32 = u32::from_be_bytes(data); Ok(SecuritySupportTemplate { dsc }) } else { - Err(anyhow!("Failed to process SecuritySupportTemplate")) + Err(Error::NotFound( + "Failed to process SecuritySupportTemplate".to_string(), + )) } } @@ -168,7 +169,7 @@ impl<'a> OpenPgpTransaction<'a> { } /// Get "Algorithm Information" - pub fn algorithm_information(&mut self) -> Result> { + pub fn algorithm_information(&mut self) -> Result, Error> { let resp = apdu::send_command(self.tx(), commands::algo_info(), true)?; resp.check_ok()?; @@ -177,7 +178,7 @@ impl<'a> OpenPgpTransaction<'a> { } /// Firmware Version (YubiKey specific (?)) - pub fn firmware_version(&mut self) -> Result> { + pub fn firmware_version(&mut self) -> Result, Error> { let resp = apdu::send_command(self.tx(), commands::firmware_version(), true)?; Ok(resp.data()?.into()) @@ -187,7 +188,7 @@ impl<'a> OpenPgpTransaction<'a> { /// [see: /// /// ] - pub fn set_identity(&mut self, id: u8) -> Result> { + pub fn set_identity(&mut self, id: u8) -> Result, Error> { let resp = apdu::send_command(self.tx(), commands::set_identity(id), false); // Apparently it's normal to get "NotTransacted" from pcsclite when @@ -218,7 +219,7 @@ impl<'a> OpenPgpTransaction<'a> { /// Get data from "private use" DO. /// /// `num` must be between 1 and 4. - pub fn private_use_do(&mut self, num: u8) -> Result> { + pub fn private_use_do(&mut self, num: u8) -> Result, Error> { assert!((1..=4).contains(&num)); let cmd = commands::private_use_do(num); @@ -234,7 +235,7 @@ impl<'a> OpenPgpTransaction<'a> { /// Access condition: /// - 1/3 need PW1 (82) /// - 2/4 need PW3 - pub fn set_private_use_do(&mut self, num: u8, data: Vec) -> Result> { + pub fn set_private_use_do(&mut self, num: u8, data: Vec) -> Result, Error> { assert!((1..=4).contains(&num)); let cmd = commands::put_private_use_do(num, data); @@ -260,7 +261,7 @@ impl<'a> OpenPgpTransaction<'a> { /// (However, e.g. vanilla Gnuk doesn't support this functionality. /// Gnuk needs to be built with the `--enable-factory-reset` /// option to the `configure` script to enable this functionality). - pub fn factory_reset(&mut self) -> Result<()> { + pub fn factory_reset(&mut self) -> Result<(), Error> { // send 4 bad requests to verify pw1 // [apdu 00 20 00 81 08 40 40 40 40 40 40 40 40] for _ in 0..4 { @@ -270,7 +271,9 @@ impl<'a> OpenPgpTransaction<'a> { || resp.status() == StatusBytes::AuthenticationMethodBlocked || matches!(resp.status(), StatusBytes::PasswordNotChecked(_))) { - return Err(anyhow!("Unexpected status for reset, at pw1.")); + return Err(Error::InternalError( + "Unexpected status for reset, at pw1.".into(), + )); } } @@ -284,7 +287,9 @@ impl<'a> OpenPgpTransaction<'a> { || resp.status() == StatusBytes::AuthenticationMethodBlocked || matches!(resp.status(), StatusBytes::PasswordNotChecked(_))) { - return Err(anyhow!("Unexpected status for reset, at pw3.")); + return Err(Error::InternalError( + "Unexpected status for reset, at pw3.".into(), + )); } } @@ -321,7 +326,7 @@ impl<'a> OpenPgpTransaction<'a> { /// access condition is only valid for one PSO:CDS command or remains /// valid for several attempts. pub fn verify_pw1_for_signing_pinpad(&mut self) -> Result<(), Error> { - let res = self.tx().pinpad_verify(0x81)?; + let res = self.tx().pinpad_verify(PinType::Sign)?; RawResponse::try_from(res)?.try_into() } @@ -350,7 +355,7 @@ impl<'a> OpenPgpTransaction<'a> { /// an error is returned. pub fn verify_pw1_pinpad(&mut self) -> Result<(), Error> { - let res = self.tx().pinpad_verify(0x82)?; + let res = self.tx().pinpad_verify(PinType::User)?; RawResponse::try_from(res)?.try_into() } @@ -377,7 +382,7 @@ impl<'a> OpenPgpTransaction<'a> { /// Verify PW3 (admin) using a pinpad on the card reader. If no usable /// pinpad is found, an error is returned. pub fn verify_pw3_pinpad(&mut self) -> Result<(), Error> { - let res = self.tx().pinpad_verify(0x83)?; + let res = self.tx().pinpad_verify(PinType::Admin)?; RawResponse::try_from(res)?.try_into() } @@ -406,10 +411,12 @@ impl<'a> OpenPgpTransaction<'a> { apdu::send_command(self.tx(), change, false)?.try_into() } - /// Change the value of PW1 (user password) using a pinpad on the + /// Change the value of PW1 (0x81) using a pinpad on the /// card reader. If no usable pinpad is found, an error is returned. pub fn change_pw1_pinpad(&mut self) -> Result<(), Error> { - let res = self.tx().pinpad_modify(0x81)?; + // Note: for change PW, only 0x81 and 0x83 are used! + // 0x82 is implicitly the same as 0x81. + let res = self.tx().pinpad_modify(PinType::Sign)?; RawResponse::try_from(res)?.try_into() } @@ -428,7 +435,7 @@ impl<'a> OpenPgpTransaction<'a> { /// Change the value of PW3 (admin password) using a pinpad on the /// card reader. If no usable pinpad is found, an error is returned. pub fn change_pw3_pinpad(&mut self) -> Result<(), Error> { - let res = self.tx().pinpad_modify(0x83)?; + let res = self.tx().pinpad_modify(PinType::Admin)?; RawResponse::try_from(res)?.try_into() } diff --git a/openpgp-card/src/tlv.rs b/openpgp-card/src/tlv.rs index 4053a61..3df3915 100644 --- a/openpgp-card/src/tlv.rs +++ b/openpgp-card/src/tlv.rs @@ -72,7 +72,7 @@ impl Tlv { } impl TryFrom<&[u8]> for Tlv { - type Error = anyhow::Error; + type Error = crate::Error; fn try_from(input: &[u8]) -> Result { complete(Tlv::parse(input)) diff --git a/openpgp-card/src/tlv/value.rs b/openpgp-card/src/tlv/value.rs index efff1a7..fe49c5f 100644 --- a/openpgp-card/src/tlv/value.rs +++ b/openpgp-card/src/tlv/value.rs @@ -37,7 +37,7 @@ impl Value { } } - pub fn from(data: &[u8], constructed: bool) -> Result { + pub fn from(data: &[u8], constructed: bool) -> Result { complete(Self::parse(data, constructed)) } diff --git a/pcsc/src/lib.rs b/pcsc/src/lib.rs index 3be3c0d..6389a06 100644 --- a/pcsc/src/lib.rs +++ b/pcsc/src/lib.rs @@ -5,13 +5,12 @@ //! `openpgp-card`. It uses the PCSC middleware to access the OpenPGP //! application on smart cards. -use anyhow::{anyhow, Result}; use iso7816_tlv::simple::Tlv; use std::collections::HashMap; use std::convert::TryInto; use openpgp_card::card_do::ApplicationRelatedData; -use openpgp_card::{CardBackend, CardCaps, CardTransaction, Error, SmartcardError}; +use openpgp_card::{CardBackend, CardCaps, CardTransaction, Error, PinType, SmartcardError}; const FEATURE_VERIFY_PIN_DIRECT: u8 = 0x06; const FEATURE_MODIFY_PIN_DIRECT: u8 = 0x07; @@ -181,23 +180,21 @@ impl<'b> PcscTransaction<'b> { } /// Get the minimum pin length for pin_id. - fn min_pin_len(&self, pin_id: u8) -> Result { - match pin_id { - 0x81 | 0x82 => Ok(6), - 0x83 => Ok(8), - _ => Err(anyhow!("Unexpected pin_id {}", pin_id)), + fn min_pin_len(&self, pin: PinType) -> u8 { + match pin { + PinType::User | PinType::Sign => 6, + PinType::Admin => 8, } } /// Get the maximum pin length for pin_id. - fn max_pin_len(&self, pin_id: u8) -> Result { + fn max_pin_len(&self, pin: PinType) -> Result { if let Some(card_caps) = self.card_caps { - match pin_id { - 0x81 | 0x82 => Ok(card_caps.pw1_max_len()), - 0x83 => Ok(card_caps.pw3_max_len()), - _ => Err(anyhow!("Unexpected pin_id {}", pin_id)), + match pin { + PinType::User | PinType::Sign => Ok(card_caps.pw1_max_len()), + PinType::Admin => Ok(card_caps.pw3_max_len()), } } else { - Err(anyhow!("card_caps is None")) + Err(Error::InternalError("card_caps is None".into())) } } } @@ -235,9 +232,9 @@ impl CardTransaction for PcscTransaction<'_> { self.reader_caps.contains_key(&FEATURE_MODIFY_PIN_DIRECT) } - fn pinpad_verify(&mut self, pin_id: u8) -> Result> { - let pin_min_size = self.min_pin_len(pin_id)?; - let pin_max_size = self.max_pin_len(pin_id)?; + fn pinpad_verify(&mut self, pin: PinType) -> Result, Error> { + let pin_min_size = self.min_pin_len(pin); + let pin_max_size = self.max_pin_len(pin)?; // Default to varlen, for now. // (NOTE: Some readers don't support varlen, and need explicit length @@ -249,7 +246,7 @@ impl CardTransaction for PcscTransaction<'_> { 0x00, /* CLA */ 0x20, /* INS: VERIFY */ 0x00, /* P1 */ - pin_id, /* P2 */ + pin.id(), /* P2 */ fixedlen, /* Lc: 'fixedlen' data bytes */ ]; ab_data.extend([0xff].repeat(fixedlen as usize)); @@ -311,22 +308,26 @@ impl CardTransaction for PcscTransaction<'_> { let verify_ioctl: [u8; 4] = self .reader_caps .get(&FEATURE_VERIFY_PIN_DIRECT) - .ok_or_else(|| anyhow!("no reader_capability"))? + .ok_or_else(|| Error::Smartcard(SmartcardError::Error("no reader_capability".into())))? .value() - .try_into()?; + .try_into() + .map_err(|e| Error::ParseError(format!("unexpected feature data: {:?}", e)))?; let res = self .tx - .control(u32::from_be_bytes(verify_ioctl).into(), &send, &mut recv)?; + .control(u32::from_be_bytes(verify_ioctl).into(), &send, &mut recv) + .map_err(|e: pcsc::Error| { + Error::Smartcard(SmartcardError::Error(format!("pcsc Error: {:?}", e))) + })?; log::debug!(" <- pcsc pinpad_verify result: {:x?}", res); Ok(res.to_vec()) } - fn pinpad_modify(&mut self, pin_id: u8) -> Result> { - let pin_min_size = self.min_pin_len(pin_id)?; - let pin_max_size = self.max_pin_len(pin_id)?; + fn pinpad_modify(&mut self, pin: PinType) -> Result, Error> { + let pin_min_size = self.min_pin_len(pin); + let pin_max_size = self.max_pin_len(pin)?; // Default to varlen, for now. // (NOTE: Some readers don't support varlen, and need explicit length @@ -338,7 +339,7 @@ impl CardTransaction for PcscTransaction<'_> { 0x00, /* CLA */ 0x24, /* INS: CHANGE_REFERENCE_DATA */ 0x00, /* P1 */ - pin_id, /* P2 */ + pin.id(), /* P2 */ fixedlen * 2, /* Lc: 'fixedlen' data bytes */ ]; ab_data.extend([0xff].repeat(fixedlen as usize * 2)); @@ -410,13 +411,17 @@ impl CardTransaction for PcscTransaction<'_> { let modify_ioctl: [u8; 4] = self .reader_caps .get(&FEATURE_MODIFY_PIN_DIRECT) - .ok_or_else(|| anyhow!("no reader_capability"))? + .ok_or_else(|| Error::Smartcard(SmartcardError::Error("no reader_capability".into())))? .value() - .try_into()?; + .try_into() + .map_err(|e| Error::ParseError(format!("unexpected feature data: {:?}", e)))?; let res = self .tx - .control(u32::from_be_bytes(modify_ioctl).into(), &send, &mut recv)?; + .control(u32::from_be_bytes(modify_ioctl).into(), &send, &mut recv) + .map_err(|e: pcsc::Error| { + Error::Smartcard(SmartcardError::Error(format!("pcsc Error: {:?}", e))) + })?; log::debug!(" <- pcsc pinpad_modify result: {:x?}", res); @@ -602,7 +607,7 @@ impl PcscBackend { /// Initialized a PcscCard: /// - Obtain and store feature lists from reader (pinpad functionality). /// - Get ARD from card, set CardCaps based on ARD. - fn initialize_card(mut self) -> Result { + fn initialize_card(mut self) -> Result { log::debug!("pcsc initialize_card"); let mut h: HashMap = HashMap::default(); diff --git a/scdc/src/lib.rs b/scdc/src/lib.rs index 260c3c9..4686c2d 100644 --- a/scdc/src/lib.rs +++ b/scdc/src/lib.rs @@ -5,7 +5,6 @@ //! `openpgp-card` crate. //! It uses GnuPG's scdaemon (via GnuPG Agent) to access OpenPGP cards. -use anyhow::{anyhow, Result}; use futures::StreamExt; use lazy_static::lazy_static; use sequoia_ipc::assuan::Response; @@ -13,7 +12,7 @@ use sequoia_ipc::gnupg::{Agent, Context}; use std::sync::Mutex; use tokio::runtime::Runtime; -use openpgp_card::{CardBackend, CardCaps, CardTransaction, Error}; +use openpgp_card::{CardBackend, CardCaps, CardTransaction, Error, PinType, SmartcardError}; lazy_static! { static ref RT: Mutex = Mutex::new(tokio::runtime::Runtime::new().unwrap()); @@ -82,7 +81,7 @@ impl ScdBackend { /// Helper fn that shuts down scdaemon via GnuPG Agent. /// This may be useful to obtain access to a Smard card via PCSC. - pub fn shutdown_scd(agent: Option) -> Result<()> { + pub fn shutdown_scd(agent: Option) -> Result<(), Error> { let mut scdc = Self::new(agent, false)?; scdc.send("SCD RESTART")?; @@ -96,13 +95,23 @@ impl ScdBackend { /// /// If `agent` is None, a Context with the default GnuPG home directory /// is used. - fn new(agent: Option, init: bool) -> Result { + fn new(agent: Option, init: bool) -> Result { let agent = if let Some(agent) = agent { agent } else { // Create and use a new Agent based on a default Context - let ctx = Context::new()?; - RT.lock().unwrap().block_on(Agent::connect(&ctx))? + let ctx = Context::new().map_err(|e| { + Error::Smartcard(SmartcardError::Error(format!("Context::new failed {}", e))) + })?; + RT.lock() + .unwrap() + .block_on(Agent::connect(&ctx)) + .map_err(|e| { + Error::Smartcard(SmartcardError::Error(format!( + "Agent::connect failed {}", + e + ))) + })? }; let mut scdc = Self { @@ -117,13 +126,22 @@ impl ScdBackend { Ok(scdc) } + fn send2(&mut self, cmd: &str) -> Result<(), Error> { + self.agent.send(cmd).map_err(|e| { + Error::Smartcard(SmartcardError::Error(format!( + "scdc agent send failed: {}", + e + ))) + }) + } + /// Call "SCD SERIALNO", which causes scdaemon to be started by gpg /// agent (if it's not running yet). - fn serialno(&mut self) -> Result<()> { + fn serialno(&mut self) -> Result<(), Error> { let mut rt = RT.lock().unwrap(); let send = "SCD SERIALNO"; - self.agent.send(send)?; + self.send2(send)?; while let Some(response) = rt.block_on(self.agent.next()) { log::debug!("init res: {:x?}", response); @@ -138,14 +156,16 @@ impl ScdBackend { } } - Err(anyhow!("SCDC init() failed")) + Err(Error::Smartcard(SmartcardError::Error( + "SCDC init() failed".into(), + ))) } /// Ask scdameon to switch to using a specific OpenPGP card, based on /// its `serial`. - fn select_card(&mut self, serial: &str) -> Result<()> { + fn select_card(&mut self, serial: &str) -> Result<(), Error> { let send = format!("SCD SERIALNO --demand={}", serial); - self.agent.send(send)?; + self.send2(&send)?; let mut rt = RT.lock().unwrap(); @@ -153,7 +173,9 @@ impl ScdBackend { log::debug!("select res: {:x?}", response); if response.is_err() { - return Err(anyhow!("Card not found")); + return Err(Error::Smartcard(SmartcardError::CardNotFound( + serial.into(), + ))); } if let Ok(Response::Status { .. }) = response { @@ -166,11 +188,13 @@ impl ScdBackend { } } - Err(anyhow!("Card not found")) + Err(Error::Smartcard(SmartcardError::CardNotFound( + serial.into(), + ))) } - fn send(&mut self, cmd: &str) -> Result<()> { - self.agent.send(cmd)?; + fn send(&mut self, cmd: &str) -> Result<(), Error> { + self.send2(cmd)?; let mut rt = RT.lock().unwrap(); @@ -178,7 +202,7 @@ impl ScdBackend { log::debug!("select res: {:x?}", response); if let Err(e) = response { - return Err(anyhow!("Err {:?}", e)); + return Err(Error::Smartcard(SmartcardError::Error(format!("{:?}", e)))); } if let Ok(..) = response { @@ -191,7 +215,10 @@ impl ScdBackend { } } - Err(anyhow!("Error sending command {}", cmd)) + Err(Error::Smartcard(SmartcardError::Error(format!( + "Error sending command {}", + cmd + )))) } } @@ -225,23 +252,23 @@ impl CardTransaction for ScdTransaction<'_> { log::debug!("SCDC command: '{}'", send); if send.len() > ASSUAN_LINELENGTH { - return Err(Error::InternalError(anyhow!( + return Err(Error::Smartcard(SmartcardError::Error(format!( "APDU command is too long ({}) to send via Assuan", send.len() - ))); + )))); } - self.scd.agent.send(send)?; + self.scd.send2(&send)?; let mut rt = RT.lock().unwrap(); while let Some(response) = rt.block_on(self.scd.agent.next()) { log::debug!("res: {:x?}", response); if response.is_err() { - return Err(Error::InternalError(anyhow!( + return Err(Error::Smartcard(SmartcardError::Error(format!( "Unexpected error response from SCD {:?}", response - ))); + )))); } if let Ok(Response::Data { partial }) = response { @@ -256,7 +283,9 @@ impl CardTransaction for ScdTransaction<'_> { } } - Err(Error::InternalError(anyhow!("no response found"))) + Err(Error::Smartcard(SmartcardError::Error( + "no response found".into(), + ))) } fn init_card_caps(&mut self, caps: CardCaps) { @@ -284,12 +313,12 @@ impl CardTransaction for ScdTransaction<'_> { } /// FIXME: not implemented yet - fn pinpad_verify(&mut self, _id: u8) -> Result> { + fn pinpad_verify(&mut self, _id: PinType) -> Result, Error> { unimplemented!() } /// FIXME: not implemented yet - fn pinpad_modify(&mut self, _id: u8) -> Result> { + fn pinpad_modify(&mut self, _id: PinType) -> Result, Error> { unimplemented!() } } diff --git a/tools/src/bin/opgpcard/main.rs b/tools/src/bin/opgpcard/main.rs index 0912e2a..e9a5af2 100644 --- a/tools/src/bin/opgpcard/main.rs +++ b/tools/src/bin/opgpcard/main.rs @@ -372,7 +372,7 @@ fn factory_reset(ident: &str) -> Result<()> { let mut pgp = OpenPgp::new(&mut card); let mut open = Open::new(pgp.transaction()?)?; - open.factory_reset() + open.factory_reset().map_err(|e| anyhow!(e)) } fn key_import_yolo(mut admin: Admin, key: &Cert) -> Result<()> {