rpgp / rpgp

OpenPGP implemented in pure Rust, permissively licensed
https://docs.rs/pgp
Apache License 2.0
798 stars 76 forks source link

signature time is not equal before and after parsing #95

Open drahnr opened 4 years ago

drahnr commented 4 years ago

I tried the following where signing key and verification key are both loaded successfully.

        let digest = &[0u8; 32][..];

        // stage 1
        let signature = signing_key
            .create_signature(passwd_fn, ::pgp::crypto::HashAlgorithm::SHA2_256, digest)
            .expect("Failed to crate signature");

        verification_key
            .verify_signature(
                ::pgp::crypto::HashAlgorithm::SHA2_256,
                digest,
                &signature,
            )
            .expect("Failed to validate signature");

        // stage 2: check parsing success
        let wrapped = ::pgp::Signature::new(
            ::pgp::types::Version::Old,
            ::pgp::packet::SignatureVersion::V4,
            ::pgp::packet::SignatureType::Binary,
            ::pgp::crypto::public_key::PublicKeyAlgorithm::RSA,
             ::pgp::crypto::hash::HashAlgorithm::SHA2_256,
            [digest[0], digest[1]],
            signature,
            vec![
                ::pgp::packet::Subpacket::SignatureCreationTime(::chrono::offset::Utc::now()),
                ::pgp::packet::Subpacket::Issuer(signing_key.key_id()),
            ],
             vec![],
        );

        let mut x = Vec::with_capacity(1024);
        use std::io::Cursor;

        let mut buff = Cursor::new(&mut x);
        ::pgp::packet::write_packet(&mut buff, &wrapped).expect("Write should be ok");

        log::debug!("{:02x?}", &x[0..15]);
        use itertools::Itertools;
        let mut parser = ::pgp::packet::PacketParser::new(x.as_slice());
        assert!(parser.any(|packet| {
            match packet {
                Ok(::pgp::packet::Packet::Signature(sig_packet)) => { dbg!(sig_packet) == wrapped },
                x => { let _ = dbg!(x); false}
            }
        }));

but the equivalence sign never seems to trigger, there is one instance of a key packet, and from the debugger view it seems to be equivalent.

Is there any issue in the above code?

drahnr commented 4 years ago
[src/crypto/pgp.rs:225] sig_packet = Signature {
    packet_version: Old,
    config: SignatureConfig {
        version: V4,
        typ: Binary,
        pub_alg: RSA,
        hash_alg: SHA2_256,
        created: None,
        issuer: None,
        unhashed_subpackets: [],
        hashed_subpackets: [
            SignatureCreationTime(
                2020-04-17T17:27:22Z,
            ),
            Issuer(
                KeyId(cfd331925ab27f39),
            ),
        ],
    },
    signed_hash_value: "0102",
    signature: ["3165303b7917d174d5e2bf9c346fbab2ec556e265522a4262e83127ab120784e07e03888116b6a75f08ccd1750de36c255bd9a9c9574c388c7068560faac595be8b22d28a9dde88b22da09296294f68235713bb9e7f52f0d782e25230ef29ea4b42838348b53e108944368c2bd160ac8d72c0e32b0c31d1ca93d4d60a682f3991b03bcd0e523ca8f1c28e43602ddd427fa26a776253e9c18c3d4bd9afca0b3b0558bbbad37ca38deeeccaf5909de8f2d32b1127bb3af68b2f0788bd125336cf200ac119260b0b6236e9e6611a5f8ba051309699331cead713738f1543120dd793343c47dde82ce426bcce1e84fb1811bbdd7ea204b11c2c33efa33e059d9e2d3"],
}
[src/crypto/pgp.rs:225] &wrapped = Signature {
    packet_version: Old,
    config: SignatureConfig {
        version: V4,
        typ: Binary,
        pub_alg: RSA,
        hash_alg: SHA2_256,
        created: None,
        issuer: None,
        unhashed_subpackets: [],
        hashed_subpackets: [
            SignatureCreationTime(
                2020-04-17T17:27:22.838539440Z,
            ),
            Issuer(
                KeyId(cfd331925ab27f39),
            ),
        ],
    },
    signed_hash_value: "0102",
    signature: ["3165303b7917d174d5e2bf9c346fbab2ec556e265522a4262e83127ab120784e07e03888116b6a75f08ccd1750de36c255bd9a9c9574c388c7068560faac595be8b22d28a9dde88b22da09296294f68235713bb9e7f52f0d782e25230ef29ea4b42838348b53e108944368c2bd160ac8d72c0e32b0c31d1ca93d4d60a682f3991b03bcd0e523ca8f1c28e43602ddd427fa26a776253e9c18c3d4bd9afca0b3b0558bbbad37ca38deeeccaf5909de8f2d32b1127bb3af68b2f0788bd125336cf200ac119260b0b6236e9e6611a5f8ba051309699331cead713738f1543120dd793343c47dde82ce426bcce1e84fb1811bbdd7ea204b11c2c33efa33e059d9e2d3"],
}
thread 'crypto::pgp::test::verify_pgp_crate' panicked at 'assertion failed: parser.any(|packet|
               {
                   match packet {
                       Ok(::pgp::packet::Packet::Signature(sig_packet)) => {
                           dbg!(sig_packet) == *dbg!(& wrapped)
                       }
                       x => { let _ = dbg!(x); false }
                   }
               })', src/crypto/pgp.rs:223:9

it seems the creation time is slightly off..

drahnr commented 4 years ago

"Workaround"

        // accuracy of serialized format is only down to seconds
        use ::chrono::offset::TimeZone;
        let now = ::chrono::offset::Utc::now();
        let now = ::chrono::offset::Utc.timestamp(now.timestamp(), 0u32);
dignifiedquire commented 4 years ago

yeah, there is limited precision in the pgp packets for this. Any suggestions how the api could be improved?

drahnr commented 4 years ago

I would simply do a custom PartialEq impl which does the above conversion or ensure the precision already during the struct construction.

dignifiedquire commented 4 years ago

Ah yes, that makes sense