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

Update SPHINCS+ "clean" suppression files #1683

Merged
merged 3 commits into from
Feb 7, 2024

Conversation

SWilson4
Copy link
Member

@SWilson4 SWilson4 commented Feb 5, 2024

Update the Sphincs+ suppression files so that constant-time tests are green. For now I've classified all of the failures as "issues"; someone with more knowledge about SPHINCS+ might want to take a look and see whether or not they're false positives.

The "SHA2-f" variants of SPHINCS+ are pretty fast, so I've added them to the weekly constant-time runs. Hopefully this keep us honest with regards to keeping suppression files up to date. It seems likely that this change will catch most constant-time errors in the "SHAKE" and "SHA2-s" variants as well.

Resolves #1666, pending merge of #1677.

  • Does this PR change the input/output behaviour of a cryptographic algorithm (i.e., does it change known answer test values)? (If so, a version bump will be required from x.y.z to x.(y+1).0.)
  • Does this PR change the list of algorithms available -- either adding, removing, or renaming? Does this PR otherwise change an API? (If so, PRs in fully supported downstream projects dependent on these, i.e., oqs-provider and OQS-OpenSSH will also need to be ready for review and merge by the time this is merged.)

Copy link
Member

@baentsch baentsch left a comment

Choose a reason for hiding this comment

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

Same comment as before, now pertaining to the "constant time" property: I also see this as a feature of "product quality" code: Why spend the compute cycles checking something again and again that OQS doesn't promise to provide? So, I'm OK with any of this -- until a clear decision is taken as to which properties OQS shall have (my goal would be towards "product usefulness", but this is a team decision). As and when that has been decided, we need to carefully comb through all tests and carefully review and enable those deemed essential (maybe only for a subset of algorithms), so for now we're good, I'd gather: Agreed, @dstebila ?

@SWilson4 SWilson4 force-pushed the sw-sphincs-suppression-files branch from a7bacfa to fcede0b Compare February 7, 2024 18:28
@SWilson4 SWilson4 force-pushed the sw-sphincs-suppression-files branch from fcede0b to cee9960 Compare February 7, 2024 18:30
@SWilson4 SWilson4 merged commit da3dab8 into main Feb 7, 2024
1 check passed
@SWilson4 SWilson4 deleted the sw-sphincs-suppression-files branch February 7, 2024 18:30
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.

Update constant-time suppression files for Sphincs+ and McEliece generic configuration
3 participants