openpgp-pqc / draft-openpgp-pqc

Repository of the WIP draft-ietf-openpgp-pqc
Other
8 stars 2 forks source link

PKESK wire format concern #45

Closed teythoon closed 7 months ago

teythoon commented 1 year ago
### Public-Key Encrypted Session Key Packets (Tag 1) {#ecc-kyber-pkesk}

The composite Kyber algorithms MUST be used only with v6 PKESK, as defined in
[I-D.ietf-openpgp-crypto-refresh] Section 5.1.2.

The algorithm-specific v6 PKESK parameters consists of:

 - A fixed-length octet string representing an ECC ephemeral public key in the
   format associated with the curve as specified in {{ecc-kem}}.

 - A fixed-length octet string of the Kyber ciphertext, whose length depends on
   the algorithm ID as specified in {{tab-kyber-artifacts}}.

 - A variable-length field containing the symmetric key:

   - A one-octet size of the following field;

   - Octet string of the wrapped symmetric key as described in
     {{ecc-kyber-encryption}}.

I think the octet count for the encrypted symmetric key is not necessary here: the algorithm-specific ciphertext is the last field in the PKESK packet anyway, and if you don't understand the composite PK algorithm you have no chance to parse the ciphertext anyway.

falko-strenzke commented 1 year ago

It is correct that the length octet is not strictly necessary, but on the other hand I think it is good practice to prefix a variable length field with its length.

teythoon commented 1 year ago

I agree that it is good practice, that is why I made sure every variable-length field has a length-prefix. It is just in this case I don't see any benefit at all.

When parsing a PKESK I see two cases: either you know the combined pk algorithm, or you don't.

If you know the pk algorithm, you know how long the ecc public key is, you know how long the kyber ciphertext is, then you find the octet count field which tells you the number of octets left in the PKESK packet, but that information is fully redundant.

If you don't know the pk algorithm, you don't know the ecc public key length, you don't know the kyber ciphertext length, you cannot find the octet count field, the whole algorithm-specific part of the PKESK packet is opaque to you.

So it doesn't help with parsing if you know the pk algorithm, and it doesn't help if you don't. The only thing it adds is a chance to be wrong, so there is now a new way in which PKESK packets can be malformed.

wussler commented 1 year ago

While there are other packets using the remainder of the packet length to encode data (compressed, literal data, user ID) I would rather be consistent with the X25519, X448, and ECDH algorithm specific data.

In case some extra data is added after the algorithm-specific data in a future version this might need to customize the algorithm-specific data based on the PKESK version, that is not great. I would try to keep the algorithm-specific data self-defined.

falko-strenzke commented 7 months ago

I agree with Aron here. Similarly to Aron's argument for future PKESK versions, this makes experimental implementations also more robust during the draft-stage, where wire format changes are likely to change the length of the packet and are then detectable.

@teythoon Are you OK with closing this issue? Otherwise, do you have further arguments?

teythoon commented 7 months ago

While there are other packets using the remainder of the packet length to encode data (compressed, literal data, user ID) I would rather be consistent with the X25519, X448, and ECDH algorithm specific data.

Fair, though somewhat depressingly the octet count in the X25519, X448, and ECDH algorithm-specific data is likewise superfluous and doesn't help with parsing (because if you don't know the PK algorithm, you cannot find the octet count, because it comes after the ephemeral key that you don't know the length of).

I retract my concern.