From d7ba06e299e3f8f76a744d9728b86c80d05f19da Mon Sep 17 00:00:00 2001 From: Justus Winter Date: Fri, 12 May 2023 16:22:29 +0200 Subject: openpgp: Deduplicate EdDSA signature verification. --- openpgp/src/crypto/backend/botan/asymmetric.rs | 24 ----------- openpgp/src/crypto/backend/cng/asymmetric.rs | 36 ---------------- openpgp/src/crypto/backend/nettle/asymmetric.rs | 35 ---------------- openpgp/src/crypto/backend/openssl/asymmetric.rs | 23 ---------- openpgp/src/crypto/backend/rust/asymmetric.rs | 34 --------------- openpgp/src/packet/key.rs | 53 +++++++++++++++++++++++- 6 files changed, 52 insertions(+), 153 deletions(-) diff --git a/openpgp/src/crypto/backend/botan/asymmetric.rs b/openpgp/src/crypto/backend/botan/asymmetric.rs index ba2f0c96..66e64b35 100644 --- a/openpgp/src/crypto/backend/botan/asymmetric.rs +++ b/openpgp/src/crypto/backend/botan/asymmetric.rs @@ -395,30 +395,6 @@ impl Key { let truncated_digest = &digest[..size.min(digest.len())]; pk.verify(truncated_digest, &sig, "Raw").unwrap() }, - (PublicKey::EdDSA { curve, q }, Signature::EdDSA { r, s }) => - match curve { - Curve::Ed25519 => { - if q.value().get(0).map(|&b| b != 0x40).unwrap_or(true) { - return Err(Error::MalformedPacket( - "Invalid point encoding".into()).into()); - } - - // OpenPGP encodes R and S separately, but our - // cryptographic library expects them to be - // concatenated. - let mut sig = Vec::with_capacity(64); - - // We need to zero-pad them at the front, because - // the MPI encoding drops leading zero bytes. - sig.extend_from_slice(&r.value_padded(32).map_err(bad)?); - sig.extend_from_slice(&s.value_padded(32).map_err(bad)?); - - let pk = Pubkey::load_ed25519(&q.value()[1..])?; - pk.verify(digest, &sig, "")? - }, - _ => return - Err(Error::UnsupportedEllipticCurve(curve.clone()).into()), - }, (PublicKey::ECDSA { curve, q }, Signature::ECDSA { s, r }) => { // OpenPGP encodes R and S separately, but our diff --git a/openpgp/src/crypto/backend/cng/asymmetric.rs b/openpgp/src/crypto/backend/cng/asymmetric.rs index e47f3911..f2a9e241 100644 --- a/openpgp/src/crypto/backend/cng/asymmetric.rs +++ b/openpgp/src/crypto/backend/cng/asymmetric.rs @@ -743,42 +743,6 @@ impl Key { Error::UnsupportedEllipticCurve(curve.clone()).into()), } }, - (mpi::PublicKey::EdDSA { curve, q }, mpi::Signature::EdDSA { r, s }) - => match curve - { - Curve::Ed25519 => { - // CNG doesn't support EdDSA, use ed25519-dalek instead - use ed25519_dalek::{PublicKey, Signature, SIGNATURE_LENGTH}; - use ed25519_dalek::{Verifier}; - - let (public, ..) = q.decode_point(&Curve::Ed25519)?; - assert_eq!(public.len(), 32); - - let key = PublicKey::from_bytes(public).map_err(|e| { - Error::InvalidKey(e.to_string()) - })?; - - // ed25519 expects full-sized signatures but OpenPGP allows - // for stripped leading zeroes, pad each part with zeroes. - let mut sig_bytes = [0u8; SIGNATURE_LENGTH]; - - // We need to zero-pad them at the front, because - // the MPI encoding drops leading zero bytes. - let half = SIGNATURE_LENGTH / 2; - sig_bytes[..half].copy_from_slice( - &r.value_padded(half).map_err(bad)?); - sig_bytes[half..].copy_from_slice( - &s.value_padded(half).map_err(bad)?); - - let signature = Signature::from(sig_bytes); - - key.verify(digest, &signature) - .map(|_| true) - .map_err(|e| Error::BadSignature(e.to_string()))? - }, - _ => return Err( - Error::UnsupportedEllipticCurve(curve.clone()).into()), - }, _ => return Err(Error::MalformedPacket(format!( "unsupported combination of key {} and signature {:?}.", self.pk_algo(), sig)).into()), diff --git a/openpgp/src/crypto/backend/nettle/asymmetric.rs b/openpgp/src/crypto/backend/nettle/asymmetric.rs index e18fa249..16c1d3d2 100644 --- a/openpgp/src/crypto/backend/nettle/asymmetric.rs +++ b/openpgp/src/crypto/backend/nettle/asymmetric.rs @@ -295,10 +295,6 @@ impl Key { { use crate::crypto::mpi::Signature; - fn bad(e: impl ToString) -> anyhow::Error { - Error::BadSignature(e.to_string()).into() - } - let ok = match (self.mpis(), sig) { (PublicKey::RSA { e, n }, Signature::RSA { s }) => { let key = rsa::PublicKey::new(n.value(), e.value())?; @@ -319,37 +315,6 @@ impl Key { dsa::verify(¶ms, &key, digest, &signature) }, - (PublicKey::EdDSA { curve, q }, Signature::EdDSA { r, s }) => - match curve { - Curve::Ed25519 => { - if q.value().get(0).map(|&b| b != 0x40).unwrap_or(true) { - return Err(Error::MalformedPacket( - "Invalid point encoding".into()).into()); - } - - // OpenPGP encodes R and S separately, but our - // cryptographic library expects them to be - // concatenated. - let mut signature = - Vec::with_capacity(ed25519::ED25519_SIGNATURE_SIZE); - - // We need to zero-pad them at the front, because - // the MPI encoding drops leading zero bytes. - let half = ed25519::ED25519_SIGNATURE_SIZE / 2; - signature.extend_from_slice( - &r.value_padded(half).map_err(bad)?); - signature.extend_from_slice( - &s.value_padded(half).map_err(bad)?); - - // Let's see if we got it right. - assert_eq!(signature.len(), - ed25519::ED25519_SIGNATURE_SIZE); - - ed25519::verify(&q.value()[1..], digest, &signature)? - }, - _ => return - Err(Error::UnsupportedEllipticCurve(curve.clone()).into()), - }, (PublicKey::ECDSA { curve, q }, Signature::ECDSA { s, r }) => { let (x, y) = q.decode_point(curve)?; diff --git a/openpgp/src/crypto/backend/openssl/asymmetric.rs b/openpgp/src/crypto/backend/openssl/asymmetric.rs index 77ee9ff5..b5e3ab02 100644 --- a/openpgp/src/crypto/backend/openssl/asymmetric.rs +++ b/openpgp/src/crypto/backend/openssl/asymmetric.rs @@ -393,29 +393,6 @@ impl Key { ctx.verify_init()?; ctx.verify(&digest, &signature.to_der()?)? } - (mpi::PublicKey::EdDSA { curve, q }, mpi::Signature::EdDSA { r, s }) => match curve { - Curve::Ed25519 => { - let public = q.decode_point(&Curve::Ed25519)?.0; - - let key = PKey::public_key_from_raw_bytes(public, openssl::pkey::Id::ED25519)?; - - const SIGNATURE_LENGTH: usize = 64; - - // ed25519 expects full-sized signatures but OpenPGP allows - // for stripped leading zeroes, pad each part with zeroes. - let mut sig_bytes = [0u8; SIGNATURE_LENGTH]; - - // We need to zero-pad them at the front, because - // the MPI encoding drops leading zero bytes. - let half = SIGNATURE_LENGTH / 2; - sig_bytes[..half].copy_from_slice(&r.value_padded(half)?); - sig_bytes[half..].copy_from_slice(&s.value_padded(half)?); - - let mut verifier = Verifier::new_without_digest(&key)?; - verifier.verify_oneshot(&sig_bytes, digest)? - } - _ => return Err(crate::Error::UnsupportedEllipticCurve(curve.clone()).into()), - }, (mpi::PublicKey::ECDSA { curve, q }, mpi::Signature::ECDSA { s, r }) => { let nid = curve.try_into()?; let group = EcGroup::from_curve_name(nid)?; diff --git a/openpgp/src/crypto/backend/rust/asymmetric.rs b/openpgp/src/crypto/backend/rust/asymmetric.rs index 9654e788..fafac2b1 100644 --- a/openpgp/src/crypto/backend/rust/asymmetric.rs +++ b/openpgp/src/crypto/backend/rust/asymmetric.rs @@ -416,40 +416,6 @@ impl Key { }, _ => Err(Error::UnsupportedEllipticCurve(curve.clone()).into()), }, - (mpi::PublicKey::EdDSA { curve, q }, - mpi::Signature::EdDSA { r, s }) => match curve { - Curve::Ed25519 => { - use ed25519_dalek::{PublicKey, Signature, SIGNATURE_LENGTH}; - use ed25519_dalek::{Verifier}; - - let (public, ..) = q.decode_point(&Curve::Ed25519)?; - assert_eq!(public.len(), 32); - - let key = PublicKey::from_bytes(public).map_err(|e| { - Error::InvalidKey(e.to_string()) - })?; - - // OpenPGP encodes R and S separately, but our - // cryptographic library expects them to be - // concatenated. - let mut sig_bytes = [0u8; SIGNATURE_LENGTH]; - - // We need to zero-pad them at the front, because - // the MPI encoding drops leading zero bytes. - let half = SIGNATURE_LENGTH / 2; - sig_bytes[..half].copy_from_slice( - &r.value_padded(half).map_err(bad)?); - sig_bytes[half..].copy_from_slice( - &s.value_padded(half).map_err(bad)?); - - let signature = Signature::from(sig_bytes); - - key.verify(digest, &signature) - .map_err(|e| Error::BadSignature(e.to_string()))?; - Ok(()) - }, - _ => Err(Error::UnsupportedEllipticCurve(curve.clone()).into()), - }, _ => Err(Error::MalformedPacket(format!( "unsupported combination of key {} and signature {:?}.", self.pk_algo(), sig)).into()), diff --git a/openpgp/src/packet/key.rs b/openpgp/src/packet/key.rs index c1d9545c..b6de1f97 100644 --- a/openpgp/src/packet/key.rs +++ b/openpgp/src/packet/key.rs @@ -748,7 +748,58 @@ impl Key /// Verifies the given signature. pub fn verify(&self, sig: &mpi::Signature, hash_algo: HashAlgorithm, digest: &[u8]) -> Result<()> { - self.verify_backend(sig, hash_algo, digest) + use crate::crypto::backend::{Backend, interface::Asymmetric}; + use crate::crypto::mpi::{PublicKey, Signature}; + + fn bad(e: impl ToString) -> anyhow::Error { + Error::BadSignature(e.to_string()).into() + } + + let ok = match (self.mpis(), sig) { + (PublicKey::EdDSA { curve, q }, Signature::EdDSA { r, s }) => + match curve { + Curve::Ed25519 => { + let (public, ..) = q.decode_point(&Curve::Ed25519)?; + assert_eq!(public.len(), 32); + + // OpenPGP encodes R and S separately, but our + // cryptographic backends expect them to be + // concatenated. + let mut signature = Vec::with_capacity(64); + + // We need to zero-pad them at the front, because + // the MPI encoding drops leading zero bytes. + signature.extend_from_slice( + &r.value_padded(32).map_err(bad)?); + signature.extend_from_slice( + &s.value_padded(32).map_err(bad)?); + + // Let's see if we got it right. + debug_assert_eq!(signature.len(), 64); + + Backend::ed25519_verify(public.try_into()?, + digest, + &signature.as_slice().try_into()?)? + }, + _ => return + Err(Error::UnsupportedEllipticCurve(curve.clone()).into()), + }, + + (PublicKey::RSA { .. }, Signature::RSA { .. }) | + (PublicKey::DSA { .. }, Signature::DSA { .. }) | + (PublicKey::ECDSA { .. }, Signature::ECDSA { .. }) => + return self.verify_backend(sig, hash_algo, digest), + + _ => return Err(Error::MalformedPacket(format!( + "unsupported combination of key {} and signature {:?}.", + self.pk_algo(), sig)).into()), + }; + + if ok { + Ok(()) + } else { + Err(Error::ManipulatedMessage.into()) + } } } -- cgit v1.2.3