From 0e44d98c184ca43616e240ad4a6d0771210a6b8f Mon Sep 17 00:00:00 2001 From: Terence Tsao Date: Wed, 25 Mar 2020 10:42:10 -0700 Subject: [PATCH 1/6] Add validateAggregatorSignature --- beacon-chain/core/blocks/block_operations.go | 37 ++++--------------- .../core/blocks/block_operations_fuzz_test.go | 4 +- beacon-chain/core/helpers/signing_root.go | 26 +++++++++++++ beacon-chain/sync/validate_aggregate_proof.go | 22 +++++++++++ 4 files changed, 57 insertions(+), 32 deletions(-) diff --git a/beacon-chain/core/blocks/block_operations.go b/beacon-chain/core/blocks/block_operations.go index 132f602104cb..9c674d7cb835 100644 --- a/beacon-chain/core/blocks/block_operations.go +++ b/beacon-chain/core/blocks/block_operations.go @@ -35,29 +35,6 @@ var log = logrus.WithField("prefix", "blocks") var eth1DataCache = cache.NewEth1DataVoteCache() -// ErrSigFailedToVerify returns when a signature of a block object(ie attestation, slashing, exit... etc) -// failed to verify. -var ErrSigFailedToVerify = errors.New("signature did not verify") - -func verifySigningRoot(obj interface{}, pub []byte, signature []byte, domain []byte) error { - publicKey, err := bls.PublicKeyFromBytes(pub) - if err != nil { - return errors.Wrap(err, "could not convert bytes to public key") - } - sig, err := bls.SignatureFromBytes(signature) - if err != nil { - return errors.Wrap(err, "could not convert bytes to signature") - } - root, err := helpers.ComputeSigningRoot(obj, domain) - if err != nil { - return errors.Wrap(err, "could not compute signing root") - } - if !sig.Verify(root[:], publicKey) { - return ErrSigFailedToVerify - } - return nil -} - // Deprecated: This method uses deprecated ssz.SigningRoot. func verifyDepositDataSigningRoot(obj *ethpb.Deposit_Data, pub []byte, signature []byte, domain []byte) error { publicKey, err := bls.PublicKeyFromBytes(pub) @@ -81,7 +58,7 @@ func verifyDepositDataSigningRoot(obj *ethpb.Deposit_Data, pub []byte, signature return errors.Wrap(err, "could not get container root") } if !sig.Verify(ctrRoot[:], publicKey) { - return ErrSigFailedToVerify + return helpers.ErrSigFailedToVerify } return nil } @@ -104,7 +81,7 @@ func verifySignature(signedData []byte, pub []byte, signature []byte, domain []b return errors.Wrap(err, "could not hash container") } if !sig.Verify(root[:], publicKey) { - return ErrSigFailedToVerify + return helpers.ErrSigFailedToVerify } return nil } @@ -247,7 +224,7 @@ func VerifyBlockHeaderSignature(beaconState *stateTrie.BeaconState, block *ethpb if err != nil { return err } - return verifySigningRoot(block.Block, proposer.PublicKey, block.Signature, domain) + return helpers.VerifySigningRoot(block.Block, proposer.PublicKey, block.Signature, domain) } // ProcessBlockHeaderNoVerify validates a block by its header but skips proposer @@ -485,7 +462,7 @@ func VerifyProposerSlashing( } headers := []*ethpb.SignedBeaconBlockHeader{slashing.Header_1, slashing.Header_2} for _, header := range headers { - if err := verifySigningRoot(header.Header, proposer.PublicKey, header.Signature, domain); err != nil { + if err := helpers.VerifySigningRoot(header.Header, proposer.PublicKey, header.Signature, domain); err != nil { return errors.Wrap(err, "could not verify beacon block header") } } @@ -862,7 +839,7 @@ func VerifyIndexedAttestation(ctx context.Context, beaconState *stateTrie.Beacon voted := len(indices) > 0 if voted && !sig.FastAggregateVerify(pubkeys, messageHash) { - return ErrSigFailedToVerify + return helpers.ErrSigFailedToVerify } return nil } @@ -1199,8 +1176,8 @@ func VerifyExit(validator *ethpb.Validator, currentSlot uint64, fork *pb.Fork, s if err != nil { return err } - if err := verifySigningRoot(exit, validator.PublicKey, signed.Signature, domain); err != nil { - return ErrSigFailedToVerify + if err := helpers.VerifySigningRoot(exit, validator.PublicKey, signed.Signature, domain); err != nil { + return helpers.ErrSigFailedToVerify } return nil } diff --git a/beacon-chain/core/blocks/block_operations_fuzz_test.go b/beacon-chain/core/blocks/block_operations_fuzz_test.go index 7f8484762b91..f6d2f9f863cb 100644 --- a/beacon-chain/core/blocks/block_operations_fuzz_test.go +++ b/beacon-chain/core/blocks/block_operations_fuzz_test.go @@ -61,8 +61,8 @@ func TestFuzzverifySigningRoot_10000(t *testing.T) { fuzzer.Fuzz(&p) fuzzer.Fuzz(&s) fuzzer.Fuzz(&d) - verifySigningRoot(state, pubkey[:], sig[:], domain[:]) - verifySigningRoot(state, p, s, d) + VerifySigningRoot(state, pubkey[:], sig[:], domain[:]) + VerifySigningRoot(state, p, s, d) } } diff --git a/beacon-chain/core/helpers/signing_root.go b/beacon-chain/core/helpers/signing_root.go index 2827b8fb6dd9..70b8324e906a 100644 --- a/beacon-chain/core/helpers/signing_root.go +++ b/beacon-chain/core/helpers/signing_root.go @@ -1,9 +1,11 @@ package helpers import ( + "github.com/pkg/errors" "github.com/prysmaticlabs/go-ssz" p2ppb "github.com/prysmaticlabs/prysm/proto/beacon/p2p/v1" pb "github.com/prysmaticlabs/prysm/proto/beacon/p2p/v1" + "github.com/prysmaticlabs/prysm/shared/bls" "github.com/prysmaticlabs/prysm/shared/bytesutil" "github.com/prysmaticlabs/prysm/shared/params" ) @@ -14,6 +16,10 @@ const ForkVersionByteLength = 4 // DomainByteLength length of domain byte array. const DomainByteLength = 4 +// ErrSigFailedToVerify returns when a signature of a block object(ie attestation, slashing, exit... etc) +// failed to verify. +var ErrSigFailedToVerify = errors.New("signature did not verify") + // ComputeSigningRoot computes the root of the object by calculating the root of the object domain tree. // // Spec pseudocode definition: @@ -38,6 +44,26 @@ func ComputeSigningRoot(object interface{}, domain []byte) ([32]byte, error) { return ssz.HashTreeRoot(container) } +// VerifySigningRoot verifies the signing root of an object given it's public key, signature and domain. +func VerifySigningRoot(obj interface{}, pub []byte, signature []byte, domain []byte) error { + publicKey, err := bls.PublicKeyFromBytes(pub) + if err != nil { + return errors.Wrap(err, "could not convert bytes to public key") + } + sig, err := bls.SignatureFromBytes(signature) + if err != nil { + return errors.Wrap(err, "could not convert bytes to signature") + } + root, err := ComputeSigningRoot(obj, domain) + if err != nil { + return errors.Wrap(err, "could not compute signing root") + } + if !sig.Verify(root[:], publicKey) { + return ErrSigFailedToVerify + } + return nil +} + // ComputeDomain returns the domain version for BLS private key to sign and verify with a zeroed 4-byte // array as the fork version. // diff --git a/beacon-chain/sync/validate_aggregate_proof.go b/beacon-chain/sync/validate_aggregate_proof.go index d386821970f7..cb7505637e2f 100644 --- a/beacon-chain/sync/validate_aggregate_proof.go +++ b/beacon-chain/sync/validate_aggregate_proof.go @@ -122,6 +122,11 @@ func (r *Service) validateAggregatedAtt(ctx context.Context, a *ethpb.AggregateA return false } + // Verify the aggregator's signature is valid. + if err := validateAggregatorSignature(s, a); err != nil { + return false + } + // Verify aggregated attestation has a valid signature. if err := blocks.VerifyAttestation(ctx, s, a.Aggregate); err != nil { traceutil.AnnotateError(span, err) @@ -244,3 +249,20 @@ func validateSelection(ctx context.Context, s *stateTrie.BeaconState, data *ethp return nil } + +// This verifies aggregator signature over the signed aggregate and proof object. +func validateAggregatorSignature(s *stateTrie.BeaconState, a *ethpb.SignedAggregateAttestationAndProof) error { + aggregator, err := s.ValidatorAtIndex(a.Message.AggregatorIndex) + if err != nil { + return err + } + + currentEpoch := helpers.SlotToEpoch(a.Message.Aggregate.Data.Slot) + domain, err := helpers.Domain(s.Fork(), currentEpoch, params.BeaconConfig().DomainAggregateAndProof, s.GenesisValidatorRoot()) + if err != nil { + return err + } + + return helpers.VerifySigningRoot(a.Message, aggregator.PublicKey, a.Signature, domain) + +} From 0a896035001c7d8aad2ccf49e34f253310541cce Mon Sep 17 00:00:00 2001 From: Terence Tsao Date: Wed, 25 Mar 2020 10:45:55 -0700 Subject: [PATCH 2/6] Use it --- beacon-chain/sync/validate_aggregate_proof.go | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/beacon-chain/sync/validate_aggregate_proof.go b/beacon-chain/sync/validate_aggregate_proof.go index 0307a9c2f6da..ff400b71a541 100644 --- a/beacon-chain/sync/validate_aggregate_proof.go +++ b/beacon-chain/sync/validate_aggregate_proof.go @@ -70,7 +70,7 @@ func (r *Service) validateAggregateAndProof(ctx context.Context, pid peer.ID, ms return false } - if !r.validateAggregatedAtt(ctx, m.Message) { + if !r.validateAggregatedAtt(ctx, m) { return false } @@ -85,11 +85,11 @@ func (r *Service) validateAggregateAndProof(ctx context.Context, pid peer.ID, ms return true } -func (r *Service) validateAggregatedAtt(ctx context.Context, a *ethpb.AggregateAttestationAndProof) bool { +func (r *Service) validateAggregatedAtt(ctx context.Context, signed *ethpb.SignedAggregateAttestationAndProof) bool { ctx, span := trace.StartSpan(ctx, "sync.validateAggregatedAtt") defer span.End() - attSlot := a.Aggregate.Data.Slot + attSlot := signed.Message.Aggregate.Data.Slot if err := validateAggregateAttTime(attSlot, uint64(r.chain.GenesisTime().Unix())); err != nil { traceutil.AnnotateError(span, err) return false @@ -111,24 +111,25 @@ func (r *Service) validateAggregatedAtt(ctx context.Context, a *ethpb.AggregateA } // Verify validator index is within the aggregate's committee. - if err := validateIndexInCommittee(ctx, s, a.Aggregate, a.AggregatorIndex); err != nil { + if err := validateIndexInCommittee(ctx, s, signed.Message.Aggregate, signed.Message.AggregatorIndex); err != nil { traceutil.AnnotateError(span, errors.Wrapf(err, "Could not validate index in committee")) return false } // Verify selection proof reflects to the right validator and signature is valid. - if err := validateSelection(ctx, s, a.Aggregate.Data, a.AggregatorIndex, a.SelectionProof); err != nil { - traceutil.AnnotateError(span, errors.Wrapf(err, "Could not validate selection for validator %d", a.AggregatorIndex)) + if err := validateSelection(ctx, s, signed.Message.Aggregate.Data, signed.Message.AggregatorIndex, signed.Message.SelectionProof); err != nil { + traceutil.AnnotateError(span, errors.Wrapf(err, "Could not validate selection for validator %d", signed.Message.AggregatorIndex)) return false } // Verify the aggregator's signature is valid. - if err := validateAggregatorSignature(s, a); err != nil { + if err := validateAggregatorSignature(s, signed); err != nil { + traceutil.AnnotateError(span, errors.Wrapf(err, "Could not verify aggregator signature %d", signed.Message.AggregatorIndex)) return false } // Verify aggregated attestation has a valid signature. - if err := blocks.VerifyAttestation(ctx, s, a.Aggregate); err != nil { + if err := blocks.VerifyAttestation(ctx, s, signed.Message.Aggregate); err != nil { traceutil.AnnotateError(span, err) return false } From 2b78a00d13e1d687c69d053d105113b13f280a8b Mon Sep 17 00:00:00 2001 From: Terence Tsao Date: Wed, 25 Mar 2020 10:56:38 -0700 Subject: [PATCH 3/6] Tests --- .../blockchain/process_attestation_helpers.go | 2 +- .../sync/pending_attestations_queue.go | 2 +- .../sync/validate_aggregate_proof_test.go | 22 +++++++++++++++++++ 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/beacon-chain/blockchain/process_attestation_helpers.go b/beacon-chain/blockchain/process_attestation_helpers.go index a21fb49cc42c..ccdfda07c36f 100644 --- a/beacon-chain/blockchain/process_attestation_helpers.go +++ b/beacon-chain/blockchain/process_attestation_helpers.go @@ -123,7 +123,7 @@ func (s *Service) verifyAttestation(ctx context.Context, baseState *stateTrie.Be } indexedAtt := attestationutil.ConvertToIndexed(ctx, a, committee) if err := blocks.VerifyIndexedAttestation(ctx, baseState, indexedAtt); err != nil { - if err == blocks.ErrSigFailedToVerify { + if err == helpers.ErrSigFailedToVerify { // When sig fails to verify, check if there's a differences in committees due to // different seeds. var aState *stateTrie.BeaconState diff --git a/beacon-chain/sync/pending_attestations_queue.go b/beacon-chain/sync/pending_attestations_queue.go index eb6b9af79b5d..0b991ae65faf 100644 --- a/beacon-chain/sync/pending_attestations_queue.go +++ b/beacon-chain/sync/pending_attestations_queue.go @@ -72,7 +72,7 @@ func (s *Service) processPendingAtts(ctx context.Context) error { if helpers.IsAggregated(att.Aggregate) { // Save the pending aggregated attestation to the pool if it passes the aggregated // validation steps. - if s.validateBlockInAttestation(ctx, signedAtt) && s.validateAggregatedAtt(ctx, att) { + if s.validateBlockInAttestation(ctx, signedAtt) && s.validateAggregatedAtt(ctx, signedAtt) { if err := s.attPool.SaveAggregatedAttestation(att.Aggregate); err != nil { return err } diff --git a/beacon-chain/sync/validate_aggregate_proof_test.go b/beacon-chain/sync/validate_aggregate_proof_test.go index c24c8a1d7fad..13d52819cd88 100644 --- a/beacon-chain/sync/validate_aggregate_proof_test.go +++ b/beacon-chain/sync/validate_aggregate_proof_test.go @@ -373,6 +373,17 @@ func TestValidateAggregateAndProof_CanValidate(t *testing.T) { } signedAggregateAndProof := ðpb.SignedAggregateAttestationAndProof{Message: aggregateAndProof} + domain, err = helpers.Domain(beaconState.Fork(), 0, params.BeaconConfig().DomainAggregateAndProof, beaconState.GenesisValidatorRoot()) + if err != nil { + t.Fatal(err) + } + signingRoot, err := helpers.ComputeSigningRoot(signedAggregateAndProof.Message, domain) + if err != nil { + t.Error(err) + } + blockSig := privKeys[33].Sign(signingRoot[:]).Marshal() + signedAggregateAndProof.Signature = blockSig[:] + if err := beaconState.SetGenesisTime(uint64(time.Now().Unix())); err != nil { t.Fatal(err) } @@ -472,6 +483,17 @@ func TestVerifyIndexInCommittee_SeenAggregatorSlot(t *testing.T) { } signedAggregateAndProof := ðpb.SignedAggregateAttestationAndProof{Message: aggregateAndProof} + domain, err = helpers.Domain(beaconState.Fork(), 0, params.BeaconConfig().DomainAggregateAndProof, beaconState.GenesisValidatorRoot()) + if err != nil { + t.Fatal(err) + } + signingRoot, err := helpers.ComputeSigningRoot(signedAggregateAndProof.Message, domain) + if err != nil { + t.Error(err) + } + blockSig := privKeys[33].Sign(signingRoot[:]).Marshal() + signedAggregateAndProof.Signature = blockSig[:] + if err := beaconState.SetGenesisTime(uint64(time.Now().Unix())); err != nil { t.Fatal(err) } From dd6150b3255eb06fd6a49bc6c5a8fc6f24da9ad0 Mon Sep 17 00:00:00 2001 From: Terence Tsao Date: Wed, 25 Mar 2020 11:02:55 -0700 Subject: [PATCH 4/6] Fixed signing root test --- .../core/blocks/block_operations_fuzz_test.go | 24 ----------------- .../core/helpers/signing_root_test.go | 26 +++++++++++++++++++ 2 files changed, 26 insertions(+), 24 deletions(-) diff --git a/beacon-chain/core/blocks/block_operations_fuzz_test.go b/beacon-chain/core/blocks/block_operations_fuzz_test.go index f6d2f9f863cb..72735580b14e 100644 --- a/beacon-chain/core/blocks/block_operations_fuzz_test.go +++ b/beacon-chain/core/blocks/block_operations_fuzz_test.go @@ -43,30 +43,6 @@ func TestFuzzProcessBlockHeader_10000(t *testing.T) { } } -func TestFuzzverifySigningRoot_10000(t *testing.T) { - fuzzer := fuzz.NewWithSeed(0) - state := ðereum_beacon_p2p_v1.BeaconState{} - pubkey := [48]byte{} - sig := [96]byte{} - domain := [4]byte{} - p := []byte{} - s := []byte{} - d := []byte{} - for i := 0; i < 10000; i++ { - fuzzer.Fuzz(state) - fuzzer.Fuzz(&pubkey) - fuzzer.Fuzz(&sig) - fuzzer.Fuzz(&domain) - fuzzer.Fuzz(state) - fuzzer.Fuzz(&p) - fuzzer.Fuzz(&s) - fuzzer.Fuzz(&d) - VerifySigningRoot(state, pubkey[:], sig[:], domain[:]) - VerifySigningRoot(state, p, s, d) - - } -} - func TestFuzzverifyDepositDataSigningRoot_10000(t *testing.T) { fuzzer := fuzz.NewWithSeed(0) ba := []byte{} diff --git a/beacon-chain/core/helpers/signing_root_test.go b/beacon-chain/core/helpers/signing_root_test.go index 4f96093a3e0f..cf34cc29987c 100644 --- a/beacon-chain/core/helpers/signing_root_test.go +++ b/beacon-chain/core/helpers/signing_root_test.go @@ -4,7 +4,9 @@ import ( "bytes" "testing" + fuzz "github.com/google/gofuzz" ethpb "github.com/prysmaticlabs/ethereumapis/eth/v1alpha1" + ethereum_beacon_p2p_v1 "github.com/prysmaticlabs/prysm/proto/beacon/p2p/v1" "github.com/prysmaticlabs/prysm/shared/params" ) @@ -55,3 +57,27 @@ func TestComputeForkDigest_OK(t *testing.T) { } } } + +func TestFuzzverifySigningRoot_10000(t *testing.T) { + fuzzer := fuzz.NewWithSeed(0) + state := ðereum_beacon_p2p_v1.BeaconState{} + pubkey := [48]byte{} + sig := [96]byte{} + domain := [4]byte{} + p := []byte{} + s := []byte{} + d := []byte{} + for i := 0; i < 10000; i++ { + fuzzer.Fuzz(state) + fuzzer.Fuzz(&pubkey) + fuzzer.Fuzz(&sig) + fuzzer.Fuzz(&domain) + fuzzer.Fuzz(state) + fuzzer.Fuzz(&p) + fuzzer.Fuzz(&s) + fuzzer.Fuzz(&d) + VerifySigningRoot(state, pubkey[:], sig[:], domain[:]) + VerifySigningRoot(state, p, s, d) + + } +} From 9257469e8910a462db1ab2f5cdaf128b7d853de1 Mon Sep 17 00:00:00 2001 From: Terence Tsao Date: Wed, 25 Mar 2020 11:04:23 -0700 Subject: [PATCH 5/6] Gaz --- beacon-chain/core/helpers/BUILD.bazel | 1 + 1 file changed, 1 insertion(+) diff --git a/beacon-chain/core/helpers/BUILD.bazel b/beacon-chain/core/helpers/BUILD.bazel index ec5a2cf0d5a3..15c36397057c 100644 --- a/beacon-chain/core/helpers/BUILD.bazel +++ b/beacon-chain/core/helpers/BUILD.bazel @@ -69,6 +69,7 @@ go_test( "//shared/params:go_default_library", "//shared/sliceutil:go_default_library", "//shared/testutil:go_default_library", + "@com_github_google_gofuzz//:go_default_library", "@com_github_prysmaticlabs_ethereumapis//eth/v1alpha1:go_default_library", "@com_github_prysmaticlabs_go_bitfield//:go_default_library", "@com_github_prysmaticlabs_go_ssz//:go_default_library", From 3e7833891f218f5f2efb2c64af4f93fdeafa331e Mon Sep 17 00:00:00 2001 From: Terence Tsao Date: Wed, 25 Mar 2020 11:47:23 -0700 Subject: [PATCH 6/6] Gaz --- beacon-chain/sync/pending_attestations_queue_test.go | 11 ++++++++++- beacon-chain/sync/validate_aggregate_proof_test.go | 8 ++++---- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/beacon-chain/sync/pending_attestations_queue_test.go b/beacon-chain/sync/pending_attestations_queue_test.go index b1a2894c73e8..cb34b9fe9d9b 100644 --- a/beacon-chain/sync/pending_attestations_queue_test.go +++ b/beacon-chain/sync/pending_attestations_queue_test.go @@ -159,6 +159,15 @@ func TestProcessPendingAtts_HasBlockSaveAggregatedAtt(t *testing.T) { Aggregate: att, AggregatorIndex: 33, } + domain, err = helpers.Domain(beaconState.Fork(), 0, params.BeaconConfig().DomainAggregateAndProof, beaconState.GenesisValidatorRoot()) + if err != nil { + t.Fatal(err) + } + signingRoot, err := helpers.ComputeSigningRoot(aggregateAndProof, domain) + if err != nil { + t.Error(err) + } + aggreSig := privKeys[33].Sign(signingRoot[:]).Marshal() if err := beaconState.SetGenesisTime(uint64(time.Now().Unix())); err != nil { t.Fatal(err) @@ -182,7 +191,7 @@ func TestProcessPendingAtts_HasBlockSaveAggregatedAtt(t *testing.T) { s, _ := beaconstate.InitializeFromProto(&pb.BeaconState{}) r.db.SaveState(context.Background(), s, r32) - r.blkRootToPendingAtts[r32] = []*ethpb.SignedAggregateAttestationAndProof{{Message: aggregateAndProof}} + r.blkRootToPendingAtts[r32] = []*ethpb.SignedAggregateAttestationAndProof{{Message: aggregateAndProof, Signature: aggreSig}} if err := r.processPendingAtts(context.Background()); err != nil { t.Fatal(err) } diff --git a/beacon-chain/sync/validate_aggregate_proof_test.go b/beacon-chain/sync/validate_aggregate_proof_test.go index 13d52819cd88..4a0142cda9a5 100644 --- a/beacon-chain/sync/validate_aggregate_proof_test.go +++ b/beacon-chain/sync/validate_aggregate_proof_test.go @@ -381,8 +381,8 @@ func TestValidateAggregateAndProof_CanValidate(t *testing.T) { if err != nil { t.Error(err) } - blockSig := privKeys[33].Sign(signingRoot[:]).Marshal() - signedAggregateAndProof.Signature = blockSig[:] + aggreSig := privKeys[33].Sign(signingRoot[:]).Marshal() + signedAggregateAndProof.Signature = aggreSig[:] if err := beaconState.SetGenesisTime(uint64(time.Now().Unix())); err != nil { t.Fatal(err) @@ -491,8 +491,8 @@ func TestVerifyIndexInCommittee_SeenAggregatorSlot(t *testing.T) { if err != nil { t.Error(err) } - blockSig := privKeys[33].Sign(signingRoot[:]).Marshal() - signedAggregateAndProof.Signature = blockSig[:] + aggreSig := privKeys[33].Sign(signingRoot[:]).Marshal() + signedAggregateAndProof.Signature = aggreSig[:] if err := beaconState.SetGenesisTime(uint64(time.Now().Unix())); err != nil { t.Fatal(err)