Refactor, Document API.

(Moved algorithm-related data structures to algorithm.rs)

card-functionality/src/tests.rs

@@ -11,9 +11,10 @@ use sequoia_openpgp::serialize::SerializeInto;
 use sequoia_openpgp::types::Timestamp;
 use sequoia_openpgp::Cert;
 
+use openpgp_card::algorithm::AlgoSimple;
 use openpgp_card::card_app::CardApp;
 use openpgp_card::errors::{OcErrorStatus, OpenpgpCardError};
-use openpgp_card::{AlgoSimple, KeyType, Sex};
+use openpgp_card::{KeyType, Sex};
 use openpgp_card_sequoia::{
     make_cert, public_key_material_to_key, public_to_fingerprint,
 };

openpgp-card-sequoia/src/lib.rs

@@ -31,14 +31,14 @@ use openpgp::types::{KeyFlags, PublicKeyAlgorithm, SignatureType};
 use openpgp::{Cert, Packet};
 use sequoia_openpgp as openpgp;
 
+use openpgp_card::algorithm::{Algo, AlgoInfo, Curve};
 use openpgp_card::apdu::response::Response;
 use openpgp_card::card_app::{CardApp, ARD};
 use openpgp_card::{
-    errors::OpenpgpCardError, Algo, AlgoInfo, ApplicationId, CardClientBox,
+    errors::OpenpgpCardError, ApplicationId, CardClientBox, CardUploadableKey,
-    CardHolder, CardUploadableKey, Curve, DecryptMe, EccKey, EccType,
+    Cardholder, DecryptMe, EccKey, EccType, ExtendedCap, ExtendedLengthInfo,
-    ExtendedCap, ExtendedLengthInfo, Features, Fingerprint, Hash, Historical,
+    Features, Fingerprint, Hash, Historical, KeySet, KeyType, PWStatus,
-    KeySet, KeyType, PWStatus, PrivateKeyMaterial, PublicKeyMaterial, RSAKey,
+    PrivateKeyMaterial, PublicKeyMaterial, RSAKey, Sex,
-    Sex,
 };
 
 use crate::signer::CardSigner;
@@ -678,7 +678,7 @@ impl CardBase {
     }
 
     // --- cardholder related data (65) ---
-    pub fn get_cardholder_related_data(&mut self) -> Result<CardHolder> {
+    pub fn get_cardholder_related_data(&mut self) -> Result<Cardholder> {
         self.card_app.get_cardholder_related_data()
     }
 

openpgp-card/src/algorithm.rs

@@ -0,0 +1,250 @@
+// SPDX-FileCopyrightText: 2021 Heiko Schaefer <heiko@schaefer.name>
+// SPDX-License-Identifier: MIT OR Apache-2.0
+
+use crate::{EccType, KeyType};
+use anyhow::anyhow;
+use std::convert::TryFrom;
+use std::fmt;
+
+/// This enum offers a shorthand way to specify algorithms (e.g. for key
+/// generation).
+///
+/// RSA variants require "number of bits in 'e'" as parameter.
+///
+/// There are (at least) two common supported values for e:
+///  e=17 [YK4, YK5]
+///  e=32 [YK5, Floss3.4, Gnuk1.2]
+#[derive(Clone, Copy, Debug)]
+pub enum AlgoSimple {
+    RSA1k(u16),
+    RSA2k(u16),
+    RSA3k(u16),
+    RSA4k(u16),
+    NIST256,
+    NIST384,
+    NIST521,
+    Curve25519,
+}
+
+impl From<&str> for AlgoSimple {
+    fn from(algo: &str) -> Self {
+        use AlgoSimple::*;
+
+        match algo {
+            "RSA2k/17" => RSA2k(17),
+            "RSA2k/32" => RSA2k(32),
+            "RSA3k/17" => RSA3k(17),
+            "RSA3k/32" => RSA3k(32),
+            "RSA4k/17" => RSA4k(17),
+            "RSA4k/32" => RSA4k(32),
+            "NIST256" => NIST256,
+            "NIST384" => NIST384,
+            "NIST521" => NIST521,
+            "Curve25519" => Curve25519,
+            _ => panic!("unexpected algo {}", algo),
+        }
+    }
+}
+
+impl AlgoSimple {
+    pub(crate) fn to_algo(&self, kt: KeyType) -> Algo {
+        let et = match kt {
+            KeyType::Signing | KeyType::Authentication => EccType::ECDSA,
+            KeyType::Decryption => EccType::ECDH,
+            _ => unimplemented!(),
+        };
+
+        match self {
+            Self::RSA1k(e) => Algo::Rsa(RsaAttrs {
+                len_n: 1024,
+                len_e: *e,
+                import_format: 0,
+            }),
+            Self::RSA2k(e) => Algo::Rsa(RsaAttrs {
+                len_n: 2048,
+                len_e: *e,
+                import_format: 0,
+            }),
+            Self::RSA3k(e) => Algo::Rsa(RsaAttrs {
+                len_n: 3072,
+                len_e: *e,
+                import_format: 0,
+            }),
+            Self::RSA4k(e) => Algo::Rsa(RsaAttrs {
+                len_n: 4096,
+                len_e: *e,
+                import_format: 0,
+            }),
+            Self::NIST256 => Algo::Ecc(EccAttrs {
+                curve: Curve::NistP256r1,
+                ecc_type: et,
+                import_format: None,
+            }),
+            Self::NIST384 => Algo::Ecc(EccAttrs {
+                curve: Curve::NistP384r1,
+                ecc_type: et,
+                import_format: None,
+            }),
+            Self::NIST521 => Algo::Ecc(EccAttrs {
+                curve: Curve::NistP521r1,
+                ecc_type: et,
+                import_format: None,
+            }),
+            Self::Curve25519 => Algo::Ecc(EccAttrs {
+                curve: match kt {
+                    KeyType::Signing | KeyType::Authentication => {
+                        Curve::Ed25519
+                    }
+                    KeyType::Decryption => Curve::Cv25519,
+                    _ => unimplemented!(),
+                },
+                ecc_type: match kt {
+                    KeyType::Signing | KeyType::Authentication => {
+                        EccType::EdDSA
+                    }
+                    KeyType::Decryption => EccType::ECDH,
+                    _ => unimplemented!(),
+                },
+                import_format: None,
+            }),
+        }
+    }
+}
+
+#[derive(Debug, Clone, Eq, PartialEq)]
+pub struct AlgoInfo(pub(crate) Vec<(KeyType, Algo)>);
+
+#[derive(Debug, Clone, Eq, PartialEq)]
+pub enum Algo {
+    Rsa(RsaAttrs),
+    Ecc(EccAttrs),
+    Unknown(Vec<u8>),
+}
+
+impl fmt::Display for Algo {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            Self::Rsa(rsa) => {
+                write!(f, "RSA {}, {} ", rsa.len_n, rsa.len_e)
+            }
+            Self::Ecc(ecc) => {
+                write!(f, "{:?} ({:?})", ecc.curve, ecc.ecc_type)
+            }
+            Self::Unknown(u) => {
+                write!(f, "Unknown: {:?}", u)
+            }
+        }
+    }
+}
+
+#[derive(Debug, Clone, Eq, PartialEq)]
+pub struct RsaAttrs {
+    pub len_n: u16,
+    pub len_e: u16,
+    pub import_format: u8,
+}
+
+#[derive(Debug, Clone, Eq, PartialEq)]
+pub struct EccAttrs {
+    pub ecc_type: EccType,
+    pub curve: Curve,
+    pub import_format: Option<u8>,
+}
+
+impl EccAttrs {
+    pub fn new(
+        ecc_type: EccType,
+        curve: Curve,
+        import_format: Option<u8>,
+    ) -> Self {
+        Self {
+            ecc_type,
+            curve,
+            import_format,
+        }
+    }
+
+    pub fn oid(&self) -> &[u8] {
+        self.curve.oid()
+    }
+}
+
+#[derive(Debug, Clone, Copy, Eq, PartialEq)]
+pub enum Curve {
+    NistP256r1,
+    NistP384r1,
+    NistP521r1,
+    BrainpoolP256r1,
+    BrainpoolP384r1,
+    BrainpoolP512r1,
+    Secp256k1,
+    Ed25519,
+    Cv25519,
+    Ed448,
+    X448,
+}
+
+impl Curve {
+    pub fn oid(&self) -> &[u8] {
+        use Curve::*;
+        match self {
+            NistP256r1 => &[0x2A, 0x86, 0x48, 0xCE, 0x3D, 0x03, 0x01, 0x07],
+            NistP384r1 => &[0x2B, 0x81, 0x04, 0x00, 0x22],
+            NistP521r1 => &[0x2B, 0x81, 0x04, 0x00, 0x23],
+            BrainpoolP256r1 => {
+                &[0x2B, 0x24, 0x03, 0x03, 0x02, 0x08, 0x01, 0x01, 0x07]
+            }
+            BrainpoolP384r1 => {
+                &[0x2B, 0x24, 0x03, 0x03, 0x02, 0x08, 0x01, 0x01, 0x0b]
+            }
+            BrainpoolP512r1 => {
+                &[0x2B, 0x24, 0x03, 0x03, 0x02, 0x08, 0x01, 0x01, 0x0d]
+            }
+            Secp256k1 => &[0x2B, 0x81, 0x04, 0x00, 0x0A],
+            Ed25519 => &[0x2B, 0x06, 0x01, 0x04, 0x01, 0xDA, 0x47, 0x0F, 0x01],
+            Cv25519 => {
+                &[0x2b, 0x06, 0x01, 0x04, 0x01, 0x97, 0x55, 0x01, 0x05, 0x01]
+            }
+            Ed448 => &[0x2b, 0x65, 0x71],
+            X448 => &[0x2b, 0x65, 0x6f],
+        }
+    }
+}
+
+impl TryFrom<&[u8]> for Curve {
+    type Error = anyhow::Error;
+
+    fn try_from(oid: &[u8]) -> Result<Self, Self::Error> {
+        use Curve::*;
+
+        let curve = match oid {
+            [0x2A, 0x86, 0x48, 0xCE, 0x3D, 0x03, 0x01, 0x07] => NistP256r1,
+            [0x2B, 0x81, 0x04, 0x00, 0x22] => NistP384r1,
+            [0x2B, 0x81, 0x04, 0x00, 0x23] => NistP521r1,
+
+            [0x2B, 0x24, 0x03, 0x03, 0x02, 0x08, 0x01, 0x01, 0x07] => {
+                BrainpoolP256r1
+            }
+            [0x2B, 0x24, 0x03, 0x03, 0x02, 0x08, 0x01, 0x01, 0x0b] => {
+                BrainpoolP384r1
+            }
+            [0x2B, 0x24, 0x03, 0x03, 0x02, 0x08, 0x01, 0x01, 0x0d] => {
+                BrainpoolP512r1
+            }
+
+            [0x2B, 0x81, 0x04, 0x00, 0x0A] => Secp256k1,
+
+            [0x2B, 0x06, 0x01, 0x04, 0x01, 0xDA, 0x47, 0x0F, 0x01] => Ed25519,
+            [0x2b, 0x06, 0x01, 0x04, 0x01, 0x97, 0x55, 0x01, 0x05, 0x01] => {
+                Cv25519
+            }
+
+            [0x2b, 0x65, 0x71] => Ed448,
+            [0x2b, 0x65, 0x6f] => X448,
+
+            _ => return Err(anyhow!("Unknown curve OID {:?}", oid)),
+        };
+
+        Ok(curve)
+    }
+}

openpgp-card/src/card_app.rs

@@ -15,16 +15,16 @@ use std::time::SystemTime;
 
 use anyhow::{anyhow, Result};
 
+use crate::algorithm::{Algo, AlgoInfo, AlgoSimple, RsaAttrs};
 use crate::apdu::{commands, response::Response};
 use crate::errors::OpenpgpCardError;
 use crate::parse::{fingerprint, key_generation_times};
 use crate::tlv::{tag::Tag, Tlv, TlvEntry};
 use crate::{
-    apdu, keys, Algo, AlgoInfo, AlgoSimple, ApplicationId, CardCaps,
+    apdu, keys, ApplicationId, CardCaps, CardClientBox, CardUploadableKey,
-    CardClientBox, CardHolder, CardUploadableKey, DecryptMe, EccType,
+    Cardholder, DecryptMe, EccType, ExtendedCap, ExtendedLengthInfo,
-    ExtendedCap, ExtendedLengthInfo, Fingerprint, Hash, Historical,
+    Fingerprint, Hash, Historical, KeyGeneration, KeySet, KeyType, PWStatus,
-    KeyGeneration, KeySet, KeyType, PWStatus, PublicKeyMaterial, RsaAttrs,
+    PublicKeyMaterial, Sex,
-    Sex,
 };
 
 pub struct ARD(Tlv);
@@ -281,12 +281,12 @@ impl CardApp {
     }
 
     // --- cardholder related data (65) ---
-    pub fn get_cardholder_related_data(&mut self) -> Result<CardHolder> {
+    pub fn get_cardholder_related_data(&mut self) -> Result<Cardholder> {
         let crd = commands::cardholder_related_data();
         let resp = apdu::send_command(&mut self.card_client, crd, true)?;
         resp.check_ok()?;
 
-        CardHolder::try_from(resp.data()?)
+        Cardholder::try_from(resp.data()?)
     }
 
     // --- security support template (7a) ---
@@ -446,7 +446,7 @@ impl CardApp {
 
     // --- sign ---
 
-    /// Sign the message in `hash`, on the card.
+    /// Sign `hash`, on the card.
     pub fn signature_for_hash(
         &mut self,
         hash: Hash,
@@ -515,7 +515,7 @@ impl CardApp {
     }
 
     pub fn set_sex(&mut self, sex: Sex) -> Result<Response, OpenpgpCardError> {
-        let put_sex = commands::put_sex(sex.as_u8());
+        let put_sex = commands::put_sex((&sex).into());
         apdu::send_command(self.card_client.borrow_mut(), put_sex, false)
     }
 
@@ -623,6 +623,8 @@ impl CardApp {
         algo_attributes
     }
 
+    /// Upload an existing private key to the card.
+    /// (This implicitly sets the algorithm info, fingerprint and timestamp)
     pub fn upload_key(
         &mut self,
         key: Box<dyn CardUploadableKey>,
@@ -666,7 +668,7 @@ impl CardApp {
         key_type: KeyType,
         algo: AlgoSimple,
     ) -> Result<(PublicKeyMaterial, u32), OpenpgpCardError> {
-        let algo = algo.get(key_type);
+        let algo = algo.to_algo(key_type);
         self.generate_key(fp_from_pub, key_type, Some(&algo))
     }
 

openpgp-card/src/keys.rs

@@ -4,17 +4,18 @@
 //! Generate and import keys
 
 use anyhow::{anyhow, Result};
+use std::convert::TryFrom;
 use std::time::{SystemTime, UNIX_EPOCH};
 
+use crate::algorithm::{Algo, AlgoInfo, Curve, EccAttrs, RsaAttrs};
 use crate::apdu::command::Command;
 use crate::apdu::commands;
 use crate::card_app::CardApp;
 use crate::errors::OpenpgpCardError;
 use crate::tlv::{tag::Tag, Tlv, TlvEntry};
-use crate::{apdu, Curve, EccPub, PublicKeyMaterial, RSAPub};
 use crate::{
-    tlv, Algo, AlgoInfo, CardUploadableKey, EccAttrs, EccKey, KeyType,
+    apdu, tlv, CardUploadableKey, EccKey, EccPub, KeyType, PrivateKeyMaterial,
-    PrivateKeyMaterial, RSAKey, RsaAttrs,
+    PublicKeyMaterial, RSAKey, RSAPub,
 };
 
 /// `gen_key_with_metadata` calculates the fingerprint for a public key
@@ -198,7 +199,7 @@ pub(crate) fn upload_key(
 
             let algo = Algo::Ecc(EccAttrs {
                 ecc_type: ecc_key.get_type(),
-                curve: Curve::from(ecc_key.get_oid()).expect("unepected oid"),
+                curve: Curve::try_from(ecc_key.get_oid())?,
                 import_format: None,
             });
 

openpgp-card/src/lib.rs

@@ -1,10 +1,11 @@
 // SPDX-FileCopyrightText: 2021 Heiko Schaefer <heiko@schaefer.name>
 // SPDX-License-Identifier: MIT OR Apache-2.0
 
+use crate::algorithm::Algo;
 use anyhow::Result;
 use std::collections::HashSet;
-use std::fmt;
 
+pub mod algorithm;
 pub mod apdu;
 pub mod card_app;
 pub mod errors;
@@ -12,14 +13,31 @@ mod keys;
 mod parse;
 mod tlv;
 
+/// The CardClient trait defines communication with an OpenPGP card via a
+/// backend implementation (e.g. the pcsc backend in the crate
+/// openpgp-card-pcsc).
 pub trait CardClient {
+    /// Transmit the command data in `cmd` to the card.
+    ///
+    /// `buf_size` is a hint to the backend (the backend may ignore it)
+    /// indicating the expected maximum response size.
     fn transmit(&mut self, cmd: &[u8], buf_size: usize) -> Result<Vec<u8>>;
+
+    /// Set the card capabilities in the CardClient.
+    ///
+    /// Setting these capabilities is typically part of a bootstrapping
+    /// process: the information about the card's capabilities is typically
+    /// requested from the card using the same CardClient instance, before
+    /// the card's capabilities have been initialized.
     fn init_caps(&mut self, caps: CardCaps);
+
+    /// Request the card's capabilities - apdu serialization makes use of
+    /// this information (e.g. to determine if extended length can be used)
     fn get_caps(&self) -> Option<&CardCaps>;
 
-    /// If a CardClient implementation introduces an inherent limit for
+    /// If a CardClient implementation introduces an additional,
-    /// maximum number of bytes per command, this fn can indicate that
+    /// backend-specific limit for maximum number of bytes per command,
-    /// limit by returning `Some(max_cmd_len)`.
+    /// this fn can indicate that limit by returning `Some(max_cmd_len)`.
     fn max_cmd_len(&self) -> Option<usize> {
         None
     }
@@ -28,12 +46,21 @@ pub trait CardClient {
 pub type CardClientBox = Box<dyn CardClient + Send + Sync>;
 
 /// Information about the capabilities of the card.
-/// (feature configuration from card metadata)
+///
+/// (This configuration is retrieved from card metadata, specifically from
+/// "Card Capabilities" and "Extended length information")
 #[derive(Clone, Copy, Debug)]
 pub struct CardCaps {
+    /// Extended Lc and Le fields
     pub ext_support: bool,
+
+    /// Command chaining
     pub chaining_support: bool,
+
+    /// Maximum number of bytes in a command APDU
     pub max_cmd_bytes: u16,
+
+    /// Maximum number of bytes in a response APDU
     pub max_rsp_bytes: u16,
 }
 
@@ -53,246 +80,6 @@ impl CardCaps {
     }
 }
 
-/// Algorithms for key generation.
-///
-/// RSA variants require "number of bits in 'e'" as parameter.
-///
-/// There are (at least) two common supported values for e:
-///  e=17 [YK4, YK5]
-///  e=32 [YK5, Floss3.4, Gnuk1.2]
-#[derive(Clone, Copy, Debug)]
-pub enum AlgoSimple {
-    RSA1k(u16),
-    RSA2k(u16),
-    RSA3k(u16),
-    RSA4k(u16),
-    NIST256,
-    NIST384,
-    NIST521,
-    Curve25519,
-}
-
-impl From<&str> for AlgoSimple {
-    fn from(algo: &str) -> Self {
-        use AlgoSimple::*;
-
-        match algo {
-            "RSA2k/17" => RSA2k(17),
-            "RSA2k/32" => RSA2k(32),
-            "RSA3k/17" => RSA3k(17),
-            "RSA3k/32" => RSA3k(32),
-            "RSA4k/17" => RSA4k(17),
-            "RSA4k/32" => RSA4k(32),
-            "NIST256" => NIST256,
-            "NIST384" => NIST384,
-            "NIST521" => NIST521,
-            "Curve25519" => Curve25519,
-            _ => panic!("unexpected algo {}", algo),
-        }
-    }
-}
-
-impl AlgoSimple {
-    fn get(&self, kt: KeyType) -> Algo {
-        let et = match kt {
-            KeyType::Signing | KeyType::Authentication => EccType::ECDSA,
-            KeyType::Decryption => EccType::ECDH,
-            _ => unimplemented!(),
-        };
-
-        match self {
-            Self::RSA1k(e) => Algo::Rsa(RsaAttrs {
-                len_n: 1024,
-                len_e: *e,
-                import_format: 0,
-            }),
-            Self::RSA2k(e) => Algo::Rsa(RsaAttrs {
-                len_n: 2048,
-                len_e: *e,
-                import_format: 0,
-            }),
-            Self::RSA3k(e) => Algo::Rsa(RsaAttrs {
-                len_n: 3072,
-                len_e: *e,
-                import_format: 0,
-            }),
-            Self::RSA4k(e) => Algo::Rsa(RsaAttrs {
-                len_n: 4096,
-                len_e: *e,
-                import_format: 0,
-            }),
-            Self::NIST256 => Algo::Ecc(EccAttrs {
-                curve: Curve::NistP256r1,
-                ecc_type: et,
-                import_format: None,
-            }),
-            Self::NIST384 => Algo::Ecc(EccAttrs {
-                curve: Curve::NistP384r1,
-                ecc_type: et,
-                import_format: None,
-            }),
-            Self::NIST521 => Algo::Ecc(EccAttrs {
-                curve: Curve::NistP521r1,
-                ecc_type: et,
-                import_format: None,
-            }),
-            Self::Curve25519 => Algo::Ecc(EccAttrs {
-                curve: match kt {
-                    KeyType::Signing | KeyType::Authentication => {
-                        Curve::Ed25519
-                    }
-                    KeyType::Decryption => Curve::Cv25519,
-                    _ => unimplemented!(),
-                },
-                ecc_type: match kt {
-                    KeyType::Signing | KeyType::Authentication => {
-                        EccType::EdDSA
-                    }
-                    KeyType::Decryption => EccType::ECDH,
-                    _ => unimplemented!(),
-                },
-                import_format: None,
-            }),
-        }
-    }
-}
-
-#[derive(Debug, Clone, Eq, PartialEq)]
-pub struct AlgoInfo(Vec<(KeyType, Algo)>);
-
-#[derive(Debug, Clone, Eq, PartialEq)]
-pub enum Algo {
-    Rsa(RsaAttrs),
-    Ecc(EccAttrs),
-    Unknown(Vec<u8>),
-}
-
-impl fmt::Display for Algo {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        match self {
-            Self::Rsa(rsa) => {
-                write!(f, "RSA {}, {} ", rsa.len_n, rsa.len_e)
-            }
-            Self::Ecc(ecc) => {
-                write!(f, "{:?} ({:?})", ecc.curve, ecc.ecc_type)
-            }
-            Self::Unknown(u) => {
-                write!(f, "Unknown: {:?}", u)
-            }
-        }
-    }
-}
-
-#[derive(Debug, Clone, Eq, PartialEq)]
-pub struct RsaAttrs {
-    pub len_n: u16,
-    pub len_e: u16,
-    pub import_format: u8,
-}
-
-#[derive(Debug, Clone, Eq, PartialEq)]
-pub struct EccAttrs {
-    pub ecc_type: EccType,
-    pub curve: Curve,
-    pub import_format: Option<u8>,
-}
-
-impl EccAttrs {
-    pub fn new(
-        ecc_type: EccType,
-        curve: Curve,
-        import_format: Option<u8>,
-    ) -> Self {
-        Self {
-            ecc_type,
-            curve,
-            import_format,
-        }
-    }
-
-    pub fn oid(&self) -> &[u8] {
-        self.curve.oid()
-    }
-}
-
-#[derive(Debug, Clone, Copy, Eq, PartialEq)]
-pub enum Curve {
-    NistP256r1,
-    NistP384r1,
-    NistP521r1,
-    BrainpoolP256r1,
-    BrainpoolP384r1,
-    BrainpoolP512r1,
-    Secp256k1,
-    Ed25519,
-    Cv25519,
-    Ed448,
-    X448,
-}
-
-impl Curve {
-    pub fn oid(&self) -> &[u8] {
-        use Curve::*;
-        match self {
-            NistP256r1 => &[0x2A, 0x86, 0x48, 0xCE, 0x3D, 0x03, 0x01, 0x07],
-            NistP384r1 => &[0x2B, 0x81, 0x04, 0x00, 0x22],
-            NistP521r1 => &[0x2B, 0x81, 0x04, 0x00, 0x23],
-            BrainpoolP256r1 => {
-                &[0x2B, 0x24, 0x03, 0x03, 0x02, 0x08, 0x01, 0x01, 0x07]
-            }
-            BrainpoolP384r1 => {
-                &[0x2B, 0x24, 0x03, 0x03, 0x02, 0x08, 0x01, 0x01, 0x0b]
-            }
-            BrainpoolP512r1 => {
-                &[0x2B, 0x24, 0x03, 0x03, 0x02, 0x08, 0x01, 0x01, 0x0d]
-            }
-            Secp256k1 => &[0x2B, 0x81, 0x04, 0x00, 0x0A],
-            Ed25519 => &[0x2B, 0x06, 0x01, 0x04, 0x01, 0xDA, 0x47, 0x0F, 0x01],
-            Cv25519 => {
-                &[0x2b, 0x06, 0x01, 0x04, 0x01, 0x97, 0x55, 0x01, 0x05, 0x01]
-            }
-            Ed448 => &[0x2b, 0x65, 0x71],
-            X448 => &[0x2b, 0x65, 0x6f],
-        }
-    }
-
-    // FIXME impl trait?
-    pub fn from(oid: &[u8]) -> Option<Self> {
-        use Curve::*;
-        match oid {
-            [0x2A, 0x86, 0x48, 0xCE, 0x3D, 0x03, 0x01, 0x07] => {
-                Some(NistP256r1)
-            }
-            [0x2B, 0x81, 0x04, 0x00, 0x22] => Some(NistP384r1),
-            [0x2B, 0x81, 0x04, 0x00, 0x23] => Some(NistP521r1),
-
-            [0x2B, 0x24, 0x03, 0x03, 0x02, 0x08, 0x01, 0x01, 0x07] => {
-                Some(BrainpoolP256r1)
-            }
-            [0x2B, 0x24, 0x03, 0x03, 0x02, 0x08, 0x01, 0x01, 0x0b] => {
-                Some(BrainpoolP384r1)
-            }
-            [0x2B, 0x24, 0x03, 0x03, 0x02, 0x08, 0x01, 0x01, 0x0d] => {
-                Some(BrainpoolP512r1)
-            }
-
-            [0x2B, 0x81, 0x04, 0x00, 0x0A] => Some(Secp256k1),
-
-            [0x2B, 0x06, 0x01, 0x04, 0x01, 0xDA, 0x47, 0x0F, 0x01] => {
-                Some(Ed25519)
-            }
-            [0x2b, 0x06, 0x01, 0x04, 0x01, 0x97, 0x55, 0x01, 0x05, 0x01] => {
-                Some(Cv25519)
-            }
-
-            [0x2b, 0x65, 0x71] => Some(Ed448),
-            [0x2b, 0x65, 0x6f] => Some(X448),
-
-            _ => None,
-        }
-    }
-}
-
 /// An OpenPGP key generation Time
 #[derive(Clone, Eq, PartialEq, Debug)]
 pub struct KeyGeneration(u32);
@@ -354,6 +141,30 @@ pub trait CardUploadableKey {
     fn get_fp(&self) -> [u8; 20];
 }
 
+/// Algorithm-independent container for private key material to upload to
+/// an OpenPGP card
+pub enum PrivateKeyMaterial {
+    R(Box<dyn RSAKey>),
+    E(Box<dyn EccKey>),
+}
+
+/// RSA-specific container for private key material to upload to an OpenPGP
+/// card.
+pub trait RSAKey {
+    fn get_e(&self) -> &[u8];
+    fn get_n(&self) -> &[u8];
+    fn get_p(&self) -> &[u8];
+    fn get_q(&self) -> &[u8];
+}
+
+/// ECC-specific container for private key material to upload to an OpenPGP
+/// card.
+pub trait EccKey {
+    fn get_oid(&self) -> &[u8];
+    fn get_scalar(&self) -> &[u8];
+    fn get_type(&self) -> EccType;
+}
+
 /// Algorithm-independent container for public key material retrieved from
 /// an OpenPGP card
 #[derive(Debug)]
@@ -379,30 +190,6 @@ pub struct EccPub {
     pub algo: Algo,
 }
 
-/// Algorithm-independent container for private key material to upload to
-/// an OpenPGP card
-pub enum PrivateKeyMaterial {
-    R(Box<dyn RSAKey>),
-    E(Box<dyn EccKey>),
-}
-
-/// RSA-specific container for private key material to upload to an OpenPGP
-/// card.
-pub trait RSAKey {
-    fn get_e(&self) -> &[u8];
-    fn get_n(&self) -> &[u8];
-    fn get_p(&self) -> &[u8];
-    fn get_q(&self) -> &[u8];
-}
-
-/// ECC-specific container for private key material to upload to an OpenPGP
-/// card.
-pub trait EccKey {
-    fn get_oid(&self) -> &[u8];
-    fn get_scalar(&self) -> &[u8];
-    fn get_type(&self) -> EccType;
-}
-
 /// A marker to distinguish between elliptic curve algorithms (ECDH, ECDSA,
 /// EdDSA)
 #[derive(PartialEq, Eq, Debug, Clone, Copy)]
@@ -423,6 +210,7 @@ pub enum DecryptMe<'a> {
 
 // ----------
 
+/// Application identifier (AID)
 #[derive(Debug, Eq, PartialEq)]
 pub struct ApplicationId {
     pub application: u8,
@@ -440,6 +228,7 @@ pub struct ApplicationId {
     pub serial: u32,
 }
 
+/// Card Capabilities (73)
 #[derive(Debug)]
 pub struct CardCapabilities {
     command_chaining: bool,
@@ -447,8 +236,9 @@ pub struct CardCapabilities {
     extended_length_information: bool,
 }
 
+/// Card service data (31)
 #[derive(Debug)]
-pub struct CardSeviceData {
+pub struct CardServiceData {
     select_by_full_df_name: bool,
     select_by_partial_df_name: bool,
     dos_available_in_ef_dir: bool,
@@ -457,21 +247,23 @@ pub struct CardSeviceData {
     mf: bool,
 }
 
+/// Historical Bytes
 #[derive(Debug)]
 pub struct Historical {
-    // category indicator byte
+    /// category indicator byte
     cib: u8,
 
-    // Card service data (31)
+    /// Card service data (31)
-    csd: Option<CardSeviceData>,
+    csd: Option<CardServiceData>,
 
-    // Card Capabilities (73)
+    /// Card Capabilities (73)
     cc: Option<CardCapabilities>,
 
-    // status indicator byte (o-card 3.4.1, pg 44)
+    /// status indicator byte (o-card 3.4.1, pg 44)
     sib: u8,
 }
 
+/// Extended Capabilities
 #[derive(Debug, Eq, PartialEq)]
 pub struct ExtendedCap {
     pub features: HashSet<Features>,
@@ -483,6 +275,7 @@ pub struct ExtendedCap {
     mse_command: bool,
 }
 
+/// Features (first byte of Extended Capabilities)
 #[derive(Copy, Clone, Debug, Eq, PartialEq, Hash)]
 pub enum Features {
     SecureMessaging,
@@ -495,19 +288,22 @@ pub enum Features {
     KdfDo,
 }
 
+/// Extended length information
 #[derive(Debug, Eq, PartialEq)]
 pub struct ExtendedLengthInfo {
     pub max_command_bytes: u16,
     pub max_response_bytes: u16,
 }
 
+/// Cardholder Related Data
 #[derive(Debug)]
-pub struct CardHolder {
+pub struct Cardholder {
     pub name: Option<String>,
     pub lang: Option<Vec<[char; 2]>>,
     pub sex: Option<Sex>,
 }
 
+/// Sex (according to ISO 5218)
 #[derive(Debug, PartialEq)]
 pub enum Sex {
     NotKnown,
@@ -516,9 +312,9 @@ pub enum Sex {
     NotApplicable,
 }
 
-impl Sex {
+impl From<&Sex> for u8 {
-    pub fn as_u8(&self) -> u8 {
+    fn from(sex: &Sex) -> u8 {
-        match self {
+        match sex {
             Sex::NotKnown => 0x30,
             Sex::Male => 0x31,
             Sex::Female => 0x32,
@@ -538,6 +334,7 @@ impl From<u8> for Sex {
     }
 }
 
+/// PW status Bytes
 #[derive(Debug)]
 pub struct PWStatus {
     pub(crate) pw1_cds_multi: bool,
@@ -554,6 +351,7 @@ pub struct PWStatus {
 #[derive(Clone, Eq, PartialEq)]
 pub struct Fingerprint([u8; 20]);
 
+/// A KeySet binds together a triple of information about each Key on a card
 #[derive(Clone, Debug, Eq, PartialEq)]
 pub struct KeySet<T> {
     signature: Option<T>,
@@ -561,19 +359,12 @@ pub struct KeySet<T> {
     authentication: Option<T>,
 }
 
-/// Enum to identify one of the Key-slots on an OpenPGP card
+/// Enum to identify the Key-slots on an OpenPGP card
 #[derive(Debug, Clone, Copy, Eq, PartialEq)]
 pub enum KeyType {
-    // Algorithm attributes signature (C1)
     Signing,
-
-    // Algorithm attributes decryption (C2)
     Decryption,
-
-    // Algorithm attributes authentication (C3)
     Authentication,
-
-    // Algorithm attributes Attestation key (DA, Yubico)
     Attestation,
 }
 

openpgp-card/src/parse/algo_attrs.rs

@@ -9,7 +9,8 @@ use nom::bytes::complete::tag;
 use nom::combinator::map;
 use nom::{branch, bytes::complete as bytes, number::complete as number};
 
-use crate::{parse, Algo, Curve, EccAttrs, EccType, RsaAttrs};
+use crate::algorithm::{Algo, Curve, EccAttrs, RsaAttrs};
+use crate::{parse, EccType};
 
 fn parse_oid_cv25519(input: &[u8]) -> nom::IResult<&[u8], Curve> {
     map(tag(Curve::Cv25519.oid()), |_| Curve::Cv25519)(input)

openpgp-card/src/parse/algo_info.rs

@@ -9,8 +9,9 @@ use nom::combinator::map;
 use nom::{branch, bytes::complete as bytes, combinator, multi, sequence};
 use std::fmt;
 
+use crate::algorithm::AlgoInfo;
 use crate::parse::algo_attrs;
-use crate::{Algo, AlgoInfo, KeyType};
+use crate::{Algo, KeyType};
 
 impl AlgoInfo {
     pub fn get_by_keytype(&self, kt: KeyType) -> Vec<&Algo> {
@@ -100,10 +101,9 @@ impl TryFrom<&[u8]> for AlgoInfo {
 mod test {
     use std::convert::TryFrom;
 
+    use crate::algorithm::{Algo::*, Curve::*, EccAttrs, RsaAttrs};
     use crate::parse::algo_info::AlgoInfo;
-    use crate::{
+    use crate::{EccType::*, KeyType::*};
-        Algo::*, Curve::*, EccAttrs, EccType::*, KeyType::*, RsaAttrs,
-    };
 
     #[test]
     fn test_gnuk() {

openpgp-card/src/parse/cardholder.rs

@@ -7,9 +7,9 @@ use anyhow::Result;
 
 use crate::tlv::tag::Tag;
 use crate::tlv::{Tlv, TlvEntry};
-use crate::{CardHolder, Sex};
+use crate::{Cardholder, Sex};
 
-impl TryFrom<&[u8]> for CardHolder {
+impl TryFrom<&[u8]> for Cardholder {
     type Error = anyhow::Error;
 
     fn try_from(data: &[u8]) -> Result<Self> {
@@ -34,6 +34,6 @@ impl TryFrom<&[u8]> for CardHolder {
             .filter(|v| v.len() == 1)
             .map(|v| Sex::from(v[0]));
 
-        Ok(CardHolder { name, lang, sex })
+        Ok(Cardholder { name, lang, sex })
     }
 }

openpgp-card/src/parse/historical.rs

@@ -2,7 +2,7 @@
 // SPDX-License-Identifier: MIT OR Apache-2.0
 
 use crate::errors::OpenpgpCardError;
-use crate::{CardCapabilities, CardSeviceData, Historical};
+use crate::{CardCapabilities, CardServiceData, Historical};
 use anyhow::{anyhow, Result};
 
 impl CardCapabilities {
@@ -33,7 +33,7 @@ impl CardCapabilities {
     }
 }
 
-impl CardSeviceData {
+impl CardServiceData {
     pub fn from(data: u8) -> Self {
         let select_by_full_df_name = data & 0x80 != 0;
         let select_by_partial_df_name = data & 0x40 != 0;
@@ -120,7 +120,7 @@ impl Historical {
             // It's unclear if these status bytes are ever useful to process.
 
             let cc = cc.map(CardCapabilities::from);
-            let csd = csd.map(CardSeviceData::from);
+            let csd = csd.map(CardServiceData::from);
 
             Ok(Self { cib, csd, cc, sib })
         } else {