Implement low-level derand API for Kyber #1552
Conversation
| memcpy(sk+KYBER_INDCPA_SECRETKEYBYTES, pk, KYBER_PUBLICKEYBYTES); | ||
| hash_h(sk+KYBER_SECRETKEYBYTES-2*KYBER_SYMBYTES, pk, KYBER_PUBLICKEYBYTES); | ||
| /* Value z for pseudo-random output on reject */ | ||
| memcpy(sk+KYBER_SECRETKEYBYTES-KYBER_SYMBYTES, coins+KYBER_SYMBYTES, KYBER_SYMBYTES); |
There was a problem hiding this comment.
Also is there a way to test/ensure this is using a separate part of the coins? I verified the firs call up on 30 indcpa_keypair_derand(pk, sk, coins); uses KYBER_SYMBYTES and then here you offset that KYBER_SYMBYTES. But this seems fragile and could a future update to indcpa_keypair_derand read more data from the coins and we reuse a part of the coins in a dangerous way? Do you think it's reasonable for crypto_kem_keypair_derand to take two coin variables so it's easier to keep them separate? If we messed up this accounting would the KATs break?
There was a problem hiding this comment.
I would rather leave more invasive changes for later when we decide to do proper refactoring of Kyber/ML-KEM. Right now, I'm just trying to apply what's done in pq-crystals/kyber@289b852.
| pq_custom_randombytes(coins, KYBER_SYMBYTES); | ||
| pq_custom_randombytes(coins + KYBER_SYMBYTES, KYBER_SYMBYTES); |
There was a problem hiding this comment.
There a reason not for this to just be a single call for 2*KYBER_SYMBYTES?
There was a problem hiding this comment.
yes, Andrew asked the same thing :) #1552 (comment).
Issues:
N/A
Description of changes:
Implement low-level derand API for Kyber.
Based on: pq-crystals/kyber@289b852
Call-outs:
Point out areas that need special attention or support during the review process. Discuss architecture or design changes.
Testing:
How is this change tested (unit tests, fuzz tests, etc.)? Are there any testing steps to be verified by the reviewer?
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.