Pull Neon implementation of Falcon from PQClean

[SWilson4]

Pull the ARM Neon implementation of Falcon from PQClean on behalf of @cothan. See PQClean/PQClean#493, PQClean/PQClean#497

• 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.)
• 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.)

[SWilson4]

@cothan It looks like there are build errors: https://app.circleci.com/pipelines/github/open-quantum-safe/liboqs/2788/workflows/46cbb0ce-3a37-4cfb-ba32-a33f2bced86f/jobs/22376

[ducnguyen-sb]

@SWilson4 Thank you, it's type confusion between signed and unsigned integer. Should I make a fix in this PR, or is it better to make a PR in pqclean and update here?

[SWilson4]

@SWilson4 Thank you, it's type confusion between signed and unsigned integer. Should I make a fix in this PR, or is it better to make a PR in pqclean and update here?

I would say it's better to fix it in PQClean and then run copy_from_upstream again, but there's been some unrelated activity in PQClean since your initial two commits, and we presumably wouldn't want to pull that in at the same time. So I'm unsure. Thoughts on this @dstebila?

Either way, it's probably best if you can get CI to pass on this branch before making a PR in PQClean, since the CI over there evidently didn't complain about the type confusion. It will save having to iterate through the process again if another build error sneaks in.

[dstebila]

@SWilson4 Thank you, it's type confusion between signed and unsigned integer. Should I make a fix in this PR, or is it better to make a PR in pqclean and update here?

I would say it's better to fix it in PQClean and then run copy_from_upstream again, but there's been some unrelated activity in PQClean since your initial two commits, and we presumably wouldn't want to pull that in at the same time. So I'm unsure. Thoughts on this @dstebila?

It's not a problem if there are other PQClean updates upstream alongside it, that's sometimes unavoidable based on how our pull-from-upstream process works.

[cothan]

Commit 91065c8 makes CI/CD green.

So I made a PR to fix type confusion in PQClean PQClean/PQClean#506.

[ducnguyen-sb]

The PR PQClean/PQClean#506 is merged recently. I think it's alright to proceed.

[SWilson4]

I ran copy_from_upstream at commit 8e220a87308154d48fdfac40abbb191ac7fce06a of PQClean and everything is passing.

Besides the Neon implementation of Falcon, there are some minor changes to McEliece from PQClean commit afcebe31ae6c636f39ce6852b8ee95c3f6e7670d. A bunch of newlines were deleted, which has made the diff light up like a Christmas tree. Another slight change there is outlined in PQClean/PQClean#503: it makes one of our patches unnecessary.

PQClean/PQClean#501 made another patch unnecessary. I deleted these patches and updated copy_from_upstream accordingly.

I also took the liberty of adding scripts/copy_from_upstream/verify_from_upstream to the .gitignore to prevent me from accidentally committing it after running ./copy_from_upstream -k verify (oops).

[baentsch]

Basically LGTM. Code license is clearly documented as Apache2 -- but I didn't find SPDX identifiers in the files. Could those be added going forward, please? This in support of (implementing) #1514 (eventually)?

[cothan]

Thanks @baentsch , sure I will update license of Falcon going forward.
Sorry I didn't know, I was following the Falcon ref License header example.

[baentsch]

I will update license of Falcon going forward.

Thanks very much! Please let us know as and when landed in PQClean so we can update suitably.

[cothan]

So I made a PR to add SPDX header in PQClean/PQClean#514

[baentsch]

So I made a PR to add SPDX header in PQClean/PQClean#514

Much appreciated. But CI isn't exactly happy: How can this be? The diffs are only comments...

[cothan]

I totally agree with you. I think the PR changes in Falcon, so it triggers all implementations in Falcon tested. Turn out, AVX2 used defined macros (as it always is from the start) that makes become red.

[ducnguyen-sb]

It's been a few weeks PQClean/PQClean#514, I think the other approach can be like this comment: PQClean/PQClean#514 (comment)

[baentsch]

It's been a few weeks PQClean/PQClean#514, I think the other approach can be like this comment: PQClean/PQClean#514 (comment)

Agreed. But with the Linux Foundation driven sidelining of independent contributors I in turn sidelined caring about #1514: Should the people paid by entities restricting the breadth of licenses look into this, not people interested in progress of open source.