diff options
author | Justus Winter <justus@sequoia-pgp.org> | 2021-03-22 14:49:19 +0100 |
---|---|---|
committer | Justus Winter <justus@sequoia-pgp.org> | 2021-03-22 14:57:38 +0100 |
commit | e55f5ab5440962f705ec47da7bf8283e1d6b660c (patch) | |
tree | fac77d0f5ba53f7af36535a65d583b5ba468fd3d | |
parent | 2779c2d60678ca36d0553bab3a2b18f25803cf0e (diff) |
openpgp: Add test demonstrating that canonicalization is robust.
- At some point, invalid self-signatures would be mis-classified as
third-party certifications by Cert::canonicalize. As a side-effect,
invalid self-revocations would be considered third-party
revocations, changing the certificates revocation status to
CouldBe. Confusingly, also changing the digest prefix would break
this mis-classification, resulting in a revocation status of
NotAsFarAsWeKnow.
- The underlying issue was fixed in
7afee60b7cf0f19559bfccd8c42fdc77f6b9c655.
- Add a test that demonstrates that bad signatures are now
recognized as such, and that the confusing behavior previously
observed is now consistent.
- Fixes #486.
-rw-r--r-- | openpgp/src/cert.rs | 69 | ||||
-rw-r--r-- | openpgp/src/packet_pile.rs | 2 |
2 files changed, 70 insertions, 1 deletions
diff --git a/openpgp/src/cert.rs b/openpgp/src/cert.rs index 4ef6d27a..010bb919 100644 --- a/openpgp/src/cert.rs +++ b/openpgp/src/cert.rs @@ -6160,4 +6160,73 @@ Pu1xwz57O4zo1VYf6TqHJzVC3OMvMUM2hhdecMUe5x6GorNaj6g= assert_eq!(cert, cert_); Ok(()) } + + /// Checks that messing with a revocation signature merely + /// invalidates the signature and keeps the cert's revocation + /// status unchanged. + #[test] + fn issue_486() -> Result<()> { + use crate::{ + crypto::mpi, + types::RevocationStatus::*, + packet::signature::Signature4, + policy::StandardPolicy, + }; + let p = &StandardPolicy::new(); + + let (cert, revocation) = CertBuilder::new().generate()?; + + // Base case. + let c = cert.clone().insert_packets(Some(revocation.clone()))?; + if let Revoked(_) = c.revocation_status(p, None) { + // cert is considered revoked + } else { + panic!("Should be revoked, but is not: {:?}", + c.revocation_status(p, None)); + } + + // Breaking the revocation signature by changing the MPIs. + let c = cert.clone().insert_packets(Some( + Signature4::new( + revocation.typ(), + revocation.pk_algo(), + revocation.hash_algo(), + revocation.hashed_area().clone(), + revocation.unhashed_area().clone(), + *revocation.digest_prefix(), + // MPI is replaced with a dummy one + mpi::Signature::RSA { + s: mpi::MPI::from(vec![1, 2, 3]) + })))?; + if let NotAsFarAsWeKnow = c.revocation_status(p, None) { + assert_eq!(c.bad_signatures().count(), 1); + } else { + panic!("Should not be revoked, but is: {:?}", + c.revocation_status(p, None)); + } + + // Breaking the revocation signature by changing the MPIs and + // the digest prefix. + let c = cert.clone().insert_packets(Some( + Signature4::new( + revocation.typ(), + revocation.pk_algo(), + revocation.hash_algo(), + revocation.hashed_area().clone(), + revocation.unhashed_area().clone(), + // Prefix replaced with a dummy one + [0, 1], + // MPI is replaced with a dummy one + mpi::Signature::RSA { + s: mpi::MPI::from(vec![1, 2, 3]) + })))?; + if let NotAsFarAsWeKnow = c.revocation_status(p, None) { + assert_eq!(c.bad_signatures().count(), 1); + } else { + panic!("Should not be revoked, but is: {:?}", + c.revocation_status(p, None)); + } + + Ok(()) + } } diff --git a/openpgp/src/packet_pile.rs b/openpgp/src/packet_pile.rs index 2eb841d0..8aef9e82 100644 --- a/openpgp/src/packet_pile.rs +++ b/openpgp/src/packet_pile.rs @@ -96,7 +96,7 @@ use crate::parse::Cookie; /// /// let cert = Cert::try_from(pp)?; /// if let NotAsFarAsWeKnow = cert.revocation_status(policy, None) { -/// // revocation signature is broken and the key is not definitely revoked +/// // revocation signature is broken and the cert is not revoked /// assert_eq!(cert.bad_signatures().count(), 1); /// } /// # else { |