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.

4 files changed, 74 insertions, 24 deletions

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<R> Key4<SecretParts, R>
         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<R> Key4<SecretParts, R>
         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<R> Key4<SecretParts, R>
                 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<R> Key4<SecretParts, R>
 
             (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::<ecc::Secp256r1, _>(&mut rng);
 
-                            (pv, HashAlgorithm::SHA256, 256)
+                            (pv, 256)
                         }
                         Curve::NistP384 => {
                             let pv =
                                 ecc::Scalar::new_random::<ecc::Secp384r1, _>(&mut rng);
 
-                            (pv, HashAlgorithm::SHA384, 384)
+                            (pv, 384)
                         }
                         Curve::NistP521 => {
                             let pv =
                                 ecc::Scalar::new_random::<ecc::Secp521r1, _>(&mut rng);
 
-                            (pv, HashAlgorithm::SHA512, 521)
+                            (pv, 521)
                         }
                         _ => unreachable!(),
                     };
@@ -549,7 +554,7 @@ impl<R> Key4<SecretParts, R>
                         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<R> Key4<SecretParts, R>
         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<R> Key4<SecretParts, R>
     /// `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<Self> {
+        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<R> Key4<SecretParts, R>
                 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<R> Key4<SecretParts, R>
                 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