openpgp: Fix how text signatures are hashed.

  - When hashing text signatures in which `cr`, `lf`, and `crlf` are
    normalized to `crlf`, if a `crlf` was split across two hash
    updates, two `crlf`s would be hashed (one for the final `cr` in
    the first update, and one for the leading `lf` in the second
    update) instead of just one.

  - Fix it.

  - Fixes #960.

7 files changed, 247 insertions, 34 deletions

diff --git a/.gitattributes b/.gitattributes
index 6313b56c..0fd8d54d 100644
--- a/.gitattributes
+++ b/.gitattributes
@@ -1 +1,6 @@
 * text=auto eol=lf
+
+# Don't do line ending conversions on these files: these are reference data
+# and we really want them as-is.
+openpgp/tests/data/messages/crlf-straddles-chunks.txt binary
+openpgp/tests/data/messages/text-signature-notation-has-lf.txt binary
diff --git a/openpgp/src/parse/hashed_reader.rs b/openpgp/src/parse/hashed_reader.rs
index 3ac13b2b..e122a1db 100644
--- a/openpgp/src/parse/hashed_reader.rs
+++ b/openpgp/src/parse/hashed_reader.rs
@@ -29,6 +29,10 @@ pub(crate) enum HashingMode<T> {
     ///
     /// The data is hashed with line endings normalized to `\r\n`.
     Text(T),
+
+    /// Like Text, but the last character that we hashed was a '\r'
+    /// that we converted to a '\r\n'.
+    TextLastWasCr(T),
 }
 
 impl<T: std::fmt::Debug> std::fmt::Debug for HashingMode<T> {
@@ -37,6 +41,7 @@ impl<T: std::fmt::Debug> std::fmt::Debug for HashingMode<T> {
         match self {
             Binary(t) => write!(f, "Binary({:?})", t),
             Text(t) => write!(f, "Text({:?})", t),
+            TextLastWasCr(t) => write!(f, "Text(last was CR, {:?})", t),
         }
     }
 }
@@ -46,7 +51,12 @@ impl<T: PartialEq> PartialEq for HashingMode<T> {
         use self::HashingMode::*;
         match (self, other) {
             (Binary(s), Binary(o)) => s.eq(o),
+
             (Text(s), Text(o)) => s.eq(o),
+            (TextLastWasCr(s), Text(o)) => s.eq(o),
+            (Text(s), TextLastWasCr(o)) => s.eq(o),
+            (TextLastWasCr(s), TextLastWasCr(o)) => s.eq(o),
+
             _ => false,
         }
     }
@@ -58,6 +68,7 @@ impl<T> HashingMode<T> {
         match self {
             Binary(t) => Binary(f(t)),
             Text(t) => Text(f(t)),
+            TextLastWasCr(t) => TextLastWasCr(f(t)),
         }
     }
 
@@ -66,6 +77,7 @@ impl<T> HashingMode<T> {
         match self {
             Binary(t) => t,
             Text(t) => t,
+            TextLastWasCr(t) => t,
         }
     }
 
@@ -74,6 +86,7 @@ impl<T> HashingMode<T> {
         match self {
             Binary(t) => t,
             Text(t) => t,
+            TextLastWasCr(t) => t,
         }
     }
 
@@ -90,45 +103,81 @@ impl<T> HashingMode<T> {
         match self {
             Binary(t) => t,
             Text(t) => t,
+            TextLastWasCr(t) => t,
         }
     }
 }
 
 impl<D> HashingMode<D>
-    where D: Digest
+    where D: Digest + Clone
 {
     /// Updates the given hash context.  When in text mode, normalize
     /// the line endings to "\r\n" on the fly.
     pub(crate) fn update(&mut self, data: &[u8]) {
-        match self {
-            HashingMode::Text(h) => {
-                let mut line = data;
-                while ! line.is_empty() {
-                    let mut next = 0;
-                    for (i, c) in line.iter().cloned().enumerate() {
-                        match c {
-                            b'\r' | b'\n' => {
-                                h.update(&line[..i]);
-                                h.update(b"\r\n");
-                                next = i + 1;
-                                if c == b'\r' && line.get(next) == Some(&b'\n') {
-                                    next += 1;
-                                }
-                                break;
-                            },
-                            _ => (),
-                        }
-                    }
+        if data.is_empty() {
+            // This isn't just a short circuit.  It preserves
+            // `last_was_cr`, which running through the code would
+            // not.
+            return;
+        }
 
-                    if next > 0 {
-                        line = &line[next..];
-                    } else {
-                        h.update(line);
+        let (h, mut last_was_cr) = match self {
+            HashingMode::Text(h) => (h, false),
+            HashingMode::TextLastWasCr(h) => (h, true),
+            HashingMode::Binary(h) => return h.update(data),
+        };
+
+        let mut line = data;
+        let last_is_cr = line.last() == Some(&b'\r');
+        while ! line.is_empty() {
+            let mut next = 0;
+            for (i, c) in line.iter().cloned().enumerate() {
+                match c {
+                    b'\n' if last_was_cr => {
+                        // We already hash the \n.
+                        assert_eq!(i, 0);
+                        assert_eq!(next, 0);
+                        next = 1;
+                        break;
+                    },
+                    b'\r' | b'\n' => {
+                        h.update(&line[..i]);
+                        h.update(b"\r\n");
+                        next = i + 1;
+                        if c == b'\r' && line.get(next) == Some(&b'\n') {
+                            next += 1;
+                        }
                         break;
-                    }
+                    },
+                    _ => (),
                 }
+                last_was_cr = false;
+            }
+
+            if next > 0 {
+                line = &line[next..];
+            } else {
+                h.update(line);
+                break;
             }
-            HashingMode::Binary(h) => h.update(data),
+        }
+
+        match (&mut *self, last_is_cr) {
+            (&mut HashingMode::Text(_), false) => {
+                // This is the common case.  Getting a crlf that is
+                // split across two chunks is extremely rare.  Hence,
+                // the clones used to change the variant are rarely
+                // needed.
+            },
+            (&mut HashingMode::Text(ref mut h), true) => {
+                *self = HashingMode::TextLastWasCr(h.clone());
+            }
+            (&mut HashingMode::TextLastWasCr(ref mut h), false) => {
+                *self = HashingMode::Text(h.clone());
+            },
+            (&mut HashingMode::TextLastWasCr(_), true) => (),
+
+            _ => unreachable!("handled above"),
         }
     }
 }
@@ -527,14 +576,20 @@ mod test {
             "one\rtwo\rthree",
             "one\ntwo\r\nthree",
         ] {
-            let mut ctx = HashingMode::Text(HashAlgorithm::SHA256.context()?);
-            ctx.update(text.as_bytes());
-            let mut ctx = ctx.into_inner();
-            let mut digest = vec![0; ctx.digest_size()];
-            let _ = ctx.digest(&mut digest);
-            assert_eq!(
-                &crate::fmt::hex::encode(&digest),
-                "5536758151607BB81CE8D6F49189B2E84763DA9EA84965AB7327E704DAE415EB");
+            for chunk_size in &[ text.len(), 1 ] {
+                let mut ctx
+                    = HashingMode::Text(HashAlgorithm::SHA256.context()?);
+                for chunk in text.as_bytes().chunks(*chunk_size) {
+                    ctx.update(chunk);
+                }
+                let mut ctx = ctx.into_inner();
+                let mut digest = vec![0; ctx.digest_size()];
+                let _ = ctx.digest(&mut digest);
+                assert_eq!(
+                    &crate::fmt::hex::encode(&digest),
+                    "5536758151607BB81CE8D6F49189B2E84763DA9EA84965AB7327E704DAE415EB",
+                    "{:?}, chunk size: {}", text, chunk_size);
+            }
         }
         Ok(())
     }
diff --git a/openpgp/src/parse/stream.rs b/openpgp/src/parse/stream.rs
index 8013e107..cd9818c9 100644
--- a/openpgp/src/parse/stream.rs
+++ b/openpgp/src/parse/stream.rs
@@ -3174,6 +3174,21 @@ mod test {
              true,
              Some(crate::frozen_time()),
              VHelper::new(0, 0, 0, 1, certs.clone())),
+            // A signed message where the signature type is text and a
+            // crlf straddles two chunks.
+            (crate::tests::message("crlf-straddles-chunks.txt.sig").to_vec(),
+             crate::tests::message("crlf-straddles-chunks.txt").to_vec(),
+             false,
+             None,
+             VHelper::new(1, 0, 0, 0, certs.clone())),
+            // Like crlf-straddles-chunks, but the signature includes a
+            // notation with a '\n'.  Make sure it is not converted to
+            // a '\r\n'.
+            (crate::tests::message("text-signature-notation-has-lf.txt.sig").to_vec(),
+             crate::tests::message("text-signature-notation-has-lf.txt").to_vec(),
+             false,
+             None,
+             VHelper::new(1, 0, 0, 0, certs.clone())),
         ];
 
         for (i, (signed, reference, test_decryptor, time, r))
diff --git a/openpgp/tests/data/messages/crlf-straddles-chunks.txt b/openpgp/tests/data/messages/crlf-straddles-chunks.txt
new file mode 100644
index 00000000..97ee4e4b
--- /dev/null
+++ b/openpgp/tests/data/messages/crlf-straddles-chunks.txt
@@ -0,0 +1,39 @@
+***-****: *********/*****; ********="*********************************"

+

+--*********************************

+*******-****: ****/*****; *******=*******-****

+*******-********: **-**

+*******-********-********: ******-*********

+

+** **.**.** ** **:** ******* **** *. ********:

+>=**

+>=**

+***** ****,

+

+****** ******* *** ***** **** ***** *****, ********** ***** *** ********

+************* *** **** *** *****.

+*=******** ** *** *** ***** ******* ****** *********** ************?

+

+****** **** *** ***** **=**=***, ****

+

+--=**

+**. **** *****=******

+- ********************* -

+*****************************************

+

+************** ***|****

+**************=*** **

+***** ******

+***  *** *** ** **-*

+***  *** *** ** **-**

+************** *********: ****://***.**************-***-****.**/*********=

+

+

+***** **** - ************ *=*** *********** ***** *** ************

+******** (**=*******)

+**: *=***** ******* *** **. ****** ******* | ** ************** *** ******=

+

+* | ***-****. ** *********

+

+

+--*********************************--

diff --git a/openpgp/tests/data/messages/crlf-straddles-chunks.txt.sig b/openpgp/tests/data/messages/crlf-straddles-chunks.txt.sig
new file mode 100644
index 00000000..82954bc2
--- /dev/null
+++ b/openpgp/tests/data/messages/crlf-straddles-chunks.txt.sig
@@ -0,0 +1,30 @@
+-----BEGIN PGP MESSAGE-----
+
+kA0DAQgWIuP6/pa1bDIBy+p0AGOWT3cqKiotKioqKjogKioqKioqKioqLyoqKioq
+OyAqKioqKioqKj0iKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqIg0K
+DQotLSoqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKg0KKioqKioqKi0q
+KioqOiAqKioqLyoqKioqOyAqKioqKioqPSoqKioqKiotKioqKg0KKioqKioqKi0q
+KioqKioqKjogKiotKioNCioqKioqKiotKioqKioqKiotKioqKioqKio6ICoqKioq
+Ki0qKioqKioqKioNCg0KKiogKiouKiouKiogKiogKio6KiogKioqKioqKiAqKioq
+ICouICoqKioqKioqOg0KPj0qKg0KPj0qKg0KKioqKiogKioqKiwNCg0KKioqKioq
+ICoqKioqKiogKioqICoqKioqICoqKiogKioqKiogKioqKiosICoqKioqKioqKiog
+KioqKiogKioqICoqKioqKioqDQoqKioqKioqKioqKioqICoqKiAqKioqICoqKiAq
+KioqKi4NCio9KioqKioqKiogKiogKioqICoqKiAqKioqKiAqKioqKioqICoqKioq
+KiAqKioqKioqKioqKiAqKioqKioqKioqKio/DQoNCioqKioqKiAqKioqICoqKiAq
+KioqKiAqKj0qKj0qKiosICoqKioNCg0KLS09KioNCioqLiAqKioqICoqKioqPSoq
+KioqKg0KLSAqKioqKioqKioqKioqKioqKioqKiogLQ0KKioqKioqKioqKioqKioq
+KioqKioqKioqKioqKioqKioqKioqKioqKioNCg0KKioqKioqKioqKioqKiogKioq
+fCoqKioNCioqKioqKioqKioqKioqPSoqKiAqKg0KKioqKiogKioqKioqDQoqKiog
+ICoqKiAqKiogKiogKiotKg0KKioqICAqKiogKioqICoqICoqLSoqDQoqKioqKioq
+KioqKioqKiAqKioqKioqKio6ICoqKio6Ly8qKiouKioqKioqKioqKioqKiotKioq
+LSoqKiouKiovKioqKioqKioqPQ0KDQoNCioqKioqICoqKiogLSAqKioqKioqKioq
+KiogKj0qKiogKioqKioqKioqKiogKioqKiogKioqICoqKioqKioqKioqKg0KKioq
+KioqKiogKCoqPSoqKioqKiopDQoqKjogKj0qKioqKiAqKioqKioqICoqKiAqKi4g
+KioqKioqICoqKioqKiogfCAqKiAqKioqKioqKioqKioqKiAqKiogKioqKioqPQ0K
+DQoqIHwgKioqLSoqKiouICoqICoqKioqKioqKg0KDQoNKAotLSoqKioqKioqKioq
+KioqKioqKioqKioqKioqKioqKioqKi0tDQqIdQQBFggAHRYhBAYcPKRK/w7FjcZu
+lSLj+v6WtWwyBQJjlk93AAoJECLj+v6WtWwy0m0BALhtJyecpEXQxawZqwjQTU0c
+URq8XixAuZeXmZjl3b6PAQDfGduoy+AmNUxPyOkzqVWllQwIi+ee5SlRj1WDHZD+
+Bg==
+=OJc6
+-----END PGP MESSAGE-----
diff --git a/openpgp/tests/data/messages/text-signature-notation-has-lf.txt b/openpgp/tests/data/messages/text-signature-notation-has-lf.txt
new file mode 100644
index 00000000..97ee4e4b
--- /dev/null
+++ b/openpgp/tests/data/messages/text-signature-notation-has-lf.txt
@@ -0,0 +1,39 @@
+***-****: *********/*****; ********="*********************************"

+

+--*********************************

+*******-****: ****/*****; *******=*******-****

+*******-********: **-**

+*******-********-********: ******-*********

+

+** **.**.** ** **:** ******* **** *. ********:

+>=**

+>=**

+***** ****,

+

+****** ******* *** ***** **** ***** *****, ********** ***** *** ********

+************* *** **** *** *****.

+*=******** ** *** *** ***** ******* ****** *********** ************?

+

+****** **** *** ***** **=**=***, ****

+

+--=**

+**. **** *****=******

+- ********************* -

+*****************************************

+

+************** ***|****

+**************=*** **

+***** ******

+***  *** *** ** **-*

+***  *** *** ** **-**

+************** *********: ****://***.**************-***-****.**/*********=

+

+

+***** **** - ************ *=*** *********** ***** *** ************

+******** (**=*******)

+**: *=***** ******* *** **. ****** ******* | ** ************** *** ******=

+

+* | ***-****. ** *********

+

+

+--*********************************--

diff --git a/openpgp/tests/data/messages/text-signature-notation-has-lf.txt.sig b/openpgp/tests/data/messages/text-signature-notation-has-lf.txt.sig
new file mode 100644
index 00000000..54086207
--- /dev/null
+++ b/openpgp/tests/data/messages/text-signature-notation-has-lf.txt.sig
@@ -0,0 +1,30 @@
+-----BEGIN PGP MESSAGE-----
+
+kA0DAQgWIuP6/pa1bDIBy+p0AGOWUTcqKiotKioqKjogKioqKioqKioqLyoqKioq
+OyAqKioqKioqKj0iKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqIg0K
+DQotLSoqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKg0KKioqKioqKi0q
+KioqOiAqKioqLyoqKioqOyAqKioqKioqPSoqKioqKiotKioqKg0KKioqKioqKi0q
+KioqKioqKjogKiotKioNCioqKioqKiotKioqKioqKiotKioqKioqKio6ICoqKioq
+Ki0qKioqKioqKioNCg0KKiogKiouKiouKiogKiogKio6KiogKioqKioqKiAqKioq
+ICouICoqKioqKioqOg0KPj0qKg0KPj0qKg0KKioqKiogKioqKiwNCg0KKioqKioq
+ICoqKioqKiogKioqICoqKioqICoqKiogKioqKiogKioqKiosICoqKioqKioqKiog
+KioqKiogKioqICoqKioqKioqDQoqKioqKioqKioqKioqICoqKiAqKioqICoqKiAq
+KioqKi4NCio9KioqKioqKiogKiogKioqICoqKiAqKioqKiAqKioqKioqICoqKioq
+KiAqKioqKioqKioqKiAqKioqKioqKioqKio/DQoNCioqKioqKiAqKioqICoqKiAq
+KioqKiAqKj0qKj0qKiosICoqKioNCg0KLS09KioNCioqLiAqKioqICoqKioqPSoq
+KioqKg0KLSAqKioqKioqKioqKioqKioqKioqKiogLQ0KKioqKioqKioqKioqKioq
+KioqKioqKioqKioqKioqKioqKioqKioqKioNCg0KKioqKioqKioqKioqKiogKioq
+fCoqKioNCioqKioqKioqKioqKioqPSoqKiAqKg0KKioqKiogKioqKioqDQoqKiog
+ICoqKiAqKiogKiogKiotKg0KKioqICAqKiogKioqICoqICoqLSoqDQoqKioqKioq
+KioqKioqKiAqKioqKioqKio6ICoqKio6Ly8qKiouKioqKioqKioqKioqKiotKioq
+LSoqKiouKiovKioqKioqKioqPQ0KDQoNCioqKioqICoqKiogLSAqKioqKioqKioq
+KiogKj0qKiogKioqKioqKioqKiogKioqKiogKioqICoqKioqKioqKioqKg0KKioq
+KioqKiogKCoqPSoqKioqKiopDQoqKjogKj0qKioqKiAqKioqKioqICoqKiAqKi4g
+KioqKioqICoqKioqKiogfCAqKiAqKioqKioqKioqKioqKiAqKiogKioqKioqPQ0K
+DQoqIHwgKioqLSoqKiouICoqICoqKioqKioqKg0KDQoNKAotLSoqKioqKioqKioq
+KioqKioqKioqKioqKioqKioqKioqKi0tDQqIkgQBFggAOhYhBAYcPKRK/w7FjcZu
+lSLj+v6WtWwyBQJjllE3HBSAAAAAABAAA2NybGZAZXhhbXBsZS5vcmd4CnkACgkQ
+IuP6/pa1bDIfFwD/UqobVcQPz912db//JwoVK2Cv39ws6hCKOVa+FE21JCQA/2Gg
+bURJotQMIZVKy1cdC3C4F5/YOThxfot3JAwLBtkD
+=LmbW
+-----END PGP MESSAGE-----