Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use a simpler formula for Kyber division by q #3959

Merged
merged 1 commit into from
Apr 2, 2024

Conversation

randombit
Copy link
Owner

Simpler approach than #3898

@randombit randombit requested a review from FAlbertDev March 30, 2024 11:30
@coveralls
Copy link

Coverage Status

coverage: 92.083% (-0.005%) from 92.088%
when pulling 8352a22 on jack/simpler-kyber-div-q
into 5f47e7b on master.

Copy link
Collaborator

@FAlbertDev FAlbertDev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm okay with using this more straightforward approach. We can stay with this simple method until we need other CT divisions in the future.
Since we already have a test for our ct_div function in #3898, we may also want to apply it here.

// src/tests/test_kyber.cpp
namespace {

std::vector<Test::Result> test_kyber_utilities() {
   return {
      Botan_Tests::CHECK("constant-time division",
                         [](Test::Result& result) {
                            constexpr auto Q = Botan::KyberConstants::Q;
                            // Check ct_int_div_kyber_q with all possible inputs that may appear in Kyber.
                            for(uint32_t i = 0; i < (1 << 18); ++i) {
                               result.test_is_eq(
                                  Botan::fmt("{}/Q", i), Botan::ct_int_div_kyber_q(i), i / Q);
                            }
                         }),
   };
}

}  // namespace

BOTAN_REGISTER_TEST_FN("kyber", "kyber_utils", test_kyber_utilities);

@randombit
Copy link
Owner Author

The function isn't currently exposed anywhere and I'm inclined not to expose it simply to test it, considering it is quite small and should already be very well tested as a side effect of the Kyber tests themselves. I did do an offline test that the expression works for all integers < 2**20.

@randombit randombit merged commit 1e92459 into master Apr 2, 2024
43 checks passed
@randombit randombit deleted the jack/simpler-kyber-div-q branch April 2, 2024 22:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants