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 McEliece (& Falcon) #1459

Closed
wants to merge 1 commit into from
Closed

Update McEliece (& Falcon) #1459

wants to merge 1 commit into from

Conversation

baentsch
Copy link
Member

Would fix #1314.

HOWEVER, this is meant to remain a Draft PR as it doesn't pull from PQClean "master" and only meant for answering this question of @praveksharma

I'm new to both PQClean and liboqs so I'm not sure what to make of this. Does this PR require to include these other implementations before it can be merged?

There were some not quite obvious quirks before the contents of the McEliece update in PQClean/PQClean#469 could be integrated to liboqs: Please see the changes particularly in docs/algorithms/kem/classic_mceliece.yml, scripts/copy_from_upstream/copy_from_upstream.py, scripts/copy_from_upstream/copy_from_upstream.yml and the addition of src/common/pqclean_shims/crypto_declassify.h.

Particularly on the latter file, asking @dstebila as to whether we really need this -- and whether the addition of the SPDX line is appropriate (wasn't there): Shall this be done upstream before PQClean/PQClean#469 is merged?

Final question: This inadvertently also changes Falcon -- and there, the KATs. Why is this so? Is the McEliece branch in PQClean older than PQClean/PQClean#476 ? One more reason to not merge this. But at least CI will show us which impact merging PQClean/PQClean#469 would have on liboqs.

  • [Yes] 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.)
  • [No] 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 oqs-provider, OQS-OpenSSL, OQS-BoringSSL, and OQS-OpenSSH will also need to be ready for review and merge by the time this is merged.)

@dstebila
Copy link
Member

Would fix #1314.

HOWEVER, this is meant to remain a Draft PR as it doesn't pull from PQClean "master" and only meant for answering this question of @praveksharma

I'm new to both PQClean and liboqs so I'm not sure what to make of this. Does this PR require to include these other implementations before it can be merged?

There were some not quite obvious quirks before the contents of the McEliece update in PQClean/PQClean#469 could be integrated to liboqs: Please see the changes particularly in docs/algorithms/kem/classic_mceliece.yml, scripts/copy_from_upstream/copy_from_upstream.py, scripts/copy_from_upstream/copy_from_upstream.yml and the addition of src/common/pqclean_shims/crypto_declassify.h.

Particularly on the latter file, asking @dstebila as to whether we really need this -- and whether the addition of the SPDX line is appropriate (wasn't there): Shall this be done upstream before PQClean/PQClean#469 is merged?

Yes we would need to add crypto_declassify.h to pqclean_shims. The only alternative I can see would be to patch the #include "crypto_declassify.h" lines out of all the places they're called, which seems annoying.

As for the SPDX line, according to the License section of the PQClean README I think the right line would be CC0.

Final question: This inadvertently also changes Falcon -- and there, the KATs. Why is this so? Is the McEliece branch in PQClean older than PQClean/PQClean#476 ? One more reason to not merge this. But at least CI will show us which impact merging PQClean/PQClean#469 would have on liboqs.

Yes, that's the case: the PQClean McEliece branch was made before the Falcon updates landed, but we're doing things in the reverse order.

This is somewhat moot now, as yesterday I merged the PQClean McEliece branch into master, so we can re-do this work based on PQClean master and the Falcon KATs would no longer change.

@baentsch
Copy link
Member Author

This is somewhat moot now, as yesterday I merged the PQClean McEliece branch into master, so we can re-do this work based on PQClean master and the Falcon KATs would no longer change.

Good to hear. Would someone (@praveksharma ?) want to do a new PR based on this (and the current liboqs main after #1420 landed) using some of the lessons from this PR? This might give us also McEliece for v0.8.0. If this is not done today, I'll give it a try tomorrow morning (my time).

@praveksharma
Copy link
Member

@baentsch Yes, I want to make a new PR for this. I'm unable to do this today, however, I will do it tomorrow if this altered schedule works for you.

@dstebila
Copy link
Member

I'm closing this PR since Pravek will be creating a new one.

@dstebila dstebila closed this May 16, 2023
@dstebila dstebila deleted the mb-mceliece branch August 11, 2023 14:02
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.

Upgrade Classic McEliece to NIST round 4 specification
3 participants