expose kyber 512/768/1024 in the ffi interface

[alexanderkjall]

I had a need to use the Kyber algorithm from rust, and noticed that it's not exposed in the ffi code.

This PR exposes functions for Kyber768, and I see it as a draft, if you think this is a good approach I am happy to add 512 and 1024 variants also.

I haven't written c++ in a very long time, so any feedback is appreciated.

For testing I used the two first test vectors generated from the test_vectors768_ref binary from here https://github.com/pq-crystals/kyber/tree/main

I adapted the FFI_X25519_Test into some testing of the ffi code, and the botan_pk_op_key_agreement didn't function. So that isn't in this pr, I'm not 100% sure why that is.

[randombit]

I adapted the FFI_X25519_Test into some testing of the ffi code, and the botan_pk_op_key_agreement didn't function. So that isn't in this pr, I'm not 100% sure why that is.

Kyber doesn't do key agreement, instead it performs what is called key encapsulation (aka KEM). Basically how it works is the kem encryption operation takes in a public key and returns two things, an "encapsulated key" plus some random secret key. Then the KEM decryption operation takes the encapsulated key, and returns the same random secret key. It's fairly close conceptually to key agreement, except there two different keys are involved.

The Kyber KEM operations already are exposed in the FFI layer, though apparently we are only tested this in the Python tests currently.

Given the wide proliferation of Kyber uses that use the raw key rather than any DER encoding we probably do have a need for some functionality like this in the FFI layer. I don't have time to review this today but I'll take a look soon.

[coveralls]

Coverage: 92.218% (+0.008%) from 92.209% when pulling 25cf4e1 on alexanderkjall:expose-kyber768-in-ffi into 057bcbc on randombit:master.

[mouse07410]

I support the idea of exposing Kyber via FFI, and think that it makes no sense to provide only Kyber768.

Recommend adding Kyber 1024, and if there's support for it - Kyber512.

[alexanderkjall]

Thanks for the feedback, I'll update this pr with the other kyber sizes.

[randombit]

So broadly this looks very good and again I do think this is functionality we want. But, seeing the PR for the Rust library, and thinking about how this will work when calling from other languages, I think the following changes are in order:

Combine all of the botan_privkey_kyberXXX_get_privkey and botan_pubkey_kyberXXX_get_pubkey functions into just two, using the view functions. Something like

botan_privkey_view_kyber_raw_key(botan_privkey_t, botan_view_ctx, botan_view_bin_fn)
and
botan_pubkey_view_kyber_raw_key(botan_pubkey_t, botan_view_ctx, botan_view_bin_fn)

We already have nice infrastructure for using these in the Python and Rust wrappers. This reduces the number of API end points, and also handles the lengths in a more generic way. In the current approach, any FFI layer that calls these functions has to "know" the lengths of the various Kyber keys. In the view callback, instead the FFI layer tells the caller what the length is, without any guesswork. The implementation of for instance botan_privkey_view_der should demonstrate what to do.

For botan_{privkey,pubkey}_load_kyber again I think we should consolidate down to two functions instead of creating a new function for each mode. Something like
botan_privkey_load_kyber(uint8_t key[], size_t key_len), and we just decide which parameter set the key is based on the length. This allows us to handle all three parameters sets somewhat generically, and also allows us (within the FFI layer) to check that the length is valid and return an appropriate error otherwise.

Also (after doing the above API combination), please add the relevant declarations to the Python wrapper in src/python/botan3.py, and add a simple test from Python.

[alexanderkjall]

PR is reworked, and I think the interface became a lot better this way :)

I added python bindings, and a python test, but since I don't program python I don't really have a good gut feeling if those bindings are good enough. It feels a bit strange to have a view_kyber_raw_key function in a PublicKey abstraction, but I don't know of a better way to do it either.

[randombit]

Thanks - this looks really good! Due to the recent merging of #3502 you'll have to rebase against latest master, and reformat using clang-format, which can be done by rerunning configure.py and then make fmt.

[randombit]

LGTM!

[randombit]

There are a few warnings from pylint causing CI to fail, can you take a look?

[alexanderkjall]

Sorry for not responding sooner, had a shortage of free time. Thanks for merging this and do you want me to do a separate PR to fix the pylint issues?

[randombit]

@alexanderkjall No problem. The fix was easy so I just merged and then committed a fix for the pylint warnings immediately after. Thanks again for your contribution! I'll review your Rust patches asap