Derive keypair

[tedeaton]

This Pull Request is intended to add deterministic key derivation to liboqs, specifically with Kyber. It adds a new compile flag, "OQS_HAZARDOUS_ENABLE_DERIVE_KEYPAIR", which exposes deterministic key derivation. This is intended to be used with RFC 9180 for hybrid public key encryption, which defines KEMs to have a "DeriveKeyPair" algorithm (in addition to key generation, encapsulation, and decapsulation).

Adds functionality requested in #1206

• Does this PR change the input/output behaviour of a cryptographic algorithm (i.e., does it change known answer test values)? (If so, a version bump will be required from x.y.z to x.(y+1).0.)
• Does this PR change the list of algorithms available -- either adding, removing, or renaming? Does this PR otherwise change an API? (If so, PRs in oqs-provider, OQS-OpenSSL, OQS-BoringSSL, and OQS-OpenSSH will also need to be ready for review and merge by the time this is merged.)

This adds a "derive_keypair" to the KEM API when the compile flag OQS_HAZARDOUS_ENABLE_DERIVE_KEYPAIR is enabled.

I tried to have it so that if the OQS_HAZARDOUS_ENABLE_DERIVE_KEYPAIR is not defined, then no changes happen to liboqs. This results in some code replication. A smarter way to do it would be to have regular key generation just call deterministic key generation with a random seed, but this would mean altering how regular key generation works so I stayed away from that (for now). It should be an easy change everywhere else if that's eventually modified.

As well, I've added a test_derive_keypair script, which just derives a keypair twice from the same, random seed and compares them to check that the same secret and public key is derived each time.

There should be some sort of check to make sure that everything other than kyber is disabled, since this only adds deterministic key derivation for kyber, but I wasn't sure about the best way to add that.

[bhess]

The changes to src/kem/kyber/... would probably be overwritten after pulling updates with copy_from_upstream. There's a patch mechanism to apply changes after copying from upstream.

I wonder if there is a more generic way to add this API to libOQS without patching the upstream sources. All KEMs would otherwise also need to be patched to enable this.
It seems that this API basically replaces the calls to randombytes by providing fixed randomness. If randombytes was configurable (e.g. either using system randomness, or a deterministic PRNG), wouldn't this have the same effect?

[dstebila]

The changes to src/kem/kyber/... would probably be overwritten after pulling updates with copy_from_upstream. There's a patch mechanism to apply changes after copying from upstream.

Yes this is problematic, either the deterministic API needs to be added upstream or added here via the patch mechanism.

I wonder if there is a more generic way to add this API to libOQS without patching the upstream sources. All KEMs would otherwise also need to be patched to enable this. It seems that this API basically replaces the calls to randombytes by providing fixed randomness. If randombytes was configurable (e.g. either using system randomness, or a deterministic PRNG), wouldn't this have the same effect?

Well, OQS_randombytes is user-configurable, but not in a local way, only globally, so it wouldn't be thread-safe.

[baentsch]

Yes this is problematic, either the deterministic API needs to be added upstream or added here via the patch mechanism.

Agreed -- and in any case, this changes the liboqs ABI, so needs an update to the library version (see #1497 and #1081) -- and having a library version dependent on a compile flag is not really desirable, I'd think.

[dstebila]

Agreed -- and in any case, this changes the liboqs ABI, so needs an update to the library version (see #1497 and #1081) -- and having a library version dependent on a compile flag is not really desirable, I'd think.

Adding a function to the ABI doesn't require an SOVERSION bump, only changing the behaviour of the existing ABI.

[dstebila]

Before merging this, we'd also need some documentation updates explaining this new functionality (in the README and may be examples) as well as adding it to the algorithm data sheets.

And as pointed out in #1491 (comment), also need to either make changes upstream or add them to our patch mechanism so they don't get wiped out next time we do a copy-from-upstream.

[baentsch]

@tedeaton Just checking the status of this PR: Do you still want to move this forward? Any particular timeline you're working towards? Any open questions from your side regarding the feedback above?

[baentsch]

Suggest to close this PR: It's pretty stale (lots of conflicts) and @tedeaton has never chimed in to this discussion.

[dstebila]

Agreed. It's still a desirable feature but this PR is stale at this point.