openpgp: Accept slightly malformed MPIs when reading secrets.

  - Apparently, some OpenPGP implementations create malformed secret
    keys that have MPIs with leading zeros.  Not accepting those seems
    not helpful.

  - Fixes #1024.

3 files changed, 42 insertions, 6 deletions

diff --git a/openpgp/NEWS b/openpgp/NEWS
index 7ccf8727..0b889aab 100644
--- a/openpgp/NEWS
+++ b/openpgp/NEWS
@@ -4,6 +4,10 @@
 #+STARTUP: content hidestars
 
 * Changes in 1.17.0
+** Notable fixes
+   - Sequoia now ignores some formatting errors when reading secret
+     keys.  Being lenient in this case helps the user recover their
+     valuable key material.
 ** New functionality
    - types::AEADAlgorithm::GCM
 ** Deprecated functionality
diff --git a/openpgp/src/parse.rs b/openpgp/src/parse.rs
index d273eb44..11480d75 100644
--- a/openpgp/src/parse.rs
+++ b/openpgp/src/parse.rs
@@ -2955,18 +2955,22 @@ impl MPI {
     fn parse(name_len: &'static str,
              name: &'static str,
              php: &mut PacketHeaderParser<'_>) -> Result<Self> {
-        Ok(MPI::parse_common(name_len, name, false, php)?.into())
+        Ok(MPI::parse_common(name_len, name, false, false, php)?.into())
     }
 
     /// Parses an OpenPGP MPI.
     ///
-    /// See [Section 3.2 of RFC 4880] for details.
+    /// If `parsing_secrets` is `true`, errors are normalized as not
+    /// to reveal parts of the plaintext to the caller.
     ///
-    ///   [Section 3.2 of RFC 4880]: https://tools.ietf.org/html/rfc4880#section-3.2
+    /// If `lenient_parsing` is `true`, this function will accept MPIs
+    /// that are not well-formed (notably, issues related to leading
+    /// zeros).
     fn parse_common(
         name_len: &'static str,
         name: &'static str,
         parsing_secrets: bool,
+        lenient_parsing: bool,
         php: &mut PacketHeaderParser<'_>)
                  -> Result<Vec<u8>> {
         // When we are parsing secrets, we don't want to leak it
@@ -3017,7 +3021,7 @@ impl MPI {
             let mask = !((1 << (8 - unused_bits)) - 1);
             let unused_value = value[0] & mask;
 
-            if unused_value != 0 {
+            if unused_value != 0 && ! lenient_parsing {
                 return uniform_error_for_secrets(
                     Error::MalformedMPI(
                         format!("{} unused bits not zeroed: ({:x})",
@@ -3027,7 +3031,7 @@ impl MPI {
         }
 
         let first_used_bit = 8 - unused_bits;
-        if value[0] & (1 << (first_used_bit - 1)) == 0 {
+        if value[0] & (1 << (first_used_bit - 1)) == 0 && ! lenient_parsing {
             return uniform_error_for_secrets(
                 Error::MalformedMPI(
                     format!("leading bit is not set: \
@@ -3068,7 +3072,7 @@ impl ProtectedMPI {
     fn parse(name_len: &'static str,
              name: &'static str,
              php: &mut PacketHeaderParser<'_>) -> Result<Self> {
-        Ok(MPI::parse_common(name_len, name, true, php)?.into())
+        Ok(MPI::parse_common(name_len, name, true, true, php)?.into())
     }
 }
 impl PKESK {
@@ -6506,4 +6510,16 @@ heLBX8Pq0kUBwQz2iFAzRwOdgTBvH5KsDU9lmE
 -----END PGP MESSAGE-----
 ");
     }
+
+    /// Tests issue 1024.
+    #[test]
+    fn parse_secret_with_leading_zeros() -> Result<()> {
+        crate::Cert::from_bytes(
+            crate::tests::key("leading-zeros-private.pgp"))?
+            .primary_key().key().clone()
+            .parts_into_secret()?
+            .decrypt_secret(&("hunter22"[..]).into())?
+            .into_keypair()?;
+        Ok(())
+    }
 }
diff --git a/openpgp/tests/data/keys/leading-zeros-private.pgp b/openpgp/tests/data/keys/leading-zeros-private.pgp
new file mode 100644
index 00000000..b21b3600
--- /dev/null
+++ b/openpgp/tests/data/keys/leading-zeros-private.pgp
@@ -0,0 +1,16 @@
+-----BEGIN PGP PRIVATE KEY BLOCK-----
+
+lIYEZHeoxhYJKwYBBAHaRw8BAQdA2jbzJ3Ol+88vmu6SM52bEGvxfft/j/+8y+89
+r9aUJXb+BwMCyL9vofICblT/6SFwgNGSk7c1vEA92d4yWFC08QTQhVZ6I100v4Po
+ml0uQ26F/iQl2c2nYWdlY0qizQJDxVBCmINMIuNMqNZH11oDwFTmtLQScGFzc3dk
+QGV4YW1wbGUuY29tiJkEExYKAEEWIQRVYU+0V0KlZhEucuCg9CG4lHDkpwUCZHeo
+xgIbAwUJA8JnAAULCQgHAgIiAgYVCgkICwIEFgIDAQIeBwIXgAAKCRCg9CG4lHDk
+pxwyAP9lDs654VolDwhO3HDBfLm8vKd5UxtB3B5cO7RZGG4OZQD/RzsK7gY4ZLk1
+7FRmt2tyoGkDZ1VMqiw+GwuQgf/TbQSciwRkd6jGEgorBgEEAZdVAQUBAQdAin8I
+t5hyfn1f19Cx4xTt9id46Ym4ZCy7x1vUnAuoxgMDAQgH/gcDAsRux8aWmWYI/1gL
+mAtVu4lyKQLWlL3eeYi0uMg202vDPLOrof8xR6imGoEw6sSqeuv9i8EsrXJksIOP
+cyWhUQCHx3qr8ZWXimEsJmaqDsiIeAQYFgoAIBYhBFVhT7RXQqVmES5y4KD0IbiU
+cOSnBQJkd6jGAhsMAAoJEKD0IbiUcOSnZwQBAKsYHNXJOQI2suV5QUDCNuVcY4Hc
+KceLlPfzxDx/MmCwAQC0eoFExqefCSSymcidjyHDy23QR+LulFXgxpFnvXGvCQ==
+=dayD
+-----END PGP PRIVATE KEY BLOCK-----