diff options
author | Justus Winter <justus@sequoia-pgp.org> | 2023-06-19 17:19:50 +0200 |
---|---|---|
committer | Justus Winter <justus@sequoia-pgp.org> | 2023-06-20 12:05:59 +0200 |
commit | f4b7910a960f36b9525c871c894295e29c3fc9b6 (patch) | |
tree | 48c1bef336ee706631eaab403a2e990b00e4ffa6 | |
parent | cbdab5a9a53e721b63c91d9d0c7c3ad0d5a5abf0 (diff) |
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.
-rw-r--r-- | openpgp/NEWS | 4 | ||||
-rw-r--r-- | openpgp/src/parse.rs | 28 | ||||
-rw-r--r-- | openpgp/tests/data/keys/leading-zeros-private.pgp | 16 |
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----- |