Page MenuHome GnuPG

Importing a key with badly ordered packets doesn't reorder it, and while --edit-key does reorder it doesn't move the signature packets to the right place
Closed, ResolvedPublic

Description

After importing a new key in an empty GNUPGHOME (this key was minimal and didn't
contain any signatures other than the self ones), I refreshed it from a
keyserver to download other sigs. I guess something went wrong (a race
condition perhaps), because signature packets were placed after the subkeys,
which in turn messed up the parsing of gpg's output:

  $ gpg2 --homedir /tmp/gnupg-test --with-colons --list-sigs 06EAA066E397832F |
grep -E '^(pub|sub|uid|sig:([^:]*:){3}(06EAA066E397832F|39278DA8109E6244)):'
  pub:-:4096:1:06EAA066E397832F:1246459499:::-:::scESCA:::::::
  uid:-::::1286747091::B41FA634ADD68A6717D380A790190CB3BC80005B::Luca Capello <luca@pca.it>:::::::::
  sig:::1:06EAA066E397832F:1286747091::::Luca Capello <luca@pca.it>:13x:::::8:
  sig:::1:39278DA8109E6244:1360031056::::Guilhem Moulin:10x:::::10:
  sig:::1:06EAA066E397832F:1246459499::::Luca Capello <luca@pca.it>:13x:::::8:
  sig:::1:06EAA066E397832F:1246460297::::Luca Capello <luca@pca.it>:13x:::::8:
  uid:-::::1286747538::3590ECEB44695F2B0D4E5B2E85EDBBF99C3A90C6::Luca Capello <gismo@debian.org>:::::::::
  sig:::1:06EAA066E397832F:1286747538::::Luca Capello <luca@pca.it>:13x:::::8:
  sig:::1:39278DA8109E6244:1360031056::::Guilhem Moulin:10x:::::10:
  sig:::1:39278DA8109E6244:1360031056::::Guilhem Moulin:10x:::::10:
  sig:::1:06EAA066E397832F:1246459499::::Luca Capello <luca@pca.it>:13x:::::8:
  sig:::1:06EAA066E397832F:1246460232::::Luca Capello <luca@pca.it>:13x:::::8:
  sig:::1:06EAA066E397832F:1246460297::::Luca Capello <luca@pca.it>:13x:::::8:
  sig:::1:06EAA066E397832F:1286747091::::Luca Capello <luca@pca.it>:13x:::::8:
  uid:-::::1453646682::8523545E8C0C86F63F6FC3387DE2D188A55481AF::Luca Capello <luca.capello@infomaniak.ch>:::::::::
  sig:::1:06EAA066E397832F:1453646682::::Luca Capello <luca@pca.it>:13x:::::10:
  uid:-::::1454107799::45C4E00E6D5D53EDE22B1CC8D2B44DCE3E3E93B5::Luca Capello <luca.capello@infomaniak.com>:::::::::
  sig:::1:06EAA066E397832F:1454107799::::Luca Capello <luca@pca.it>:13x:::::10:
  sub:-:4096:1:D91D57A03BE9F36D:1246460943::::::esa::::::
  sig:::1:06EAA066E397832F:1246460943::::Luca Capello <luca@pca.it>:18x:::::8:
  sig:::1:39278DA8109E6244:1360031056::::Guilhem Moulin:10x:::::10:
  sig:::1:06EAA066E397832F:1246459499::::Luca Capello <luca@pca.it>:13x:::::8:
  sig:::1:06EAA066E397832F:1246460297::::Luca Capello <luca@pca.it>:13x:::::8:
  sig:::1:06EAA066E397832F:1286747091::::Luca Capello <luca@pca.it>:13x:::::8:
  sub:-:4096:1:90C02DEC2BB95F4B:1246460155::::::e::::::
  sig:::1:06EAA066E397832F:1246460155::::Luca Capello <luca@pca.it>:18x:::::8:

Editing the key moves the packet to the right place:

  ~$ gpg2 --homedir /tmp/gnupg-test --edit-key 06EAA066E397832F
  gpg: moving a key signature to the correct place
  […]
  gpg> save
  ~$ gpg2 --homedir /tmp/gnupg-test --with-colons --list-sigs 06EAA066E397832F |
grep -E '^(pub|sub|uid|sig:([^:]*:){3}(06EAA066E397832F|39278DA8109E6244)):'
  pub:-:4096:1:06EAA066E397832F:1246459499:::-:::scESCA:::::::
  uid:-::::1286747091::B41FA634ADD68A6717D380A790190CB3BC80005B::Luca Capello <luca@pca.it>:::::::::
  sig:::1:06EAA066E397832F:1286747091::::Luca Capello <luca@pca.it>:13x:::::8:
  sig:::1:39278DA8109E6244:1360031056::::Guilhem Moulin:10x:::::10:
  sig:::1:06EAA066E397832F:1246459499::::Luca Capello <luca@pca.it>:13x:::::8:
  sig:::1:06EAA066E397832F:1246460297::::Luca Capello <luca@pca.it>:13x:::::8:
  uid:-::::1286747538::3590ECEB44695F2B0D4E5B2E85EDBBF99C3A90C6::Luca Capello <gismo@debian.org>:::::::::
  sig:::1:06EAA066E397832F:1286747538::::Luca Capello <luca@pca.it>:13x:::::8:
  sig:::1:39278DA8109E6244:1360031056::::Guilhem Moulin:10x:::::10:
  sig:::1:39278DA8109E6244:1360031056::::Guilhem Moulin:10x:::::10:
  sig:::1:06EAA066E397832F:1246459499::::Luca Capello <luca@pca.it>:13x:::::8:
  sig:::1:06EAA066E397832F:1246460232::::Luca Capello <luca@pca.it>:13x:::::8:
  sig:::1:06EAA066E397832F:1246460297::::Luca Capello <luca@pca.it>:13x:::::8:
  sig:::1:06EAA066E397832F:1286747091::::Luca Capello <luca@pca.it>:13x:::::8:
  uid:-::::1453646682::8523545E8C0C86F63F6FC3387DE2D188A55481AF::Luca Capello <luca.capello@infomaniak.ch>:::::::::
  sig:::1:06EAA066E397832F:1453646682::::Luca Capello <luca@pca.it>:13x:::::10:
  uid:-::::1454107799::45C4E00E6D5D53EDE22B1CC8D2B44DCE3E3E93B5::Luca Capello <luca.capello@infomaniak.com>:::::::::
  sig:::1:06EAA066E397832F:1454107799::::Luca Capello <luca@pca.it>:13x:::::10:
  sig:::1:06EAA066E397832F:1286747091::::Luca Capello <luca@pca.it>:13x:::::8:
  sig:::1:06EAA066E397832F:1246460297::::Luca Capello <luca@pca.it>:13x:::::8:
  sig:::1:06EAA066E397832F:1246459499::::Luca Capello <luca@pca.it>:13x:::::8:
  sig:::1:39278DA8109E6244:1360031056::::Guilhem Moulin:10x:::::10:
  sub:-:4096:1:D91D57A03BE9F36D:1246460943::::::esa::::::
  sig:::1:06EAA066E397832F:1246460943::::Luca Capello <luca@pca.it>:18x:::::8:
  sub:-:4096:1:90C02DEC2BB95F4B:1246460155::::::e::::::
  sig:::1:06EAA066E397832F:1246460155::::Luca Capello <luca@pca.it>:18x:::::8:

I understand that one might not want to open the keyring in write mode with --list-*
commands. But --import/--refresh-keys should probably fix the key, and
--list-sigs could have printed a warning regarding the bad input.

Details

Version
2.1.11

Event Timeline

guilhem set Version to 2.1.11.
guilhem added a subscriber: guilhem.

In fact this is reproducible with Luca's key (but strangely not with mine):

~$ gpg2 --version
gpg (GnuPG) 2.1.11
~$ gnupghome=$(mktemp -d)
~$ key=0x06EAA066E397832F
~$ gpg2 --homedir="$gnupghome" --keyserver hkp://pool.sks-keyservers.net

--recv-key "$key"

~$ gpg2 --homedir="$gnupghome" --edit-key "$key" minimize 4 deluid save
~$ gpg2 --homedir="$gnupghome" --keyserver hkp://pool.sks-keyservers.net

--recv-key "$key"

  ~$ gpg2 --homedir="$gnupghome" --with-colons --list-sigs "$key"

The last command shows a lot of signatures under the last subkey. This not only
messes up the parsing, but also confuses GnuPG: for instance it refuses to let
me sign the 4th UID because it thinks I already did.

guilhem renamed this task from Importing badly ordered packets yields unparsable --list-sigs output to Importing a key with incorrectly ordered packets yields wrong listing output.Feb 1 2016, 2:05 AM

In fact Luca' key can currently be found on the keyserver pool with badly
ordered packets (I can provide a copy if need be):

~$ gpg2 --homedir="$gnupghome" --keyserver-options import-minimal --keyserver

hkp://pool.sks-keyservers.net --recv-key "$key"

~$ gpg2 --homedir="$gnupghome" --with-colons --list-sigs "$key" | grep -E

'^(pub|sub|uid|sig:([^:]*:){3}(06EAA066E397832F|39278DA8109E6244)):'

pub:-:4096:1:06EAA066E397832F:1246459499:::-:::scESCA:::::::
uid:-::::1286747091::B41FA634ADD68A6717D380A790190CB3BC80005B::Luca Capello

<luca@pca.it>:::::::::

sig:::1:06EAA066E397832F:1286747091::::Luca Capello <luca@pca.it>:13x:::::8:
uid:-::::1286747538::3590ECEB44695F2B0D4E5B2E85EDBBF99C3A90C6::Luca Capello

<gismo@debian.org>:::::::::

sig:::1:06EAA066E397832F:1286747538::::Luca Capello <luca@pca.it>:13x:::::8:
uid:-::::1453646682::8523545E8C0C86F63F6FC3387DE2D188A55481AF::Luca Capello

<luca.capello@infomaniak.ch>:::::::::

sig:::1:06EAA066E397832F:1453646682::::Luca Capello <luca@pca.it>:13x:::::10:
uid:-::::1454107799::45C4E00E6D5D53EDE22B1CC8D2B44DCE3E3E93B5::Luca Capello

<luca.capello@infomaniak.com>:::::::::

  sig:::1:06EAA066E397832F:1454107799::::Luca Capello <luca@pca.it>:13x:::::10:
  sub:-:4096:1:90C02DEC2BB95F4B:1246460155::::::e::::::
  sig:::1:06EAA066E397832F:1246460155::::Luca Capello <luca@pca.it>:18x:::::8:
  sub:-:4096:1:D91D57A03BE9F36D:1246460943::::::esa::::::
  sig:::1:39278DA8109E6244:1360031056::::[User ID not found]:10x:::::10:
  sig:::1:06EAA066E397832F:1246459499::::Luca Capello <luca@pca.it>:13x:::::8:
  sig:::1:06EAA066E397832F:1246460297::::Luca Capello <luca@pca.it>:13x:::::8:
  sig:::1:06EAA066E397832F:1286747091::::Luca Capello <luca@pca.it>:13x:::::8:
  sig:::1:06EAA066E397832F:1246460943::::Luca Capello <luca@pca.it>:18x:::::8:

Is there any reason why --import/--recv-key didn't move the packets to their
proper place? After all the keyring is then open in write mode.

Moreover while --edit attempts to reorder the packets, it places the signature
packets under the wrong UID:

~$ gpg2 --homedir="$gnupghome" --edit "$key" save
[…]
gpg: moving a key signature to the correct place
~$ gpg2 --homedir="$gnupghome" --with-colons --list-sigs "$key" | grep -E

'^(pub|sub|uid|sig:([^:]*:){3}(06EAA066E397832F|39278DA8109E6244)):'

pub:-:4096:1:06EAA066E397832F:1246459499:::-:::scESCA:::::::
uid:-::::1286747091::B41FA634ADD68A6717D380A790190CB3BC80005B::Luca Capello

<luca@pca.it>:::::::::

sig:::1:06EAA066E397832F:1286747091::::Luca Capello <luca@pca.it>:13x:::::8:
uid:-::::1286747538::3590ECEB44695F2B0D4E5B2E85EDBBF99C3A90C6::Luca Capello

<gismo@debian.org>:::::::::

sig:::1:06EAA066E397832F:1286747538::::Luca Capello <luca@pca.it>:13x:::::8:
uid:-::::1453646682::8523545E8C0C86F63F6FC3387DE2D188A55481AF::Luca Capello

<luca.capello@infomaniak.ch>:::::::::

sig:::1:06EAA066E397832F:1453646682::::Luca Capello <luca@pca.it>:13x:::::10:
uid:-::::1454107799::45C4E00E6D5D53EDE22B1CC8D2B44DCE3E3E93B5::Luca Capello

<luca.capello@infomaniak.com>:::::::::

  sig:::1:06EAA066E397832F:1454107799::::Luca Capello <luca@pca.it>:13x:::::10:
  sig:::1:06EAA066E397832F:1286747091::::Luca Capello <luca@pca.it>:13x:::::8:
  sig:::1:06EAA066E397832F:1246460297::::Luca Capello <luca@pca.it>:13x:::::8:
  sig:::1:06EAA066E397832F:1246459499::::Luca Capello <luca@pca.it>:13x:::::8:
  sig:::1:39278DA8109E6244:1360031056::::[User ID not found]:10x:::::10:
  sub:-:4096:1:90C02DEC2BB95F4B:1246460155::::::e::::::
  sig:::1:06EAA066E397832F:1246460155::::Luca Capello <luca@pca.it>:18x:::::8:
  sub:-:4096:1:D91D57A03BE9F36D:1246460943::::::esa::::::
  sig:::1:06EAA066E397832F:1246460943::::Luca Capello <luca@pca.it>:18x:::::8:

I (0x39278DA8109E6244) did *not* sign Luca's 4th UID. I'm unsure (based on
--list-packets' output) which of the 2 first UIDs my signature applies to, but
certainly not to the last two, which were created 3 years after my sig was
issued.

guilhem renamed this task from Importing a key with incorrectly ordered packets yields wrong listing output to Importing a key with badly ordered packets doesn't reorder it, and while --edit-key does reorder it doesn't move the signature packets to the right place.Feb 2 2016, 1:57 AM

I found the following comment above g10/keyedit.c' fix_key_signature_order
function, which is responsible for printing the "moving a key signature to the
correct place" lines upon --edit-key.

  /*
   * There are some keys out (due to a bug in gnupg), where the sequence
   * of the packets is wrong.  This function fixes that.
   * Returns: true if the keyblock has been fixed.
   *        
   * Note:  This function does not work if there is more than one user ID.
   */

When was the mentioned bug introduced and fixed? git blame tells me the comment
predates Thu Jun 5 07:14:21 2003 +0000, date when cvs2svn created branch
'GNUPG-1-9-BRANCH' (commit 72503314). But Lucas' key was generated much later,
on 2009-07-01.

Of the 100 keys with lowest MSD http://pgp.cs.uu.nl/doc/top_1000.html, 27 have
badly ordered packets; 26 of these have multiple UIDs; and only 20 of the 120
uids with a creation date have been generated before 2009-07-01.

Is there a technical reason which "This function does not work if there is more
than one user ID"? (I guess gpg could try to verify the sig against each
UID/UAT to find out which one it binds to.) Or is Luca's key (among many
others) broken from a WoT perspective due to the multiple UIDs?

The branch neal/issue2236 contains an initial fix. It does two things:

  • It identifies duplicate signatures (based on their message digest) and removes

duplicates.

  • Instead of blindly moving signatures around, this systematically tests each

signature against its alleged component (= primary key / subkey / user id) and
if it is bad, it tries the other components in the key block and moves it if
appropriate. (If it doesn't belong to any components, then the sig is just left
where it is and GnuPG will ignore it).

I've tested this with a few keys and it seems to work well. Lucas' key just has
a lot of duplicate signatures.

Starting program: /home/us/neal/work/gpg/build/gnupg/g10/gpg2 --check-key
0x06EAA066E397832F
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
gpg: WARNING: unsafe permissions on homedir '/tmp/luca'
gpg: NOTE: THIS IS A DEVELOPMENT VERSION!
gpg: It is only intended for test purposes and should NOT be
gpg: used in a production environment or with production keys!
gpg: Ignored 852 duplicate signatures (total: 2079).
gpg: public key E397832F: timestamp: 2009-07-01 14:44:59 (1246459499)
gpg: user id: Luca Capello <luca@pca.it>
gpg: sig: class: 0x10, issuer: 109E6244, timestamp: 2013-02-05 02:24:16
(1360031056), digest: eb c3
gpg: Good signature over last major component!
gpg: sig: class: 0x13, issuer: E397832F, timestamp: 2009-07-01 14:44:59
(1246459499), digest: 93 7a
gpg: Good signature over last major component!
gpg: sig: class: 0x13, issuer: E397832F, timestamp: 2009-07-01 14:58:17
(1246460297), digest: 53 4f
gpg: Good signature over last major component!
gpg: sig: class: 0x13, issuer: E397832F, timestamp: 2010-10-10 21:44:51
(1286747091), digest: be d5
gpg: Good signature over last major component!
gpg: user id: Luca Capello <gismo@debian.org>
gpg: sig: class: 0x10, issuer: 109E6244, timestamp: 2013-02-05 02:24:16
(1360031056), digest: 4e 92
gpg: Good signature over last major component!
gpg: sig: class: 0x13, issuer: E397832F, timestamp: 2009-07-01 14:57:12
(1246460232), digest: 9c 3d
gpg: Good signature over last major component!
gpg: sig: class: 0x13, issuer: E397832F, timestamp: 2010-10-10 21:52:18
(1286747538), digest: 54 c1
gpg: Good signature over last major component!
gpg: user id: Luca Capello <luca.capello@infomaniak.ch>
gpg: sig: class: 0x13, issuer: E397832F, timestamp: 2016-01-24 14:44:42
(1453646682), digest: 79 a4
gpg: Good signature over last major component!
gpg: user id: Luca Capello <luca.capello@infomaniak.com>
gpg: sig: class: 0x13, issuer: E397832F, timestamp: 2016-01-29 22:49:59
(1454107799), digest: 43 19
gpg: Good signature over last major component!
gpg: subkey 2BB95F4B: timestamp: 2009-07-01 14:55:55 (1246460155)
gpg: sig: class: 0x18, issuer: E397832F, timestamp: 2009-07-01 14:55:55
(1246460155), digest: 4b d9
gpg: Good signature over last major component!
gpg: subkey 3BE9F36D: timestamp: 2009-07-01 15:09:03 (1246460943)
gpg: sig: class: 0x18, issuer: E397832F, timestamp: 2009-07-01 15:09:03
(1246460943), digest: c2 f9
gpg: Good signature over last major component!
gpg: Couldn't check 1216 signatures due to missing issuer keys.

Interestingly, your key contains a bad signature (the hash has been corrupted).

The reason that I haven't pushed this to master is that I need to work our how
the output should look. Also, this functionality will probably only be
available via the --edit-key menu. This patch includes an argument --check-key,
which will probably be removed.

If you have an opportunity to test this, I'd appreciate it.

I've pushed a slightly different version of this patch (2d1d795). Please test
not only that --edit-key detects duplicates and reorders out of place
signatures, but also that revocation certifications, self-sigs, etc. are
correctly checked. Thanks!

neal added a project: Restricted Project.Feb 19 2016, 6:57 PM

Hi Neal,

Thanks for the patch, works great on the couple of keys I tried it on.
Unfortunately I'm unsure how to build OpenPGP keys with deliberately wrongly
ordered packets, so my tests are probably not exhaustive :-( But looking at
your code (from an outsider's perspective), I can't see how revocation
certificates etc would be handled differently from certificate signatures.

I found two issues though:

+ ndataa = pubkey_get_nsig (a->pubkey_algo);
+ ndatab = pubkey_get_nsig (a->pubkey_algo);

I guess it should be "b->pubkey_algo" on the second line.

Also, since the "check" command of the GnuPG prompt can modify the keyblock, it
should set "modify" accordingly:

-8<----------------------------------------------------------------------------------->8-
diff --git a/g10/keyedit.c b/g10/keyedit.c
index d7c2a4b..ede350a 100644

  • a/g10/keyedit.c

+++ b/g10/keyedit.c
@@ -2190,8 +2190,9 @@ keyedit_menu (ctrl_t ctrl, const char *username, strlist_t
locusr,

         break;

       case cmdCHECK:
  • check_all_keysigs (keyblock, count_selected_uids (keyblock),
  • !strcmp (arg_string, "selfsig"));

+ if (check_all_keysigs (keyblock, count_selected_uids (keyblock),
+ !strcmp (arg_string, "selfsig")))
+ modified = 1;

         break;

       case

cmdSIGN:-8<----------------------------------------------------------------------------------->8-

I understand that by default only selfsigs are reordered for performance
reasons. May I suggest to also consider the key to sign with (for instance
specified with "--local-user")? This can be useful, otherwise in order to avoid
potential duplicates signers might have to type "check" before signing a key.

Also (repeating what we discussed about on IRC so it gets indexed on the web :-)
Due to the append-only nature of keyservers, an uploaded badly ordered key
can't be fixed on the keyserver. As a consequence, with the current algorithm
each refresh would undo fixing the packets' order and removing the duplicates.
Ideally keys would be reordered upon import, and the merge algorithm would avoid
duplicate (for instance it could assume the local copy to be properly ordered,
and not add a packet to the local copy if said packet was found elsewhere on the
keyblock).

2.1.12 does a verbose check and fix during --edit-key. We will eventually call
that reorder function during import. But let's wait for bug reports with the
--edit-key triggered code.

I see. I just pushed two fixes.

From the ML:

Hi there,

Some keys are found on the keyserver network with non-self signatures
incorrectly attached to a subkey instead of a UID (cf. Issue2236).

Since 2.1.13 it's possible to reorder fix these keys by running the
‘check’ command of the gpg shell. However the procedure currently has
to be repeated after refreshing the keyring, since each --refresh-keys
command downloads the badly ordered key again.

In T2236 (wk on May 06 2016, 08:18 PM / Roundup) Werner wrote that “We will eventually call that reorder
function during import. But let's wait for bug reports with the
--edit-key triggered code.” This code has been working fine for me
since 2.1.13, so I was wondering if it could be activated for --import
(and --recv-key) in 2.1.18? (So we get this in the next Debian stable
:-)

Moreover, as Neal pointed out to me privately, there is no overhead for
keys that don't have incorrectly placed signature packets.

Thanks!

Cheers,

Guilhem.

werner removed a project: Restricted Project.Jan 23 2017, 11:17 PM
dkg raised the priority of this task from Normal to High.Apr 26 2017, 4:03 AM
dkg added a subscriber: dkg.

Can we activate this for --import and --recv-key as guilhem requested?

justus edited projects, added gnupg (gpg22); removed gnupg.

Justus: When you have implemented that, can you please do a test with my key before and after? As you may know, I have hundreds of vanity signatures so that I need to have

import-filter drop-sig=   sig_created_d=2015-12-24
import-filter drop-sig=|| sig_created_d=2016-03-16
import-filter drop-sig=|| sig_created_d=2016-03-19
import-filter drop-sig=|| sig_created_d=2016-03-20

in my gpg.conf. I hope that the extra clean step does not make it worse. Eventually we may need to have an option to enable the import of key signatures.

It doesn't seem to impact performance significantly:

$ export GNUPGHOME=$(mktemp -d)
$ time g10/gpg --import ../../werners-key.gpg
gpg: keybox '/tmp/tmp.SEwfTct27t/pubring.kbx' created
key F2AD85AC1E42B367:
4709 signatures not checked due to missing keys
gpg: /tmp/tmp.SEwfTct27t/trustdb.gpg: trustdb created
gpg: key F2AD85AC1E42B367: public key "Werner Koch <wk@gnupg.org>" imported
gpg: Total number processed: 1
gpg:               imported: 1
gpg: no ultimately trusted keys found
g10/gpg --import ../../werners-key.gpg  0.33s user 0.02s system 25% cpu 1.351 total
$ export GNUPGHOME=$(mktemp -d)
$ time g10/gpg --import-options no-repair-keys --import ../../werners-key.gpg
gpg: keybox '/tmp/tmp.QMwINCy3r1/pubring.kbx' created
gpg: /tmp/tmp.QMwINCy3r1/trustdb.gpg: trustdb created
gpg: key F2AD85AC1E42B367: public key "Werner Koch <wk@gnupg.org>" imported
gpg: Total number processed: 1
gpg:               imported: 1
gpg: no ultimately trusted keys found
g10/gpg --import-options no-repair-keys --import ../../werners-key.gpg  0.30s user 0.02s system 23% cpu 1.317 total
$ g10/gpg --list-signatures werner|wc --lines
4733
$ g10/gpg --list-signatures werner|grep 2015-12-24 |wc --lines
814