diff options
author | Neal H. Walfield <neal@pep.foundation> | 2020-12-11 14:41:17 +0100 |
---|---|---|
committer | Neal H. Walfield <neal@pep.foundation> | 2020-12-11 14:46:30 +0100 |
commit | 35119b755db270ab43a8e1ec13577bc0f9846546 (patch) | |
tree | 2499fe86c242b8aa7e05df02f56640e11e8e920b /openpgp | |
parent | 582a079f1cccc07bd74432ceb55da09e698da2d0 (diff) |
openpgp: Pass the hash algo's security reqs to Policy::signature.
- If the signer controls the data that is being signed, then the
hash algorithm only needs second pre-image resistance.
- This observation can be used to extend the life of hash algorithms
that have been weakened, as is the case for SHA-1.
- Introduces a new `enum HashAlgoSecurity`, which is now passed to
`Policy::signature`.
- See #595.
Diffstat (limited to 'openpgp')
-rw-r--r-- | openpgp/src/cert.rs | 16 | ||||
-rw-r--r-- | openpgp/src/cert/amalgamation.rs | 9 | ||||
-rw-r--r-- | openpgp/src/cert/bundle.rs | 24 | ||||
-rw-r--r-- | openpgp/src/cert/parser/low_level/grammar.lalrpop | 10 | ||||
-rw-r--r-- | openpgp/src/packet/key.rs | 30 | ||||
-rw-r--r-- | openpgp/src/packet/unknown.rs | 30 | ||||
-rw-r--r-- | openpgp/src/packet/user_attribute.rs | 30 | ||||
-rw-r--r-- | openpgp/src/packet/userid.rs | 105 | ||||
-rw-r--r-- | openpgp/src/parse/stream.rs | 5 | ||||
-rw-r--r-- | openpgp/src/policy.rs | 328 |
10 files changed, 566 insertions, 21 deletions
diff --git a/openpgp/src/cert.rs b/openpgp/src/cert.rs index 78dad9b1..ca010f19 100644 --- a/openpgp/src/cert.rs +++ b/openpgp/src/cert.rs @@ -1267,15 +1267,25 @@ impl Cert { { let mut keys = std::collections::HashSet::new(); + let pk_sec = self.primary_key().hash_algo_security(); + // All user ids. self.userids() .flat_map(|ua| { // All valid self-signatures. - ua.self_signatures().iter() + let sec = ua.hash_algo_security; + ua.self_signatures() + .iter() + .filter(move |sig| { + policy.signature(sig, sec).is_ok() + }) }) // All direct-key signatures. - .chain(self.primary_key().self_signatures() .iter()) - .filter(|sig| policy.signature(sig).is_ok()) + .chain(self.primary_key() + .self_signatures().iter() + .filter(|sig| { + policy.signature(sig, pk_sec).is_ok() + })) .flat_map(|sig| sig.revocation_keys()) .for_each(|rk| { keys.insert(rk); }); diff --git a/openpgp/src/cert/amalgamation.rs b/openpgp/src/cert/amalgamation.rs index 78efb4cc..7e065e4d 100644 --- a/openpgp/src/cert/amalgamation.rs +++ b/openpgp/src/cert/amalgamation.rs @@ -895,15 +895,20 @@ impl<'a, C> ComponentAmalgamation<'a, C> { let mut keys = std::collections::HashSet::new(); for rk in self.self_signatures().iter() .filter(|sig| { - policy.signature(sig).is_ok() + policy + .signature(sig, self.hash_algo_security) + .is_ok() }) .flat_map(|sig| sig.revocation_keys()) { keys.insert(rk); } + let pk_sec = self.cert().primary_key().hash_algo_security(); for rk in self.cert().primary_key().self_signatures().iter() .filter(|sig| { - policy.signature(sig).is_ok() + policy + .signature(sig, pk_sec) + .is_ok() }) .flat_map(|sig| sig.revocation_keys()) { diff --git a/openpgp/src/cert/bundle.rs b/openpgp/src/cert/bundle.rs index 3693ef1e..272088ce 100644 --- a/openpgp/src/cert/bundle.rs +++ b/openpgp/src/cert/bundle.rs @@ -91,6 +91,7 @@ use crate::{ packet::UserAttribute, packet::Unknown, Packet, + policy::HashAlgoSecurity, policy::Policy, Result, }; @@ -112,6 +113,8 @@ use super::{ pub struct ComponentBundle<C> { pub(crate) component: C, + pub(crate) hash_algo_security: HashAlgoSecurity, + // Self signatures. pub(crate) self_signatures: Vec<Signature>, @@ -302,7 +305,8 @@ impl<C> ComponentBundle<C> { continue; } - if let Err(e) = policy.signature(s) { + if let Err(e) = policy.signature(s, self.hash_algo_security) + { if error.is_none() { error = Some(e); } @@ -329,7 +333,9 @@ impl<C> ComponentBundle<C> { continue 'next_backsig; } - if let Err(e) = policy.signature(backsig) { + if let Err(e) = policy + .signature(backsig, self.hash_algo_security) + { if error.is_none() { error = Some(e); } @@ -523,9 +529,11 @@ impl<C> ComponentBundle<C> { selfsig.signature_alive(t, time::Duration::new(0, 0)).is_ok()); } - let check = |revs: &'a [Signature]| -> Option<Vec<&'a Signature>> { + let check = |revs: &'a [Signature], sec: HashAlgoSecurity| + -> Option<Vec<&'a Signature>> + { let revs = revs.iter().filter_map(|rev| { - if let Err(err) = policy.signature(rev) { + if let Err(err) = policy.signature(rev, sec) { t!(" revocation rejected by caller policy: {}", err); None } else if hard_revocations_are_final @@ -580,9 +588,13 @@ impl<C> ComponentBundle<C> { } }; - if let Some(revs) = check(&self.self_revocations) { + if let Some(revs) + = check(&self.self_revocations, self.hash_algo_security) + { RevocationStatus::Revoked(revs) - } else if let Some(revs) = check(&self.other_revocations) { + } else if let Some(revs) + = check(&self.other_revocations, Default::default()) + { RevocationStatus::CouldBe(revs) } else { RevocationStatus::NotAsFarAsWeKnow diff --git a/openpgp/src/cert/parser/low_level/grammar.lalrpop b/openpgp/src/cert/parser/low_level/grammar.lalrpop index 7280ddf5..47c9da75 100644 --- a/openpgp/src/cert/parser/low_level/grammar.lalrpop +++ b/openpgp/src/cert/parser/low_level/grammar.lalrpop @@ -44,10 +44,12 @@ pub Cert: Option<Cert> = { _ => unreachable!(), }; let c = c.unwrap(); + let sec = key.hash_algo_security(); let mut cert = Cert { primary: PrimaryKeyBundle { component: key, + hash_algo_security: sec, self_signatures: vec![], certifications: sigs, self_revocations: vec![], @@ -156,9 +158,11 @@ Component: Option<Component> = { match key { Some(key) => { let sigs = sigs.unwrap(); + let sec = key.hash_algo_security(); Some(Component::SubkeyBundle(SubkeyBundle { component: key, + hash_algo_security: sec, self_signatures: vec![], certifications: sigs, self_revocations: vec![], @@ -173,9 +177,11 @@ Component: Option<Component> = { match u { Some(u) => { let sigs = sigs.unwrap(); + let sec = u.hash_algo_security(); Some(Component::UserIDBundle(UserIDBundle { component: u, + hash_algo_security: sec, self_signatures: vec![], certifications: sigs, self_revocations: vec![], @@ -190,9 +196,11 @@ Component: Option<Component> = { match u { Some(u) => { let sigs = sigs.unwrap(); + let sec = u.hash_algo_security(); Some(Component::UserAttributeBundle(UserAttributeBundle { component: u, + hash_algo_security: sec, self_signatures: vec![], certifications: sigs, self_revocations: vec![], @@ -207,9 +215,11 @@ Component: Option<Component> = { match u { Some(u) => { let sigs = sigs.unwrap(); + let sec = u.hash_algo_security(); Some(Component::UnknownBundle(UnknownBundle { component: u, + hash_algo_security: sec, self_signatures: vec![], certifications: sigs, self_revocations: vec![], diff --git a/openpgp/src/packet/key.rs b/openpgp/src/packet/key.rs index 4003e507..87745997 100644 --- a/openpgp/src/packet/key.rs +++ b/openpgp/src/packet/key.rs @@ -110,6 +110,7 @@ use crate::crypto::Password; use crate::KeyID; use crate::Fingerprint; use crate::KeyHandle; +use crate::policy::HashAlgoSecurity; mod conversions; @@ -832,6 +833,35 @@ impl<P, R> Key4<P, R> where P: key::KeyParts, R: key::KeyRole, { + /// The security requirements of the hash algorithm for + /// self-signatures. + /// + /// A cryptographic hash algorithm usually has [three security + /// properties]: pre-image resistance, second pre-image + /// resistance, and collision resistance. If an attacker can + /// influence the signed data, then the hash algorithm needs to + /// have both second pre-image resistance, and collision + /// resistance. If not, second pre-image resistance is + /// sufficient. + /// + /// [three security properties]: https://en.wikipedia.org/wiki/Cryptographic_hash_function#Properties + /// + /// In general, an attacker may be able to influence third-party + /// signatures. But direct key signatures, and binding signatures + /// are only over data fully determined by signer. And, an + /// attacker's control over self signatures over User IDs is + /// limited due to their structure. + /// + /// These observations can be used to extend the life of a hash + /// algorithm after its collision resistance has been partially + /// compromised, but not completely broken. For more details, + /// please refer to the documentation for [HashAlgoSecurity]. + /// + /// [HashAlgoSecurity]: ../policy/enum.HashAlgoSecurity.html + pub fn hash_algo_security(&self) -> HashAlgoSecurity { + HashAlgoSecurity::SecondPreImageResistance + } + /// Compares the public bits of two keys. /// /// This returns `Ordering::Equal` if the public MPIs, creation diff --git a/openpgp/src/packet/unknown.rs b/openpgp/src/packet/unknown.rs index 086f1b32..86758601 100644 --- a/openpgp/src/packet/unknown.rs +++ b/openpgp/src/packet/unknown.rs @@ -4,6 +4,7 @@ use std::cmp::Ordering; use crate::packet::Tag; use crate::packet; use crate::Packet; +use crate::policy::HashAlgoSecurity; /// Holds an unknown packet. /// @@ -73,6 +74,35 @@ impl Unknown { } } + /// The security requirements of the hash algorithm for + /// self-signatures. + /// + /// A cryptographic hash algorithm usually has [three security + /// properties]: pre-image resistance, second pre-image + /// resistance, and collision resistance. If an attacker can + /// influence the signed data, then the hash algorithm needs to + /// have both second pre-image resistance, and collision + /// resistance. If not, second pre-image resistance is + /// sufficient. + /// + /// [three security properties]: https://en.wikipedia.org/wiki/Cryptographic_hash_function#Properties + /// + /// In general, an attacker may be able to influence third-party + /// signatures. But direct key signatures, and binding signatures + /// are only over data fully determined by signer. And, an + /// attacker's control over self signatures over User IDs is + /// limited due to their structure. + /// + /// These observations can be used to extend the life of a hash + /// algorithm after its collision resistance has been partially + /// compromised, but not completely broken. For more details, + /// please refer to the documentation for [HashAlgoSecurity]. + /// + /// [HashAlgoSecurity]: ../policy/enum.HashAlgoSecurity.html + pub fn hash_algo_security(&self) -> HashAlgoSecurity { + HashAlgoSecurity::CollisionResistance + } + /// Gets the unknown packet's tag. pub fn tag(&self) -> Tag { self.tag diff --git a/openpgp/src/packet/user_attribute.rs b/openpgp/src/packet/user_attribute.rs index 851e3126..57d93e82 100644 --- a/openpgp/src/packet/user_attribute.rs +++ b/openpgp/src/packet/user_attribute.rs @@ -20,6 +20,7 @@ use crate::packet::{ header::BodyLength, }; use crate::Packet; +use crate::policy::HashAlgoSecurity; use crate::serialize::Marshal; use crate::serialize::MarshalInto; @@ -74,6 +75,35 @@ impl UserAttribute { }) } + /// The security requirements of the hash algorithm for + /// self-signatures. + /// + /// A cryptographic hash algorithm usually has [three security + /// properties]: pre-image resistance, second pre-image + /// resistance, and collision resistance. If an attacker can + /// influence the signed data, then the hash algorithm needs to + /// have both second pre-image resistance, and collision + /// resistance. If not, second pre-image resistance is + /// sufficient. + /// + /// [three security properties]: https://en.wikipedia.org/wiki/Cryptographic_hash_function#Properties + /// + /// In general, an attacker may be able to influence third-party + /// signatures. But direct key signatures, and binding signatures + /// are only over data fully determined by signer. And, an + /// attacker's control over self signatures over User IDs is + /// limited due to their structure. + /// + /// These observations can be used to extend the life of a hash + /// algorithm after its collision resistance has been partially + /// compromised, but not completely broken. For more details, + /// please refer to the documentation for [HashAlgoSecurity]. + /// + /// [HashAlgoSecurity]: ../policy/enum.HashAlgoSecurity.html + pub fn hash_algo_security(&self) -> HashAlgoSecurity { + HashAlgoSecurity::CollisionResistance + } + /// Gets the user attribute packet's raw, unparsed value. /// /// Most likely you will want to use [`subpackets()`] to iterate diff --git a/openpgp/src/packet/userid.rs b/openpgp/src/packet/userid.rs index 44e9b510..49d189bf 100644 --- a/openpgp/src/packet/userid.rs +++ b/openpgp/src/packet/userid.rs @@ -15,6 +15,7 @@ use crate::Result; use crate::packet; use crate::Packet; use crate::Error; +use crate::policy::HashAlgoSecurity; /// A conventionally parsed UserID. #[derive(Clone, Debug)] @@ -474,6 +475,8 @@ pub struct UserID { /// Use `UserID::default()` to get a UserID with a default settings. value: Vec<u8>, + hash_algo_security: HashAlgoSecurity, + parsed: Mutex<RefCell<Option<ConventionallyParsedUserID>>>, } assert_send_and_sync!(UserID); @@ -482,6 +485,7 @@ impl From<Vec<u8>> for UserID { fn from(u: Vec<u8>) -> Self { UserID { common: Default::default(), + hash_algo_security: UserID::determine_hash_algo_security(&u), value: u, parsed: Mutex::new(RefCell::new(None)), } @@ -571,6 +575,7 @@ impl Clone for UserID { fn clone(&self) -> Self { UserID { common: self.common.clone(), + hash_algo_security: self.hash_algo_security, value: self.value.clone(), parsed: Mutex::new(RefCell::new(None)), } @@ -684,6 +689,78 @@ impl UserID { Ok(UserID::from(value)) } + /// The security requirements of the hash algorithm for + /// self-signatures. + /// + /// A cryptographic hash algorithm usually has [three security + /// properties]: pre-image resistance, second pre-image + /// resistance, and collision resistance. If an attacker can + /// influence the signed data, then the hash algorithm needs to + /// have both second pre-image resistance, and collision + /// resistance. If not, second pre-image resistance is + /// sufficient. + /// + /// [three security properties]: https://en.wikipedia.org/wiki/Cryptographic_hash_function#Properties + /// + /// In general, an attacker may be able to influence third-party + /// signatures. But direct key signatures, and binding signatures + /// are only over data fully determined by signer. And, an + /// attacker's control over self signatures over User IDs is + /// limited due to their structure. + /// + /// In the case of self signatures over User IDs, an attacker may + /// be able to control the content of the User ID packet. + /// However, unlike an image, there is no easy way to hide large + /// amounts of arbitrary data (e.g., the 512 bytes needed by the + /// [SHA-1 is a Shambles] attack) from the user. Further, normal + /// User IDs are short and encoded using UTF-8. + /// + /// [SHA-1 is a Shambles]: https://sha-mbles.github.io/ + /// + /// These observations can be used to extend the life of a hash + /// algorithm after its collision resistance has been partially + /// compromised, but not completely broken. Specifically for the + /// case of User IDs, we relax the requirement for strong + /// collision resistance for self signatures over User IDs if: + /// + /// - The User ID is at most 96 bytes long, + /// - It contains valid UTF-8, and + /// - It doesn't contain a UTF-8 control character (this includes + /// the NUL byte). + /// + /// + /// For more details, please refer to the documentation for + /// [HashAlgoSecurity]. + /// + /// [HashAlgoSecurity]: ../policy/enum.HashAlgoSecurity.html + pub fn hash_algo_security(&self) -> HashAlgoSecurity { + self.hash_algo_security + } + + // See documentation for hash_algo_security. + fn determine_hash_algo_security(u: &[u8]) -> HashAlgoSecurity { + // SHA-1 has 64 byte (512-bit) blocks. A block and a half (96 + // bytes) is more than enough for all but malicious users. + if u.len() > 96 { + return HashAlgoSecurity::CollisionResistance; + } + + // Check that the User ID is valid UTF-8. + match str::from_utf8(u) { + Ok(s) => { + // And doesn't contain control characters. + if s.chars().any(char::is_control) { + return HashAlgoSecurity::CollisionResistance; + } + } + Err(_err) => { + return HashAlgoSecurity::CollisionResistance; + } + } + + HashAlgoSecurity::SecondPreImageResistance + } + /// Constructs a User ID. /// /// This does a basic check and any necessary escaping to form a @@ -1280,4 +1357,32 @@ mod tests { .unwrap().value(), b"Foo Q. Bar <foo@bar.com>"); } + + #[test] + fn hash_algo_security() { + // Acceptable. + assert_eq!(UserID::from("Alice Lovelace <alice@lovelace.org>") + .hash_algo_security(), + HashAlgoSecurity::SecondPreImageResistance); + + // Embedded NUL. + assert_eq!(UserID::from(&b"Alice Lovelace <alice@lovelace.org>\0"[..]) + .hash_algo_security(), + HashAlgoSecurity::CollisionResistance); + assert_eq!( + UserID::from( + &b"Alice Lovelace <alice@lovelace.org>\0Hidden!"[..]) + .hash_algo_security(), + HashAlgoSecurity::CollisionResistance); + + // Long strings. + assert_eq!( + UserID::from(String::from_utf8(vec!['a' as u8; 90]).unwrap()) + .hash_algo_security(), + HashAlgoSecurity::SecondPreImageResistance); + assert_eq!( + UserID::from(String::from_utf8(vec!['a' as u8; 100]).unwrap()) + .hash_algo_security(), + HashAlgoSecurity::CollisionResistance); + } } diff --git a/openpgp/src/parse/stream.rs b/openpgp/src/parse/stream.rs index f799a4ce..e9d26205 100644 --- a/openpgp/src/parse/stream.rs +++ b/openpgp/src/parse/stream.rs @@ -2719,7 +2719,10 @@ impl<'a, H: VerificationHelper + DecryptionHelper> Decryptor<'a, H> { } else { match sig.verify(ka.key()) { Ok(()) => { - if let Err(error) = self.policy.signature(&sig) { + if let Err(error) + = self.policy.signature( + &sig, Default::default()) + { t!("{:02X}{:02X}: signature rejected by policy: {}", sigid[0], sigid[1], error); VerificationErrorInternal::BadSignature { diff --git a/openpgp/src/policy.rs b/openpgp/src/policy.rs index 29715451..c763abd9 100644 --- a/openpgp/src/policy.rs +++ b/openpgp/src/policy.rs @@ -77,7 +77,9 @@ pub trait Policy : fmt::Debug + Send + Sync { /// signatures, one should be more liberal when considering /// revocations: if you reject a revocation certificate, it may /// inadvertently make something else valid! - fn signature(&self, _sig: &Signature) -> Result<()> { + fn signature(&self, _sig: &Signature, _sec: HashAlgoSecurity) + -> Result<()> + { Ok(()) } @@ -140,6 +142,300 @@ pub trait Policy : fmt::Debug + Send + Sync { } } +/// Whether the signed data requires a hash algorithm with collision +/// resistance. +/// +/// Since the context of a signature is not passed to +/// `Policy::signature`, it is not possible to determine from that +/// function whether the signature requires a hash algorithm with +/// collision resistance. This enum indicates this. +/// +/// In short, many self signatures only require second pre-image +/// resistance. This can be used to extend the life of hash +/// algorithms whose collision resistance has been partially +/// compromised. Be careful. Read the background and the warning +/// before accepting the use of weak hash algorithms! +/// +/// # Warning +/// +/// Although distinguishing whether signed data requires collision +/// resistance can be used to permit the continued use of a hash +/// algorithm in certain situations, once attacks against a hash +/// algorithm are known, it is imperative to retire the use of the +/// hash algorithm as soon as it is feasible. Cryptoanalytic attacks +/// improve quickly, as demonstrated by the attacks on SHA-1. +/// +/// # Background +/// +/// Cryptographic hash functions normally have three security +/// properties: +/// +/// - Pre-image resistance, +/// - Second pre-image resistance, and +/// - Collision resistance. +/// +/// A hash algorithm has pre-image resistance if given a hash `h`, it +/// is impractical for an attacker to find a message `m` such that `h +/// = hash(m)`. In other words, a hash algorithm has pre-image +/// resistance if it is hard to invert. A hash algorithm has second +/// pre-image resistance if it is impractical for an attacker to find +/// a second message with the same hash as the first. That is, given +/// `m1`, it is hard for an attacker to find an `m2` such that +/// `hash(m1) = hash(m2)`. And, a hash algorithm has collision +/// resistance if it is impractical for an attacker to find two +/// messages with the same hash. That is, it is hard for an attacker +/// to find an `m1` and an `m2` such that `hash(m1) = hash(m2)`. +/// +/// In the context of verifying an OpenPGP signature, we don't need a +/// hash algorithm with pre-image resistance. Pre-image resistance is +/// only required when the message is a secret, e.g., a password. We +/// always need a hash algorithm with second pre-image resistance, +/// because an attacker must not be able to repurpose an arbitrary +/// signature, i.e., create a collision with respect to a *known* +/// hash. And, we need collision resistance when a signature is over +/// data that could have been influenced by an attacker: if an +/// attacker creates a pair of colliding messages and convinces the +/// user to sign one of them, then the attacker can copy the signature +/// to the other message. +/// +/// Collision resistance implies second pre-image resistance, but not +/// vice versa. If an attacker can find a second message with the +/// same hash as some known message, they can also create a collision +/// by choosing an arbitrary message and using their pre-image attack +/// to find a colliding message. Thus, a context that requires +/// collision resistance also requires second pre-image resistance. +/// +/// Because collision resistance is with respect to two arbitrary +/// messages, collision resistance is always susceptible to a +/// [birthday paradox]. This means that the security margin of a hash +/// algorithm's collision resistance is half of the security margin of +/// its second pre-image resistance. And, in practice, the collision +/// resistance of industry standard hash algorithms has been +/// practically attacked multiple times. In the context of SHA-1, +/// Wang et al. described how to find collisions in SHA-1 in their +/// 2005 paper [Finding Collisions in the Full SHA-1]. In 2017, +/// Stevens et al. published [The First Collision for Full SHA-1], +/// which demonstrates the first practical attack on SHA-1's collision +/// resistance, an identical-prefix collision attack. This attack +/// only gives the attacker limited control over the content of the +/// collided messages, which limits its applicability. However, in +/// 2020, Leurent and Peyrin published [SHA-1 is a Shambles], which +/// demonstrates a practical chosen-prefix collision attack. This +/// attack gives the attacker complete control over the prefixes of +/// the collided messages. +/// +/// [birthday paradox]: https://en.wikipedia.org/wiki/Birthday_attack#Digital_signature_susceptibility +/// [Finding Collisions in the Full SHA-1]: https://link.springer.com/chapter/10.1007/11535218_2 +/// [The first collision for full SHA-1]: https://shattered.io/ +/// [SHA-1 is a Shambles]: https://sha-mbles.github.io/ +/// +/// A chosen-prefix collision attack works as follows: an attacker +/// chooses two arbitrary message prefixes, and then searches for +/// so-called near collision blocks. These near collision blocks +/// cause the internal state of the hashes to converge and eventually +/// result in a collision, i.e., an identical hash value. The attack +/// described in the [SHA-1 is a Shambles] paper requires 8 to 10 near +/// collision blocks (512 to 640 bytes) to fully synchronize the +/// internal state. +/// +/// SHA-1 is a [Merkle-Damgård hash function]. This means that the +/// hash function processes blocks one after the other, and the +/// internal state of the |