From afda68a38acca37becb8ba6d8982d03fee9559a0 Mon Sep 17 00:00:00 2001 From: Arya Tabaie Date: Thu, 22 Aug 2024 04:40:23 -0500 Subject: [PATCH] feat add random mask to groth16 commitment (#1245) * feat add random mask to groth16 commitment * fix register rand hint * update latest.stats * test: add regression test * test: fail if witness found * test: multicommit with zero commitments * test: multicommit with all backends --------- Co-authored-by: Arya Tabaie <15056835+Tabaie@users.noreply.github.com> Co-authored-by: Ivo Kubjas --- constraint/solver/hint_registry.go | 3 +- frontend/cs/r1cs/api.go | 14 +++ internal/hints/hints.go | 20 ++++ .../advisory-9xcg/advisory_test.go | 89 ++++++++++++++++++ internal/stats/latest.stats | Bin 1934 -> 1934 bytes std/multicommit/nativecommit_test.go | 9 +- 6 files changed, 130 insertions(+), 5 deletions(-) create mode 100644 internal/hints/hints.go create mode 100644 internal/security_tests/advisory-9xcg/advisory_test.go diff --git a/constraint/solver/hint_registry.go b/constraint/solver/hint_registry.go index 9ea3b573a4..fa5d44d357 100644 --- a/constraint/solver/hint_registry.go +++ b/constraint/solver/hint_registry.go @@ -2,6 +2,7 @@ package solver import ( "fmt" + "github.com/consensys/gnark/internal/hints" "math/big" "sync" @@ -9,7 +10,7 @@ import ( ) func init() { - RegisterHint(InvZeroHint) + RegisterHint(InvZeroHint, hints.Randomize) } var ( diff --git a/frontend/cs/r1cs/api.go b/frontend/cs/r1cs/api.go index a23665fbb8..095ee6e528 100644 --- a/frontend/cs/r1cs/api.go +++ b/frontend/cs/r1cs/api.go @@ -19,6 +19,7 @@ package r1cs import ( "errors" "fmt" + "github.com/consensys/gnark/internal/hints" "path/filepath" "reflect" "runtime" @@ -687,6 +688,19 @@ func (builder *builder) Compiler() frontend.Compiler { func (builder *builder) Commit(v ...frontend.Variable) (frontend.Variable, error) { + // add a random mask to v + { + vCp := make([]frontend.Variable, len(v)+1) + copy(vCp, v) + mask, err := builder.NewHint(hints.Randomize, 1) + if err != nil { + return nil, err + } + vCp[len(v)] = mask[0] + builder.cs.AddR1C(builder.newR1C(mask[0], builder.eOne, mask[0]), builder.genericGate) // the variable needs to be involved in a constraint otherwise it will not affect the commitment + v = vCp + } + commitments := builder.cs.GetCommitments().(constraint.Groth16Commitments) existingCommitmentIndexes := commitments.CommitmentIndexes() privateCommittedSeeker := utils.MultiListSeeker(commitments.GetPrivateCommitted()) diff --git a/internal/hints/hints.go b/internal/hints/hints.go new file mode 100644 index 0000000000..1513706dac --- /dev/null +++ b/internal/hints/hints.go @@ -0,0 +1,20 @@ +package hints + +import ( + "crypto/rand" + "errors" + "math/big" +) + +func Randomize(mod *big.Int, ins, outs []*big.Int) error { + if len(ins) != 0 { + return errors.New("randomize takes no input") + } + var err error + for i := range outs { + if outs[i], err = rand.Int(rand.Reader, mod); err != nil { + return err + } + } + return nil +} diff --git a/internal/security_tests/advisory-9xcg/advisory_test.go b/internal/security_tests/advisory-9xcg/advisory_test.go new file mode 100644 index 0000000000..fd7488048f --- /dev/null +++ b/internal/security_tests/advisory-9xcg/advisory_test.go @@ -0,0 +1,89 @@ +// Package advisory9xcg implements a test for advisory GHSA-9xcg-3q8v-7fq6. +package advisory9xcg + +import ( + "crypto/rand" + "fmt" + "math/big" + "testing" + + "github.com/consensys/gnark-crypto/ecc" + "github.com/consensys/gnark-crypto/ecc/bn254" + "github.com/consensys/gnark/backend/groth16" + groth16_bn254 "github.com/consensys/gnark/backend/groth16/bn254" + "github.com/consensys/gnark/frontend" + "github.com/consensys/gnark/frontend/cs/r1cs" + "github.com/consensys/gnark/test" +) + +type Circuit struct { + SecretWitness frontend.Variable `gnark:",private"` +} + +func (circuit *Circuit) Define(api frontend.API) error { + // the goal of the test is to show that we are able to predict the private + // input solely from the stored commitment. + commitCompiler, ok := api.Compiler().(frontend.Committer) + if !ok { + return fmt.Errorf("compiler does not commit") + } + + commit, err := commitCompiler.Commit(circuit.SecretWitness) + if err != nil { + return err + } + + api.AssertIsDifferent(commit, 0) + api.AssertIsDifferent(circuit.SecretWitness, 0) + return nil +} + +func TestAdvisory_ghsa_9xcg_3q8v_7fq6(t *testing.T) { + assert := test.NewAssert(t) + // the goal of the test is to show that we are able to predict the private + // input solely from the stored commitment + + // Generating a random secret witness. + var bound int64 = 1024 // ten bits of entropy for testing + secretWitness, err := rand.Int(rand.Reader, big.NewInt(bound)) + assert.NoError(err, "random generation failed") + assert.Log("random secret witness: ", secretWitness) + + // Assigning some values. + assignment := Circuit{SecretWitness: secretWitness} + witness, err := frontend.NewWitness(&assignment, ecc.BN254.ScalarField()) + assert.NoError(err, "witness creation failed") + witnessPublic, err := witness.Public() + assert.NoError(err, "witness public failed") + + // Setup circuit + ccs, err := frontend.Compile(ecc.BN254.ScalarField(), r1cs.NewBuilder, &Circuit{}) + assert.NoError(err, "compilation failed") + + // run the setup and prover + pk, vk, err := groth16.Setup(ccs) + assert.NoError(err, "setup failed") + proof, err := groth16.Prove(ccs, pk, witness) + assert.NoError(err, "proof failed") + + // sanity check, check that the proof verifies + err = groth16.Verify(proof, vk, witnessPublic) + assert.NoError(err, "verification failed") + + // we're ready to set up the attack. For that first we need to assert the + // exact types for being able to extract the proving key information. + pkConcrete, ok := pk.(*groth16_bn254.ProvingKey) + assert.True(ok, "unexpected type for proving key") + proofConcrete, ok := proof.(*groth16_bn254.Proof) + assert.True(ok, "unexpected type for proof") + + var guessedCommitment bn254.G1Affine + for i := int64(0); i < bound; i++ { + // We check our guess for the secret witness. + guessedCommitment.ScalarMultiplication(&pkConcrete.CommitmentKeys[0].Basis[0], big.NewInt(int64(i))) + if guessedCommitment.Equal(&proofConcrete.Commitments[0]) { + assert.Fail("secret witness found: ", i) + return + } + } +} diff --git a/internal/stats/latest.stats b/internal/stats/latest.stats index dc2f8a08aae29df1dd358d7af74fce31b316bfcf..c156102ed0baab988339cffd4b14a99ef885e30d 100644 GIT binary patch delta 183 zcmeC<@8h44IoXV5+GJ6t`I94=k787 zUFb6bEHe2rbMoZ1YzrozVc9(~bHQXiHd!_)#=nN?Zj%GqEZKH3{+<5w!{iBUvXi$l j#!qf%)wGLY{Ks#@!1#}Y3r8Z^gj)kAn;b1IJ`VcHPM~?81{zvC6VZF#a`6cbWW_)sk%o=k787UFb6bCIkQ&PCmN; diff --git a/std/multicommit/nativecommit_test.go b/std/multicommit/nativecommit_test.go index 4b570b7f33..b78f518651 100644 --- a/std/multicommit/nativecommit_test.go +++ b/std/multicommit/nativecommit_test.go @@ -4,7 +4,6 @@ import ( "testing" "github.com/consensys/gnark-crypto/ecc" - "github.com/consensys/gnark/backend" "github.com/consensys/gnark/frontend" "github.com/consensys/gnark/frontend/cs/r1cs" "github.com/consensys/gnark/test" @@ -52,7 +51,7 @@ func TestMultipleCommitments(t *testing.T) { circuit := multipleCommitmentCircuit{} assignment := multipleCommitmentCircuit{X: 10} assert := test.NewAssert(t) - assert.ProverSucceeded(&circuit, &assignment, test.WithCurves(ecc.BN254), test.WithBackends(backend.GROTH16)) // right now PLONK doesn't implement commitment + assert.ProverSucceeded(&circuit, &assignment, test.WithCurves(ecc.BN254)) // right now PLONK doesn't implement commitment } type noCommitVariable struct { @@ -64,9 +63,11 @@ func (c *noCommitVariable) Define(api frontend.API) error { return nil } +// TestNoCommitVariable checks that a circuit that doesn't use the commitment variable +// compiles and prover succeeds. This is due to the randomization of the commitment. func TestNoCommitVariable(t *testing.T) { circuit := noCommitVariable{} + assignment := noCommitVariable{X: 10} assert := test.NewAssert(t) - _, err := frontend.Compile(ecc.BN254.ScalarField(), r1cs.NewBuilder, &circuit) - assert.Error(err) + assert.ProverSucceeded(&circuit, &assignment, test.WithCurves(ecc.BN254)) }