Skip to content

LibOQS integration #14

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

Merged
merged 2 commits into from
Jul 30, 2025
Merged

LibOQS integration #14

merged 2 commits into from
Jul 30, 2025

Conversation

h2parson
Copy link
Contributor

I have added an integration directory which contain all additional files required for use of slh_dsa_c in the libOQS repository.

Additionally I have changed the maximum context size for prehash signing to 255 from 256 to get signature tests to pass in libOQS.

Copy link
Contributor

@mkannwischer mkannwischer left a comment

Choose a reason for hiding this comment

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

Thanks! Can you move it to integration/liboqs, please? We may have more integrations in the future. Also please squash your commits, so that each commit corresponds to one logical change.

Don't we also need a list of files to be imported and compiler flags to be set? Something along the lines of https://github.com/pq-code-package/mlkem-native/blob/main/integration/liboqs/ML-KEM-768_META.yml?
It's probably a good idea to maintain that all in this repo in case files change in the future.

@h2parson
Copy link
Contributor Author

Thanks! Can you move it to integration/liboqs, please? We may have more integrations in the future. Also please squash your commits, so that each commit corresponds to one logical change.

Don't we also need a list of files to be imported and compiler flags to be set? Something along the lines of https://github.com/pq-code-package/mlkem-native/blob/main/integration/liboqs/ML-KEM-768_META.yml? It's probably a good idea to maintain that all in this repo in case files change in the future.

Hi Matthias,
I can move this to integration/liboqs and squash the commit history.

As for including a list of files, I am currently pulling the tarball of the slh_dsa_c commit listed in libOQS so it will copy the entire repo over once without requiring a list of files to pull.
I also do not think that setting different compile options will be necessary as the parameter set can be specified through a variable at runtime.

@mkannwischer
Copy link
Contributor

Thanks! Can you move it to integration/liboqs, please? We may have more integrations in the future. Also please squash your commits, so that each commit corresponds to one logical change.
Don't we also need a list of files to be imported and compiler flags to be set? Something along the lines of https://github.com/pq-code-package/mlkem-native/blob/main/integration/liboqs/ML-KEM-768_META.yml? It's probably a good idea to maintain that all in this repo in case files change in the future.

Hi Matthias, I can move this to integration/liboqs and squash the commit history.

Thanks!

As for including a list of files, I am currently pulling the tarball of the slh_dsa_c commit listed in libOQS so it will copy the entire repo over once without requiring a list of files to pull. I also do not think that setting different compile options will be necessary as the parameter set can be specified through a variable at runtime.

But we don't want all the files in slhdsa-c in liboqs? E.g., the test files shouldn't go into liboqs, but also the Makefile and other files shouldn't. How do you control which files get imported?

@h2parson
Copy link
Contributor Author

Thanks! Can you move it to integration/liboqs, please? We may have more integrations in the future. Also please squash your commits, so that each commit corresponds to one logical change.
Don't we also need a list of files to be imported and compiler flags to be set? Something along the lines of https://github.com/pq-code-package/mlkem-native/blob/main/integration/liboqs/ML-KEM-768_META.yml? It's probably a good idea to maintain that all in this repo in case files change in the future.

Hi Matthias, I can move this to integration/liboqs and squash the commit history.

Thanks!

As for including a list of files, I am currently pulling the tarball of the slh_dsa_c commit listed in libOQS so it will copy the entire repo over once without requiring a list of files to pull. I also do not think that setting different compile options will be necessary as the parameter set can be specified through a variable at runtime.

But we don't want all the files in slhdsa-c in liboqs? E.g., the test files shouldn't go into liboqs, but also the Makefile and other files shouldn't. How do you control which files get imported?

That is true. I will rework the yml file to explicitly list these files.

@h2parson h2parson force-pushed the main branch 2 times, most recently from ae847b0 to 1f1713c Compare June 25, 2025 19:18
@mkannwischer
Copy link
Contributor

As for including a list of files, I am currently pulling the tarball of the slh_dsa_c commit listed in libOQS so it will copy the entire repo over once without requiring a list of files to pull. I also do not think that setting different compile options will be necessary as the parameter set can be specified through a variable at runtime.

But we don't want all the files in slhdsa-c in liboqs? E.g., the test files shouldn't go into liboqs, but also the Makefile and other files shouldn't. How do you control which files get imported?

That is true. I will rework the yml file to explicitly list these files.

Thanks!

Is this PR ready for review or are you planning further changes before it should be merged? If so please mark it as draft until it is ready.

@h2parson
Copy link
Contributor Author

As for including a list of files, I am currently pulling the tarball of the slh_dsa_c commit listed in libOQS so it will copy the entire repo over once without requiring a list of files to pull. I also do not think that setting different compile options will be necessary as the parameter set can be specified through a variable at runtime.

But we don't want all the files in slhdsa-c in liboqs? E.g., the test files shouldn't go into liboqs, but also the Makefile and other files shouldn't. How do you control which files get imported?

That is true. I will rework the yml file to explicitly list these files.

Thanks!

Is this PR ready for review or are you planning further changes before it should be merged? If so please mark it as draft until it is ready.

I will mark it as a draft for now since my PR is not yet ready to merge on the libOQS side.

@h2parson h2parson closed this Jun 26, 2025
@h2parson h2parson reopened this Jun 26, 2025
@h2parson h2parson marked this pull request as draft June 26, 2025 13:56
@h2parson h2parson marked this pull request as ready for review July 28, 2025 20:18
@h2parson h2parson force-pushed the main branch 2 times, most recently from 546eb95 to 14a4327 Compare July 28, 2025 21:08
Copy link
Contributor

@mkannwischer mkannwischer left a comment

Choose a reason for hiding this comment

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

Thanks @h2parson. The last two commits look good to me, but the PR contains a bunch of changes that look unintentional (the first 6 commits). You will also have to rebase on top of main.
Could you please clean that up and request a review again?

Copy link
Contributor

@mkannwischer mkannwischer left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you!

@mkannwischer mkannwischer merged commit 29c1ec1 into pq-code-package:main Jul 30, 2025
16 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.

2 participants