Refactor determine_ecc_attrs() for reusability.

Use in AlgoSimple::determine_algo().

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(keys::determine_ecc_attrs(
-            Self::NIST256 => Algo::Ecc(EccAttrs {
+                Curve::NistP256r1.oid(),
-                curve: Curve::NistP256r1,
+                Self::ecc_type(key_type),
-                ecc_type: Self::ecc_type(key_type),
+                key_type,
-                import_format: None,
+                algo_info,
-            }),
+            )?),
-            Self::NIST384 => Algo::Ecc(EccAttrs {
+            Self::NIST384 => Algo::Ecc(keys::determine_ecc_attrs(
-                curve: Curve::NistP384r1,
+                Curve::NistP384r1.oid(),
-                ecc_type: Self::ecc_type(key_type),
+                Self::ecc_type(key_type),
-                import_format: None,
+                key_type,
-            }),
+                algo_info,
-            Self::NIST521 => Algo::Ecc(EccAttrs {
+            )?),
-                curve: Curve::NistP521r1,
+            Self::NIST521 => Algo::Ecc(keys::determine_ecc_attrs(
-                ecc_type: Self::ecc_type(key_type),
+                Curve::NistP521r1.oid(),
-                import_format: None,
+                Self::ecc_type(key_type),
-            }),
+                key_type,
-            Self::Curve25519 => Algo::Ecc(EccAttrs {
+                algo_info,
-                curve: Self::curve_for_25519(key_type),
+            )?),
-                ecc_type: Self::ecc_type_25519(key_type),
+            Self::Curve25519 => Algo::Ecc(keys::determine_ecc_attrs(
-                import_format: None,
+                Self::curve_for_25519(key_type).oid(),
-            }),
+                Self::ecc_type_25519(key_type),
+                key_type,
+                algo_info,
+            )?),
         };
 
         Ok(algo)

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,
+    CardUploadableKey, EccKey, EccPub, EccType, PrivateKeyMaterial,
-    RSAKey, RSAPub,
+    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 =
+            let ecc_attrs = determine_ecc_attrs(
-                determine_ecc_attrs(&*ecc_key, key_type, algo_list)?;
+                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.
+/// Derive EccAttrs from `oid` and `ecc_type`, check if the OID is listed in
-fn determine_ecc_attrs(
+/// algo_list.
-    ecc_key: &dyn EccKey,
+pub(crate) fn determine_ecc_attrs(
+    oid: &[u8],
+    ecc_type: EccType,
     key_type: KeyType,
     algo_list: Option<AlgoInfo>,
 ) -> Result<EccAttrs> {
     // 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(
+    Ok(EccAttrs::new(ecc_type, Curve::try_from(oid)?, None))
-        ecc_key.ecc_type(),
-        Curve::try_from(ecc_key.oid())?,
-        None,
-    ))
 }
 
 /// Look up RsaAttrs parameters in algo_list based on key_type and rsa_bits