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

first cut removing default algs, biker2, adding biker3 #26

Merged
merged 4 commits into from
Jul 10, 2021
Merged

Conversation

baentsch
Copy link
Member

@baentsch baentsch commented Jul 8, 2021

Aims to fix #27

@baentsch baentsch marked this pull request as ready for review July 8, 2021 13:00
@baentsch baentsch requested a review from bhess July 8, 2021 13:01
@dstebila
Copy link
Member

dstebila commented Jul 8, 2021

Are the NIDs in generate_extras.yml manually synced with the NIDs table in the main OQS-OpenSSL repo?

@baentsch
Copy link
Member Author

baentsch commented Jul 8, 2021

Are the NIDs in generate_extras.yml manually synced with the NIDs table in the main OQS-OpenSSL repo?

No. See the discussion in #27. They now simply moved 0x40 up (and away from the "official" code points -- my mistake to call these NIDs).

@baentsch baentsch requested a review from dstebila July 8, 2021 14:52
Copy link
Member

@bhess bhess left a comment

Choose a reason for hiding this comment

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

Are the NIDs in generate_extras.yml manually synced with the NIDs table in the main OQS-OpenSSL repo?

The code points in generate_extras.yml were for hybrid combinations with x25519/x448, which OQS-OpenSSL111 didn't support, so there wasn't any syncing necessary. Once OQS-OpenSSL111 supports all the extra combinations, the generate_extras.yml will become obsolete. In the meanwhile I think it's best to keep the generate_extras.yml.

The cleanest way could be to fetch the available x25519/x448 code points from the OSSL111 yml and only provide extra ones in the local generate_extras.yml.

oqs-template/generate-extras.yml Outdated Show resolved Hide resolved
Copy link
Member

@bhess bhess left a comment

Choose a reason for hiding this comment

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

#26 (comment)
...
yes, the nid_ecx_hybrid fields for kyber512, sikep434 and bike are already defined in OQS-OSSL111 generate.yml and yes, it would be cleaner to drop them from the generate-extras.yml. I will commit to this branch tomorrow with a proposed update.

oqs-template/generate-extras.yml Outdated Show resolved Hide resolved
oqs-template/generate-extras.yml Outdated Show resolved Hide resolved
Removes sikep434 and kyber512 ecx pairs from generate-extras.yml (they are defined in the OQS-OpenSSL generate.yml).
@bhess
Copy link
Member

bhess commented Jul 9, 2021

a02569b changes generate.py so that it gives priority to the extra code points for ecx from OQS-OpenSSL generate.yml. Also removes the obsolete ones for sikep434 and kyber512 ecx-hybrids from generate-extras.yml.
If there are duplicate/conflicting entries in generate-extras.yml, a warning is printed during a generate.py run.

running the interop tests at the moment...

@bhess
Copy link
Member

bhess commented Jul 9, 2021

running the interop tests at the moment...

The BIKE and Kyber interop tests against test.openquantumsafe.org still fail. The server might not yet accept the new code points.

@baentsch
Copy link
Member Author

baentsch commented Jul 9, 2021

The server might not yet accept the new code points.

It doesn't: "This corresponds to the OQS release version 0.6.0-rc3 ". Let me upgrade it now that oqs-demos seems to mostly build again (except oqs-provider....)

@baentsch
Copy link
Member Author

baentsch commented Jul 9, 2021

@bhess test.openquantumsafe.org now running 0.7.0-dev code: Interop tests should pass now. If so, please approve PR so that also oqs-demos can pass in toto.

@bhess bhess self-requested a review July 9, 2021 14:23
Copy link
Member

@bhess bhess left a comment

Choose a reason for hiding this comment

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

Thanks, looks good to me.
Interop tests seem to work fine. Just the assignments.json entries seem to be off by a few ports (http://test.openquantumsafe.org/assignments.json) E.g. frodo640aes is listed under port 6003, but it is actually port 6001. However, I tried a few ports manually and it looks to work fine with oqs-provider.

@baentsch
Copy link
Member Author

assignments.json entries seem to be off by a few ports

Thanks for the report: For some reason, the new JSON didn't get moved into the download folder. Now fixed. Thanks also for the review, fix and approval. Now settled.

@baentsch baentsch merged commit 040a10d into main Jul 10, 2021
@baentsch baentsch deleted the mb-060update branch July 10, 2021 05:05
@baentsch
Copy link
Member Author

@bhess -- in implementing #111 I noticed "surprising" ID choices for BIKE x-hybrids in https://github.com/open-quantum-safe/oqs-provider/blob/main/oqs-template/generate.yml. How did you make those choices? Second question: Can't we completely do away with https://github.com/open-quantum-safe/oqs-provider/blob/main/oqs-template/generate-extras.yml?

@bhess
Copy link
Member

bhess commented Feb 13, 2023

I think the BIKE x-hybrids were added in open-quantum-safe/openssl#313 but I don't know how the ID/code point choice was made.

The code points I added were the extra ones for the provider in https://github.com/open-quantum-safe/oqs-provider/blob/main/oqs-template/generate-extras.yml. The idea was to express the ones separately that are only supported in oqs-provider. It seems you now detached the generate.yml from oqs-openssl, so I also think that generate.yml and generate-extras.yml can be merged.

@baentsch
Copy link
Member Author

baentsch commented Feb 13, 2023

Thanks for the pointer to open-quantum-safe/openssl#313: I didn't find that. Indeed, one generate.yml would be good. We can do a YML-cross-comparator if there is still an interest to maintain oqs-openssl111 (and keep it in sync with oqs-provider). I'll remove generate-extras.yml then when resolving #111. Edit/Add: Tagging @dstebila FYI (and asking how the X-hybrid IDs were chosen).

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.

Remove default algs, BIKEr2; add BIKEr3
3 participants