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

Serial backends w/formally-verified field arithmetic for 32 & 64 bits #342

Merged
merged 8 commits into from
Apr 13, 2021

Conversation

huitseeker
Copy link
Contributor

@huitseeker huitseeker commented Jan 11, 2021

Contribution

This sets up two non-default serial backends, fiat_u32_backendand fiat_u64_backend. Those backends use field arithmetic generated in Rust from proofs of functional correctness checked by the Coq theorem prover, as part of the fiat-crypto project. More details can be found in the fiat-crypto paper.

This Rust code is located in the fiat-crypto crate maintained by fiat-crypto team, and the present PR simply provides the plumbing necessary to integrate with curve25519-dalek's backend architecture.

Motivation

This contribution could offer to curve25519-dalek an opt-in backend providing a "gold standard" of correctness, and already used for this reason in Wireguard, Go, BoringSSL, etc. Dalek developers could also use these backends as a fuzzing target, in order to check the correctness upcoming optimizations of the serial code.

Performance

The performance is lower, but within a single digit of the existing basic operations, and only rises to a ~15% slowdown on composite operations.

https://gist.github.com/huitseeker/d3c7a30fa90fdb43d2815eb4b63a7205

Maintenability

The backends collectively comprise less than 600 SLoC. They reuse the scalar and constant definitions to offer a minimal code increment.

The fiat-crypto project already runs the curve25519-dalek unit tests on the fiat_u64 backend as part of its CI. Hence the probability that it would break this backend is very low. I would be happy to contribute the few lines of adjustments necessary there to make it run on the fiat_u32 backend as well, should the present PR be merged.

Base automatically changed from master to main March 25, 2021 03:33
@isislovecruft isislovecruft self-requested a review April 13, 2021 22:59
Copy link
Member

@isislovecruft isislovecruft left a comment

Choose a reason for hiding this comment

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

Thanks @huitseeker! I'm pleasantly surprised by how small the LOC footprint is to add Fiat support. Hopefully I'll get more time at some point to dig into their output Rust code again and see if there's other optimisations to be made, but ~15% slow down is pretty impressive.

@isislovecruft isislovecruft added this to the 3.1 milestone Apr 13, 2021
@isislovecruft isislovecruft changed the base branch from main to develop April 13, 2021 23:23
@isislovecruft isislovecruft merged commit cecc821 into dalek-cryptography:develop Apr 13, 2021
huitseeker added a commit to huitseeker/ed25519-dalek that referenced this pull request May 3, 2021
This allows the fiat backends introduced in
[curve25519-dalek/#342](dalek-cryptography/curve25519-dalek#342)
to be used from an ed25519 import without cumbersome overrides.
huitseeker added a commit to huitseeker/x25519-dalek that referenced this pull request May 3, 2021
This allows the fiat backends introduced in [curve25519-dalek/#342](dalek-cryptography/curve25519-dalek#342) to be used from an x25519 import without cumbersome overrides.
@spitters
Copy link

@isislovecruft @huitseeker This paper speeds up curve25519: https://cs.adelaide.edu.au/~yval/pdfs/KuepperEGSWCCGWY23.pdf

They say it's the fastest implementation on Intel i9 12G.

mkj pushed a commit to mkj/ed25519-dalek that referenced this pull request Mar 29, 2023
This allows the fiat backends introduced in
[curve25519-dalek/#342](dalek-cryptography/curve25519-dalek#342)
to be used from an ed25519 import without cumbersome overrides.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants