From acc96e988fb02f2de6decb4f8b85f775c7e23c8b Mon Sep 17 00:00:00 2001 From: BlackHoleFox Date: Mon, 1 Jun 2020 22:42:04 -0500 Subject: [PATCH 1/4] Refactor key import function --- src/key.rs | 171 +++++++++++++++++++++++++++++++++++------------------ 1 file changed, 115 insertions(+), 56 deletions(-) diff --git a/src/key.rs b/src/key.rs index 4be5278..f1ac7b3 100644 --- a/src/key.rs +++ b/src/key.rs @@ -69,6 +69,9 @@ const TAG_RSA_MODULUS: u8 = 0x81; const TAG_RSA_EXP: u8 = 0x82; const TAG_ECC_POINT: u8 = 0x86; +#[cfg(feature = "untested")] +const KEYDATA_LEN: usize = 1024; + /// Slot identifiers. /// #[derive(Clone, Copy, Debug, PartialEq)] @@ -380,6 +383,24 @@ impl AlgorithmId { pub(crate) fn write(self, buf: &mut [u8]) -> Result { Tlv::write(buf, 0x80, &[self.into()]) } + + #[cfg(feature = "untested")] + fn get_elem_len(self) -> usize { + match self { + AlgorithmId::Rsa1024 => 64, + AlgorithmId::Rsa2048 => 128, + AlgorithmId::EccP256 => 32, + AlgorithmId::EccP384 => 48, + } + } + + #[cfg(feature = "untested")] + fn get_param_tag(self) -> u8 { + match self { + AlgorithmId::Rsa1024 | AlgorithmId::Rsa2048 => 0x01, + AlgorithmId::EccP256 | AlgorithmId::EccP384 => 0x6, + } + } } /// PIV cryptographic keys stored in a YubiKey @@ -633,72 +654,26 @@ pub fn generate( } } -/// Import a private encryption or signing key into the YubiKey -// TODO(tarcieri): refactor this into separate methods per key type #[cfg(feature = "untested")] -#[allow(clippy::too_many_arguments)] -pub fn import( +fn write_key( yubikey: &mut YubiKey, - key: SlotId, - algorithm: AlgorithmId, - p: Option<&[u8]>, - q: Option<&[u8]>, - dp: Option<&[u8]>, - dq: Option<&[u8]>, - qinv: Option<&[u8]>, - ec_data: Option<&[u8]>, + slot: SlotId, + params: Vec<&[u8]>, pin_policy: PinPolicy, touch_policy: TouchPolicy, + algorithm: AlgorithmId, ) -> Result<(), Error> { - let mut key_data = Zeroizing::new(vec![0u8; 1024]); - let templ = [0, Ins::ImportKey.code(), algorithm.into(), key.into()]; - - let (elem_len, params, param_tag) = match algorithm { - AlgorithmId::Rsa1024 | AlgorithmId::Rsa2048 => match (p, q, dp, dq, qinv) { - (Some(p), Some(q), Some(dp), Some(dq), Some(qinv)) => { - if p.len() + q.len() + dp.len() + dq.len() + qinv.len() >= key_data.len() { - return Err(Error::SizeError); - } - - ( - match algorithm { - AlgorithmId::Rsa1024 => 64, - AlgorithmId::Rsa2048 => 128, - _ => unreachable!(), - }, - vec![p, q, dp, dq, qinv], - 0x01, - ) - } - _ => return Err(Error::GenericError), - }, - AlgorithmId::EccP256 | AlgorithmId::EccP384 => match ec_data { - Some(ec_data) => { - if ec_data.len() >= key_data.len() { - // This can never be true, but check to be explicit. - return Err(Error::SizeError); - } - - ( - match algorithm { - AlgorithmId::EccP256 => 32, - AlgorithmId::EccP384 => 48, - _ => unreachable!(), - }, - vec![ec_data], - 0x06, - ) - } - _ => return Err(Error::GenericError), - }, - }; - + let mut key_data = Zeroizing::new(vec![0u8; KEYDATA_LEN]); + let templ = [0, Ins::ImportKey.code(), algorithm.into(), slot.into()]; let mut offset = 0; + let elem_len = algorithm.get_elem_len(); + let param_tag = algorithm.get_param_tag(); + for (i, param) in params.into_iter().enumerate() { offset += Tlv::write_as( &mut key_data[offset..], - param_tag + i as u8, + param_tag + (i as u8), elem_len, |buf| { let padding = elem_len - param.len(); @@ -726,6 +701,90 @@ pub fn import( } } +/// The key data that makes up an RSA key. +#[cfg(feature = "untested")] +pub struct RsaKeyData<'a> { + /// The secret prime `p`. + pub p: &'a [u8], + /// The secret prime, `q`. + pub q: &'a [u8], + /// D mod (P-1) + pub dp: &'a [u8], + /// D mod (Q-1) + pub dq: &'a [u8], + /// Q^-1 mod P + pub qinv: &'a [u8], +} + +#[cfg(feature = "untested")] +impl RsaKeyData<'_> { + fn total_len(&self) -> usize { + self.p.len() + self.q.len() + self.dp.len() + self.qinv.len() + } +} + +/// Imports a private RSA encryption or signing key into the YubiKey. +/// +/// Errors if `algorithm` isn't `AlgorithmId::Rsa1024` or `AlgorithmId::Rsa2048`. +#[cfg(feature = "untested")] +pub fn import_rsa_key( + yubikey: &mut YubiKey, + slot: SlotId, + algorithm: AlgorithmId, + key_data: RsaKeyData<'_>, + touch_policy: TouchPolicy, + pin_policy: PinPolicy, +) -> Result<(), Error> { + match algorithm { + AlgorithmId::Rsa1024 | AlgorithmId::Rsa2048 => (), + _ => return Err(Error::AlgorithmError), + } + + if key_data.total_len() > KEYDATA_LEN { + return Err(Error::SizeError); + } + + let params = vec![ + key_data.p, + key_data.q, + key_data.dp, + key_data.dq, + key_data.qinv, + ]; + + write_key(yubikey, slot, params, pin_policy, touch_policy, algorithm)?; + + Ok(()) +} + +/// Imports a private ECC encryption or signing key into the YubiKey. +/// +/// Errors if `algorithm` isn't `AlgorithmId::EccP256` or ` AlgorithmId::EccP384`. +#[cfg(feature = "untested")] +pub fn import_ecc_key( + yubikey: &mut YubiKey, + slot: SlotId, + algorithm: AlgorithmId, + key_data: &[u8], + touch_policy: TouchPolicy, + pin_policy: PinPolicy, +) -> Result<(), Error> { + match algorithm { + AlgorithmId::EccP256 | AlgorithmId::EccP384 => (), + _ => return Err(Error::AlgorithmError), + } + + if key_data.len() > KEYDATA_LEN { + return Err(Error::SizeError); + } + + let params = vec![key_data]; + + write_key(yubikey, slot, params, pin_policy, touch_policy, algorithm)?; + + Ok(()) +} + /// Generate an attestation certificate for a stored key. /// #[cfg(feature = "untested")] From 0f907ebd5c0419624f3209f272b09166553e9d4d Mon Sep 17 00:00:00 2001 From: BlackHoleFox Date: Mon, 8 Jun 2020 21:48:25 -0500 Subject: [PATCH 2/4] Implement RSA key precomputation --- Cargo.lock | 3 +++ Cargo.toml | 5 +++- src/key.rs | 72 ++++++++++++++++++++++++++++++++++++++++++++---------- 3 files changed, 66 insertions(+), 14 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 372451f..a1ad037 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1064,6 +1064,9 @@ dependencies = [ "log", "nom", "num-bigint", + "num-bigint-dig", + "num-integer", + "num-traits", "p256", "p384", "pbkdf2", diff --git a/Cargo.toml b/Cargo.toml index c61244d..ae2d756 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -31,7 +31,10 @@ getrandom = "0.1" hmac = "0.7" log = "0.4" nom = "5" -num-bigint = { version = "0.2", features = ["rand"] } +num-bigint_rsa = { version = "0.6", features = ["rand"], package = "num-bigint-dig" } +num-bigint = { version = "0.2", features = ["rand"] } +num-traits = "0.2" +num-integer = "0.1" pbkdf2 = "0.3" p256 = "0.2" p384 = "0.1" diff --git a/src/key.rs b/src/key.rs index f1ac7b3..510db55 100644 --- a/src/key.rs +++ b/src/key.rs @@ -58,6 +58,12 @@ use crate::{ }; use elliptic_curve::weierstrass::PublicKey as EcPublicKey; use log::{error, warn}; +#[cfg(feature = "untested")] +use num_bigint_rsa::traits::ModInverse; +#[cfg(feature = "untested")] +use num_integer::Integer; +#[cfg(feature = "untested")] +use num_traits::{FromPrimitive, One}; use rsa::{BigUint, RSAPublicKey}; #[cfg(feature = "untested")] use zeroize::Zeroizing; @@ -72,6 +78,9 @@ const TAG_ECC_POINT: u8 = 0x86; #[cfg(feature = "untested")] const KEYDATA_LEN: usize = 1024; +#[cfg(feature = "untested")] +const KEYDATA_RSA_EXP: u64 = 65537; + /// Slot identifiers. /// #[derive(Clone, Copy, Debug, PartialEq)] @@ -703,21 +712,58 @@ fn write_key( /// The key data that makes up an RSA key. #[cfg(feature = "untested")] -pub struct RsaKeyData<'a> { +pub struct RsaKeyData { /// The secret prime `p`. - pub p: &'a [u8], + p: Zeroizing>, /// The secret prime, `q`. - pub q: &'a [u8], + q: Zeroizing>, /// D mod (P-1) - pub dp: &'a [u8], + dp: Zeroizing>, /// D mod (Q-1) - pub dq: &'a [u8], + dq: Zeroizing>, /// Q^-1 mod P - pub qinv: &'a [u8], + qinv: Zeroizing>, } #[cfg(feature = "untested")] -impl RsaKeyData<'_> { +impl RsaKeyData { + /// Generates a new RSA key data set from two randomly generated, secret, primes. + /// + /// Panics if `secret_p` or `secret_q` are invalid primes. + pub fn new(secret_p: &[u8], secret_q: &[u8]) -> Self { + let p = BigUint::from_bytes_be(secret_p); + let q = BigUint::from_bytes_be(secret_q); + + let totient = { + let p_t = &p - BigUint::one(); + let q_t = &p - BigUint::one(); + + p_t.lcm(&q_t) + }; + + let exp = BigUint::from_u64(KEYDATA_RSA_EXP).unwrap(); + + let d = exp.mod_inverse(&totient).unwrap(); + let d = d.to_biguint().unwrap(); + + // We calculate the optimization values ahead of time, instead of making the user + // do so. + + let dp = &d % (&p - BigUint::one()); + let dq = &d % (&q - BigUint::one()); + + let qinv = q.clone().mod_inverse(&p).unwrap(); + let (_, qinv) = qinv.to_bytes_be(); + + RsaKeyData { + p: Zeroizing::new(p.to_bytes_be()), + q: Zeroizing::new(q.to_bytes_be()), + dp: Zeroizing::new(dp.to_bytes_be()), + dq: Zeroizing::new(dq.to_bytes_be()), + qinv: Zeroizing::new(qinv), + } + } + fn total_len(&self) -> usize { self.p.len() + self.q.len() + self.dp.len() + self.qinv.len() } @@ -731,7 +777,7 @@ pub fn import_rsa_key( yubikey: &mut YubiKey, slot: SlotId, algorithm: AlgorithmId, - key_data: RsaKeyData<'_>, + key_data: RsaKeyData, touch_policy: TouchPolicy, pin_policy: PinPolicy, ) -> Result<(), Error> { @@ -745,11 +791,11 @@ pub fn import_rsa_key( } let params = vec![ - key_data.p, - key_data.q, - key_data.dp, - key_data.dq, - key_data.qinv, + key_data.p.as_slice(), + key_data.q.as_slice(), + key_data.dp.as_slice(), + key_data.dq.as_slice(), + key_data.qinv.as_slice(), ]; write_key(yubikey, slot, params, pin_policy, touch_policy, algorithm)?; From 6e3560c10f874ae182b693fd295e79db8ca40d73 Mon Sep 17 00:00:00 2001 From: BlackHoleFox Date: Mon, 8 Jun 2020 22:09:57 -0500 Subject: [PATCH 3/4] Switch to buffer alias --- src/key.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/key.rs b/src/key.rs index 510db55..e38e1f9 100644 --- a/src/key.rs +++ b/src/key.rs @@ -672,7 +672,7 @@ fn write_key( touch_policy: TouchPolicy, algorithm: AlgorithmId, ) -> Result<(), Error> { - let mut key_data = Zeroizing::new(vec![0u8; KEYDATA_LEN]); + let mut key_data = Buffer::new(vec![0u8; KEYDATA_LEN]); let templ = [0, Ins::ImportKey.code(), algorithm.into(), slot.into()]; let mut offset = 0; @@ -714,15 +714,15 @@ fn write_key( #[cfg(feature = "untested")] pub struct RsaKeyData { /// The secret prime `p`. - p: Zeroizing>, + p: Buffer, /// The secret prime, `q`. - q: Zeroizing>, + q: Buffer, /// D mod (P-1) - dp: Zeroizing>, + dp: Buffer, /// D mod (Q-1) - dq: Zeroizing>, + dq: Buffer, /// Q^-1 mod P - qinv: Zeroizing>, + qinv: Buffer, } #[cfg(feature = "untested")] From 556b9cb6719c6593d208ee9c1c188a07fe6244e6 Mon Sep 17 00:00:00 2001 From: BlackHoleFox Date: Tue, 9 Jun 2020 18:42:56 -0500 Subject: [PATCH 4/4] Remove dependency on regular num-bigint --- Cargo.lock | 2 -- Cargo.toml | 3 +-- src/certificate.rs | 3 ++- src/key.rs | 2 +- 4 files changed, 4 insertions(+), 6 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a1ad037..e9739ba 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -424,7 +424,6 @@ dependencies = [ "autocfg 1.0.0", "num-integer", "num-traits", - "rand 0.5.6", ] [[package]] @@ -1063,7 +1062,6 @@ dependencies = [ "lazy_static", "log", "nom", - "num-bigint", "num-bigint-dig", "num-integer", "num-traits", diff --git a/Cargo.toml b/Cargo.toml index ae2d756..e60c7e5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -31,8 +31,7 @@ getrandom = "0.1" hmac = "0.7" log = "0.4" nom = "5" -num-bigint_rsa = { version = "0.6", features = ["rand"], package = "num-bigint-dig" } -num-bigint = { version = "0.2", features = ["rand"] } +num-bigint = { version = "0.6", features = ["rand"], package = "num-bigint-dig" } num-traits = "0.2" num-integer = "0.1" pbkdf2 = "0.3" diff --git a/src/certificate.rs b/src/certificate.rs index 4f07f4e..da1d08e 100644 --- a/src/certificate.rs +++ b/src/certificate.rs @@ -476,7 +476,8 @@ impl Certificate { _ => return Err(Error::InvalidObject), }; - let serial = parsed_cert.tbs_certificate.serial.into(); + let serial = Serial::try_from(parsed_cert.tbs_certificate.serial.to_bytes_be().as_slice()) + .map_err(|_| Error::InvalidObject)?; let issuer = parsed_cert.tbs_certificate.issuer.to_string(); let subject = parsed_cert.tbs_certificate.subject.to_string(); let subject_pki = PublicKeyInfo::parse(&parsed_cert.tbs_certificate.subject_pki)?; diff --git a/src/key.rs b/src/key.rs index e38e1f9..d3ab85b 100644 --- a/src/key.rs +++ b/src/key.rs @@ -59,7 +59,7 @@ use crate::{ use elliptic_curve::weierstrass::PublicKey as EcPublicKey; use log::{error, warn}; #[cfg(feature = "untested")] -use num_bigint_rsa::traits::ModInverse; +use num_bigint::traits::ModInverse; #[cfg(feature = "untested")] use num_integer::Integer; #[cfg(feature = "untested")]