Re: [openpgp] Expected client behaviour ambiguity in signature verification

[Andrew Gallagher <[e-mail]>]

On 15 Jul 2022, at 15:25, Daniel Huigens <d.huigens=[e-mail]> wrote:
> 
> I understood that GitHub has one, generated
> using OpenPGP.php, would it be possible to ask them to generate a new
> key, or modify their existing one?

I decided to test this infamous GitHub signing key. It appears to be 0x4AEE18F83AFDEB23 and can be found at https://github.com/web-flow.gpg <https://github.com/web-flow.gpg> . Unfortunately, its self-sig appears to have multiple issues.

First, it has a bad quick-test hash prefix as already discussed, although it is not broken in the way that openpgp-php breaks things (by using the first two bytes of the signature MPI). This points towards a similar but independent bug.

Secondly, it has a malformed MPI length:

```
$ curl -s https://github.com/web-flow.gpg | sq packet dump
Public-Key Packet, new CTB, 269 bytes
    Version: 4
    Creation time: 2017-08-16 15:44:01 UTC
    Pk algo: RSA (Encrypt or Sign)
    Pk size: 2048 bits
    Fingerprint: 5DE3E0509C47EA3CF04A42D34AEE18F83AFDEB23
    KeyID: 4AEE18F83AFDEB23

User ID Packet, new CTB, 53 bytes
    Value: GitHub (web-flow commit signing) <[e-mail]>

Unknown or Unsupported Packet, new CTB, 290 bytes
    Tag: Signature Packet
    Error: Malformed MPI: leading bit is not set: expected bit 8 to be set in   100000 (20)
```

Examining the key shows that the MPI begins [08 00 20 …] where the length is an integer multiple of 8 bits, but the first two bits of the first value byte are zero. Sequoia fails on this, while gnupg ignores it:

```
$ curl -s https://github.com/web-flow.gpg | gpg --list-packets
# off=0 ctb=c6 tag=6 hlen=3 plen=269 new-ctb
:public key packet:
	version 4, algo 1, created 1502898241, expires 0
	pkey[0]: [2048 bits]
	pkey[1]: [17 bits]
	keyid: 4AEE18F83AFDEB23
# off=272 ctb=cd tag=13 hlen=2 plen=53 new-ctb
:user ID packet: "GitHub (web-flow commit signing) <[e-mail]>"
# off=327 ctb=c2 tag=2 hlen=3 plen=290 new-ctb
:signature packet: algo 1, keyid 4AEE18F83AFDEB23
	version 4, created 1502898241, md5len 0, sigclass 0x13
	digest algo 8, begin of digest 99 01
	hashed subpkt 2 len 4 (sig created 2017-08-16)
	hashed subpkt 16 len 8 (issuer key ID 4AEE18F83AFDEB23)
	hashed subpkt 27 len 1 (key flags: 03)
	hashed subpkt 25 len 1 (primary user ID)
	data: [2046 bits]
```

This again throws up the question of how strict clients should be when parsing noncompliant yet unambiguous data. From the RFC section 3.2:

```
The length field of an MPI describes the length starting from its
most significant non-zero bit.  Thus, the MPI [00 02 01] is not
formed correctly.  It should be [00 01 01].
```

This is pretty clear re what generating software should do (but not SHOULD do?), while leaving the door open for receiving software to be lenient about zero leading bits. Is it a good idea to leave so much to implementation discretion, particularly when it means that latent interop problems can go unnoticed?

(Aside: why are MPI lengths measured in bits even though they’re only ever stored as whole bytes? I accept we’re probably stuck with it now but it’s yet another example of needless added complexity in the standard)

A