From 18c2f6805cd7878c0974404f45965773ffee09a1 Mon Sep 17 00:00:00 2001 From: Justus Winter Date: Thu, 5 May 2022 16:36:51 +0200 Subject: openpgp: Fix ECDH parameter selection on generation and import. - Select an appropriate hash algorithm for the ECDH KDF and an appropriate cipher for the ECDH KEK depending on the curve. Harmonize that for import and generation. - Fixes #841. --- openpgp/src/crypto/backend/cng/asymmetric.rs | 23 ++++++++++------- openpgp/src/crypto/backend/nettle/asymmetric.rs | 23 ++++++++++------- openpgp/src/crypto/backend/rust/asymmetric.rs | 18 ++++++++----- openpgp/src/crypto/ecdh.rs | 34 +++++++++++++++++++++++++ 4 files changed, 74 insertions(+), 24 deletions(-) (limited to 'openpgp/src/crypto') diff --git a/openpgp/src/crypto/backend/cng/asymmetric.rs b/openpgp/src/crypto/backend/cng/asymmetric.rs index fe987dbe..e8b52c1f 100644 --- a/openpgp/src/crypto/backend/cng/asymmetric.rs +++ b/openpgp/src/crypto/backend/cng/asymmetric.rs @@ -722,13 +722,16 @@ where let mut private = blob.d().to_vec(); private.reverse(); + use crate::crypto::ecdh; Self::with_secret( ctime.into().unwrap_or_else(crate::now), PublicKeyAlgorithm::ECDH, mpi::PublicKey::ECDH { curve: Curve::Cv25519, - hash: hash.into().unwrap_or(HashAlgorithm::SHA512), - sym: sym.into().unwrap_or(SymmetricAlgorithm::AES256), + hash: hash.into().unwrap_or_else( + || ecdh::default_ecdh_kdf_hash(&Curve::Cv25519)), + sym: sym.into().unwrap_or_else( + || ecdh::default_ecdh_kek_cipher(&Curve::Cv25519)), q: mpi::MPI::new(&public), }, mpi::SecretKeyMaterial::ECDH { scalar: private.into() }.into() @@ -866,12 +869,15 @@ where use cng::asymmetric::{ecc, Export}; use cng::asymmetric::{AsymmetricKey, AsymmetricAlgorithmId, Ecdh}; + let hash = crate::crypto::ecdh::default_ecdh_kdf_hash(&curve); + let sym = crate::crypto::ecdh::default_ecdh_kek_cipher(&curve); + let (algo, public, private) = match (curve.clone(), for_signing) { (Curve::NistP256, ..) | (Curve::NistP384, ..) | (Curve::NistP521, ..) => { - let (cng_curve, hash) = match curve { - Curve::NistP256 => (ecc::NamedCurve::NistP256, HashAlgorithm::SHA256), - Curve::NistP384 => (ecc::NamedCurve::NistP384, HashAlgorithm::SHA384), - Curve::NistP521 => (ecc::NamedCurve::NistP521, HashAlgorithm::SHA512), + let cng_curve = match curve { + Curve::NistP256 => ecc::NamedCurve::NistP256, + Curve::NistP384 => ecc::NamedCurve::NistP384, + Curve::NistP521 => ecc::NamedCurve::NistP521, _ => unreachable!() }; @@ -900,7 +906,6 @@ where mpi::SecretKeyMaterial::ECDSA { scalar: scalar.into() }, ) } else { - let sym = SymmetricAlgorithm::AES256; ( ECDH, mpi::PublicKey::ECDH { curve, q, hash, sym }, @@ -927,8 +932,8 @@ where mpi::PublicKey::ECDH { curve, q: mpi::MPI::new(&public), - hash: HashAlgorithm::SHA256, - sym: SymmetricAlgorithm::AES256, + hash, + sym, }, mpi::SecretKeyMaterial::ECDH { scalar: private.into() } ) diff --git a/openpgp/src/crypto/backend/nettle/asymmetric.rs b/openpgp/src/crypto/backend/nettle/asymmetric.rs index 0b4e1835..9efad861 100644 --- a/openpgp/src/crypto/backend/nettle/asymmetric.rs +++ b/openpgp/src/crypto/backend/nettle/asymmetric.rs @@ -343,13 +343,16 @@ impl Key4 let mut private_key = Vec::from(private_key); private_key.reverse(); + use crate::crypto::ecdh; Self::with_secret( ctime.into().unwrap_or_else(crate::now), PublicKeyAlgorithm::ECDH, mpi::PublicKey::ECDH { curve: Curve::Cv25519, - hash: hash.into().unwrap_or(HashAlgorithm::SHA512), - sym: sym.into().unwrap_or(SymmetricAlgorithm::AES256), + hash: hash.into().unwrap_or_else( + || ecdh::default_ecdh_kdf_hash(&Curve::Cv25519)), + sym: sym.into().unwrap_or_else( + || ecdh::default_ecdh_kek_cipher(&Curve::Cv25519)), q: MPI::new_compressed_point(&public_key), }, mpi::SecretKeyMaterial::ECDH { @@ -443,6 +446,8 @@ impl Key4 use crate::PublicKeyAlgorithm::*; let mut rng = Yarrow::default(); + let hash = crate::crypto::ecdh::default_ecdh_kdf_hash(&curve); + let sym = crate::crypto::ecdh::default_ecdh_kek_cipher(&curve); let (mpis, secret, pk_algo) = match (curve.clone(), for_signing) { (Curve::Ed25519, true) => { @@ -476,8 +481,8 @@ impl Key4 let public_mpis = PublicKey::ECDH { curve: Curve::Cv25519, q: MPI::new_compressed_point(&public), - hash: HashAlgorithm::SHA256, - sym: SymmetricAlgorithm::AES256, + hash, + sym, }; let private_mpis = mpi::SecretKeyMaterial::ECDH { scalar: private.into(), @@ -522,24 +527,24 @@ impl Key4 (Curve::NistP256, false) | (Curve::NistP384, false) | (Curve::NistP521, false) => { - let (private, hash, field_sz) = match curve { + let (private, field_sz) = match curve { Curve::NistP256 => { let pv = ecc::Scalar::new_random::(&mut rng); - (pv, HashAlgorithm::SHA256, 256) + (pv, 256) } Curve::NistP384 => { let pv = ecc::Scalar::new_random::(&mut rng); - (pv, HashAlgorithm::SHA384, 384) + (pv, 384) } Curve::NistP521 => { let pv = ecc::Scalar::new_random::(&mut rng); - (pv, HashAlgorithm::SHA512, 521) + (pv, 521) } _ => unreachable!(), }; @@ -549,7 +554,7 @@ impl Key4 curve, q: MPI::new_point(&pub_x, &pub_y, field_sz), hash, - sym: SymmetricAlgorithm::AES256, + sym, }; let private_mpis = mpi::SecretKeyMaterial::ECDH{ scalar: MPI::new(&private.as_bytes()).into(), diff --git a/openpgp/src/crypto/backend/rust/asymmetric.rs b/openpgp/src/crypto/backend/rust/asymmetric.rs index 5ca2cce8..7c4785e2 100644 --- a/openpgp/src/crypto/backend/rust/asymmetric.rs +++ b/openpgp/src/crypto/backend/rust/asymmetric.rs @@ -366,13 +366,16 @@ impl Key4 let mut private_key = Vec::from(private_key); private_key.reverse(); + use crate::crypto::ecdh; Self::with_secret( ctime.into().unwrap_or_else(crate::now), PublicKeyAlgorithm::ECDH, mpi::PublicKey::ECDH { curve: Curve::Cv25519, - hash: hash.into().unwrap_or(HashAlgorithm::SHA512), - sym: sym.into().unwrap_or(SymmetricAlgorithm::AES256), + hash: hash.into().unwrap_or_else( + || ecdh::default_ecdh_kdf_hash(&Curve::Cv25519)), + sym: sym.into().unwrap_or_else( + || ecdh::default_ecdh_kek_cipher(&Curve::Cv25519)), q: MPI::new_compressed_point(&*public_key.as_bytes()), }, mpi::SecretKeyMaterial::ECDH { @@ -495,6 +498,9 @@ impl Key4 /// `curve == Cv25519` will produce an error. Likewise /// `for_signing == false` and `curve == Ed25519` will produce an error. pub fn generate_ecc(for_signing: bool, curve: Curve) -> Result { + let hash = crate::crypto::ecdh::default_ecdh_kdf_hash(&curve); + let sym = crate::crypto::ecdh::default_ecdh_kek_cipher(&curve); + let (algo, public, private) = match (&curve, for_signing) { (Curve::Ed25519, true) => { use ed25519_dalek::Keypair; @@ -528,8 +534,8 @@ impl Key4 let public_mpis = mpi::PublicKey::ECDH { curve: Curve::Cv25519, q: MPI::new_compressed_point(&*public_key.as_bytes()), - hash: HashAlgorithm::SHA256, - sym: SymmetricAlgorithm::AES256, + hash, + sym, }; let private_mpis = mpi::SecretKeyMaterial::ECDH { scalar: private_key.into(), @@ -566,8 +572,8 @@ impl Key4 let public_mpis = mpi::PublicKey::ECDH { curve, q: MPI::new(public.as_bytes()), - hash: HashAlgorithm::SHA256, - sym: SymmetricAlgorithm::AES256, + hash, + sym, }; let private_mpis = mpi::SecretKeyMaterial::ECDH { scalar: Vec::from(secret.to_bytes().as_slice()).into(), diff --git a/openpgp/src/crypto/ecdh.rs b/openpgp/src/crypto/ecdh.rs index 5c2162e1..a4c73985 100644 --- a/openpgp/src/crypto/ecdh.rs +++ b/openpgp/src/crypto/ecdh.rs @@ -22,6 +22,40 @@ use crate::utils::{read_be_u64, write_be_u64}; pub(crate) use crate::crypto::backend::ecdh::{encrypt, decrypt}; +/// Returns the default ECDH KDF hash function. +pub(crate) fn default_ecdh_kdf_hash(curve: &Curve) -> HashAlgorithm { + match curve { + Curve::Cv25519 => HashAlgorithm::SHA256, + // From RFC6637: + Curve::NistP256 => HashAlgorithm::SHA256, + Curve::NistP384 => HashAlgorithm::SHA384, + Curve::NistP521 => HashAlgorithm::SHA512, + // Extrapolated from RFC6637: + Curve::BrainpoolP256 => HashAlgorithm::SHA256, + Curve::BrainpoolP512 => HashAlgorithm::SHA512, + // Conservative default. + Curve::Ed25519 // Odd: Not an encryption algo. + | Curve::Unknown(_) => HashAlgorithm::SHA512, + } +} + +/// Returns the default ECDH KEK cipher. +pub(crate) fn default_ecdh_kek_cipher(curve: &Curve) -> SymmetricAlgorithm { + match curve { + Curve::Cv25519 => SymmetricAlgorithm::AES128, + // From RFC6637: + Curve::NistP256 => SymmetricAlgorithm::AES128, + Curve::NistP384 => SymmetricAlgorithm::AES192, + Curve::NistP521 => SymmetricAlgorithm::AES256, + // Extrapolated from RFC6637: + Curve::BrainpoolP256 => SymmetricAlgorithm::AES128, + Curve::BrainpoolP512 => SymmetricAlgorithm::AES256, + // Conservative default. + Curve::Ed25519 // Odd: Not an encryption algo. + | Curve::Unknown(_) => SymmetricAlgorithm::AES256, + } +} + /// Wraps a session key. /// /// After using Elliptic-curve Diffie-Hellman to compute the shared -- cgit v1.2.3