View Issue Details

IDProjectCategoryView StatusLast Update
0000266file[All Projects] Generalpublic2021-06-08 18:13
Reporterj2jAssigned To 
PrioritynormalSeverityminorReproducibilityalways
Status newResolutionopen 
Product Version5.40 
Target VersionFixed in Version 
Summary0000266: False hits by Magdir/pgp-binary-keys
Descriptionwhen 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
message text.

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.zip.
TagsPGP

Activities

j2j

2021-05-20 22:01

reporter  

OpenPGP-bad.zip (66,795 bytes)
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)

neal

2021-05-29 21:27

reporter   ~0003608

(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.

neal

2021-06-08 18:12

reporter   ~0003612

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.

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" <neal@gnu.org>
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

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" <neal@gnu.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

neal

2021-06-08 18:13

reporter   ~0003613

(I'll take a look at pgp and prune the secret subkey detection and some other stuff that is not actually useful in practice.)

Issue History

Date Modified Username Field Change
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