Key browser displays incorrect subkey expiry dates

[andrewgdotcom]

Browse my key on e.g. de.pgpkeys.eu (built from current master):
https://de.pgpkeys.eu/pks/lookup?search=0x00CC54C6A0C601691AF4931FFB73E21AF1163937&fingerprint=on&exact=on&op=index

At the bottom, note the sbind expiry dates:

sub rsa4096/0539428d4ef7ff24fe16c2916b09069314549d4b 2013-07-02T17:04:50Z            
sig sbind fb73e21af1163937 2020-12-09T09:23:57Z ____________________ 2022-12-09T09:23:57Z []
sig sbind fb73e21af1163937 2019-01-08T15:05:22Z ____________________ 2021-01-07T15:05:22Z []
sig sbind fb73e21af1163937 2017-01-11T18:49:28Z ____________________ 2019-01-11T18:49:28Z []
sig sbind fb73e21af1163937 2015-04-26T16:08:48Z ____________________ 2017-04-25T16:08:48Z []
sig sbind fb73e21af1163937 2013-07-02T17:04:50Z ____________________ ____________________ []

sub rsa4096/291e79a1dc55ae27a52eef835c1ec404d5906629 2015-04-26T16:01:24Z            
sig sbind fb73e21af1163937 2020-12-09T09:23:57Z ____________________ 2021-02-14T10:27:23Z []
sig sbind fb73e21af1163937 2020-12-09T09:23:57Z ____________________ 2021-02-14T10:27:23Z []
sig sbind fb73e21af1163937 2019-01-08T15:05:22Z ____________________ 2019-03-16T16:08:48Z []
sig sbind fb73e21af1163937 2017-01-11T18:50:06Z ____________________ 2017-03-19T19:53:32Z []
sig sbind fb73e21af1163937 2015-04-26T16:01:24Z ____________________ 2015-07-02T17:04:50Z []

sub rsa4096/eefb8d7c6e3f401c4820ffb285fdf561da8c0c46 2015-04-26T16:18:28Z            
sig sbind fb73e21af1163937 2020-12-09T09:23:57Z ____________________ 2021-02-14T10:10:19Z []
sig sbind fb73e21af1163937 2019-01-08T15:05:22Z ____________________ 2019-03-16T15:51:44Z []
sig sbind fb73e21af1163937 2017-01-11T18:50:42Z ____________________ 2017-03-19T19:37:04Z []
sig sbind fb73e21af1163937 2015-04-26T16:18:28Z ____________________ 2015-07-02T17:04:50Z []

These are incorrect, as can be demonstrated by importing to a fresh gnupg keybox:

andrewg@whippet:~$ gpg --no-default-keyring --keyring foo --keyserver de.pgpkeys.eu --fetch-keys 'https://de.pgpkeys.eu/pks/lookup?search=0x00CC54C6A0C601691AF4931FFB73E21AF1163937&op=get'
gpg: keybox '/home/andrewg/.gnupg/foo' created
gpg: requesting key from 'https://de.pgpkeys.eu/pks/lookup?search=0x00CC54C6A0C601691AF4931FFB73E21AF1163937&op=get'
gpg: key FB73E21AF1163937: 1 duplicate signature removed
gpg: key FB73E21AF1163937: public key "Andrew Gallagher <andrewg@andrewg.com>" imported
gpg: Total number processed: 1
gpg:               imported: 1
andrewg@whippet:~$ gpg --no-default-keyring --keyring foo -k
/home/andrewg/.gnupg/foo
------------------------
pub   rsa4096 2013-07-02 [SCA] [expires: 2022-12-09]
      00CC54C6A0C601691AF4931FFB73E21AF1163937
uid           [ultimate] Andrew Gallagher <andrewg@andrewg.com>
uid           [ultimate] Andrew Gallagher <andrew.gallagher@siren.io>
uid           [ultimate] Andrew Gallagher <andrewg@llagher.net>
uid           [ultimate] Andrew Gallagher <ab.gallagher@gmail.com>
uid           [ultimate] Andrew Gallagher <andrew.gallagher@siren.solutions>
sub   rsa4096 2013-07-02 [E] [expires: 2022-12-09]
sub   rsa4096 2015-04-26 [S] [expires: 2022-12-09]
sub   rsa4096 2015-04-26 [A] [expires: 2022-12-09]

or by requesting the same key from an SKS server, e.g. sks.pgpkeys.eu:

http://sks.pgpkeys.eu/pks/lookup?op=vindex&fingerprint=on&search=0xFB73E21AF1163937

sub  4096R/14549D4B 2013-07-02            
sig sbind  F1163937 2013-07-02 __________ __________ []
sig sbind  F1163937 2015-04-26 __________ 2017-04-25 []
sig sbind  F1163937 2017-01-11 __________ 2019-01-11 []
sig sbind  F1163937 2019-01-08 __________ 2021-01-07 []
sig sbind  F1163937 2020-12-09 __________ 2022-12-09 []

sub  4096R/D5906629 2015-04-26            
sig sbind  F1163937 2015-04-26 __________ 2017-04-25 []
sig sbind  F1163937 2017-01-11 __________ 2019-01-11 []
sig sbind  F1163937 2019-01-08 __________ 2021-01-07 []
sig sbind  F1163937 2020-12-09 __________ 2022-12-09 []
sig sbind  F1163937 2020-12-09 __________ 2022-12-09 []

sub  4096R/DA8C0C46 2015-04-26            
sig sbind  F1163937 2015-04-26 __________ 2017-04-25 []
sig sbind  F1163937 2017-01-11 __________ 2019-01-11 []
sig sbind  F1163937 2019-01-08 __________ 2021-01-07 []
sig sbind  F1163937 2020-12-09 __________ 2022-12-09 []

[andrewgdotcom]

This is exactly reproducible on any hockeypuck server - the exact same timestamp discrepancies can be seen on keyserver.ubuntu.com and pgp.cyberbits.eu - so it is a deterministic bug in the subkey parsing code. There are similar errors on many other keys, although the error amount differs. For example, @cmars has an old key with subkeys that appear to have negative lifetimes:

https://de.pgpkeys.eu/pks/lookup?search=0x81279eee7ec89fb781702adaf79362da44a2d1db&fingerprint=on&op=index

I'm looking into it now to see if there's a mathematical pattern.

[andrewgdotcom]

I've finally worked out what's going on.

PGP (sub)key expiry is stored as an interval in seconds since the (sub)key creation date, which is itself stored in Unix epoch format. In order to display an expiry date in human readable format, we must add the expiry interval (in seconds) to the creation date (in seconds since the epoch) and interpret the result as seconds since the epoch.

What seems to be happening in hockeypuck is that for subkeys, it is adding the expiry interval of that subkey to the creation date of either the first subkey or the primary key (the timestamps are usually the same). This can be seen in the attached spreadsheet, which uses my key and that of another keyserver operator as references. Only keys with more than one subkey are therefore affected.

hockeypuck-date-discrepancy.xlsx

The columns are populated from the creation and expiry dates as shown by hockeypuck, compared with the corresponding dates as reported by sq packet dump.

[CORRECTED: it is the creation time of either the first subkey or the primary key that is erroneously used, not the previous subkey]

[andrewgdotcom]

The error appears to be at src/hockeypuck/openpgp/io.go line 181:

case 2: //packet.PacketTypeSignature:
	if signablePacket == nil {
		log.Debugf("signature out of context")
		badPacket = opkt
	} else {
		sig, err := ParseSignature(opkt, pubkey.Creation, pubkey.UUID, signablePacket.uuid())
		if err != nil {
			log.Debugf("unreadable signature packet in key 0x%s: %v", pubkey.KeyID(), err)
			badPacket = opkt
		} else {
			signablePacket.appendSignature(sig)
		}
	}

ParseSignature() eventually calls Signature.setSignature() (src/hockeypuck/openpgp/signature.go line 116) which overloads sig.Expiration with the key expiration date if s.KeyLifetimeSecs is non-nil:

	// Expiration time
	if s.SigLifetimeSecs != nil {
		sig.Expiration = s.CreationTime.Add(
			time.Duration(*s.SigLifetimeSecs) * time.Second)
	} else if s.KeyLifetimeSecs != nil {
		sig.Expiration = keyCreationTime.Add(
			time.Duration(*s.KeyLifetimeSecs) * time.Second)
	}

Note that this is dependent on having been passed the appropriate keyCreationTime in the first place. But the call to ParseSignature() passes in the primary key's Creation date regardless of context. This results in non-standard behaviour. According to RFC4880 5.2.3.3:

Subpackets that appear in a certification self-signature apply to the user name, and subpackets that appear in the subkey self-signature apply to the subkey. Lastly, subpackets on the direct-key signature apply to the entire key.

This could be read ambiguously (I'll open an issue with the -bis WG now) but the common interpretation is that if a direct sig is made on the primary key, the expiration is calculated relative to the creation of the primary key, but for an sbind, the relevant creation is that of the subkey. This distinction is not made in io.go, and so Signature.setSignature() does not calculate the timestamps as expected.

IMO, overloading the semantics of sig.Expiration is fundamentally confusing, however we don't need to refactor that deeply to fix this particular issue. We just need to declare a variable keyCreation outside the main loop of OpaquePacket.Parse() (~ io.go line 130), set it appropriately when encountering a primary or subkey packet (lines 143 and 154), and pass it into ParseSignature() instead of pubkey.Creation (line 181). I'll open a PR shortly.

[andrewgdotcom]

The ambiguity in the spec is tracked in https://gitlab.com/openpgp-wg/rfc4880bis/-/merge_requests/113