View Issue Details
|ID||Project||Category||View Status||Date Submitted||Last Update|
|0000266||file||General||public||2021-05-20 22:01||2021-06-08 18:13|
|Summary||0000266: False hits by Magdir/pgp-binary-keys|
|Description||when i run file command version 5.40 on some files with -k option i|
often get also misidentification messages starting with "OpenPGP". See
appended output OpenPGP-bad-k.txt.
When looking inside sources i see that such messages are triggered by
magic lines inside Magdir/pgp-binary-keys. These magic lines should
identify OpenPGP files.
The above mentioned examples are handled by starting lines like
0 ubyte&0xFC =0x94 OpenPGP Secret Key
>&-1 use primary_key_length_old
After inspecting just one byte print a message starting with "OpenPGP
Secret Key" and then do some additional check by calling sub routine
like primary_key_length_old. Obviously checking only 1 byte is not
sufficient. So non PGP examples with starting byte 95h like
mathemusic, PEDE and samples starting with 97h like Event.Tdf,
RIRE6.SPL, Rx.GS and Welcome.Snd are misidentified.
The consistence check is done later by sub routine
pgp_binary_key_pk_check which checks for valid versions range (2-7)
and valid time stamps (after 1990).
The correct way would be to check some possible PGP packets for valid
version and time stamp. If this succeeds then afterwards display some
Furthermore the samples starting with 97h are also described inside
Magdir/pgp in a more unreliable way by line starting with
0 byte 0x97 PGP Secret Sub-key -
So when check and describing part is done by Magdir/pgp-binary-keys
then remove the lines from Magdir/pgp.
Furthermore with --extension option the 31 byte string pgp/gpg/pkr/asd
is shown. For public "foo" extension pkr is used whereas for secret
"foo" the extension "skr" is used. So skr file is missing in the
following magic line: !:ext pgp/gpg/pkr/asd And when doing effort in
inspecting PGP packet for "OpenPGP Public Key" and "OpenPGP Secret
Key" then it would be a good thing to display afterwards the right
file name extension ( pkr or skr).
Furthermore the extension asd is listed as a possibility. As far as i
know i no PGP or related file exist with that extension.
My misidentified examples are stored in appended archive
OpenPGP-bad-k.txt (578 bytes)
Event.Tdf: PGP Secret Sub-key - 1024b created on Fri Sep 21 14:01:04 2057 - IDEA\012- OpenPGP Secret Key\012- data mathemusic: OpenPGP Secret Key\012- (Lepton 3.x), scale 18676-4, spot sensor temperature -0.000000, color scheme 8, calibration: offset 0.000000, slope -302575422733272930000000000000000000.000000\012- data PEDE: OpenPGP Secret Key\012- data RIRE6.SPL: PGP Secret Sub-key -\012- OpenPGP Secret Key\012- data Rx.GS: PGP Secret Sub-key -\012- OpenPGP Secret Key\012- data Welcome.Snd: PGP Secret Sub-key -\012- OpenPGP Secret Key\012- data
OpenPGP-bad-k.txt (578 bytes)
OpenPGP-bad.zip (66,795 bytes)
(I wrote pgp-binary-keys.)
Thanks for the thorough report. I tested a lot of true positives (a large portion of the SKS dump), but it seems I failed to test enough false positives. The code checks a lot of bits, so it should be unambiguous. I suspect the problem is that I just emit "OpenPGP Secret Key" too early.
As for the file extensions, I'm only aware of .pgp and .gpg. The other variants existed in the old version of the code, so I kept them assuming that they used to be used.
I'll take a look in the next few days.
The issue identifies three problems:
1. Descriptions in pgp-binary-key are printed too eagerly.
2. Descriptions in pgp (PGP Secret Sub-key) are printed too eagerly.
3. The extensions listed in pgp-binary-key are wrong.
I've fixed one as j2j suggested. Unfortunately, I can't figure out how to distinguish public and secret keys anymore, because the first byte of the file is not accessible from a function ("use").
The other patch fixes 3. I've changed it to only report pgp and gpg as valid extensions. I've never actually seen srk, prk or adf used in practice and I've been doing PGP stuff for nearly a decade.
0001-Show-information-about-a-PGP-key-only-if-we-have-a-s.patch (2,356 bytes)
From f16113d6d2dd7f6bccc4bf10a4f87af1615acea1 Mon Sep 17 00:00:00 2001 From: "Neal H. Walfield" <firstname.lastname@example.org> Date: Tue, 8 Jun 2021 13:42:59 +0200 Subject: [PATCH 1/2] Show information about a PGP key only if we have a strong match. - OpenPGP is a packet-based format with little information at fixed offsets. - Currently, we print out interesting fields as we encounter them. Unfortunately, the certificate type (public or secret) is the first field. Thus, we've matched less than a byte of data before we print something, which results in false positives. - Delay printing this information until we've matched on the packet's type, sanity checked its size, its version field, and its creation time. - Note: because we use functions ('use' and 'name') we are not able to access the first byte of the file, and thus can no longer indicate whether the key is a public or a private key. - See: https://bugs.astron.com/view.php?id=266 --- magic/Magdir/pgp-binary-keys | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/magic/Magdir/pgp-binary-keys b/magic/Magdir/pgp-binary-keys index 1ce76d90..20e10aaa 100644 --- a/magic/Magdir/pgp-binary-keys +++ b/magic/Magdir/pgp-binary-keys @@ -160,16 +160,16 @@ # The first packet has to be a public key or a secret key. # # New-Style Public Key -0 ubyte =0xC6 OpenPGP Public Key +0 ubyte =0xC6 >&0 use primary_key_length_new # New-Style Secret Key -0 ubyte =0xC5 OpenPGP Secret Key +0 ubyte =0xC5 >&0 use primary_key_length_new # Old-Style Public Key -0 ubyte&0xFC =0x98 OpenPGP Public Key +0 ubyte&0xFC =0x98 >&-1 use primary_key_length_old # Old-Style Secret Key -0 ubyte&0xFC =0x94 OpenPGP Secret Key +0 ubyte&0xFC =0x94 >&-1 use primary_key_length_old # Parse the length, check the packet's body and finally advance to the @@ -228,10 +228,12 @@ # key format in a decade or so :D. >&0 ubyte >1 >>&-1 ubyte <8 ->>>&-1 byte x Version %d # Check that keys were created after 1990. # (1990 - 1970) * 365.2524 * 24 * 60 * 60 = 631156147 ->>>&0 bedate >631156147 \b, Created %s +>>>&0 bedate >631156147 +>>>>0 ubyte x OpenPGP Key +>>>>&-5 byte x Version %d +>>>>&-4 bedate x \b, Created %s >>>>&-5 ubyte >3 >>>>>&4 use pgp_binary_key_algo >>>>&-5 ubyte <4 -- 2.20.1
0002-For-binary-PGP-keys-only-use-the-pgp-and-gpg-extensi.patch (792 bytes)
From c6378fe5dd3be0b3c452a31173bc16786b3a2228 Mon Sep 17 00:00:00 2001 From: "Neal H. Walfield" <email@example.com> Date: Tue, 8 Jun 2021 13:59:22 +0200 Subject: [PATCH 2/2] For binary PGP keys, only use the pgp and gpg extensions. - Remove the pkr and ask extensions, which at least haven't been canonical for the past decade (if ever), as far as I know. --- magic/Magdir/pgp-binary-keys | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/magic/Magdir/pgp-binary-keys b/magic/Magdir/pgp-binary-keys index 20e10aaa..2c7c377e 100644 --- a/magic/Magdir/pgp-binary-keys +++ b/magic/Magdir/pgp-binary-keys @@ -387,4 +387,4 @@ 0 name pgp_binary_keys_end >0 byte x \b; OpenPGP Certificate !:mime application/pgp-keys -!:ext pgp/gpg/pkr/asd +!:ext pgp/gpg -- 2.20.1
||(I'll take a look at pgp and prune the secret subkey detection and some other stuff that is not actually useful in practice.)|
|2021-05-20 22:01||j2j||New Issue|
|2021-05-20 22:01||j2j||File Added: OpenPGP-bad-k.txt|
|2021-05-20 22:01||j2j||File Added: OpenPGP-bad.zip|
|2021-05-20 22:01||j2j||Tag Attached: PGP|
|2021-05-29 21:27||neal||Note Added: 0003608|
|2021-06-08 18:12||neal||File Added: 0001-Show-information-about-a-PGP-key-only-if-we-have-a-s.patch|
|2021-06-08 18:12||neal||File Added: 0002-For-binary-PGP-keys-only-use-the-pgp-and-gpg-extensi.patch|
|2021-06-08 18:12||neal||Note Added: 0003612|
|2021-06-08 18:13||neal||Note Added: 0003613|