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

Dilithium: fix check hint #7803

Merged
merged 1 commit into from
Aug 6, 2024

Conversation

SparkiDev
Copy link
Contributor

Description

When all indeces are 0, then don't check hints against indeces.

Testing

./configure '--disable-shared' '--enable-experimental' '--enable-dilithium'
valgrind ./tests/unit.test -test_wc_dilithium_verify
(new test case in test_wc_dilithium_verify() will have buffer overrun without fix)

Checklist

  • added tests
  • updated/added doxygen
  • updated appropriate READMEs
  • Updated manual and documentation

@SparkiDev SparkiDev self-assigned this Jul 29, 2024
@bandi13
Copy link
Contributor

bandi13 commented Jul 29, 2024

retest this please

@SparkiDev SparkiDev assigned wolfSSL-Bot and unassigned SparkiDev Aug 5, 2024
@SparkiDev SparkiDev requested a review from wolfSSL-Bot August 5, 2024 00:28
Copy link
Contributor

@douzzer douzzer left a comment

Choose a reason for hiding this comment

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

not passing unit test (see the build config below):

   284: test_wc_dilithium_der                               :
ERROR - tests/api.c line 33410 failed with:
    expected: wc_Dilithium_PrivateKeyToDer(key , ((void *)0), 0 ) == BAD_FUNC_ARG
    result:   2590 != -173

The test, both before and after the PR, is also rendering a slew of hex digits to stdout:



    0xc5, 0x57, 0x8b, 0x6b, 0x17, 0x21, 0xcb, 0x73,
    0xcf, 0xc7, 0x0b, 0xe6, 0x81, 0x5c, 0x63, 0xf0,
[...]
 failed (  0.00398)

ERROR - tests/api.c line 85884 failed with:
    expected: Test failed

config (from multi-test quantum-safe-wolfssl-all-gcc-latest):

'--enable-all' '--enable-testcert' '--enable-srtp' '--enable-intelasm' '--enable-sp-asm' '--enable-experimental' '--enable-kyber' '--enable-lms' '--enable-xmss' '--enable-dilithium' '--disable-qt' 'CPPFLAGS=-DNO_WOLFSSL_CIPHER_SUITE_TEST -DWOLFSSL_OLD_PRIME_CHECK -pedantic'

@douzzer douzzer assigned SparkiDev and unassigned douzzer and wolfSSL-Bot Aug 5, 2024
When all indeces are 0, then don't check hints against indeces.
@SparkiDev SparkiDev force-pushed the dilithium_hint_check_fix branch from 61e958d to fc19c36 Compare August 5, 2024 22:32
@SparkiDev SparkiDev assigned douzzer and wolfSSL-Bot and unassigned SparkiDev Aug 5, 2024
@douzzer douzzer merged commit 6fea4f1 into wolfSSL:master Aug 6, 2024
122 checks passed
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.

5 participants