diff options
author | Justus Winter <justus@sequoia-pgp.org> | 2024-02-13 15:46:50 +0100 |
---|---|---|
committer | Justus Winter <justus@sequoia-pgp.org> | 2024-02-13 15:47:18 +0100 |
commit | aab11c2361223c9db3e1db722d7dcb95f3d940c0 (patch) | |
tree | 8c635abae222fde6e99218d21b3865e3dd649f0f | |
parent | 4e4aaa021c1ff14691ff515f1f2dab0b97223016 (diff) |
openpgp: Clamp the secret key in Key4::import_secret_cv25519.
- Fixes #1087.
-rw-r--r-- | openpgp/NEWS | 6 | ||||
-rw-r--r-- | openpgp/src/crypto/backend/interface.rs | 16 | ||||
-rw-r--r-- | openpgp/src/packet/key.rs | 34 |
3 files changed, 52 insertions, 4 deletions
diff --git a/openpgp/NEWS b/openpgp/NEWS index 8d8c4624..572dce4f 100644 --- a/openpgp/NEWS +++ b/openpgp/NEWS @@ -3,6 +3,12 @@ #+TITLE: sequoia-openpgp NEWS – history of user-visible changes #+STARTUP: content hidestars +* Changes in 1.19.0 +** Notable fixes + - Key4::import_secret_cv25519 will now clamp some bits of the given + secret scalar to make the generated secret key packet more + compatible with implementations that do not implicitly do the + clamping before decryption. * Changes in 1.18.0 ** New functionality - ComponentAmalgamation::certifications_by_key diff --git a/openpgp/src/crypto/backend/interface.rs b/openpgp/src/crypto/backend/interface.rs index bf6f876a..864b2714 100644 --- a/openpgp/src/crypto/backend/interface.rs +++ b/openpgp/src/crypto/backend/interface.rs @@ -51,6 +51,22 @@ pub trait Asymmetric { /// Returns a tuple containing the secret and public key. fn x25519_generate_key() -> Result<(Protected, [u8; 32])>; + /// Clamp the X25519 secret key scalar. + /// + /// X25519 does the clamping implicitly, but OpenPGP's ECDH over + /// Curve25519 requires the secret to be clamped. To increase + /// compatibility with OpenPGP implementations that do not + /// implicitly clamp the secrets before use, we do that before we + /// store the secrets in OpenPGP data structures. + /// + /// Note: like every function in this trait, this function expects + /// `secret` to be in native byte order. + fn x25519_clamp_secret(secret: &mut Protected) { + secret[0] &= 0b1111_1000; + secret[31] &= !0b1000_0000; + secret[31] |= 0b0100_0000; + } + /// Computes the public key for a given secret key. fn x25519_derive_public(secret: &Protected) -> Result<[u8; 32]>; diff --git a/openpgp/src/packet/key.rs b/openpgp/src/packet/key.rs index 118428fe..787c5101 100644 --- a/openpgp/src/packet/key.rs +++ b/openpgp/src/packet/key.rs @@ -1139,6 +1139,10 @@ impl<R> Key4<SecretParts, R> /// algorithm `sym`. If one or both are `None` secure defaults /// will be used. The key will have it's creation date set to /// `ctime` or the current time if `None` is given. + /// + /// The given `private_key` is expected to be in the native X25519 + /// representation, i.e. as opaque byte string of length 32. It + /// is transformed into OpenPGP's representation during import. pub fn import_secret_cv25519<H, S, T>(private_key: &[u8], hash: H, sym: S, ctime: T) -> Result<Self> where H: Into<Option<HashAlgorithm>>, @@ -1150,6 +1154,22 @@ impl<R> Key4<SecretParts, R> let mut private_key = Protected::from(private_key); let public_key = Backend::x25519_derive_public(&private_key)?; + // Clamp the X25519 secret key scalar. + // + // X25519 does the clamping implicitly, but OpenPGP's ECDH + // over Curve25519 requires the secret to be clamped. To + // increase compatibility with OpenPGP implementations that do + // not implicitly clamp the secrets before use, we do that + // before we store the secrets in OpenPGP data structures. + Backend::x25519_clamp_secret(&mut private_key); + + // Reverse the scalar. + // + // X25519 stores the secret as opaque byte string representing + // a little-endian scalar. OpenPGP's ECDH over Curve25519 on + // the other hand stores it as big-endian scalar, as was + // customary in OpenPGP. See + // https://lists.gnupg.org/pipermail/gnupg-devel/2018-February/033437.html. private_key.reverse(); use crate::crypto::ecdh; @@ -2441,10 +2461,14 @@ FwPoSAbbsLkNS/iNN2MDGAVYvezYn2QZ } #[test] - #[cfg(not(windows))] // see: https://gitlab.com/sequoia-pgp/sequoia/-/issues/958 - fn cv25519_secret_is_reversed() { - let private_key: &[u8] = &crate::crypto::SessionKey::new(32); - let key: Key4<_, UnspecifiedRole> = Key4::import_secret_cv25519(private_key, None, None, None).unwrap(); + fn cv25519_secret_is_reversed() -> Result<()> { + use crate::crypto::backend::{Backend, interface::Asymmetric}; + + let (mut private_key, _) = Backend::x25519_generate_key()?; + Backend::x25519_clamp_secret(&mut private_key); + + let key: Key4<_, UnspecifiedRole> = + Key4::import_secret_cv25519(&private_key, None, None, None)?; if let crate::packet::key::SecretKeyMaterial::Unencrypted(key) = key.secret() { key.map(|secret| { if let mpi::SecretKeyMaterial::ECDH { scalar } = secret { @@ -2458,6 +2482,8 @@ FwPoSAbbsLkNS/iNN2MDGAVYvezYn2QZ } else { unreachable!(); } + + Ok(()) } #[test] |