openpgp-pqc / draft-openpgp-pqc

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

Add test vectors #96

Closed wussler closed 5 months ago

wussler commented 5 months ago

Add test vectors for

TJ-91 commented 5 months ago

Thanks for providing the test vectors!

  1. I think it'd be good to make separate files instead of putting it into the main file directly. See for example #65.
  2. Adding the public key might be useful as well.
  3. Your v6 key doesn't have the SEIPDv2 Feature Flag and I think therefore the encrypted message is a PKESKv3+SEIPDv1 message. It should include the SEIPDv2 flag and create a PKESKv6+SEIPDv2 message.
  4. Also, the symmetric and AEAD algo preferences should include AES 256 (which we recommend in the draft).
  5. Intermediate Values would be good, too.
  6. You should also remove the checksums of the ASCII armor. We can decide to leave it for the v4 keys / PKESKv3+SEIPDv1, I'm not sure about that.

If you don't have the time to fix points 3 and 4, I can provide the v6 key + v6 PKESK/v2 SEIPD as well, just let me know.

intermediate values that I currently print for the encrypted message to the v4 key:

[LOG] KMAC256 domSeparation: 4f70656e504750436f6d706f736974654b657944657269766174696f6e46756e6374696f6e
[LOG] KMAC256 customizationString: 4b4446
[LOG] KMAC256 encData: 
[LOG]  - counter: 00000001
[LOG]  - eccKeyShare: e0a9be0a0c0180ec7359fce38a2a73144d759337d8d79f3baf67d358e0644c39
[LOG]  - eccCipherText: 16844f4dee0c8ca335e50193ce078f7d634bc4a4b96ed3fdf63bf30419380f3d
[LOG]  - kyberKeyShare: fdfef7b8ef1323ba5ab948c2605f47bf6eec3f9765d224c024228ad7ebdc10b6
[LOG]  - kyberCipherText: 3168f4452a32b61340f4b258efbe3d665ba7c65f3deb0b37b487efcbb173319d3fea43c462f00365bcbd930a6f5fa21adc33402a586225f5ccee859768f510feb6569cb9be7849e19a45ea13cd95cf79df9a5ee1de4e4b409a3a7f8dcb5f6f9ebaa1c3321771a5f7cc0f520d90004dd53b73b61fdecd0dbfe54a1bfd8bf14f89513869814e5f60b9af7114dd96a3870cc5ee57c4b632f10529fa41f943e11d3e65bd1d816edd8035c451002ecb66c7dc4cda8460280f38419515c5c06641e9076c5897918a4655a670a9aff7a03a935b6b50bea0ee7128569305c1af20d8acfaa671238275b3a5c96c561d14da9a0bc1cb4df9d5d9c45534fea66f4360392999d6f44e80693de6c2f2ccbf5875959912cb92f50f7be2069202ffd5e84d30f56f10009149c5791af898becdba36bba87c93105d9dc16f1011719fbebce4d55e5fa40bbc7e2db40e4b8800c8ddd175f052c6b28f1f62c1608ec271dd69525feed009d2e9f92c0d3ea7a1f102c70be42e4b3f084986c966b13427c055500fcfc4b88377fc8f184ae23b515a10f43e963730b087ddbba0265613cd09b6279f4d4fd2a79bef4f5d22a7cbfe4654d3a97f015f58b82b2b9f32e18e437cad055611272e9c16caa578f72e2e8c9116a7749df4d482c3ea08ce2f57c0776e8f524fed3f90667dc40f9b96e7de90e7e907a62bf59e383825127bc492bdc02e75238ee4526929a68d04af4e03531ff585f5f13a68ad55c9b30dea3c0b0549b229f9e9a73d50f8582c7c93472edda3f7007dde67c533525012360c1f74df36f249a2372461a0133c4e734466a073452aef5a8eb2bdb8166b30c8d3d2d21b443fa85a2378875b5856b3c95594cb89664629a56bd0f24204df2b488f711f1f7ff156a9a7da2bd2d37f5c14dfe089f4fe098efd1afae7fe3a025f2f916ae07296364c5b81f2f77281451cae06e82c765e8e75b25153b6f8f6c486d6c4d8f339b1404b08b2b9535c50a619bbd9bc2ec24d9cc598a1b24f7c2fcd5b865f33f720cdc1e8b4b90d9faeef86a69fba70a7c169dd1e5dc0bd2bc585c0aa6caee7836aedd4f1a1bd814a92169032f705616bfc5a8267566054a5d5de18b416a7a4652146b943f62491521549521b70140c2a8699c4ffa2a2243a62e373bd32ad517b7622bc08d1fcf7ae6963f363d493a946469edf654385b1f56949c3d158f1f53c422bd6196ee29f186837f4f5a2bd56c7688a74ae8e7ba64706d283ca9cbd7f0b5a9b448ec93df50c576ade9a294573d69db8733d9c60cc6b3bf5ae367c565b4c3ea2cf9d057ae25819f1f1f1a8473d094f6de992e09849d0170576db7da737c9ded030c5f02784626b25427f30e6dafb7dae8631588b0694ee8eeeaf66a4d4ad1eaaa3779005b84300d0469b0968c70ef897698120d575812438cc853cba9ff2c5948e35d40954ab72df5123a4f942f520b207438c4d3a6cc7a9e8494bba0e8ba27c27ca39da51b4a088a1b52e81f51cde3896d5feb1389372b411d31cafe2a0ed265b3072deac8dcd
[LOG]  - fixedInfo: 69
[LOG] KMAC256 Output: ed4e6e2c8d995420735c58eee861c4e68e83efc4d3ae180e79176c89ebf3e27e
[LOG] Session Key: 3d0d4978d67436238d5713fab95ff6ccae07ce3d0a7cade0e71fab96e5b066b5
wussler commented 5 months ago
  1. Okay
  2. Will fix
  3. Good catch, will also fix
  4. I'll add it, but I also like the implicit version
  5. Will add as well
  6. Only for V6, V4 technically requires them (the whole point is backwards compat)
wussler commented 5 months ago

I ended up removing the checksum everywhere, they are test vectors anyway

TJ-91 commented 5 months ago

Thanks, this looks good to me! I can process all the non-ML-DSA keys and messages and get the same intermediate outputs. ML-DSA I can't process since I don't have it.

Just a few minor things:

  1. A newline at the end of the test vector files would help. RNP doesn't like it otherwise
  2. I'd replace ML-KEM-768 by ML-KEM-ipd-768 and also write a note at the beginning of the section that the test vectors use ML-KEM-ipd and not the final standard.
  3. Same for ML-DSA.
  4. Typo: ML-DSA-67 does not exist :)
dkg commented 5 months ago

can i discourage you from using references to Golang in the user ID? it makes the test vector seem implementation-specific, and the goal of a test vector is promoting cross-implementation interoperability. (not the end of the world if it slips into some version of the draft, but it'd be better to avoid it)

dkg commented 5 months ago

Let me review the purpose of the v4 EdDSALegacy+PQ/T test vector (test-vectors/v4-eddsa-sample*):

In summary, this test vector represents a "compromise key" that is supposed to interoperate with legacy implementations, while also being usable by fully up-to-date implementations in 2024 that encrypt with the best-known mechanisms.

If that's the goal, then i draw the following conclusions:

Test vectors have a way of setting defacto standards, and it would be great to start off with something as close to what we think implementations should be generating as possible.

Having the "compromise key" test vector be as plausible as possible will also help us to evaluate whether it actually serves the "compromise" purpose or not, by testing using it with legacy implementations (similar to the interop test suite's "Mock PQ Subkey" test).

It's possible that my three requests above aren't the only things needed to make a good "compromise key". Can you update this test vector to be as close as possible to what you think would be a plausible "compromise key"?

TJ-91 commented 5 months ago

An implementation that cannot parse v6 will surely also not be able to parse or encrypt to the ML-KEM-768+X25519

That's not exactly true. There is the possibility to leave out v6 keys, v6 PKESK and v2 SEIPD entirely (parsing and generating), but to include PQ/T encryption.

In summary, this test vector represents a "compromise key" that is supposed to interoperate with legacy implementations, while also being usable by fully up-to-date implementations in 2024 that encrypt with the best-known mechanisms.

That's exactly the point, yes.

the "compromise key" should also have an ECDH encryption-capable subkey, presumably using Curve25519Legacy, so that legacy implementations can encrypt to it. I recommend having this second encryption-capable subkey with a creation timestamp and subkey binding signature with earlier timestamps than the PQ/T encryption-capable subkey.

Agreed. Our intent was to add an ECDH subkey, good that you noticed that it's missing here. The timestamp thing is an interesting idea. We should add this to the migration considerations (if we think it's useful, might as well not have an impact at all).

The ASCII-armored representation of the "compromise key" should include the deprecated checksum.

I agree.

dkg commented 5 months ago

@TJ-91 wrote:

There is the possibility to leave out v6 keys, v6 PKESK and v2 SEIPD entirely (parsing and generating), but to include PQ/T encryption.

Sure, but i think the goal should be interoperating with actual legacy software, not interoperating with an implementer practicing deliberate avoidance. There is no way to interoperate with an implementer who is trying to not interoperate, so there is no point in targeting them. And the v6 key and sig formats, and v2 SEIPD are now stable, well-specified, and simple to implement compared with anything like ML-KEM.

wussler commented 5 months ago

Okay, I'll do the following changes to the vectors:

Regarding

I recommend having this second encryption-capable subkey with a creation timestamp and subkey binding signature with earlier timestamps than the PQ/T encryption-capable subkey.

Note that in the spec we explicitly state

Implementations SHOULD prefer PQ(/T) keys when multiple options are available.

Not setting an earlier date can also be a test for this case.

TJ-91 commented 5 months ago

@dkg

there is no way to interoperate with an implementer who is trying to not interoperate, so there is no point in targeting them.

I'm not sure whether it's helpful to frame it like this. Trying to not interoperate is not the same as waiting until everything plays out (see e.g. Kai Engerts views on this). Anyway, I think it'll not make much difference: If a legacy implementation is interoperable, then also a "v4-PQC-only" implementation can manage to be interoperable.

@wussler

Note that in the spec we explicitly state

Implementations SHOULD prefer PQ(/T) keys when multiple options are available.

Good point. Anyway, I think we shouldn't rush the timestamp thing. Let's properly discuss this, it's not crucial for the test vectors now.

wussler commented 5 months ago

I ended up doing:

@TJ-91 PR should be ready and rebased on main. Can you please check if the artifacts are correct on your side?

@dkg do you see any further issue with the keys?

@falko-strenzke @fluppe2 as agreed I removed the (unverified) vectors for ML-DSA.