From eaf46e6bbbd129eae199c4d789cdba426e56cc84 Mon Sep 17 00:00:00 2001 From: Heiko Schaefer Date: Thu, 26 Aug 2021 17:59:54 +0200 Subject: [PATCH] Make fields private, add accessor methods. --- card-functionality/src/tests.rs | 14 +++--- openpgp-card-sequoia/Cargo.toml | 2 +- openpgp-card-sequoia/src/lib.rs | 32 ++++++++------ openpgp-card/src/algorithm.rs | 44 ++++++++++++++++--- openpgp-card/src/card_app.rs | 14 +++--- openpgp-card/src/card_do/algo_attrs.rs | 9 +--- openpgp-card/src/card_do/application_id.rs | 12 ++++- openpgp-card/src/card_do/cardholder.rs | 14 ++++++ openpgp-card/src/card_do/extended_cap.rs | 10 +++++ .../src/card_do/extended_length_info.rs | 8 ++++ .../src/card_do/key_generation_times.rs | 24 +++++----- openpgp-card/src/card_do/mod.rs | 39 +++++++--------- openpgp-card/src/crypto_data.rs | 35 +++++++++++++-- openpgp-card/src/keys.rs | 40 +++++++---------- 14 files changed, 192 insertions(+), 105 deletions(-) diff --git a/card-functionality/src/tests.rs b/card-functionality/src/tests.rs index 234e104..3fc10b2 100644 --- a/card-functionality/src/tests.rs +++ b/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< // SPDX-License-Identifier: MIT OR Apache-2.0 -//! This library supports using openpgp-card functionality with -//! sequoia_openpgp data structures. +//! This is a higher-level wrapper around the openpgp-card crate. +//! 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> { 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()) diff --git a/openpgp-card/src/algorithm.rs b/openpgp-card/src/algorithm.rs index fef58b9..ceda377 100644 --- a/openpgp-card/src/algorithm.rs +++ b/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, - pub len_e: u16, - pub import_format: u8, + len_n: u16, + len_e: u16, + 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, - pub curve: Curve, - pub import_format: Option, + ecc_type: EccType, + curve: Curve, + import_format: Option, } 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() } diff --git a/openpgp-card/src/card_app.rs b/openpgp-card/src/card_app.rs index 638c3cd..d0839ae 100644 --- a/openpgp-card/src/card_app.rs +++ b/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)) } diff --git a/openpgp-card/src/card_do/algo_attrs.rs b/openpgp-card/src/card_do/algo_attrs.rs index adbb4a6..6ebc668 100644 --- a/openpgp-card/src/card_do/algo_attrs.rs +++ b/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(( - input, - Algo::Rsa(RsaAttrs { - len_n, - len_e, - import_format, - }), - )) + Ok((input, Algo::Rsa(RsaAttrs::new(len_n, len_e, import_format)))) } fn parse_import_format(input: &[u8]) -> nom::IResult<&[u8], Option> { diff --git a/openpgp-card/src/card_do/application_id.rs b/openpgp-card/src/card_do/application_id.rs index e65322d..8263aab 100644 --- a/openpgp-card/src/card_do/application_id.rs +++ b/openpgp-card/src/card_do/application_id.rs @@ -38,14 +38,22 @@ impl TryFrom<&[u8]> for ApplicationId { } impl ApplicationId { - pub fn serial(&self) -> u32 { - self.serial + pub fn application(&self) -> u8 { + 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). /// diff --git a/openpgp-card/src/card_do/cardholder.rs b/openpgp-card/src/card_do/cardholder.rs index 53145c4..2e6b0a4 100644 --- a/openpgp-card/src/card_do/cardholder.rs +++ b/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 { + self.sex + } +} + impl TryFrom<&[u8]> for Cardholder { type Error = anyhow::Error; diff --git a/openpgp-card/src/card_do/extended_cap.rs b/openpgp-card/src/card_do/extended_cap.rs index dbe7a80..94064e0 100644 --- a/openpgp-card/src/card_do/extended_cap.rs +++ b/openpgp-card/src/card_do/extended_cap.rs @@ -57,6 +57,16 @@ fn parse( )))(input) } +impl ExtendedCap { + pub fn features(&self) -> HashSet { + self.features.clone() + } + + pub fn max_len_special_do(&self) -> u16 { + self.max_len_special_do + } +} + impl TryFrom<&[u8]> for ExtendedCap { type Error = OpenpgpCardError; diff --git a/openpgp-card/src/card_do/extended_length_info.rs b/openpgp-card/src/card_do/extended_length_info.rs index 42f0022..d4d8649 100644 --- a/openpgp-card/src/card_do/extended_length_info.rs +++ b/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 { let eli = complete(parse(input))?; diff --git a/openpgp-card/src/card_do/key_generation_times.rs b/openpgp-card/src/card_do/key_generation_times.rs index f8d24f7..2bcb40d 100644 --- a/openpgp-card/src/card_do/key_generation_times.rs +++ b/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 for DateTime { - fn from(kg: KeyGeneration) -> Self { +impl From for DateTime { + 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 { - fn from(kg: &KeyGeneration) -> Self { +impl From<&KeyGenerationTime> for u32 { + fn from(kg: &KeyGenerationTime) -> Self { kg.0 } } -impl From for KeyGeneration { +impl From 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> { +fn key_generation( + input: &[u8], +) -> nom::IResult<&[u8], Option> { 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> { +) -> nom::IResult<&[u8], KeySet> { combinator::into(sequence::tuple(( key_generation, key_generation, @@ -50,7 +52,9 @@ fn key_generation_set( )))(input) } -pub fn from(input: &[u8]) -> Result, OpenpgpCardError> { +pub fn from( + input: &[u8], +) -> Result, 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 diff --git a/openpgp-card/src/card_do/mod.rs b/openpgp-card/src/card_do/mod.rs index fd02b0e..c0c6108 100644 --- a/openpgp-card/src/card_do/mod.rs +++ b/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, OpenpgpCardError> { + ) -> Result, 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, - - // GnuPG says: - // if (app->appversion >= 0x0200) - // 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, + application: u8, + version: u16, + manufacturer: u16, + 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: HashSet, 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, - pub max_response_bytes: u16, + max_command_bytes: u16, + max_response_bytes: u16, } /// Cardholder Related Data #[derive(Debug)] pub struct Cardholder { - pub name: Option, - pub lang: Option>, - pub sex: Option, + name: Option, + lang: Option>, + sex: Option, } /// Sex (according to ISO 5218) -#[derive(Debug, PartialEq)] +#[derive(Debug, PartialEq, Clone, Copy)] pub enum Sex { NotKnown, Male, diff --git a/openpgp-card/src/crypto_data.rs b/openpgp-card/src/crypto_data.rs index c26f286..169dfbe 100644 --- a/openpgp-card/src/crypto_data.rs +++ b/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, + n: Vec, /// Public exponent (a number denoted as v, e.g. 65537 dec.) - pub v: Vec, + v: Vec, +} + +impl RSAPub { + pub fn new(n: Vec, v: Vec) -> 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, - pub algo: Algo, + data: Vec, + algo: Algo, +} + +impl EccPub { + pub fn new(data: Vec, 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, diff --git a/openpgp-card/src/keys.rs b/openpgp-card/src/keys.rs index 817b997..a9181f4 100644 --- a/openpgp-card/src/keys.rs +++ b/openpgp-card/src/keys.rs @@ -75,21 +75,15 @@ fn tlv_to_pubkey(tlv: &Tlv, algo: &Algo) -> Result { match (n, v, ec) { (Some(n), Some(v), None) => { - let rsa = RSAPub { - n: n.serialize(), - v: v.serialize(), - }; - + let rsa = RSAPub::new(n.serialize(), 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 { - data, - algo: algo.clone(), - })) + let ecc = EccPub::new(data, algo.clone()); + Ok(PublicKeyMaterial::E(ecc)) } (_, _, _) => { @@ -168,10 +162,8 @@ pub(crate) fn upload_key( let algo = ard.get_algorithm_attributes(key_type)?; - if let Algo::Rsa(mut rsa) = algo { - rsa.len_n = rsa_bits; - - rsa + if let Algo::Rsa(rsa) = algo { + RsaAttrs::new(rsa_bits, rsa.len_e(), rsa.import_format()) } 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 { - ecc_type: ecc_key.get_type(), - curve: Curve::try_from(ecc_key.get_oid())?, - import_format: None, - }); + let algo = Algo::Ecc(EccAttrs::new( + ecc_key.get_type(), + Curve::try_from(ecc_key.get_oid())?, + 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<_> = - rsa_algos.iter().filter(|&a| a.len_n == rsa_bits).collect(); + let algo: Vec<_> = rsa_algos + .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_q_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; value.push(0x92); // len p in bytes, TLV-encoded