From 2310819af54cf2849861675e6f5c8783c7d55608 Mon Sep 17 00:00:00 2001 From: "Neal H. Walfield" Date: Tue, 15 Dec 2020 11:43:06 +0100 Subject: openpgp: Extend StandardPolicy's hash policy API. - A `Policy` now knows whether the use of a hash requires collision resistance or only second pre-image resistance. - Extend `StandardPolicy`'s hash policy API to allow a user to express a more nuanced policy that takes this information into account. - See #595. --- openpgp/src/cert.rs | 13 ++- openpgp/src/policy.rs | 262 +++++++++++++++++++++++++++++++++++++++++--------- 2 files changed, 223 insertions(+), 52 deletions(-) diff --git a/openpgp/src/cert.rs b/openpgp/src/cert.rs index e7eb32c3..bc32f0b6 100644 --- a/openpgp/src/cert.rs +++ b/openpgp/src/cert.rs @@ -3482,6 +3482,7 @@ mod test { use crate::policy::StandardPolicy as P; use crate::types::Curve; use crate::packet::signature; + use crate::policy::HashAlgoSecurity; use super::*; use crate::{ @@ -5322,10 +5323,13 @@ Pu1xwz57O4zo1VYf6TqHJzVC3OMvMUM2hhdecMUe5x6GorNaj6g= #[test] fn canonicalize_with_v3_sig() -> Result<()> { // This test relies on being able to validate SHA-1 - // signatures. The standard policy reject SHA-1. So, use a + // signatures. The standard policy rejects SHA-1. So, use a // custom policy. let p = &P::new(); - let sha1 = p.hash_cutoff(HashAlgorithm::SHA1).unwrap(); + let sha1 = + p.hash_cutoff( + HashAlgorithm::SHA1, HashAlgoSecurity::CollisionResistance) + .unwrap(); let p = &P::at(sha1 - std::time::Duration::from_secs(1)); let cert = Cert::from_bytes( @@ -5657,8 +5661,9 @@ Pu1xwz57O4zo1VYf6TqHJzVC3OMvMUM2hhdecMUe5x6GorNaj6g= Cert::from_bytes(crate::tests::key("peter-sha1-backsig.pgp"))?; let p = &crate::policy::NullPolicy::new(); assert_eq!(cert.with_policy(p, None)?.keys().for_signing().count(), 1); - let p = &crate::policy::StandardPolicy::new(); - assert_eq!(cert.with_policy(p, None)?.keys().for_signing().count(), 0); + let mut p = crate::policy::StandardPolicy::new(); + p.reject_hash(HashAlgorithm::SHA1); + assert_eq!(cert.with_policy(&p, None)?.keys().for_signing().count(), 0); Ok(()) } diff --git a/openpgp/src/policy.rs b/openpgp/src/policy.rs index 4c66ef19..2842f9cb 100644 --- a/openpgp/src/policy.rs +++ b/openpgp/src/policy.rs @@ -409,12 +409,13 @@ pub enum HashAlgoSecurity { /// - Primary key binding signatures /// - Self revocations /// - /// Due to the structure of User IDs (they are short UTF-8 encoded - /// RFC 2822 mailboxes), self signatures over short, reasonable - /// User IDs (**not** User Attributes) also don't require - /// collision resistance: + /// Due to the structure of User IDs (they are normally short, + /// UTF-8 encoded RFC 2822 mailboxes), self signatures over short, + /// reasonable User IDs (**not** User Attributes) also don't + /// require strong collision resistance. Thus, we also only + /// require a signature with second pre-image resistance for: /// - /// - Self signatures over User IDs + /// - Self signatures over reasonable User IDs SecondPreImageResistance, /// The signed data requires collision resistance. /// @@ -424,7 +425,9 @@ pub enum HashAlgoSecurity { /// and third-party revocations. /// /// Note: collision resistance implies second pre-image - /// resistance. + /// resistance. Thus, when evaluating whether a hash algorithm + /// has collision resistance, we also check whether it has second + /// pre-image resistance. CollisionResistance, } @@ -556,7 +559,10 @@ pub struct StandardPolicy<'a> { time: Option, // Hash algorithms. - hash_algos: HashCutoffList, + collision_resistant_hash_algos: + CollisionResistantHashCutoffList, + second_pre_image_resistant_hash_algos: + SecondPreImageResistantHashCutoffList, hash_revocation_tolerance: types::Duration, // Critical subpacket tags. @@ -592,7 +598,10 @@ impl<'a> From<&'a StandardPolicy<'a>> for Option<&'a dyn Policy> { } } -a_cutoff_list!(HashCutoffList, HashAlgorithm, 12, +// Signatures that require a hash with collision Resistance and second +// Pre-image Resistance. See the documentation for HashAlgoSecurity +// for more details. +a_cutoff_list!(CollisionResistantHashCutoffList, HashAlgorithm, 12, [ REJECT, // 0. Not assigned. Some(Timestamp::Y1997), // 1. MD5 @@ -607,6 +616,24 @@ a_cutoff_list!(HashCutoffList, HashAlgorithm, 12, ACCEPT, // 10. SHA512 ACCEPT, // 11. SHA224 ]); +// Signatures that *only* require a hash with Second Pre-image +// Resistance. See the documentation for HashAlgoSecurity for more +// details. +a_cutoff_list!(SecondPreImageResistantHashCutoffList, HashAlgorithm, 12, + [ + REJECT, // 0. Not assigned. + Some(Timestamp::Y2004), // 1. MD5 + Some(Timestamp::Y2023), // 2. SHA-1 + Some(Timestamp::Y2013), // 3. RIPE-MD/160 + REJECT, // 4. Reserved. + REJECT, // 5. Reserved. + REJECT, // 6. Reserved. + REJECT, // 7. Reserved. + ACCEPT, // 8. SHA256 + ACCEPT, // 9. SHA384 + ACCEPT, // 10. SHA512 + ACCEPT, // 11. SHA224 + ]); a_cutoff_list!(SubpacketTagCutoffList, SubpacketTag, 36, [ @@ -754,7 +781,10 @@ impl<'a> StandardPolicy<'a> { const EMPTY_LIST: &'static [&'static str] = &[]; Self { time: None, - hash_algos: HashCutoffList::Default(), + collision_resistant_hash_algos: + CollisionResistantHashCutoffList::Default(), + second_pre_image_resistant_hash_algos: + SecondPreImageResistantHashCutoffList::Default(), // There are 365.2425 days in a year. Use a reasonable // approximation. hash_revocation_tolerance: @@ -814,37 +844,120 @@ impl<'a> StandardPolicy<'a> { } /// Always considers `h` to be secure. + /// + /// A cryptographic hash algorithm normally has three security + /// properties: + /// + /// - Pre-image resistance, + /// - Second pre-image resistance, and + /// - Collision resistance. + /// + /// A hash algorithm should only be unconditionally accepted if it + /// has all three of these properties. See the documentation for + /// [`HashAlgoSecurity`] for more details. + /// + /// [`HashAlgoSecurity`]: enum.HashAlgoSecurity.html pub fn accept_hash(&mut self, h: HashAlgorithm) { - self.hash_algos.set(h, ACCEPT); + self.collision_resistant_hash_algos.set(h, ACCEPT); + self.second_pre_image_resistant_hash_algos.set(h, ACCEPT); } - /// Always considers `h` to be insecure. + /// Considers `h` to be insecure in all security contexts. + /// + /// A cryptographic hash algorithm normally has three security + /// properties: + /// + /// - Pre-image resistance, + /// - Second pre-image resistance, and + /// - Collision resistance. + /// + /// This method causes the hash algorithm to be considered unsafe + /// in all security contexts. + /// + /// See the documentation for [`HashAlgoSecurity`] for more + /// details. + /// + /// [`HashAlgoSecurity`]: enum.HashAlgoSecurity.html + /// + /// To express a more nuanced policy, use + /// [`StandardPolicy::reject_hash_at`] or + /// [`StandardPolicy::reject_hash_property_at`]. + /// + /// [`StandardPolicy::reject_hash_at`]: #method.reject_hash_at + /// [`StandardPolicy::reject_hash_property_at`]: #method.reject_hash_property_at pub fn reject_hash(&mut self, h: HashAlgorithm) { - self.hash_algos.set(h, REJECT); + self.collision_resistant_hash_algos.set(h, REJECT); + self.second_pre_image_resistant_hash_algos.set(h, REJECT); + } + + /// Considers `h` to be insecure in all security contexts starting + /// at time `t`. + /// + /// A cryptographic hash algorithm normally has three security + /// properties: + /// + /// - Pre-image resistance, + /// - Second pre-image resistance, and + /// - Collision resistance. + /// + /// This method causes the hash algorithm to be considered unsafe + /// in all security contexts starting at time `t`. + /// + /// See the documentation for [`HashAlgoSecurity`] for more + /// details. + /// + /// [`HashAlgoSecurity`]: enum.HashAlgoSecurity.html + /// + /// To express a more nuanced policy, use + /// [`StandardPolicy::reject_hash_property_at`]. + /// + /// [`StandardPolicy::reject_hash_property_at`]: #method.reject_hash_property_at + pub fn reject_hash_at(&mut self, h: HashAlgorithm, t: T) + where T: Into>, + { + let t = t.into().and_then(system_time_cutoff_to_timestamp); + self.collision_resistant_hash_algos.set(h, t); + self.second_pre_image_resistant_hash_algos.set(h, t); } - /// Considers `h` to be insecure starting at `normal` for normal - /// signatures and at `revocation` for revocation certificates. + /// Considers `h` to be insecure starting at `t` for the specified + /// security property. + /// + /// A hash algorithm is considered secure if it has all of the + /// following security properties: + /// + /// - Pre-image resistance, + /// - Second pre-image resistance, and + /// - Collision resistance. + /// + /// Some contexts only require a subset of these security + /// properties. Specifically, if an attacker is unable to + /// influence the data that a user signs, then the hash algorithm + /// only needs second pre-image resistance; it doesn't need + /// collision resistance. See the documentation for + /// [`HashAlgoSecurity`] for more details. + /// + /// [`HashAlgoSecurity`]: enum.HashAlgoSecurity.html /// - /// For each algorithm, there are two different cutoffs: when the - /// algorithm is no longer safe for normal use (e.g., binding - /// signatures, document signatures), and when the algorithm is no - /// longer safe for revocations. Normally, an algorithm should be - /// allowed for use in a revocation longer than it should be - /// allowed for normal use, because once we consider a revocation - /// certificate to be invalid, it may cause something else to be - /// considered valid! + /// This method makes it possible to specify different policies + /// depending on the security requirements. /// /// A cutoff of `None` means that there is no cutoff and the - /// algorithm has no known vulnerabilities. + /// algorithm has no known vulnerabilities for the specified + /// security policy. /// - /// As a rule of thumb, we want to stop accepting a Hash algorithm - /// for normal signature when there is evidence that it is broken, - /// and we want to stop accepting it for revocations shortly - /// before collisions become practical. + /// As a rule of thumb, collision resistance is easier to attack + /// than second pre-image resistance. And in practice there are + /// practical attacks against several widely-used hash algorithms' + /// collision resistance, but only theoretical attacks against + /// their second pre-image resistance. Nevertheless, once one + /// property of a hash has been compromised, we want to deprecate + /// its use as soon as it is feasible. Unfortunately, because + /// OpenPGP certificates are long-lived, this can take years. /// - /// As such, we start rejecting [MD5] in 1997 and completely - /// reject it starting in 2004: + /// Given this, we start rejecting [MD5] in cases where collision + /// resistance is required in 1997 and completely reject it + /// starting in 2004: /// /// > In 1996, Dobbertin announced a collision of the /// > compression function of MD5 (Dobbertin, 1996). While this @@ -859,10 +972,11 @@ impl<'a> StandardPolicy<'a> { /// > /// > (Accessed Feb. 2020.) /// - /// [MD5]: https://en.wikipedia.org/wiki/MD5 + /// [MD5]: https://en.wikipedia.org/wiki/MD5 /// - /// And we start rejecting [SHA-1] in 2013 and completely reject - /// it in 2020: + /// And we start rejecting [SHA-1] in cases where collision + /// resistance is required in 2013, and completely reject it in + /// 2023: /// /// > Since 2005 SHA-1 has not been considered secure against /// > well-funded opponents, as of 2010 many organizations have @@ -876,23 +990,52 @@ impl<'a> StandardPolicy<'a> { /// > /// > (Accessed Feb. 2020.) /// - /// [SHA-1]: https://en.wikipedia.org/wiki/SHA-1 + /// [SHA-1]: https://en.wikipedia.org/wiki/SHA-1 + /// + /// There are two main reasons why we have decided to accept SHA-1 + /// for so long. First, as of the end of 2020, there are still a + /// large number of [certificates that rely on SHA-1]. Second, + /// Sequoia uses a variant of SHA-1 called [SHA1CD], which is able + /// to detect and *mitigate* the known attacks on SHA-1's + /// collision resistance. + /// + /// [certificates that rely on SHA-1]: https://gitlab.com/sequoia-pgp/sequoia/-/issues/595 + /// [SHA1CD]: https://github.com/cr-marcstevens/sha1collisiondetection /// /// Since RIPE-MD is structured similarly to SHA-1, we - /// conservatively consider it to be broken as well. - pub fn reject_hash_at(&mut self, h: HashAlgorithm, t: T) + /// conservatively consider it to be broken as well. But, because + /// it is not widely used in the OpenPGP ecosystem, we don't make + /// provisions for it. + /// + /// Note: if a context indicates that it requires collision + /// resistance, then it requires both collision resistance and + /// second pre-image resistance, and both policies must indicate + /// that the hash algorithm can be safely used at the specified + /// time. + pub fn reject_hash_property_at(&mut self, h: HashAlgorithm, + sec: HashAlgoSecurity, t: T) where T: Into>, { - self.hash_algos.set( - h, - t.into().and_then(system_time_cutoff_to_timestamp)); + let t = t.into().and_then(system_time_cutoff_to_timestamp); + match sec { + HashAlgoSecurity::CollisionResistance => + self.collision_resistant_hash_algos.set(h, t), + HashAlgoSecurity::SecondPreImageResistance => + self.second_pre_image_resistant_hash_algos.set(h, t), + } } - /// Returns the cutoff times for the specified hash algorithm. - pub fn hash_cutoff(&self, h: HashAlgorithm) + /// Returns the cutoff time for the specified hash algorithm and + /// security policy. + pub fn hash_cutoff(&self, h: HashAlgorithm, sec: HashAlgoSecurity) -> Option { - self.hash_algos.cutoff(h).map(|t| t.into()) + match sec { + HashAlgoSecurity::CollisionResistance => + self.collision_resistant_hash_algos.cutoff(h), + HashAlgoSecurity::SecondPreImageResistance => + self.second_pre_image_resistant_hash_algos.cutoff(h), + }.map(|t| t.into()) } /// Sets the amount of time to continue to accept revocation @@ -1146,7 +1289,7 @@ impl<'a> StandardPolicy<'a> { } impl<'a> Policy for StandardPolicy<'a> { - fn signature(&self, sig: &Signature, _sec: HashAlgoSecurity) -> Result<()> { + fn signature(&self, sig: &Signature, sec: HashAlgoSecurity) -> Result<()> { let time = self.time.unwrap_or_else(Timestamp::now); let rev = match sig.typ() { @@ -1156,18 +1299,41 @@ impl<'a> Policy for StandardPolicy<'a> { _ => false, }; + // Note: collision resistance requires 2nd pre-image resistance. + if sec == HashAlgoSecurity::CollisionResistance { + if rev { + self + .collision_resistant_hash_algos + .check(sig.hash_algo(), time, + Some(self.hash_revocation_tolerance)) + .context(format!( + "Policy rejected revocation signature ({}) requiring \ + collision resistance", sig.typ()))? + } else { + self + .collision_resistant_hash_algos + .check(sig.hash_algo(), time, None) + .context(format!( + "Policy rejected non-revocation signature ({}) requiring \ + collision resistance", sig.typ()))? + } + } + if rev { self - .hash_algos.check( - sig.hash_algo(), time, Some(self.hash_revocation_tolerance)) + .second_pre_image_resistant_hash_algos + .check(sig.hash_algo(), time, + Some(self.hash_revocation_tolerance)) .context(format!( - "Policy rejected revocation signature ({})", sig.typ()))? + "Policy rejected revocation signature ({}) requiring \ + second pre-image resistance", sig.typ()))? } else { self - .hash_algos.check( - sig.hash_algo(), time, None) + .second_pre_image_resistant_hash_algos + .check(sig.hash_algo(), time, None) .context(format!( - "Policy rejected non-revocation signature ({})", sig.typ()))? + "Policy rejected non-revocation signature ({}) requiring \ + second pre-image resistance", sig.typ()))? } for csp in sig.hashed_area().iter().filter(|sp| sp.critical()) { -- cgit v1.2.3