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 |
| Reporter | j2j | Assigned To | |||
| Priority | normal | Severity | minor | Reproducibility | always |
| Status | new | Resolution | open | ||
| Product Version | 5.40 | ||||
| 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 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. | ||||
| Tags | PGP | ||||
|
|
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 |
|
|
(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" <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
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
|
|
|
(I'll take a look at pgp and prune the secret subkey detection and some other stuff that is not actually useful in practice.) |
| 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 |
