Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

pull kyber from upstream: dda29cc63af721981ee2c831cf00822e69be3220 (#1631) #1633

Merged
merged 2 commits into from
Dec 19, 2023

Conversation

dstebila
Copy link
Member

Applies fix from #1631 to dev branch for 0.9.1 release.

  • [No] 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 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.)

@dstebila dstebila self-assigned this Dec 19, 2023
@dstebila dstebila added this to the 0.9.1 milestone Dec 19, 2023
Copy link
Member

@SWilson4 SWilson4 left a comment

Choose a reason for hiding this comment

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

It looks like the Travis build is failing due since #1629 is not merged into either branch. We know that Travis passes on the current main branch, which includes this patch, so I don't think it's necessary for those tests to be green on this branch.

@dstebila
Copy link
Member Author

It looks like the Travis build is failing due since #1629 is not merged into either branch. We know that Travis passes on the current main branch, which includes this patch, so I don't think it's necessary for those tests to be green on this branch.

Thanks Spencer. I've cherry-picked that commit over to this branch as well.

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.

LGTM. But why doesn't this target "main"?

@dstebila
Copy link
Member Author

LGTM. But why doesn't this target "main"?

These have already been merged to main. The point here is to do a quick 0.9.1 security release that takes the 0.9.0 release and just ports back the Kyber security fix (plus one extra commit that makes things build cleanly on Travis).

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.

Note that the same issue still exists in the aarch64 code that we pull from PQClean:
https://github.com/open-quantum-safe/liboqs/blob/8449e546491e77b52ca5b2f1451086ef564e16aa/src/kem/kyber/oldpqclean_kyber1024_aarch64/poly.c#L217C1-L217C1

If you agree, I'll create two PRs with a patch: one targeting main, and one targeting the 0.9.1 branch.

@dstebila
Copy link
Member Author

Note that the same issue still exists in the aarch64 code that we pull from PQClean: https://github.com/open-quantum-safe/liboqs/blob/8449e546491e77b52ca5b2f1451086ef564e16aa/src/kem/kyber/oldpqclean_kyber1024_aarch64/poly.c#L217C1-L217C1

If you agree, I'll create two PRs with a patch: one targeting main, and one targeting the 0.9.1 branch.

That would be great, thanks Basil.

@bhess
Copy link
Member

bhess commented Dec 19, 2023

#1637 contains a patch for the aarch64 issue targeting the ds-091-kyber branch (#1636 is the same patch but targeting main, cherry-picking it wouldn't be straight-forward because of the change in #1595)

@dstebila dstebila merged commit e68dbc6 into dev-091 Dec 19, 2023
52 checks passed
@dstebila dstebila deleted the ds-091-kyber branch December 22, 2023 20:28
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants