From ddcd8888340ede215b306a26c5d5fb214a82f009 Mon Sep 17 00:00:00 2001 From: Heiko Schaefer Date: Thu, 2 Dec 2021 15:44:30 +0100 Subject: [PATCH] Refactor determine_ecc_attrs() for reusability. Use in AlgoSimple::determine_algo(). --- openpgp-card/src/algorithm.rs | 50 ++++++++++++++++++++--------------- openpgp-card/src/keys.rs | 38 ++++++++++++++------------ 2 files changed, 50 insertions(+), 38 deletions(-) diff --git a/openpgp-card/src/algorithm.rs b/openpgp-card/src/algorithm.rs index 490efda..cc709ce 100644 --- a/openpgp-card/src/algorithm.rs +++ b/openpgp-card/src/algorithm.rs @@ -82,6 +82,11 @@ impl AlgoSimple { } } + /// Return the appropriate Algo for this AlgoSimple. + /// + /// This mapping differs between cards, based on `ard` and `algo_info` + /// (e.g. the exact Algo variant can have a different size for e, in RSA; + /// also, the import_format can differ). pub(crate) fn determine_algo( &self, key_type: KeyType, @@ -101,27 +106,30 @@ impl AlgoSimple { Self::RSA4k => Algo::Rsa(keys::determine_rsa_attrs( 4096, key_type, ard, algo_info, )?), - - Self::NIST256 => Algo::Ecc(EccAttrs { - curve: Curve::NistP256r1, - ecc_type: Self::ecc_type(key_type), - import_format: None, - }), - Self::NIST384 => Algo::Ecc(EccAttrs { - curve: Curve::NistP384r1, - ecc_type: Self::ecc_type(key_type), - import_format: None, - }), - Self::NIST521 => Algo::Ecc(EccAttrs { - curve: Curve::NistP521r1, - ecc_type: Self::ecc_type(key_type), - import_format: None, - }), - Self::Curve25519 => Algo::Ecc(EccAttrs { - curve: Self::curve_for_25519(key_type), - ecc_type: Self::ecc_type_25519(key_type), - import_format: None, - }), + Self::NIST256 => Algo::Ecc(keys::determine_ecc_attrs( + Curve::NistP256r1.oid(), + Self::ecc_type(key_type), + key_type, + algo_info, + )?), + Self::NIST384 => Algo::Ecc(keys::determine_ecc_attrs( + Curve::NistP384r1.oid(), + Self::ecc_type(key_type), + key_type, + algo_info, + )?), + Self::NIST521 => Algo::Ecc(keys::determine_ecc_attrs( + Curve::NistP521r1.oid(), + Self::ecc_type(key_type), + key_type, + algo_info, + )?), + Self::Curve25519 => Algo::Ecc(keys::determine_ecc_attrs( + Self::curve_for_25519(key_type).oid(), + Self::ecc_type_25519(key_type), + key_type, + algo_info, + )?), }; Ok(algo) diff --git a/openpgp-card/src/keys.rs b/openpgp-card/src/keys.rs index 88a8f6a..9b7be8b 100644 --- a/openpgp-card/src/keys.rs +++ b/openpgp-card/src/keys.rs @@ -13,8 +13,8 @@ use crate::apdu::commands; use crate::card_app::CardApp; use crate::card_do::{ApplicationRelatedData, Fingerprint, KeyGenerationTime}; use crate::crypto_data::{ - CardUploadableKey, EccKey, EccPub, PrivateKeyMaterial, PublicKeyMaterial, - RSAKey, RSAPub, + CardUploadableKey, EccKey, EccPub, EccType, PrivateKeyMaterial, + PublicKeyMaterial, RSAKey, RSAPub, }; use crate::tlv::{length::tlv_encode_length, value::Value, Tlv}; use crate::Error; @@ -199,8 +199,12 @@ pub(crate) fn key_import( (Algo::Rsa(rsa_attrs), key_cmd) } PrivateKeyMaterial::E(ecc_key) => { - let ecc_attrs = - determine_ecc_attrs(&*ecc_key, key_type, algo_list)?; + let ecc_attrs = determine_ecc_attrs( + ecc_key.oid(), + ecc_key.ecc_type(), + key_type, + algo_list, + )?; let key_cmd = ecc_key_import_cmd(key_type, ecc_key, &ecc_attrs)?; @@ -225,7 +229,8 @@ pub(crate) fn key_import( Ok(()) } -/// Determine RsaAttrs for the current card, for an `rsa_bits` sized key. +/// Determine suitable RsaAttrs for the current card, for an `rsa_bits` +/// sized key. /// /// If available, via lookup in `algo_list`, otherwise the current /// algorithm attributes are checked. If neither method yields a @@ -272,15 +277,16 @@ pub(crate) fn determine_rsa_attrs( Ok(rsa_attrs) } -/// Derive EccAttrs from `ecc_key`, check if the OID is listed in algo_list. -fn determine_ecc_attrs( - ecc_key: &dyn EccKey, +/// Derive EccAttrs from `oid` and `ecc_type`, check if the OID is listed in +/// algo_list. +pub(crate) fn determine_ecc_attrs( + oid: &[u8], + ecc_type: EccType, key_type: KeyType, algo_list: Option, ) -> Result { // If we have an algo_list, refuse upload if oid is not listed if let Some(algo_list) = algo_list { - let oid = ecc_key.oid(); let algos = check_card_algo_ecc(algo_list, key_type, oid); if algos.is_empty() { // If oid is not in algo_list, return error. @@ -290,15 +296,17 @@ fn determine_ecc_attrs( )); } - // (Looking up ecc_type in the card's "Algorithm Information" + // Note: Looking up ecc_type in the card's "Algorithm Information" // seems to do more harm than good, so we don't do it. // Some cards report erroneous information about supported algorithms // - e.g. Yubikey 5 reports support for EdDSA over Cv25519 and - // Ed25519, but not ECDH). + // Ed25519, but not ECDH. + // + // We do however, use import_format from algorithm information. if !algos.is_empty() { return Ok(EccAttrs::new( - ecc_key.ecc_type(), + ecc_type, Curve::try_from(oid)?, algos[0].import_format(), )); @@ -308,11 +316,7 @@ fn determine_ecc_attrs( // Return a default when we have no algo_list. // (Do cards that support ecc but have no algo_list exist?) - Ok(EccAttrs::new( - ecc_key.ecc_type(), - Curve::try_from(ecc_key.oid())?, - None, - )) + Ok(EccAttrs::new(ecc_type, Curve::try_from(oid)?, None)) } /// Look up RsaAttrs parameters in algo_list based on key_type and rsa_bits