Make fields private, add accessor methods.

card-functionality/src/tests.rs

@@ -2,6 +2,7 @@
 // SPDX-License-Identifier: MIT OR Apache-2.0
 
 use anyhow::{Error, Result};
+use std::convert::TryInto;
 use std::str::FromStr;
 use std::string::FromUtf8Error;
 use thiserror::Error;
@@ -345,9 +346,12 @@ pub fn test_set_user_data(
     // read all the fields back again, expect equal data
     let ch = ca.get_cardholder_related_data()?;
 
-    assert_eq!(ch.name, Some("Bar<<Foo".to_string()));
+    assert_eq!(ch.name(), Some("Bar<<Foo"));
-    assert_eq!(ch.lang, Some(vec![['d', 'e'], ['e', 'n']]));
+    assert_eq!(
-    assert_eq!(ch.sex, Some(Sex::Female));
+        ch.lang().expect("Language setting is None"),
+        &[['d', 'e'], ['e', 'n']]
+    );
+    assert_eq!(ch.sex(), Some(Sex::Female));
 
     let url = ca.get_url()?;
     assert_eq!(url, "https://duckduckgo.com/".to_string());
@@ -518,7 +522,7 @@ pub fn test_verify(
     ca.set_name("Admin<<Hello")?;
 
     let cardholder = ca.get_cardholder_related_data()?;
-    assert_eq!(cardholder.name, Some("Admin<<Hello".to_string()));
+    assert_eq!(cardholder.name(), Some("Admin<<Hello"));
 
     ca.verify_pw1("123456")?;
 
@@ -536,7 +540,7 @@ pub fn test_verify(
     ca.set_name("There<<Hello")?;
 
     let cardholder = ca.get_cardholder_related_data()?;
-    assert_eq!(cardholder.name, Some("There<<Hello".to_string()));
+    assert_eq!(cardholder.name(), Some("There<<Hello"));
 
     Ok(out)
 }

openpgp-card-sequoia/Cargo.toml

@@ -14,7 +14,7 @@ documentation = "https://docs.rs/crate/openpgp-card-sequoia"
 [dependencies]
 sequoia-openpgp = "1.3"
 openpgp-card = { path = "../openpgp-card", version = "0.0.3" }
-#openpgp-card-scdc = { path = "../scdc" }
+openpgp-card-scdc = { path = "../scdc", version = "0.0.1" }
 openpgp-card-pcsc = { path = "../pcsc", version = "0.0.2" }
 chrono = "0.4"
 anyhow = "1"

openpgp-card-sequoia/src/lib.rs

@@ -1,8 +1,8 @@
 // SPDX-FileCopyrightText: 2021 Heiko Schaefer <heiko@schaefer.name>
 // SPDX-License-Identifier: MIT OR Apache-2.0
 
-//! This library supports using openpgp-card functionality with
+//! This is a higher-level wrapper around the openpgp-card crate.
-//! sequoia_openpgp data structures.
+//! It uses sequoia_openpgp for OpenPGP operations.
 
 use anyhow::{anyhow, Context, Result};
 use std::convert::TryFrom;
@@ -96,14 +96,14 @@ pub fn public_key_material_to_key(
 ) -> Result<Key<PublicParts, UnspecifiedRole>> {
     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))?;
 
             Ok(k4.into())
         }
         PublicKeyMaterial::E(ecc) => {
-            let algo = ecc.algo.clone(); // FIXME?
+            let algo = ecc.algo().clone(); // FIXME?
             if let Algo::Ecc(algo_ecc) = algo {
-                let curve = match algo_ecc.curve {
+                let curve = match algo_ecc.curve() {
                     Curve::NistP256r1 => openpgp::types::Curve::NistP256,
                     Curve::NistP384r1 => openpgp::types::Curve::NistP384,
                     Curve::NistP521r1 => openpgp::types::Curve::NistP521,
@@ -114,10 +114,10 @@ pub fn public_key_material_to_key(
 
                 match key_type {
                     KeyType::Authentication | KeyType::Signing => {
-                        if algo_ecc.curve == Curve::Ed25519 {
+                        if algo_ecc.curve() == Curve::Ed25519 {
                             // EdDSA
                             let k4 =
-                                Key4::import_public_ed25519(&ecc.data, time)?;
+                                Key4::import_public_ed25519(ecc.data(), time)?;
 
                             Ok(Key::from(k4))
                         } else {
@@ -127,7 +127,7 @@ pub fn public_key_material_to_key(
                                 PublicKeyAlgorithm::ECDSA,
                                 mpi::PublicKey::ECDSA {
                                     curve,
-                                    q: mpi::MPI::new(&ecc.data),
+                                    q: mpi::MPI::new(ecc.data()),
                                 },
                             )?;
 
@@ -135,10 +135,13 @@ pub fn public_key_material_to_key(
                         }
                     }
                     KeyType::Decryption => {
-                        if algo_ecc.curve == Curve::Cv25519 {
+                        if algo_ecc.curve() == Curve::Cv25519 {
                             // EdDSA
                             let k4 = Key4::import_public_cv25519(
-                                &ecc.data, None, None, time,
+                                ecc.data(),
+                                None,
+                                None,
+                                time,
                             )?;
 
                             Ok(k4.into())
@@ -152,7 +155,7 @@ pub fn public_key_material_to_key(
                                 PublicKeyAlgorithm::ECDH,
                                 mpi::PublicKey::ECDH {
                                     curve,
-                                    q: mpi::MPI::new(&ecc.data),
+                                    q: mpi::MPI::new(ecc.data()),
                                     hash: Default::default(),
                                     sym: Default::default(),
                                 },
@@ -249,7 +252,8 @@ pub fn make_cert(
     let cardholder = ca.get_cardholder_related_data()?;
 
     // FIXME: process name field? accept email as argument?!
-    let uid: UserID = cardholder.name.expect("expecting name on card").into();
+    let uid: UserID =
+        cardholder.name().expect("expecting name on card").into();
 
     pp.push(uid.clone().into());
 
@@ -690,7 +694,7 @@ impl CardBase {
         // The DO "Algorithm Information" (Tag FA) shall be present if
         // Algorithm attributes can be changed
         let ec = self.get_extended_capabilities()?;
-        if !ec.features.contains(&Features::AlgoAttrsChangeable) {
+        if !ec.features().contains(&Features::AlgoAttrsChangeable) {
             // Algorithm attributes can not be changed,
             // list_supported_algo is not supported
             return Ok(None);
@@ -896,7 +900,7 @@ impl CardAdmin {
         // Check for max len
         let ec = self.get_extended_capabilities()?;
 
-        if url.len() < ec.max_len_special_do as usize {
+        if url.len() < ec.max_len_special_do() as usize {
             self.card_app.set_url(url)
         } else {
             Err(anyhow!("URL too long").into())

openpgp-card/src/algorithm.rs

@@ -57,7 +57,7 @@ impl From<&str> for AlgoSimple {
 }
 
 impl AlgoSimple {
-    pub(crate) fn to_algo(&self, kt: KeyType) -> Algo {
+    pub(crate) fn get_algo(&self, kt: KeyType) -> Algo {
         let et = match kt {
             KeyType::Signing | KeyType::Authentication => EccType::ECDSA,
             KeyType::Decryption => EccType::ECDH,
@@ -165,17 +165,39 @@ impl fmt::Display for Algo {
 /// RSA specific attributes of [`Algo`] ("Algorithm Attributes")
 #[derive(Debug, Clone, Eq, PartialEq)]
 pub struct RsaAttrs {
-    pub len_n: u16,
+    len_n: u16,
-    pub len_e: u16,
+    len_e: u16,
-    pub import_format: u8,
+    import_format: u8,
+}
+
+impl RsaAttrs {
+    pub fn new(len_n: u16, len_e: u16, import_format: u8) -> Self {
+        RsaAttrs {
+            len_n,
+            len_e,
+            import_format,
+        }
+    }
+
+    pub fn len_n(&self) -> u16 {
+        self.len_n
+    }
+
+    pub fn len_e(&self) -> u16 {
+        self.len_e
+    }
+
+    pub fn import_format(&self) -> u8 {
+        self.import_format
+    }
 }
 
 /// ECC specific attributes of [`Algo`] ("Algorithm Attributes")
 #[derive(Debug, Clone, Eq, PartialEq)]
 pub struct EccAttrs {
-    pub ecc_type: EccType,
+    ecc_type: EccType,
-    pub curve: Curve,
+    curve: Curve,
-    pub import_format: Option<u8>,
+    import_format: Option<u8>,
 }
 
 impl EccAttrs {
@@ -191,6 +213,14 @@ impl EccAttrs {
         }
     }
 
+    pub fn ecc_type(&self) -> EccType {
+        self.ecc_type
+    }
+
+    pub fn curve(&self) -> Curve {
+        self.curve
+    }
+
     pub fn oid(&self) -> &[u8] {
         self.curve.oid()
     }

openpgp-card/src/card_app.rs

@@ -64,7 +64,7 @@ impl CardApp {
 
         let (max_cmd_bytes, max_rsp_bytes) =
             if let Ok(Some(eli)) = ard.get_extended_length_information() {
-                (eli.max_command_bytes, eli.max_response_bytes)
+                (eli.max_command_bytes(), eli.max_response_bytes())
             } else {
                 (255, 255)
             };
@@ -548,7 +548,7 @@ impl CardApp {
 
         let data = match algo {
             Algo::Rsa(rsa) => Self::rsa_algo_attrs(rsa)?,
-            Algo::Ecc(ecc) => Self::ecc_algo_attrs(ecc.oid(), ecc.ecc_type),
+            Algo::Ecc(ecc) => Self::ecc_algo_attrs(ecc.oid(), ecc.ecc_type()),
             _ => unimplemented!(),
         };
 
@@ -563,21 +563,21 @@ impl CardApp {
         let mut algo_attributes = vec![0x01];
 
         // Length of modulus n in bit
-        algo_attributes.extend(&algo_attrs.len_n.to_be_bytes());
+        algo_attributes.extend(&algo_attrs.len_n().to_be_bytes());
 
         // Length of public exponent e in bit
         algo_attributes.push(0x00);
-        algo_attributes.push(algo_attrs.len_e as u8);
+        algo_attributes.push(algo_attrs.len_e() as u8);
 
         // Import-Format of private key
         // (This fn currently assumes import_format "00 = standard (e, p, q)")
-        if algo_attrs.import_format != 0 {
+        if algo_attrs.import_format() != 0 {
             return Err(anyhow!(
                 "Unexpected RSA input format (only 0 is supported)"
             ));
         }
 
-        algo_attributes.push(algo_attrs.import_format);
+        algo_attributes.push(algo_attrs.import_format());
 
         Ok(algo_attributes)
     }
@@ -641,7 +641,7 @@ impl CardApp {
         key_type: KeyType,
         algo: AlgoSimple,
     ) -> Result<(PublicKeyMaterial, u32), OpenpgpCardError> {
-        let algo = algo.to_algo(key_type);
+        let algo = algo.get_algo(key_type);
         self.generate_key(fp_from_pub, key_type, Some(&algo))
     }
 

openpgp-card/src/card_do/algo_attrs.rs

@@ -86,14 +86,7 @@ fn parse_rsa(input: &[u8]) -> nom::IResult<&[u8], Algo> {
     let (input, len_e) = number::be_u16(input)?;
     let (input, import_format) = number::u8(input)?;
 
-    Ok((
+    Ok((input, Algo::Rsa(RsaAttrs::new(len_n, len_e, import_format))))
-        input,
-        Algo::Rsa(RsaAttrs {
-            len_n,
-            len_e,
-            import_format,
-        }),
-    ))
 }
 
 fn parse_import_format(input: &[u8]) -> nom::IResult<&[u8], Option<u8>> {

openpgp-card/src/card_do/application_id.rs

@@ -38,14 +38,22 @@ impl TryFrom<&[u8]> for ApplicationId {
 }
 
 impl ApplicationId {
-    pub fn serial(&self) -> u32 {
+    pub fn application(&self) -> u8 {
-        self.serial
+        self.application
+    }
+
+    pub fn version(&self) -> u16 {
+        self.version
     }
 
     pub fn manufacturer(&self) -> u16 {
         self.manufacturer
     }
 
+    pub fn serial(&self) -> u32 {
+        self.serial
+    }
+
     /// This ident is constructed as the concatenation of manufacturer
     /// id, a colon, and the card serial (in hexadecimal representation).
     ///

openpgp-card/src/card_do/cardholder.rs

@@ -9,6 +9,20 @@ use crate::card_do::{Cardholder, Sex};
 use crate::tlv::tag::Tag;
 use crate::tlv::{Tlv, TlvEntry};
 
+impl Cardholder {
+    pub fn name(&self) -> Option<&str> {
+        self.name.as_deref()
+    }
+
+    pub fn lang(&self) -> Option<&[[char; 2]]> {
+        self.lang.as_deref()
+    }
+
+    pub fn sex(&self) -> Option<Sex> {
+        self.sex
+    }
+}
+
 impl TryFrom<&[u8]> for Cardholder {
     type Error = anyhow::Error;
 

openpgp-card/src/card_do/extended_cap.rs

@@ -57,6 +57,16 @@ fn parse(
     )))(input)
 }
 
+impl ExtendedCap {
+    pub fn features(&self) -> HashSet<Features> {
+        self.features.clone()
+    }
+
+    pub fn max_len_special_do(&self) -> u16 {
+        self.max_len_special_do
+    }
+}
+
 impl TryFrom<&[u8]> for ExtendedCap {
     type Error = OpenpgpCardError;
 

openpgp-card/src/card_do/extended_length_info.rs

@@ -19,6 +19,14 @@ fn parse(input: &[u8]) -> nom::IResult<&[u8], (u16, u16)> {
 }
 
 impl ExtendedLengthInfo {
+    pub fn max_command_bytes(&self) -> u16 {
+        self.max_command_bytes
+    }
+
+    pub fn max_response_bytes(&self) -> u16 {
+        self.max_response_bytes
+    }
+
     pub fn from(input: &[u8]) -> Result<Self> {
         let eli = complete(parse(input))?;
 

openpgp-card/src/card_do/key_generation_times.rs

@@ -5,25 +5,25 @@ use anyhow::anyhow;
 use chrono::{DateTime, NaiveDateTime, Utc};
 use nom::{combinator, number::complete as number, sequence};
 
-use crate::card_do::KeyGeneration;
+use crate::card_do::KeyGenerationTime;
 use crate::card_do::KeySet;
 use crate::errors::OpenpgpCardError;
 
-impl From<KeyGeneration> for DateTime<Utc> {
+impl From<KeyGenerationTime> for DateTime<Utc> {
-    fn from(kg: KeyGeneration) -> Self {
+    fn from(kg: KeyGenerationTime) -> Self {
         let naive_datetime = NaiveDateTime::from_timestamp(kg.0 as i64, 0);
 
         DateTime::from_utc(naive_datetime, Utc)
     }
 }
 
-impl From<&KeyGeneration> for u32 {
+impl From<&KeyGenerationTime> for u32 {
-    fn from(kg: &KeyGeneration) -> Self {
+    fn from(kg: &KeyGenerationTime) -> Self {
         kg.0
     }
 }
 
-impl From<u32> for KeyGeneration {
+impl From<u32> for KeyGenerationTime {
     fn from(data: u32) -> Self {
         Self(data)
     }
@@ -33,16 +33,18 @@ fn gen_time(input: &[u8]) -> nom::IResult<&[u8], u32> {
     (number::be_u32)(input)
 }
 
-fn key_generation(input: &[u8]) -> nom::IResult<&[u8], Option<KeyGeneration>> {
+fn key_generation(
+    input: &[u8],
+) -> nom::IResult<&[u8], Option<KeyGenerationTime>> {
     combinator::map(gen_time, |kg| match kg {
         0 => None,
-        kg => Some(KeyGeneration(kg)),
+        kg => Some(KeyGenerationTime(kg)),
     })(input)
 }
 
 fn key_generation_set(
     input: &[u8],
-) -> nom::IResult<&[u8], KeySet<KeyGeneration>> {
+) -> nom::IResult<&[u8], KeySet<KeyGenerationTime>> {
     combinator::into(sequence::tuple((
         key_generation,
         key_generation,
@@ -50,7 +52,9 @@ fn key_generation_set(
     )))(input)
 }
 
-pub fn from(input: &[u8]) -> Result<KeySet<KeyGeneration>, OpenpgpCardError> {
+pub fn from(
+    input: &[u8],
+) -> Result<KeySet<KeyGenerationTime>, OpenpgpCardError> {
     // List of generation dates/times of key pairs, binary.
     // 4 bytes, Big Endian each for Sig, Dec and Aut. Each
     // value shall be seconds since Jan 1, 1970. Default

openpgp-card/src/card_do/mod.rs

@@ -153,7 +153,7 @@ impl ApplicationRelatedData {
     /// Generation dates/times of key pairs
     pub fn get_key_generation_times(
         &self,
-    ) -> Result<KeySet<KeyGeneration>, OpenpgpCardError> {
+    ) -> Result<KeySet<KeyGenerationTime>, OpenpgpCardError> {
         let kg = self.0.find(&Tag::from([0xCD]));
 
         if let Some(kg) = kg {
@@ -183,9 +183,9 @@ impl SecuritySupportTemplate {
 
 /// An OpenPGP key generation Time
 #[derive(Clone, Eq, PartialEq, Debug)]
-pub struct KeyGeneration(u32);
+pub struct KeyGenerationTime(u32);
 
-impl KeyGeneration {
+impl KeyGenerationTime {
     pub fn get(&self) -> u32 {
         self.0
     }
@@ -194,19 +194,10 @@ impl KeyGeneration {
 /// Application identifier (AID)
 #[derive(Debug, Eq, PartialEq)]
 pub struct ApplicationId {
-    pub application: u8,
+    application: u8,
-
+    version: u16,
-    // GnuPG says:
+    manufacturer: u16,
-    // if (app->appversion >= 0x0200)
+    serial: u32,
-    // app->app_local->extcap.is_v2 = 1;
-    //
-    // if (app->appversion >= 0x0300)
-    // app->app_local->extcap.is_v3 = 1;
-    pub version: u16,
-
-    pub manufacturer: u16,
-
-    pub serial: u32,
 }
 
 /// Card Capabilities (73)
@@ -247,11 +238,11 @@ pub struct Historical {
 /// Extended Capabilities
 #[derive(Debug, Eq, PartialEq)]
 pub struct ExtendedCap {
-    pub features: HashSet<Features>,
+    features: HashSet<Features>,
     sm_algo: u8,
     max_len_challenge: u16,
     max_len_cardholder_cert: u16,
-    pub max_len_special_do: u16,
+    max_len_special_do: u16,
     pin_block_2_format_support: bool,
     mse_command_support: bool,
 }
@@ -272,20 +263,20 @@ pub enum Features {
 /// Extended length information
 #[derive(Debug, Eq, PartialEq)]
 pub struct ExtendedLengthInfo {
-    pub max_command_bytes: u16,
+    max_command_bytes: u16,
-    pub max_response_bytes: u16,
+    max_response_bytes: u16,
 }
 
 /// Cardholder Related Data
 #[derive(Debug)]
 pub struct Cardholder {
-    pub name: Option<String>,
+    name: Option<String>,
-    pub lang: Option<Vec<[char; 2]>>,
+    lang: Option<Vec<[char; 2]>>,
-    pub sex: Option<Sex>,
+    sex: Option<Sex>,
 }
 
 /// Sex (according to ISO 5218)
-#[derive(Debug, PartialEq)]
+#[derive(Debug, PartialEq, Clone, Copy)]
 pub enum Sex {
     NotKnown,
     Male,

openpgp-card/src/crypto_data.rs

@@ -108,17 +108,44 @@ pub enum PublicKeyMaterial {
 #[derive(Debug)]
 pub struct RSAPub {
     /// Modulus (a number denoted as n coded on x bytes)
-    pub n: Vec<u8>,
+    n: Vec<u8>,
 
     /// Public exponent (a number denoted as v, e.g. 65537 dec.)
-    pub v: Vec<u8>,
+    v: Vec<u8>,
+}
+
+impl RSAPub {
+    pub fn new(n: Vec<u8>, v: Vec<u8>) -> Self {
+        Self { n, v }
+    }
+
+    pub fn n(&self) -> &[u8] {
+        &self.n
+    }
+
+    pub fn v(&self) -> &[u8] {
+        &self.v
+    }
 }
 
 /// ECC-specific container for public key material from an OpenPGP card.
 #[derive(Debug)]
 pub struct EccPub {
-    pub data: Vec<u8>,
+    data: Vec<u8>,
-    pub algo: Algo,
+    algo: Algo,
+}
+
+impl EccPub {
+    pub fn new(data: Vec<u8>, algo: Algo) -> Self {
+        Self { data, algo }
+    }
+
+    pub fn data(&self) -> &[u8] {
+        &self.data
+    }
+    pub fn algo(&self) -> &Algo {
+        &self.algo
+    }
 }
 
 /// A marker to distinguish between elliptic curve algorithms (ECDH, ECDSA,

openpgp-card/src/keys.rs

@@ -75,21 +75,15 @@ fn tlv_to_pubkey(tlv: &Tlv, algo: &Algo) -> Result<PublicKeyMaterial> {
 
     match (n, v, ec) {
         (Some(n), Some(v), None) => {
-            let rsa = RSAPub {
+            let rsa = RSAPub::new(n.serialize(), v.serialize());
-                n: n.serialize(),
-                v: v.serialize(),
-            };
-
             Ok(PublicKeyMaterial::R(rsa))
         }
         (None, None, Some(ec)) => {
             let data = ec.serialize();
             log::trace!("EC --- len {}, data {:x?}", data.len(), data);
 
-            Ok(PublicKeyMaterial::E(EccPub {
+            let ecc = EccPub::new(data, algo.clone());
-                data,
+            Ok(PublicKeyMaterial::E(ecc))
-                algo: algo.clone(),
-            }))
         }
 
         (_, _, _) => {
@@ -168,10 +162,8 @@ pub(crate) fn upload_key(
 
                 let algo = ard.get_algorithm_attributes(key_type)?;
 
-                if let Algo::Rsa(mut rsa) = algo {
+                if let Algo::Rsa(rsa) = algo {
-                    rsa.len_n = rsa_bits;
+                    RsaAttrs::new(rsa_bits, rsa.len_e(), rsa.import_format())
-
-                    rsa
                 } else {
                     // We don't expect a card to support non-RSA algos when
                     // it can't provide an algorithm list.
@@ -195,11 +187,11 @@ pub(crate) fn upload_key(
             //     // Error
             // }
 
-            let algo = Algo::Ecc(EccAttrs {
+            let algo = Algo::Ecc(EccAttrs::new(
-                ecc_type: ecc_key.get_type(),
+                ecc_key.get_type(),
-                curve: Curve::try_from(ecc_key.get_oid())?,
+                Curve::try_from(ecc_key.get_oid())?,
-                import_format: None,
+                None,
-            });
+            ));
 
             let key_cmd = ecc_key_cmd(ecc_key, key_type)?;
 
@@ -239,8 +231,10 @@ fn get_card_algo_rsa(
         .collect();
 
     // Filter card algorithms by rsa bitlength of the key we want to upload
-    let algo: Vec<_> =
+    let algo: Vec<_> = rsa_algos
-        rsa_algos.iter().filter(|&a| a.len_n == rsa_bits).collect();
+        .iter()
+        .filter(|&a| a.len_n() == rsa_bits)
+        .collect();
 
     // FIXME: handle error if no algo found
     let algo = *algo[0];
@@ -375,7 +369,7 @@ fn rsa_key_cmd(
     let mut value = vec![];
 
     // Length of e in bytes, rounding up from the bit value in algo.
-    let len_e_bytes = ((algo_attrs.len_e + 7) / 8) as u8;
+    let len_e_bytes = ((algo_attrs.len_e() + 7) / 8) as u8;
 
     value.push(0x91);
     // len_e in bytes has a value of 3-4, it doesn't need TLV encoding
@@ -383,8 +377,8 @@ fn rsa_key_cmd(
 
     // len_p and len_q are len_n/2 (value from card algorithm list).
     // transform unit from bits to bytes.
-    let len_p_bytes: u16 = algo_attrs.len_n / 2 / 8;
+    let len_p_bytes: u16 = algo_attrs.len_n() / 2 / 8;
-    let len_q_bytes: u16 = algo_attrs.len_n / 2 / 8;
+    let len_q_bytes: u16 = algo_attrs.len_n() / 2 / 8;
 
     value.push(0x92);
     // len p in bytes, TLV-encoded