-
Notifications
You must be signed in to change notification settings - Fork 93
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
Add CROSS #461
Add CROSS #461
Conversation
1571720
to
b988894
Compare
Signed-off-by: rtjk <47841774+rtjk@users.noreply.github.com>
Signed-off-by: rtjk <47841774+rtjk@users.noreply.github.com>
Signed-off-by: rtjk <47841774+rtjk@users.noreply.github.com>
Signed-off-by: rtjk <47841774+rtjk@users.noreply.github.com>
@SWilson4 Do you know how one can trigger GH CI on this PR again? Our CCI script clearly has a bug (indicating a pass while failing) but I'm not overly interested in fixing that given #248. Second question: How can one trigger a (new) CI run using the CROSS PR/branch in |
Signed-off-by: rtjk <47841774+rtjk@users.noreply.github.com>
b988894
to
6ad2a96
Compare
Signed-off-by: rtjk <47841774+rtjk@users.noreply.github.com>
6ad2a96
to
65d1eb2
Compare
Signed-off-by: rtjk <47841774+rtjk@users.noreply.github.com>
I just rebased, we should now be synched with It looks to me like CI is failing because at various points liboqs' main branch is being cloned instead of my fork; the build process fails because it can't find CROSS, which is not part of main yet. This happens in oqs-provider/scripts/fullbuild.sh Line 77 in a0f475d
oqs-provider/.github/workflows/linux.yml Line 127 in a0f475d
oqs-provider/.circleci/config.yml Line 45 in a0f475d
I just tried switching these and a few more cloning operations from I noticed for MAYO CI was initially skipped, should we do the same here? Or maybe wait for CROSS to be added to liboqs? |
The |
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.
Functionally and wrt testing LGTM. See single comments regarding "management & support".
For the first, you can go over to one of the jobs (e.g., https://github.com/open-quantum-safe/oqs-provider/actions/runs/10452640069/job/28946958204?pr=461) and hit "Re-run jobs" -> "Re-run all jobs". For the second, I wrote the '-tracker' script trigger code with release testing in mind, so I didn't take forks into account. I'm pretty sure it will only work for |
That should be easy to establish: Just push the "add-cross[-tracker]" branches into the OQS name space and add a [trigger-downstream] commit... Only "little issue": It worked fine for See status and command sequence below (& sorry for the German of my system's installation):
|
I just added an |
I could be wrong, but I think this was failing because the branch name had a hyphen and the push command used an underscore :/ |
Looking at the release test I notice it enables all algorithms, so all 18 variants of CROSS are on. oqs-provider/scripts/release-test-ci.sh Lines 22 to 23 in a0f475d
And it fails to do a TLS handshake with one parameter set only: CROSSrsdp256fast.
My first guess would be that the signature produced by this variant is too large, it looks like it's the only one in liboqs with |
One algorithm needs to be disabled. See single comment above.
Signed-off-by: rtjk <47841774+rtjk@users.noreply.github.com>
Signed-off-by: rtjk <47841774+rtjk@users.noreply.github.com>
And FWIW, this time it's only (but consistently) "CROSSrsdp256balanced" failing... When looking at the cert sizes, they're conspicuously large:
But other than that observation I'm afraid I can only recommend running the test yourself and debugging into this, @rtjk . |
Thanks for the catch @baentsch, it looks like rsdp-256-balanced is also too big for TLS (
|
Signed-off-by: rtjk <47841774+rtjk@users.noreply.github.com>
Signed-off-by: rtjk <47841774+rtjk@users.noreply.github.com>
The proposed configuration update in open-quantum-safe/tsc#77 will fix the CODEOWNERS file issue by providing @alexrow with write permissions (which is necessary for the CODEOWNERS file to work as intended). |
commit 7754b08 Author: rtjk <47841774+rtjk@users.noreply.github.com> Date: Tue Aug 27 04:36:34 2024 +0200 updated codeowners and contributors Signed-off-by: rtjk <47841774+rtjk@users.noreply.github.com> commit d1b29f4 Author: rtjk <47841774+rtjk@users.noreply.github.com> Date: Tue Aug 27 04:14:22 2024 +0200 disabled CROSSrsdp256balanced because it (also) is too large for TLS Signed-off-by: rtjk <47841774+rtjk@users.noreply.github.com> commit 008df36 Author: rtjk <47841774+rtjk@users.noreply.github.com> Date: Thu Aug 22 12:17:22 2024 +0200 format code Signed-off-by: rtjk <47841774+rtjk@users.noreply.github.com> commit e988f78 Author: rtjk <47841774+rtjk@users.noreply.github.com> Date: Thu Aug 22 11:49:15 2024 +0200 disabled CROSSrsdp256fast because it's too large for TLS Signed-off-by: rtjk <47841774+rtjk@users.noreply.github.com> commit 77c6818 Author: rtjk <47841774+rtjk@users.noreply.github.com> Date: Mon Aug 19 13:39:39 2024 +0200 format code Signed-off-by: rtjk <47841774+rtjk@users.noreply.github.com> commit 65d1eb2 Author: rtjk <47841774+rtjk@users.noreply.github.com> Date: Mon Aug 19 13:29:46 2024 +0200 re-run generate.py Signed-off-by: rtjk <47841774+rtjk@users.noreply.github.com> commit 45e961e Author: rtjk <47841774+rtjk@users.noreply.github.com> Date: Mon Aug 19 11:58:28 2024 +0200 fromat code Signed-off-by: rtjk <47841774+rtjk@users.noreply.github.com> commit 3b08dde Author: rtjk <47841774+rtjk@users.noreply.github.com> Date: Tue Aug 13 02:11:50 2024 +0200 switched to official OIDs, added @rtjk as code owner for generate.yml Signed-off-by: rtjk <47841774+rtjk@users.noreply.github.com> commit c8f447f Author: rtjk <47841774+rtjk@users.noreply.github.com> Date: Tue Aug 6 13:34:01 2024 +0200 switched to free OIDs, temporarely enabled only one CROSS variant Signed-off-by: rtjk <47841774+rtjk@users.noreply.github.com> commit c032f53 Author: rtjk <47841774+rtjk@users.noreply.github.com> Date: Mon Aug 5 11:15:40 2024 +0200 run generate.py Signed-off-by: rtjk <47841774+rtjk@users.noreply.github.com> commit 532d2ae Author: rtjk <47841774+rtjk@users.noreply.github.com> Date: Mon Aug 5 11:14:07 2024 +0200 added CROSS to generate.yml Signed-off-by: rtjk <47841774+rtjk@users.noreply.github.com>
@rtjk Can I ask you to re-base this PR to the latest main and trigger a CI re-run to validate all "turns green" now so we can proceed to merge? |
Closing since #530 has landed. |
@rtjk @alexrow Please accept my sincere apologies for closing and replacing your PR with #530: While I have not been party to this approach nor approve of it as maintainer valuing every contributor's work with an interest to see it come to completion I think it has been borne out of interest by @praveksharma and @SWilson4 to make progress and move |
Like @baentsch says, the decision to go ahead with #530 was primarily motivated by the desire to see an earlier release through with support for CROSS. My intent was not to devalue or undermine anyone's contributions to this project; although, I see how my actions could come across as such and for that I sincerely apologise to @rtjk and @alexrow. I hope I have not discouraged you from contributing to OQS projects and shall strive to work towards more organised releases moving forward. |
@baentsch @praveksharma No worries at all, I totally understand the situation. Happy to see CROSS merged in oqs-provider too :) |
Tracker for liboqs#1881 which adds the CROSS signature scheme.
NOTE: The Object Identifiers in
generate.yml
are placeholders for now, we are in the process of obtaing new OIDs for CROSS.