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

Environment-specific Classic McEliece constant-time leaks #1540

Closed
praveksharma opened this issue Sep 7, 2023 · 5 comments
Closed

Environment-specific Classic McEliece constant-time leaks #1540

praveksharma opened this issue Sep 7, 2023 · 5 comments
Assignees
Labels
bug Something isn't working; high priority to fix help wanted Asking for support from non-core team

Comments

@praveksharma
Copy link
Member

Describe the bug
The testing suite is reporting potential memory leaks (test_constant_time.txt) for Classic McEliece which are not documented in the suppression files.

To Reproduce
I am unable to reproduce this issue locally. Additionally, these errors aren't being caught by the CI during weekly constant time tests. One might try to reproduce this with following steps:

docker run -dti openquantumsafe/ci-ubuntu-focal-x86_64 /bin/bash
docker attach (output of docker run command)
cd
git clone https://github.com/open-quantum-safe/liboqs
cd liboqs
mkdir build
cd build
cmake -GNinja -DOQS_OPT_TARGET=generic -DCMAKE_BUILD_TYPE=Debug -DOQS_ENABLE_TEST_CONSTANT_TIME=ON ..
ninja
cd ..
SKIP_ALGS='BIKE*,Frodo*,HQC*,Kyber*,NTRU*,stru*,ntru*,LightSaber,Saber,FireSaber,Dilithium*,Falcon*,,SPHINCS*' python3 tests/test_constant_time.py  --verbose

Thank you for sharing this issue @bhess! Could you please share information about your environment:
Environment (please complete the following information):

  • OS: [e.g. Ubuntu 20]
  • OpenSSL version [e.g., 3.0.2]
  • Compiler version used [e.g., clang 9.0.0]
  • Build variables used [e.g., "-DOQS_ALGS_ENABLED=STD"]
  • liboqs version [e.g. 0.7.2 or main branch]
@praveksharma praveksharma added the bug Something isn't working; high priority to fix label Sep 7, 2023
@praveksharma praveksharma added this to the 0.9.0 milestone Sep 7, 2023
@praveksharma praveksharma self-assigned this Sep 7, 2023
@praveksharma
Copy link
Member Author

Addressing the (potential) leaks documented in this issue doesn't fix the larger issue of the current Classic McEliece implementation (potentially) not being constant time in certain environments. Aside from documenting this in the advisories (as in #1541) should we be taking additional measures for the final 0.9.0 release?

@bhess
Copy link
Member

bhess commented Sep 8, 2023

Thank you for picking this up @praveksharma !

The environment is:

OS: Ubuntu 22.04.3 LTS
OpenSSL version 3.0.2
Compiler version used: GCC 11.4.0
Build variables used: -DCMAKE_BUILD_TYPE=Debug -DOQS_ENABLE_TEST_CONSTANT_TIME=ON
liboqs version: main

@OfekShochat
Copy link

OfekShochat commented Sep 13, 2023

This is an interesting problem!
I have no real experience (I think I understand the McEliece cryptosystem), but if this isn't too bad to fix I can try. Any analysis yet? Why is this happening? What architectures are having this issue and on what version?

@dstebila
Copy link
Member

@praveksharma, do we consider this to be resolved?

@praveksharma praveksharma removed this from the 0.9.0 milestone Oct 31, 2023
@praveksharma praveksharma added the help wanted Asking for support from non-core team label Oct 31, 2023
@SWilson4
Copy link
Member

I am able to duplicate at least some of these CT errors (haven't gone through and checked them all line by line) running 0.9.0 in an Ubuntu 22.04 container with the same library config and OpenSSL and gcc versions as mentioned above by @bhess. I can also confirm that the errors do not occur in the same environment with the current main branch.

The CT errors occur in same env as above on commit d93a431 (immediately before the merge of #1909) but no longer occur after the merge of #1909, which updated suppression files when the constant-time CI tests moved to using Ubuntu 24. The CI tests had previously been running on Ubuntu 20, so I suspect that the environment-specific differences were somehow related to the newer Ubuntu LTS versions—possibly a newer version of Valgrind installed by default?

At any rate, I believe this can now safely be closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working; high priority to fix help wanted Asking for support from non-core team
Projects
Status: Done
Development

No branches or pull requests

5 participants