From 5417fde8caa644f301c80607adce862a4232a4f7 Mon Sep 17 00:00:00 2001 From: Heiko Schaefer Date: Tue, 21 Sep 2021 09:47:11 +0200 Subject: [PATCH] Implement support for alternate ECC import format (which includes public key data) --- openpgp-card-sequoia/src/privkey.rs | 37 ++++++++---- openpgp-card/src/algorithm.rs | 16 +++++- openpgp-card/src/crypto_data.rs | 3 +- openpgp-card/src/keys.rs | 88 ++++++++++++++++++++++------- 4 files changed, 111 insertions(+), 33 deletions(-) diff --git a/openpgp-card-sequoia/src/privkey.rs b/openpgp-card-sequoia/src/privkey.rs index bf8f5c8..a616999 100644 --- a/openpgp-card-sequoia/src/privkey.rs +++ b/openpgp-card-sequoia/src/privkey.rs @@ -80,33 +80,39 @@ impl CardUploadableKey for SequoiaKey { Ok(PrivateKeyMaterial::R(Box::new(sq_rsa))) } ( - mpi::PublicKey::ECDH { curve, .. }, + mpi::PublicKey::ECDH { curve, q, .. }, mpi::SecretKeyMaterial::ECDH { scalar }, ) => { - let sq_ecc = - SqEccKey::new(curve.oid().to_vec(), scalar, EccType::ECDH); + let sq_ecc = SqEccKey::new( + curve.oid().to_vec(), + scalar, + q, + EccType::ECDH, + ); Ok(PrivateKeyMaterial::E(Box::new(sq_ecc))) } ( - mpi::PublicKey::ECDSA { curve, .. }, + mpi::PublicKey::ECDSA { curve, q, .. }, mpi::SecretKeyMaterial::ECDSA { scalar }, ) => { let sq_ecc = SqEccKey::new( curve.oid().to_vec(), scalar, + q, EccType::ECDSA, ); Ok(PrivateKeyMaterial::E(Box::new(sq_ecc))) } ( - mpi::PublicKey::EdDSA { curve, .. }, + mpi::PublicKey::EdDSA { curve, q, .. }, mpi::SecretKeyMaterial::EdDSA { scalar }, ) => { let sq_ecc = SqEccKey::new( curve.oid().to_vec(), scalar, + q, EccType::EdDSA, ); @@ -203,15 +209,22 @@ impl RSAKey for SqRSA { /// with the `openpgp-card` crate. struct SqEccKey { oid: Vec, - scalar: ProtectedMPI, + private: ProtectedMPI, + public: MPI, ecc_type: EccType, } impl SqEccKey { - fn new(oid: Vec, scalar: ProtectedMPI, ecc_type: EccType) -> Self { + fn new( + oid: Vec, + private: ProtectedMPI, + public: MPI, + ecc_type: EccType, + ) -> Self { SqEccKey { oid, - scalar, + private, + public, ecc_type, } } @@ -222,8 +235,12 @@ impl EccKey for SqEccKey { &self.oid } - fn get_scalar(&self) -> &[u8] { - self.scalar.value() + fn get_private(&self) -> &[u8] { + self.private.value() + } + + fn get_public(&self) -> &[u8] { + self.public.value() } fn get_type(&self) -> EccType { diff --git a/openpgp-card/src/algorithm.rs b/openpgp-card/src/algorithm.rs index 3778c64..4284ed1 100644 --- a/openpgp-card/src/algorithm.rs +++ b/openpgp-card/src/algorithm.rs @@ -170,7 +170,17 @@ impl fmt::Display for Algo { ) } Self::Ecc(ecc) => { - write!(f, "{:?} ({:?})", ecc.curve, ecc.ecc_type) + write!( + f, + "{:?} ({:?}){}", + ecc.curve, + ecc.ecc_type, + if ecc.import_format == Some(0xff) { + " with pub" + } else { + "" + } + ) } Self::Unknown(u) => { write!(f, "Unknown: {:?}", u) @@ -288,6 +298,10 @@ impl EccAttrs { pub fn oid(&self) -> &[u8] { self.curve.oid() } + + pub fn import_format(&self) -> Option { + self.import_format + } } #[derive(Debug, Clone, Copy, Eq, PartialEq)] diff --git a/openpgp-card/src/crypto_data.rs b/openpgp-card/src/crypto_data.rs index 64bcab0..2f67500 100644 --- a/openpgp-card/src/crypto_data.rs +++ b/openpgp-card/src/crypto_data.rs @@ -100,7 +100,8 @@ pub trait RSAKey { /// card. pub trait EccKey { fn get_oid(&self) -> &[u8]; - fn get_scalar(&self) -> &[u8]; + fn get_private(&self) -> &[u8]; + fn get_public(&self) -> &[u8]; fn get_type(&self) -> EccType; } diff --git a/openpgp-card/src/keys.rs b/openpgp-card/src/keys.rs index cb58cf3..5247740 100644 --- a/openpgp-card/src/keys.rs +++ b/openpgp-card/src/keys.rs @@ -200,7 +200,7 @@ pub(crate) fn key_import( let ecc_attrs = determine_ecc_attrs(&*ecc_key, key_type, algo_list)?; - let key_cmd = ecc_key_import_cmd(ecc_key, key_type)?; + let key_cmd = ecc_key_import_cmd(key_type, ecc_key, &ecc_attrs)?; (Algo::Ecc(ecc_attrs), key_cmd) } @@ -284,20 +284,32 @@ fn determine_ecc_attrs( // If we have an algo_list, refuse upload if oid is not listed if let Some(algo_list) = algo_list { let oid = ecc_key.get_oid(); - if !check_card_algo_ecc(algo_list, key_type, 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. return Err(anyhow!( "Oid {:?} unsupported according to algo_list", oid )); } + + // (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). + + if !algos.is_empty() { + return Ok(EccAttrs::new( + ecc_key.get_type(), + Curve::try_from(ecc_key.get_oid())?, + algos[0].import_format(), + )); + } } - // (Precisely looking up ECC algorithms 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). + // 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.get_type(), @@ -345,12 +357,12 @@ fn get_card_algo_rsa( } } -/// Check if `oid` is supported for `key_type` in algo_list. +/// Get all entries from algo_list with matching `oid` and `key_type`. fn check_card_algo_ecc( algo_list: AlgoInfo, key_type: KeyType, oid: &[u8], -) -> bool { +) -> Vec { // Find suitable algorithm parameters (from card's list of algorithms). // Get Algos for this keytype @@ -363,8 +375,13 @@ fn check_card_algo_ecc( .flatten() .collect(); - // Check if this OID exists in the algorithm information for key_type - ecc_algos.iter().any(|e| e.oid() == oid) + // Find entries with this OID in the algorithm information for key_type + ecc_algos + .iter() + .filter(|e| e.oid() == oid) + .cloned() + .cloned() + .collect() } /// Create command for RSA key import @@ -466,21 +483,50 @@ fn rsa_key_import_cmd( /// Create command for ECC key import fn ecc_key_import_cmd( - ecc_key: Box, key_type: KeyType, + ecc_key: Box, + ecc_attrs: &EccAttrs, ) -> Result { - let scalar_data = ecc_key.get_scalar(); - let scalar_len = scalar_data.len() as u8; + let private = ecc_key.get_private(); - // 1) "Control Reference Template" + // Collect data for "Cardholder private key template" DO (7F48) + // + // (Describes the content of the Cardholder private key DO) + let mut cpkt_data = vec![]; + + // "Cardholder private key" (5F48) + // + // "The key data elements according to the definitions in the CPKT DO + // (7F48)." + let mut key_data = Vec::new(); + + // Process "scalar" + cpkt_data.push(0x92); + cpkt_data.extend_from_slice(&tlv_encode_length(private.len() as u16)); + + key_data.extend(private); + + // Process "public", if the import format requires it + if ecc_attrs.import_format() == Some(0xff) { + let p = ecc_key.get_public(); + + cpkt_data.push(0x99); + cpkt_data.extend_from_slice(&tlv_encode_length(p.len() as u16)); + + key_data.extend(p); + } + + // Assemble DOs + + // "Cardholder private key template" + let cpkt = Tlv::new([0x7F, 0x48], Value::S(cpkt_data)); + + // "Cardholder private key" + let cpk = Tlv::new([0x5F, 0x48], Value::S(key_data)); + + // "Control Reference Template" let crt = get_crt(key_type)?; - // 2) "Cardholder private key template" (7F48) - let cpkt = Tlv::new([0x7F, 0x48], Value::S(vec![0x92, scalar_len])); - - // 3) "Cardholder private key" (5F48) - let cpk = Tlv::new([0x5F, 0x48], Value::S(scalar_data.to_vec())); - // "Extended header list (DO 4D)" (contains the three inner TLV) let ehl = Tlv::new([0x4d], Value::C(vec![crt, cpkt, cpk]));