Adjust error handling in CardSigner and CardDecryptor (for better error messages in tools, such as opgpcard).

Refactor sq_utils, rename some fn for clarity.

card-functionality/src/util.rs

@@ -18,7 +18,7 @@ use sequoia_openpgp::Cert;
 
 use openpgp_card::card_do::KeyGenerationTime;
 use openpgp_card::{CardApp, KeyType};
-use openpgp_card_sequoia::sq_util::get_subkey;
+use openpgp_card_sequoia::sq_util;
 use openpgp_card_sequoia::util::vka_as_uploadable_key;
 
 pub const SP: &StandardPolicy = &StandardPolicy::new();
@@ -35,7 +35,7 @@ pub(crate) fn upload_subkeys(
         KeyType::Decryption,
         KeyType::Authentication,
     ] {
-        if let Some(vka) = get_subkey(cert, policy, *kt)? {
+        if let Some(vka) = sq_util::get_subkey_by_type(cert, policy, *kt)? {
             // store fingerprint as return-value
             let fp = vka.fingerprint().to_hex();
             // store key creation time as return-value

openpgp-card-sequoia/src/decryptor.rs

@@ -18,6 +18,7 @@ use sequoia_openpgp as openpgp;
 use openpgp_card::crypto_data::Cryptogram;
 use openpgp_card::{CardApp, Error};
 
+use crate::sq_util;
 use crate::PublicKey;
 
 pub struct CardDecryptor<'a> {
@@ -47,23 +48,23 @@ impl<'a> CardDecryptor<'a> {
             // Transform into Sequoia Fingerprint
             let fp = openpgp::Fingerprint::from_bytes(fp.as_bytes());
 
-            // Find the matching encryption-capable (sub)key in `cert`
+            if let Some(vk) =
-            let keys: Vec<_> = cert
+                sq_util::get_subkey_by_fingerprint(cert, policy, &fp)?
-                .keys()
+            {
-                .with_policy(policy, None)
+                if vk.for_storage_encryption() || vk.for_transport_encryption()
-                .for_storage_encryption()
+                {
-                .for_transport_encryption()
+                    let public = vk.key().clone();
-                .filter(|ka| ka.fingerprint() == fp)
+                    Ok(Self { ca, public })
-                .map(|ka| ka.key())
+                } else {
-                .collect();
+                    Err(Error::InternalError(anyhow!(
-
+                        "(Sub)key {} in the cert isn't encryption capable",
-            // Exactly one matching (sub)key should be found. If not, fail!
+                        fp
-            if keys.len() == 1 {
+                    )))
-                let public = keys[0].clone();
+                }
-                Ok(Self { ca, public })
             } else {
                 Err(Error::InternalError(anyhow!(
-                    "Failed to find a matching (sub)key in cert"
+                    "Failed to find (sub)key {} in cert",
+                    fp
                 )))
             }
         } else {

openpgp-card-sequoia/src/main.rs

@@ -14,7 +14,7 @@ use openpgp_card::KeyType;
 use openpgp_card_pcsc::PcscClient;
 
 use openpgp_card_sequoia::card::Open;
-use openpgp_card_sequoia::sq_util::{decryption_helper, sign_helper};
+use openpgp_card_sequoia::sq_util;
 
 // Filename of test key and test message to use
 
@@ -118,29 +118,23 @@ fn main() -> Result<(), Box<dyn Error>> {
         let cert = Cert::from_file(TEST_KEY_PATH)?;
         let p = StandardPolicy::new();
 
-        if let Some(vka) = openpgp_card_sequoia::sq_util::get_subkey(
+        if let Some(vka) =
-            &cert,
+            sq_util::get_subkey_by_type(&cert, &p, KeyType::Signing)?
-            &p,
+        {
-            KeyType::Signing,
-        )? {
             println!("Upload signing key");
             admin.upload_key(vka, KeyType::Signing, None)?;
         }
 
-        if let Some(vka) = openpgp_card_sequoia::sq_util::get_subkey(
+        if let Some(vka) =
-            &cert,
+            sq_util::get_subkey_by_type(&cert, &p, KeyType::Decryption)?
-            &p,
+        {
-            KeyType::Decryption,
-        )? {
             println!("Upload decryption key");
             admin.upload_key(vka, KeyType::Decryption, None)?;
         }
 
-        if let Some(vka) = openpgp_card_sequoia::sq_util::get_subkey(
+        if let Some(vka) =
-            &cert,
+            sq_util::get_subkey_by_type(&cert, &p, KeyType::Authentication)?
-            &p,
+        {
-            KeyType::Authentication,
-        )? {
             println!("Upload auth key");
             admin.upload_key(vka, KeyType::Authentication, None)?;
         }
@@ -179,7 +173,7 @@ fn main() -> Result<(), Box<dyn Error>> {
 
         let sp = StandardPolicy::new();
         let d = user.decryptor(&cert, &sp)?;
-        let res = decryption_helper(d, msg.into_bytes(), &sp)?;
+        let res = sq_util::decryption_helper(d, msg.into_bytes(), &sp)?;
 
         let plain = String::from_utf8_lossy(&res);
         println!("Decrypted plaintext: {}", plain);
@@ -204,7 +198,7 @@ fn main() -> Result<(), Box<dyn Error>> {
         let text = "Hello world, I am signed.";
 
         let signer = sign.signer(&cert, &StandardPolicy::new())?;
-        let sig = sign_helper(signer, &mut text.as_bytes())?;
+        let sig = sq_util::sign_helper(signer, &mut text.as_bytes())?;
 
         println!("Signature from card:\n{}", sig)
 

openpgp-card-sequoia/src/signer.rs

@@ -4,6 +4,7 @@
 use std::convert::TryInto;
 
 use anyhow::anyhow;
+
 use openpgp::crypto;
 use openpgp::crypto::mpi;
 use openpgp::policy::Policy;
@@ -13,6 +14,7 @@ use sequoia_openpgp as openpgp;
 use openpgp_card::crypto_data::Hash;
 use openpgp_card::{CardApp, Error};
 
+use crate::sq_util;
 use crate::PublicKey;
 
 pub struct CardSigner<'a> {
@@ -42,25 +44,22 @@ impl<'a> CardSigner<'a> {
             // Transform into Sequoia Fingerprint
             let fp = openpgp::Fingerprint::from_bytes(fp.as_bytes());
 
-            // Find the matching signing-capable (sub)key in `cert`
+            if let Some(vk) =
-            let keys: Vec<_> = cert
+                sq_util::get_subkey_by_fingerprint(cert, policy, &fp)?
-                .keys()
+            {
-                .with_policy(policy, None)
+                if vk.for_signing() {
-                .alive()
+                    let key = vk.key().clone();
-                .revoked(false)
+                    Ok(Self::with_pubkey(ca, key))
-                .for_signing()
+                } else {
-                .filter(|ka| ka.fingerprint() == fp)
+                    Err(Error::InternalError(anyhow!(
-                .map(|ka| ka.key())
+                        "(Sub)key {} in the cert isn't signing capable",
-                .collect();
+                        fp
-
+                    )))
-            // Exactly one matching (sub)key should be found. If not, fail!
+                }
-            if keys.len() == 1 {
-                let public = keys[0].clone();
-
-                Ok(Self::with_pubkey(ca, public))
             } else {
                 Err(Error::InternalError(anyhow!(
-                    "Failed to find a matching (sub)key in cert"
+                    "Failed to find (sub)key {} in cert",
+                    fp
                 )))
             }
         } else {

openpgp-card-sequoia/src/sq_util.rs

@@ -9,25 +9,28 @@ use anyhow::{anyhow, Context, Result};
 use std::io;
 
 use openpgp::armor;
-use openpgp::cert::amalgamation::key::ValidErasedKeyAmalgamation;
+use openpgp::cert::amalgamation::{
+    key::ValidErasedKeyAmalgamation, ValidAmalgamation, ValidateAmalgamation,
+};
 use openpgp::crypto;
-use openpgp::packet::key::SecretParts;
+use openpgp::packet::key::{PublicParts, SecretParts};
 use openpgp::parse::{
     stream::{DecryptionHelper, DecryptorBuilder, VerificationHelper},
     Parse,
 };
 use openpgp::policy::Policy;
 use openpgp::serialize::stream::{Message, Signer};
+use openpgp::types::RevocationStatus;
 use openpgp::{Cert, Fingerprint};
 use sequoia_openpgp as openpgp;
 
-use openpgp_card::KeyType;
+use openpgp_card::{Error, KeyType};
 
 /// Retrieve a (sub)key from a Cert, for a given KeyType.
 ///
 /// Returns Ok(None), if no such (sub)key exists.
 /// If multiple suitable (sub)keys are found, an error is returned.
-pub fn get_subkey<'a>(
+pub fn get_subkey_by_type<'a>(
     cert: &'a Cert,
     policy: &'a dyn Policy,
     key_type: KeyType,
@@ -60,8 +63,8 @@ pub fn get_subkey<'a>(
     }
 }
 
-/// Retrieve a (sub)key from a Cert, with a specified fingerprint.
+/// Retrieve a private (sub)key from a Cert, by fingerprint.
-pub fn get_subkey_by_fingerprint<'a>(
+pub fn get_priv_subkey_by_fingerprint<'a>(
     cert: &'a Cert,
     policy: &'a dyn Policy,
     fingerprint: &str,
@@ -90,6 +93,51 @@ pub fn get_subkey_by_fingerprint<'a>(
     }
 }
 
+/// Retrieve a public (sub)key from a Cert, by fingerprint.
+pub fn get_subkey_by_fingerprint<'a>(
+    cert: &'a Cert,
+    policy: &'a dyn Policy,
+    fp: &Fingerprint,
+) -> Result<Option<ValidErasedKeyAmalgamation<'a, PublicParts>>, Error> {
+    // Find the (sub)key in `cert` that matches the fingerprint from
+    // the Card's signing-key slot.
+    let keys: Vec<_> =
+        cert.keys().filter(|ka| &ka.fingerprint() == fp).collect();
+
+    // Exactly one matching (sub)key should be found. If not, fail!
+    if keys.len() == 1 {
+        // Check if the (sub)key is valid/alive, return error
+        // otherwise
+        let validkey = keys[0].clone().with_policy(policy, None)?;
+        validkey.alive()?;
+
+        if let RevocationStatus::Revoked(_) = validkey.revocation_status() {
+            return Err(Error::InternalError(anyhow!(
+                "(Sub)key {} in the cert is revoked",
+                fp
+            )));
+        }
+
+        Ok(Some(validkey))
+    } else {
+        if keys.len() == 0 {
+            Ok(None)
+        } else if keys.len() == 2 {
+            Err(Error::InternalError(anyhow!(
+                "Found two results for {}, probably the cert has the \
+                 primary as a subkey?",
+                fp
+            )))
+        } else {
+            Err(Error::InternalError(anyhow!(
+                "Found {} results for (sub)key {}, this is unexpected",
+                keys.len(),
+                fp
+            )))
+        }
+    }
+}
+
 /// Produce an armored signature from `input` and a Signer `s`.
 pub fn sign_helper<S>(s: S, input: &mut dyn io::Read) -> Result<String>
 where

tools/src/bin/opgpcard/main.rs

@@ -353,20 +353,11 @@ fn factory_reset(ident: &str) -> Result<()> {
 fn key_import_yolo(mut admin: Admin, key: &Cert) -> Result<()> {
     let p = StandardPolicy::new();
 
-    let sig =
+    let sig = sq_util::get_subkey_by_type(key, &p, KeyType::Signing)?;
-        openpgp_card_sequoia::sq_util::get_subkey(key, &p, KeyType::Signing)?;
 
-    let dec = openpgp_card_sequoia::sq_util::get_subkey(
+    let dec = sq_util::get_subkey_by_type(key, &p, KeyType::Decryption)?;
-        key,
-        &p,
-        KeyType::Decryption,
-    )?;
 
-    let auth = openpgp_card_sequoia::sq_util::get_subkey(
+    let auth = sq_util::get_subkey_by_type(key, &p, KeyType::Authentication)?;
-        key,
-        &p,
-        KeyType::Authentication,
-    )?;
 
     if let Some(sig) = sig {
         println!("Uploading {} as signing key", sig.fingerprint());
@@ -395,7 +386,7 @@ fn key_import_explicit(
 
     if let Some(sig_fp) = sig_fp {
         if let Some(sig) =
-            sq_util::get_subkey_by_fingerprint(key, &p, &sig_fp)?
+            sq_util::get_priv_subkey_by_fingerprint(key, &p, &sig_fp)?
         {
             println!("Uploading {} as signing key", sig.fingerprint());
             admin.upload_key(sig, KeyType::Signing, None)?;
@@ -406,7 +397,7 @@ fn key_import_explicit(
 
     if let Some(dec_fp) = dec_fp {
         if let Some(dec) =
-            sq_util::get_subkey_by_fingerprint(key, &p, &dec_fp)?
+            sq_util::get_priv_subkey_by_fingerprint(key, &p, &dec_fp)?
         {
             println!("Uploading {} as decryption key", dec.fingerprint());
             admin.upload_key(dec, KeyType::Decryption, None)?;
@@ -417,7 +408,7 @@ fn key_import_explicit(
 
     if let Some(auth_fp) = auth_fp {
         if let Some(auth) =
-            sq_util::get_subkey_by_fingerprint(key, &p, &auth_fp)?
+            sq_util::get_priv_subkey_by_fingerprint(key, &p, &auth_fp)?
         {
             println!("Uploading {} as authentication key", auth.fingerprint());
             admin.upload_key(auth, KeyType::Authentication, None)?;