From 8bc7ffd94051ebfdfcb40a46cedfd5e7af279d57 Mon Sep 17 00:00:00 2001 From: Heiko Schaefer Date: Sat, 7 Aug 2021 17:44:35 +0200 Subject: [PATCH] Refactor code for set_algorithm_attributes() - Move algorithm attribute setting out from key import code - Simplify Algo struct --- openpgp-card/src/card_app.rs | 75 ++++++++++++++++-- openpgp-card/src/keys.rs | 111 ++++++--------------------- openpgp-card/src/lib.rs | 2 +- openpgp-card/src/parse/algo_attrs.rs | 65 +++++----------- 4 files changed, 111 insertions(+), 142 deletions(-) diff --git a/openpgp-card/src/card_app.rs b/openpgp-card/src/card_app.rs index 122cb38..809f01a 100644 --- a/openpgp-card/src/card_app.rs +++ b/openpgp-card/src/card_app.rs @@ -18,15 +18,16 @@ use anyhow::{anyhow, Result}; use crate::apdu::{commands, response::Response}; use crate::errors::OpenpgpCardError; use crate::parse::{ - algo_attrs::Algo, algo_info::AlgoInfo, application_id::ApplicationId, - cardholder::CardHolder, extended_cap::ExtendedCap, - extended_length_info::ExtendedLengthInfo, fingerprint, - historical::Historical, key_generation_times, pw_status::PWStatus, KeySet, + algo_attrs::Algo, algo_attrs::RsaAttrs, algo_info::AlgoInfo, + application_id::ApplicationId, cardholder::CardHolder, + extended_cap::ExtendedCap, extended_length_info::ExtendedLengthInfo, + fingerprint, historical::Historical, key_generation_times, + pw_status::PWStatus, KeySet, }; use crate::tlv::{tag::Tag, Tlv, TlvEntry}; use crate::{ - apdu, keys, CardCaps, CardClientBox, CardUploadableKey, DecryptMe, Hash, - KeyGeneration, KeyType, PublicKeyMaterial, Sex, + apdu, keys, CardCaps, CardClientBox, CardUploadableKey, DecryptMe, + EccType, Hash, KeyGeneration, KeyType, PublicKeyMaterial, Sex, }; pub struct CardApp { @@ -545,6 +546,68 @@ impl CardApp { apdu::send_command(&mut self.card_client, time_cmd, false) } + /// Set algorithm attributes [4.4.3.9 Algorithm Attributes] + pub fn set_algorithm_attributes( + &mut self, + key_type: KeyType, + algo: &Algo, + ) -> Result { + // FIXME: caching? + let ard = self.get_app_data()?; + + // FIXME: reuse "e" from card, if no algo list is available + let _cur_algo = Self::get_algorithm_attributes(&ard, key_type)?; + + let data = match algo { + Algo::Rsa(rsa) => Self::rsa_algo_attrs(rsa)?, + Algo::Ecc(ecc) => Self::ecc_algo_attrs(&ecc.oid, ecc.t), + _ => unimplemented!(), + }; + + // Command to PUT the algorithm attributes + let cmd = commands::put_data(&[key_type.get_algorithm_tag()], data); + + apdu::send_command(&mut self.card_client, cmd, false) + } + + fn rsa_algo_attrs(algo_attrs: &RsaAttrs) -> Result> { + // Algorithm ID (01 = RSA (Encrypt or Sign)) + let mut algo_attributes = vec![0x01]; + + // Length of modulus n in bit + 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); + + // Import-Format of private key + // (This fn currently assumes import_format "00 = standard (e, p, q)") + if algo_attrs.import_format != 0 { + return Err(anyhow!( + "Unexpected RSA input format (only 0 is supported)" + )); + } + + algo_attributes.push(algo_attrs.import_format); + + Ok(algo_attributes) + } + + fn ecc_algo_attrs(oid: &[u8], ecc_type: EccType) -> Vec { + let algo_id = match ecc_type { + EccType::EdDSA => 0x16, + EccType::ECDH => 0x12, + EccType::ECDSA => 0x13, + }; + + let mut algo_attributes = vec![algo_id]; + algo_attributes.extend(oid); + // Leave Import-Format unset, for default (pg. 35) + + algo_attributes + } + pub fn upload_key( &mut self, key: Box, diff --git a/openpgp-card/src/keys.rs b/openpgp-card/src/keys.rs index 020c030..e465c4b 100644 --- a/openpgp-card/src/keys.rs +++ b/openpgp-card/src/keys.rs @@ -10,13 +10,12 @@ use crate::apdu::command::Command; use crate::apdu::commands; use crate::card_app::CardApp; use crate::errors::OpenpgpCardError; -use crate::parse::algo_attrs::{Algo, RsaAttrs}; +use crate::parse::algo_attrs::{Algo, EccAttrs, RsaAttrs}; use crate::parse::algo_info::AlgoInfo; use crate::tlv::{tag::Tag, Tlv, TlvEntry}; use crate::{apdu, EccPub, PublicKeyMaterial, RSAPub}; use crate::{ - tlv, CardUploadableKey, EccKey, EccType, KeyType, PrivateKeyMaterial, - RSAKey, + tlv, CardUploadableKey, EccKey, KeyType, PrivateKeyMaterial, RSAKey, }; /// `gen_key_with_metadata` calculates the fingerprint for a public key @@ -147,7 +146,7 @@ pub(crate) fn upload_key( key_type: KeyType, algo_list: Option, ) -> Result<(), OpenpgpCardError> { - let (algo_cmd, key_cmd) = match key.get_key()? { + let (algo, key_cmd) = match key.get_key()? { PrivateKeyMaterial::R(rsa_key) => { // RSA bitsize // (round up to 4-bytes, in case the key has 8+ leading zeros) @@ -156,7 +155,7 @@ pub(crate) fn upload_key( // Get suitable algorithm from card's list, or try to derive it // from the current algo settings on the card - let algo = if let Some(algo_list) = algo_list { + let rsa_attrs = if let Some(algo_list) = algo_list { get_card_algo_rsa(algo_list, key_type, rsa_bits) } else { // Get current settings for this KeyType and adjust the bit @@ -178,11 +177,9 @@ pub(crate) fn upload_key( } }; - let algo_cmd = rsa_algo_attrs_cmd(key_type, rsa_bits, &algo)?; - let key_cmd = rsa_key_cmd(key_type, rsa_key, &algo)?; + let key_cmd = rsa_key_cmd(key_type, rsa_key, &rsa_attrs)?; - // Return commands - (algo_cmd, key_cmd) + (Algo::Rsa(rsa_attrs), key_cmd) } PrivateKeyMaterial::E(ecc_key) => { // Initially there were checks of the following form, here. @@ -196,15 +193,15 @@ pub(crate) fn upload_key( // // Error // } - let algo_cmd = ecc_algo_attrs_cmd( - key_type, - ecc_key.get_oid(), - ecc_key.get_type(), - ); + let algo = Algo::Ecc(EccAttrs { + t: ecc_key.get_type(), + oid: ecc_key.get_oid().to_vec(), + import_format: None, + }); let key_cmd = ecc_key_cmd(ecc_key, key_type)?; - (algo_cmd, key_cmd) + (algo, key_cmd) } }; @@ -213,7 +210,7 @@ pub(crate) fn upload_key( key_type, key.get_ts(), key.get_fp(), - algo_cmd, + &algo, key_cmd, )?; @@ -265,7 +262,7 @@ fn check_card_algo_ecdh( // Get attributes let ecdh_algos: Vec<_> = keytype_algos .iter() - .map(|a| if let Algo::Ecdh(e) = a { Some(e) } else { None }) + .map(|a| if let Algo::Ecc(e) = a { Some(e) } else { None }) .flatten() .collect(); @@ -289,13 +286,7 @@ fn check_card_algo_ecdsa( // Get attributes let ecdsa_algos: Vec<_> = keytype_algos .iter() - .map(|a| { - if let Algo::Ecdsa(e) = a { - Some(e) - } else { - None - } - }) + .map(|a| if let Algo::Ecc(e) = a { Some(e) } else { None }) .flatten() .collect(); @@ -319,13 +310,7 @@ fn check_card_algo_eddsa( // Get attributes let eddsa_algos: Vec<_> = keytype_algos .iter() - .map(|a| { - if let Algo::Eddsa(e) = a { - Some(e) - } else { - None - } - }) + .map(|a| if let Algo::Ecc(e) = a { Some(e) } else { None }) .flatten() .collect(); @@ -453,78 +438,26 @@ fn rsa_key_cmd( )) } -/// Set algorithm attributes [4.4.3.9 Algorithm Attributes] -fn rsa_algo_attrs_cmd( - key_type: KeyType, - rsa_bits: u16, - algo_attrs: &RsaAttrs, -) -> Result { - // Algorithm ID (01 = RSA (Encrypt or Sign)) - let mut algo_attributes = vec![0x01]; - - // Length of modulus n in bit - algo_attributes.extend(rsa_bits.to_be_bytes()); - - // Length of public exponent e in bit - algo_attributes.push(0x00); - 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 { - return Err(anyhow!( - "Unexpected RSA input format (only 0 is supported)" - )); - } - - algo_attributes.push(algo_attrs.import_format); - - // Command to PUT the algorithm attributes - Ok(commands::put_data( - &[key_type.get_algorithm_tag()], - algo_attributes, - )) -} - -/// Set algorithm attributes [4.4.3.9 Algorithm Attributes] -fn ecc_algo_attrs_cmd( - key_type: KeyType, - oid: &[u8], - ecc_type: EccType, -) -> Command { - let algo_id = match ecc_type { - EccType::EdDSA => 0x16, - EccType::ECDH => 0x12, - EccType::ECDSA => 0x13, - }; - - let mut algo_attributes = vec![algo_id]; - algo_attributes.extend(oid); - // Leave Import-Format unset, for default (pg. 35) - - // Command to PUT the algorithm attributes - commands::put_data(&[key_type.get_algorithm_tag()], algo_attributes) -} - fn copy_key_to_card( card_app: &mut CardApp, key_type: KeyType, ts: u32, fp: Vec, - algo_cmd: Command, + algo: &Algo, key_cmd: Command, ) -> Result<(), OpenpgpCardError> { let fp_cmd = commands::put_data(&[key_type.get_fingerprint_put_tag()], fp); // Send all the commands - let card_client = card_app.card(); // FIXME: Only write algo attributes to the card if "extended // capabilities" show that they are changeable! - apdu::send_command(card_client, algo_cmd, false)?.check_ok()?; + card_app + .set_algorithm_attributes(key_type, algo)? + .check_ok()?; - apdu::send_command(card_client, key_cmd, false)?.check_ok()?; - apdu::send_command(card_client, fp_cmd, false)?.check_ok()?; + apdu::send_command(card_app.card(), key_cmd, false)?.check_ok()?; + apdu::send_command(card_app.card(), fp_cmd, false)?.check_ok()?; card_app.set_creation_time(ts, key_type)?.check_ok()?; diff --git a/openpgp-card/src/lib.rs b/openpgp-card/src/lib.rs index ffb9146..1e08fe9 100644 --- a/openpgp-card/src/lib.rs +++ b/openpgp-card/src/lib.rs @@ -178,7 +178,7 @@ pub trait EccKey { /// A marker to distinguish between elliptic curve algorithms (ECDH, ECDSA, /// EdDSA) -#[derive(Clone, Copy)] +#[derive(PartialEq, Eq, Debug, Clone, Copy)] pub enum EccType { ECDH, EdDSA, diff --git a/openpgp-card/src/parse/algo_attrs.rs b/openpgp-card/src/parse/algo_attrs.rs index d3ce257..cca4a64 100644 --- a/openpgp-card/src/parse/algo_attrs.rs +++ b/openpgp-card/src/parse/algo_attrs.rs @@ -9,14 +9,12 @@ use nom::bytes::complete::tag; use nom::combinator::map; use nom::{branch, bytes::complete as bytes, number::complete as number}; -use crate::parse; +use crate::{parse, EccType}; #[derive(Debug, Clone, Eq, PartialEq)] pub enum Algo { Rsa(RsaAttrs), - Ecdsa(EcdsaAttrs), - Eddsa(EddsaAttrs), - Ecdh(EcdhAttrs), + Ecc(EccAttrs), Unknown(Vec), } @@ -28,50 +26,16 @@ pub struct RsaAttrs { } #[derive(Debug, Clone, Eq, PartialEq)] -pub struct EcdsaAttrs { - pub curve: Curve, +pub struct EccAttrs { + pub t: EccType, pub oid: Vec, pub import_format: Option, } -impl EcdsaAttrs { - pub fn new(curve: Curve, import_format: Option) -> Self { +impl EccAttrs { + pub fn new(t: EccType, curve: Curve, import_format: Option) -> Self { Self { - curve, - oid: curve.oid().to_vec(), - import_format, - } - } -} - -#[derive(Debug, Clone, Eq, PartialEq)] -pub struct EddsaAttrs { - pub curve: Curve, - pub oid: Vec, - pub import_format: Option, -} - -impl EddsaAttrs { - pub fn new(curve: Curve, import_format: Option) -> Self { - Self { - curve, - oid: curve.oid().to_vec(), - import_format, - } - } -} - -#[derive(Debug, Clone, Eq, PartialEq)] -pub struct EcdhAttrs { - pub curve: Curve, - pub oid: Vec, - pub import_format: Option, -} - -impl EcdhAttrs { - pub fn new(curve: Curve, import_format: Option) -> Self { - Self { - curve, + t, oid: curve.oid().to_vec(), import_format, } @@ -219,7 +183,10 @@ fn parse_ecdh(input: &[u8]) -> nom::IResult<&[u8], Algo> { let (input, import_format) = alt((parse_import_format, default_import_format))(input)?; - Ok((input, Algo::Ecdh(EcdhAttrs::new(curve, import_format)))) + Ok(( + input, + Algo::Ecc(EccAttrs::new(EccType::ECDH, curve, import_format)), + )) } fn parse_ecdsa(input: &[u8]) -> nom::IResult<&[u8], Algo> { @@ -229,7 +196,10 @@ fn parse_ecdsa(input: &[u8]) -> nom::IResult<&[u8], Algo> { let (input, import_format) = alt((parse_import_format, default_import_format))(input)?; - Ok((input, Algo::Ecdsa(EcdsaAttrs::new(curve, import_format)))) + Ok(( + input, + Algo::Ecc(EccAttrs::new(EccType::ECDSA, curve, import_format)), + )) } fn parse_eddsa(input: &[u8]) -> nom::IResult<&[u8], Algo> { @@ -239,7 +209,10 @@ fn parse_eddsa(input: &[u8]) -> nom::IResult<&[u8], Algo> { let (input, import_format) = alt((parse_import_format, default_import_format))(input)?; - Ok((input, Algo::Eddsa(EddsaAttrs::new(curve, import_format)))) + Ok(( + input, + Algo::Ecc(EccAttrs::new(EccType::EdDSA, curve, import_format)), + )) } pub(crate) fn parse(input: &[u8]) -> nom::IResult<&[u8], Algo> {