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.

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) -