-
Notifications
You must be signed in to change notification settings - Fork 512
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
Update Classic McEliece suppression files #1541
Conversation
@bhess I was hoping you could help me work on this issue, since I can't reproduce these errors locally. Could you check if Valgrind still complains about potential constant time leaks? It might be a good idea to add |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for keeping updating the suppression files for this algorithm family.
As this PR adds this (IMO very correct) advisory:
Current implementation of the algorithm may not be constant-time
and looking at the length/extent of the suppression files and the above advisory I wonder whether it wouldn't be simpler to completely exclude McEliece from constant time testing until someone states interest in giving McEliece the CT property (?) We have something similar for BIKE -- although I wonder whether this is (still) correct given there's no corresponding advisory for BIKE (???).
I ran the tests and there is one error remaining. Seems to be all fine when adding one more rule.
|
I think this is a good decision and I've added Classic McEliece to the skip list in
Looking through the file history for the BIKE suppression file, it looks like the advisories weren't updated when the suppression file was created. I've added a similar advisory for BIKE. I've updated the suppression file with the remaining error, we should be good to go. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updates suppression files to include potential leaks document in #1540.
Updates Classic McEliece advisories to include information about environment-specific constant time leaks.
Removes Classic-McEliece-6XX from weekly constant time test skip list.