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

bls12: replacing big.Int by fiat-crypto arithmetic. #252

Merged
merged 5 commits into from
Jul 27, 2021

Conversation

armfazh
Copy link
Contributor

@armfazh armfazh commented Jul 23, 2021

This PR integrates formal verified code from Erbsen et al. "Simple High-Level Code For Cryptographic Arithmetic – With Proofs, Without Compromises" fiat-crypto project.

Generator

Generation of code requires manual triggering for fetching fiat codebase, all instructions are in
ecc/bls12381/ff/gen.go

Generated files

Code generated targets prime field operations in Montgomery representation: addition, subtraction, multiplication, squaring.

ecc/bls12381/ff/fpMont381.go for the base field.
ecc/bls12381/ff/scMont255.go for the scalar field.


cc: @JasonGross

@armfazh armfazh added the enhancement Improvement over something already in the project label Jul 23, 2021
@armfazh armfazh requested review from wbl and tanyav2 July 23, 2021 01:52
@armfazh armfazh self-assigned this Jul 23, 2021
@armfazh armfazh changed the title Replacing big.Int by fiat-crypto arithmetic. bls12: replacing big.Int by fiat-crypto arithmetic. Jul 23, 2021
Copy link

@wbl wbl left a comment

Choose a reason for hiding this comment

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

Some minor comments. Otherwise LGTM

ecc/bls12381/ff/common.go Outdated Show resolved Hide resolved
ecc/bls12381/ff/common.go Outdated Show resolved Hide resolved
ecc/bls12381/ff/fp12.go Show resolved Hide resolved
ecc/bls12381/ff/fp12.go Outdated Show resolved Hide resolved
ecc/bls12381/ff/fp6.go Show resolved Hide resolved
// Code Generation using fiat-crypto
//
// Download and unpack `ExtractionOCaml-Ubuntu LTS` from
// https://github.com/mit-plv/fiat-crypto/suites/3241842351/artifacts/75305272
Copy link

Choose a reason for hiding this comment

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

Youch. This sounds like a fun time on the mac. Well, that's ok: we'll manage.

Choose a reason for hiding this comment

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

The artifacts also contain the .ml files from which the binaries are built, so you could rebuild them from source. I can also get the Mac and Windows CIs to start uploading artifacts again; I seem to recall disabling them because it seemed that seemed plausibly related to the Mac CIs failing.

ecc/bls12381/g1.go Show resolved Hide resolved
func (z *Gt) Mul(x, y *Gt) { (*ff.Fp12)(z).Mul((*ff.Fp12)(x), (*ff.Fp12)(y)) }
func (z *Gt) Sqr(x *Gt) { (*ff.Fp12)(z).Sqr((*ff.Fp12)(x)) }
func (z *Gt) Inv(x *Gt) { (*ff.Fp12)(z).Inv((*ff.Fp12)(x)) }
func (z Gt) String() string { return (ff.Fp12)(z).String() }
Copy link

Choose a reason for hiding this comment

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

Gt isn't using cyclo6, which seems odd to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

take a look on the new approach.

@armfazh armfazh merged commit 70e117e into cloudflare:master Jul 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvement over something already in the project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants