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

Small fixes after adding ML-* #1702

Merged
merged 2 commits into from
Feb 23, 2024
Merged

Small fixes after adding ML-* #1702

merged 2 commits into from
Feb 23, 2024

Conversation

bhess
Copy link
Member

@bhess bhess commented Feb 21, 2024

Two small fixes after #1626:

  • The fragment to filter "STD" signature algorithms contained an error
  • The script to reproduce ML-* vectors used an incorrect directory name
  • 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.)

- Fixes fetch_values.sh script to (re-)generate ML-* vectors
.CMake/alg_support.cmake Outdated Show resolved Hide resolved
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.

Please ascertain aliases and originals operate identically. Current problem: Header-defines (can -- and do) mismatch (see single comment).

@baentsch
Copy link
Member

A (non-merge-blocking) comment, though: I understand you have a propensity to retain the "*-ipd" algorithms as "STD" algorithms, but I personally consider this logically incorrect and my suggestion is to not include them in this algorithm selection.

Rationale: As and when a v2 ipd arrives or when ipd gets approved as final, they will need to be manually removed from this list again -- in other words, no matter what, they will need to be removed eventually; in turn, the algorithm names without "-ipd" moniker are standardized and will remain the name of valid "STD" algorithms, whatever their underlying implementation.

-> If you don't want to subscribe to this logic, may I then suggest to at least add a comment to this code along the lines "TBD - manually revise as standard process moves on"? Then we could create a script scouring for "TBD" monikers and consciously decide whether or not this (and other instances containing such aide memoire) is still OK when doing a release.

@bhess
Copy link
Member Author

bhess commented Feb 23, 2024

I agree that adding the non-ipd alias in the STD list would be sufficient to track the latest versions (which I think is the goal of "STD" as the algorothms there are not in final-standard phase yet but rather "selected to be standardized and in their latest draft phase"). With #1703 / #1704 addressed, the latest version (-ipd or -ipdv2...) macros would then automatically be enabled as well, and explicitly adding -ipd to the STD list will not be needed anymore.

@bhess bhess merged commit 154ae5c into main Feb 23, 2024
61 checks passed
@bhess bhess deleted the bhe-fixafterml branch May 30, 2024 15:08
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