Re: [openpgp] padding packet as first packet in a packet sequence

[Daniel Kahn Gillmor <[e-mail]>]

On Thu 2022-12-01 11:23:17 +0100, Neal H. Walfield wrote:
>> 5.14.  Padding Packet (Tag 21)
>>
>>     The Padding packet contains random data...
>>
>>     Such a packet MUST be ignored when received.
>
>> 11.4.  Detached Signatures
>>
>>    In addition, a marker packet (Section 5.8) and a padding packet
>>    (Section 5.14) can appear anywhere in the sequence.
>
> The padding packet doesn't contain much identifying information
> (perhaps 8 bits: the two hard-wired high bits, and the 6-bit packet
> tag?).  This makes fingerprint files quite difficult.
> Ideally, a program, like file(1), should be able to look at the first
> few kilobytes of data, and quickly identify its format.  This is also
> important for an OpenPGP implementation to avoid wasting cycles.

If i'm understanding you correctly, it sounds like "fingerprint files"
here means "identifying files by looking for likely byte signatures",
and has nothing to do with OpenPGP fingerprints.

Are we talking about identifying a file as an OpenPGP sequence, or about
identifying the type of OpenPGP sequence (message, detached signatures,
certificate, secret key)?

fwiw, a unique first octet isn't terrible as far as "magic octets" goes.

> I think the draft should be changed to forbid the padding packet from
> being the first packet in a packet sequence.  Something like:
>
>   the padding packet MUST NOT occur as the first packet in a packet
>   sequence.

What would this mean in practice?  MUST NOT generate when producing and
MUST reject when consuming?  

And what about wrapped OpenPGP packet sequences?  for example, consider
an SEIPD packet that unwraps into another series of OpenPGP packets --
is this saying that the first packet within the inner series of packets
also MUST NOT be a padding packet?

What about tools that concatenate OpenPGP objects and split them apart?

For example, consider two sequences that each make up an OpenPGP
certificate ("Transferable public key").  Concatenating them forms a
"keyring" with two certs in it.  If the first certificate has a trailing
padding packet, then the new sequence contains a padding packet in the
middle.  One reasonable operation in a keyring is to remove a
certificate.  If this is done by removing the first certificate without
also removing the trailing padding packet, it sounds like this would
make the entire keyring invalid.  (similar operations are imaginable for
detached signatures, i think)

Overall, this seems like a very strong constraint if the motivation is
only to improve generic libmagic-style file sniffing.

> Alternative, I propose fixing the first few (4? 8?) bytes of the
> padding packet to some value specified by the standard.

This would enhance libmagic's ability to identify a sequence as an
OpenPGP sequence *if* the padding packet comes first, but it would do
nothing to help identify the type of OpenPGP sequence, right?  libmagic
would still need to parse the variable packet length in order to know
where to look for the magic value.

And what if the padding packet has no internal length?  (e.g. only
minimal padding was needed, so the producer generated an empty padding
packet).  What would we expect a libmagic-equivalent to do when scanning
a file containing such a packet first?

Also: if libmagic can parse the packet length, it can also skip to the
next packet header to learn more about the OpenPGP material, right?

It seems like if our goal is to improve libmagic or the equivalents, we
could give implementer guidance in the draft (or just patches to
libmagic directly), but i'm not convinced that strict yet subtle
constraints in the draft are a good idea; they might result in some
sequence rejections that make things overall more brittle than need be.

If we want strict constraints about placement of padding packets to make
sequences more predictable, we could deliberately restrict its placement
to the two locations identified in the current text, though that would
make it less useful for any future padding scheme.

I'd be fine with proposed advisory guidance here, but the MUST seems
like it would make OpenPGP interop more fragile, just for the sake of
making file(1) do a bit less work.

         --dkg