diff --git a/cluster/cluster_test.go b/cluster/cluster_test.go index 3e935a1e46..b224331c95 100644 --- a/cluster/cluster_test.go +++ b/cluster/cluster_test.go @@ -166,9 +166,19 @@ func TestEncode(t *testing.T) { require.NoError(t, err) require.Equal(t, definition, definition2) + // Signatures aren't covered by the lock hash. Pad the 32 byte values drawn from r to real lengths + // using a separate random source, so r's sequence (and thereby the golden lock hashes) is unchanged. + sigR := rand.New(rand.NewSource(2)) + padSig := func(b []byte, length int) []byte { + pad := make([]byte, length-len(b)) + _, _ = sigR.Read(pad) + + return append(b, pad...) + } + lock := cluster.Lock{ Definition: definition, - SignatureAggregate: testutil.RandomBytes32Seed(r), + SignatureAggregate: padSig(testutil.RandomBytes32Seed(r), 96), Validators: []cluster.DistValidator{ { PubKey: testutil.RandomBytes48Seed(r), @@ -189,8 +199,8 @@ func TestEncode(t *testing.T) { }, }, NodeSignatures: [][]byte{ - testutil.RandomBytes32Seed(r), - testutil.RandomBytes32Seed(r), + padSig(testutil.RandomBytes32Seed(r), 65), + padSig(testutil.RandomBytes32Seed(r), 65), }, } @@ -304,6 +314,33 @@ func TestExamples(t *testing.T) { } } +// TestTestdataStrictParsing ensures all golden testdata definitions and locks of all versions +// still parse and verify hashes with strict fixed size field length checks. +func TestTestdataStrictParsing(t *testing.T) { + files, err := filepath.Glob("testdata/cluster_*.json") + require.NoError(t, err) + require.NotEmpty(t, files) + + for _, file := range files { + t.Run(filepath.Base(file), func(t *testing.T) { + b, err := os.ReadFile(file) + require.NoError(t, err) + + if strings.HasPrefix(filepath.Base(file), "cluster_lock") { + var lock cluster.Lock + require.NoError(t, json.Unmarshal(b, &lock)) + require.NoError(t, lock.VerifyHashes()) + + return + } + + var def cluster.Definition + require.NoError(t, json.Unmarshal(b, &def)) + require.NoError(t, def.VerifyHashes()) + }) + } +} + func TestDefinitionPeers(t *testing.T) { seed := 5 random := rand.New(rand.NewSource(int64(seed))) diff --git a/cluster/definition.go b/cluster/definition.go index 3b27264a34..b71a927014 100644 --- a/cluster/definition.go +++ b/cluster/definition.go @@ -575,8 +575,72 @@ func (d *Definition) UnmarshalJSON(data []byte) error { return nil } +// verifyDefinitionFieldLengths returns an error if any fixed size field of the definition has an invalid length. +// Empty values are allowed since they represent absent values (e.g. unsigned operators), see checkBytesN. +// Presence of required values is checked by VerifyHashes and VerifySignatures. +func verifyDefinitionFieldLengths(d Definition) error { + if err := checkBytesN(d.ForkVersion, sszLenForkVersion, "fork_version"); err != nil { + return err + } + + if err := checkBytesN(d.ConfigHash, sszLenHash, "config_hash"); err != nil { + return err + } + + if err := checkBytesN(d.DefinitionHash, sszLenHash, "definition_hash"); err != nil { + return err + } + + if err := checkAddressLength(d.Creator.Address, "creator_address"); err != nil { + return err + } + + if err := validateSignatureLength(d.Version, d.Creator.ConfigSignature, "creator_config_signature"); err != nil { + return err + } + + for i, o := range d.Operators { + if err := checkAddressLength(o.Address, "operator_address"); err != nil { + return errors.Wrap(err, "invalid operator", z.Int("operator_index", i)) + } + + if err := validateSignatureLength(d.Version, o.ConfigSignature, "operator_config_signature"); err != nil { + return errors.Wrap(err, "invalid operator", z.Int("operator_index", i)) + } + + if err := validateSignatureLength(d.Version, o.ENRSignature, "operator_enr_signature"); err != nil { + return errors.Wrap(err, "invalid operator", z.Int("operator_index", i)) + } + } + + for i, v := range d.ValidatorAddresses { + if err := checkAddressLength(v.FeeRecipientAddress, "fee_recipient_address"); err != nil { + return errors.Wrap(err, "invalid validator addresses", z.Int("validator_index", i)) + } + + if err := checkAddressLength(v.WithdrawalAddress, "withdrawal_address"); err != nil { + return errors.Wrap(err, "invalid validator addresses", z.Int("validator_index", i)) + } + } + + return nil +} + +// checkAddressLength returns an error if the address is neither empty nor a 20 byte hex address. +func checkAddressLength(addr string, field string) error { + if _, err := from0xHex(addr, addressLen); err != nil { + return errors.Wrap(err, "invalid address", z.Str("field", field)) + } + + return nil +} + // VerifyHashes returns an error if hashes populated from json object doesn't matches actual hashes. func (d Definition) VerifyHashes() error { + if err := verifyDefinitionFieldLengths(d); err != nil { + return err + } + configHash, err := hashDefinition(d, true) if err != nil { return errors.Wrap(err, "config hash") diff --git a/cluster/helpers.go b/cluster/helpers.go index e23e9c14a5..6134f67b27 100644 --- a/cluster/helpers.go +++ b/cluster/helpers.go @@ -277,10 +277,27 @@ func putK1SigList(h sszutils.HashWalker, sig []byte, maxAmountSigs int, field st return nil } +// checkBytesN returns an error if b is neither empty nor exactly n bytes long. +// Empty is allowed since absent values (e.g. unsigned operators, missing deposit data or +// builder registrations) are hashed as zeros. Any other length is rejected since ssz +// would otherwise pad it, resulting in the same hash as a different (padded) value. +func checkBytesN(b []byte, n int, field string) error { + if len(b) != 0 && len(b) != n { + return errors.New("invalid fixed size bytes length", + z.Str("field", field), + z.Int("expect", n), + z.Int("actual", len(b)), + ) + } + + return nil +} + // putBytesN appends b as a ssz fixed size byte array of length n. -func putBytesN(h sszutils.HashWalker, b []byte, n int) error { - if len(b) > n { - return errors.New("bytes too long", z.Int("n", n), z.Int("l", len(b))) +// b must be either empty (hashed as zeros) or exactly n bytes long. +func putBytesN(h sszutils.HashWalker, b []byte, n int, field string) error { + if err := checkBytesN(b, n, field); err != nil { + return err } h.PutBytes(leftPad(b, n)) @@ -288,6 +305,19 @@ func putBytesN(h sszutils.HashWalker, b []byte, n int) error { return nil } +// putBytesRaw appends b as is (not padded) after checking it is either empty or exactly n bytes long. +// It is used by older hashing versions that appended fixed size fields without padding, +// where an empty value must still append nothing for backwards compatible hashes. +func putBytesRaw(h sszutils.HashWalker, b []byte, n int, field string) error { + if err := checkBytesN(b, n, field); err != nil { + return err + } + + h.PutBytes(b) + + return nil +} + // putHexBytes20 appends a 20 byte fixed size byte ssz array from the 0xhex address. func putHexBytes20(h sszutils.HashWalker, addr string) error { b, err := from0xHex(addr, addressLen) @@ -328,7 +358,7 @@ func from0xHex(s string, length int) ([]byte, error) { if err != nil { return nil, errors.Wrap(err, "decode hex") } else if len(b) != length { - return nil, errors.Wrap(err, "invalid hex length", z.Int("expect", length), z.Int("actual", len(b))) + return nil, errors.New("invalid hex length", z.Int("expect", length), z.Int("actual", len(b))) } return b, nil diff --git a/cluster/helpers_internal_test.go b/cluster/helpers_internal_test.go index 847345d842..672ef4b1bf 100644 --- a/cluster/helpers_internal_test.go +++ b/cluster/helpers_internal_test.go @@ -149,22 +149,11 @@ func TestPutBytesN(t *testing.T) { b: []byte{0x01, 0x02, 0x03, 0x04}, n: 4, }, - { - name: "short_n32", - b: []byte{0xab, 0xcd}, - n: 32, - }, { name: "full_n4", b: []byte{0xde, 0xad, 0xbe, 0xef}, n: 4, }, - { - name: "after_uint64_1", - setup: func(hh sszutils.HashWalker) { hh.PutUint64(1) }, - b: []byte{0xab, 0xcd}, - n: 32, - }, { name: "after_bytes20", setup: func(hh sszutils.HashWalker) { hh.PutBytes(make([]byte, 20)) }, @@ -177,7 +166,7 @@ func TestPutBytesN(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - fn := func(hh sszutils.HashWalker) error { return putBytesN(hh, tt.b, tt.n) } + fn := func(hh sszutils.HashWalker) error { return putBytesN(hh, tt.b, tt.n, "test") } var got string if tt.setup != nil { @@ -193,6 +182,31 @@ func TestPutBytesN(t *testing.T) { testutil.RequireGoldenJSON(t, roots) } +// TestPutBytesNInvalidLength ensures values that are neither empty nor exactly n bytes are rejected +// instead of being padded, since padding would result in the same hash as the padded value. +func TestPutBytesNInvalidLength(t *testing.T) { + tests := []struct { + name string + b []byte + n int + }{ + {name: "short_n32", b: []byte{0xab, 0xcd}, n: 32}, + {name: "address_as_creds", b: bytes.Repeat([]byte{0x11}, 20), n: 32}, + {name: "long_n4", b: []byte{0x01, 0x02, 0x03, 0x04, 0x05}, n: 4}, + {name: "short_pubkey", b: make([]byte, 47), n: 48}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + hh := hasher.DefaultHasherPool.Get() + defer hasher.DefaultHasherPool.Put(hh) + + require.ErrorContains(t, putBytesN(hh, tt.b, tt.n, "test"), "invalid fixed size bytes length") + require.ErrorContains(t, putBytesRaw(hh, tt.b, tt.n, "test"), "invalid fixed size bytes length") + }) + } +} + func TestPutHexBytes20(t *testing.T) { tests := []struct { name string diff --git a/cluster/lock.go b/cluster/lock.go index 398612bf00..ab1fc24d80 100644 --- a/cluster/lock.go +++ b/cluster/lock.go @@ -118,6 +118,63 @@ func (l *Lock) UnmarshalJSON(data []byte) error { return nil } +// verifyLockFieldLengths returns an error if any fixed size field included in the lock hash has an invalid length. +// Empty values are allowed since they represent absent values (e.g. missing deposit data), see checkBytesN. +// Presence of required values is checked by VerifyHashes and VerifySignatures. +// The definition fields are checked by Definition.VerifyHashes. +// +// Note that SignatureAggregate and NodeSignatures are not included in the lock hash +// and their lengths are checked by VerifySignatures. +func verifyLockFieldLengths(l Lock) error { + if err := checkBytesN(l.LockHash, sszLenHash, "lock_hash"); err != nil { + return err + } + + for i, val := range l.Validators { + if err := verifyDistValidatorFieldLengths(val); err != nil { + return errors.Wrap(err, "invalid distributed validator", z.Int("dv_index", i)) + } + } + + return nil +} + +// verifyDistValidatorFieldLengths returns an error if any fixed size field of the distributed validator has an invalid length. +func verifyDistValidatorFieldLengths(val DistValidator) error { + type fixedField struct { + b []byte + n int + field string + } + + fields := []fixedField{ + {val.PubKey, sszLenPubKey, "distributed_public_key"}, + {val.BuilderRegistration.Message.FeeRecipient, addressLen, "builder_registration_fee_recipient"}, + {val.BuilderRegistration.Message.PubKey, sszLenPubKey, "builder_registration_pubkey"}, + {val.BuilderRegistration.Signature, sszLenBLSSig, "builder_registration_signature"}, + } + + for _, share := range val.PubShares { + fields = append(fields, fixedField{share, sszLenPubKey, "public_shares"}) + } + + for _, dd := range val.PartialDepositData { + fields = append(fields, + fixedField{dd.PubKey, sszLenPubKey, "deposit_data_pubkey"}, + fixedField{dd.WithdrawalCredentials, sszLenWithdrawCreds, "deposit_data_withdrawal_credentials"}, + fixedField{dd.Signature, sszLenBLSSig, "deposit_data_signature"}, + ) + } + + for _, f := range fields { + if err := checkBytesN(f.b, f.n, f.field); err != nil { + return err + } + } + + return nil +} + // SetLockHash returns a copy of the lock with the lock hash populated. func (l Lock) SetLockHash() (Lock, error) { lockHash, err := hashLock(l) @@ -136,6 +193,10 @@ func (l Lock) VerifyHashes() error { return errors.Wrap(err, "invalid definition") } + if err := verifyLockFieldLengths(l); err != nil { + return err + } + if len(l.Validators) != l.NumValidators { return errors.New("lock validators count mismatch", z.Int("definition_num_validators", l.NumValidators), @@ -170,6 +231,10 @@ func (l Lock) VerifySignatures(eth1 eth1wrap.EthClientRunner) error { return errors.New("empty lock aggregate signature") } + if len(l.SignatureAggregate) != sszLenBLSSig { + return errors.New("invalid lock aggregate signature length", z.Int("length", len(l.SignatureAggregate))) + } + sig, err := tblsconv.SignatureFromBytes(l.SignatureAggregate) if err != nil { return err @@ -249,6 +314,10 @@ func (l Lock) verifyNodeSignatures() error { // Verify the node signatures for idx := range len(l.Operators) { + if len(l.NodeSignatures[idx]) != sszLenK1Sig { + return errors.New("invalid node signature length", z.Int("peer_index", idx), z.Int("length", len(l.NodeSignatures[idx]))) + } + record, err := enr.Parse(l.Operators[idx].ENR) if err != nil { return errors.Wrap(err, "operator ENR") diff --git a/cluster/lock_test.go b/cluster/lock_test.go index c757f56316..61aea3ad66 100644 --- a/cluster/lock_test.go +++ b/cluster/lock_test.go @@ -3,12 +3,20 @@ package cluster_test import ( + "bytes" + "encoding/json" "math/rand" "testing" + eth2p0 "github.com/attestantio/go-eth2-client/spec/phase0" + k1 "github.com/decred/dcrd/dcrec/secp256k1/v4" "github.com/stretchr/testify/require" + "github.com/obolnetwork/charon/app/k1util" + "github.com/obolnetwork/charon/app/z" "github.com/obolnetwork/charon/cluster" + "github.com/obolnetwork/charon/eth2util/deposit" + "github.com/obolnetwork/charon/tbls" ) func TestVerifyLock(t *testing.T) { @@ -95,3 +103,333 @@ func TestVerifyLockRejectsDuplicatePublicShares(t *testing.T) { require.Error(t, err) require.Contains(t, err.Error(), "duplicate public share") } + +// newLockWithDepositsForT returns a signed lock of the provided version populated with valid deposit data +// for each of the definition's deposit amounts (or defaults), as well as the p2p keys and secret shares. +func newLockWithDepositsForT(t *testing.T, version string, compounding bool) (cluster.Lock, []*k1.PrivateKey, [][]tbls.PrivateKey) { + t.Helper() + + const seed = 1 + + lock, p2pKeys, shares := cluster.NewForT(t, 2, 3, 4, seed, rand.New(rand.NewSource(seed)), func(d *cluster.Definition) { + d.Version = version + d.Compounding = compounding + + if isAnyVersion(version, v1_6, v1_7, v1_8, v1_9) { + d.TargetGasLimit = 0 + } + }) + + amounts := lock.DepositAmounts + if len(amounts) == 0 { + amounts = deposit.DefaultDepositAmounts(compounding) + if isAnyVersion(version, v1_6, v1_7) { + amounts = []eth2p0.Gwei{deposit.DefaultDepositAmount} + } + } + + for i := range lock.Validators { + if isAnyVersion(version, v1_6) { + lock.Validators[i].BuilderRegistration = cluster.BuilderRegistration{} // Not supported before v1.7. + } + + lock.Validators[i].PartialDepositData = nil + for _, amount := range amounts { + lock.Validators[i].PartialDepositData = append(lock.Validators[i].PartialDepositData, + signDepositForT(t, lock, shares[i], i, amount, compounding)) + } + } + + return resignLockForT(t, lock, p2pKeys, shares), p2pKeys, shares +} + +// signDepositForT returns deposit data for the validator at index i signed with the validator's root secret. +func signDepositForT(t *testing.T, lock cluster.Lock, shares []tbls.PrivateKey, i int, amount eth2p0.Gwei, compounding bool) cluster.DepositData { + t.Helper() + + shareMap := make(map[int]tbls.PrivateKey) + for idx, share := range shares { + shareMap[idx+1] = share + } + + secret, err := tbls.RecoverSecret(shareMap, uint(len(shares)), uint(lock.Threshold)) + require.NoError(t, err) + + msg, err := deposit.NewMessage(eth2p0.BLSPubKey(lock.Validators[i].PubKey), lock.ValidatorAddresses[i].WithdrawalAddress, amount, compounding) + require.NoError(t, err) + + root, err := deposit.GetMessageSigningRootByForkVersion(msg, eth2p0.Version(lock.ForkVersion)) + require.NoError(t, err) + + sig, err := tbls.Sign(secret, root[:]) + require.NoError(t, err) + + return cluster.DepositData{ + PubKey: msg.PublicKey[:], + WithdrawalCredentials: msg.WithdrawalCredentials, + Amount: int(msg.Amount), + Signature: sig[:], + } +} + +// resignLockForT returns the lock with recalculated lock hash, aggregate signature and node signatures. +func resignLockForT(t *testing.T, lock cluster.Lock, p2pKeys []*k1.PrivateKey, shares [][]tbls.PrivateKey) cluster.Lock { + t.Helper() + + lock, err := lock.SetLockHash() + require.NoError(t, err) + + var sigs []tbls.Signature + + for _, dvShares := range shares { + for _, share := range dvShares { + sig, err := tbls.Sign(share, lock.LockHash) + require.NoError(t, err) + + sigs = append(sigs, sig) + } + } + + aggSig, err := tbls.Aggregate(sigs) + require.NoError(t, err) + + lock.SignatureAggregate = aggSig[:] + + if lock.NodeSignatures != nil { + lock.NodeSignatures = nil + for _, p2pKey := range p2pKeys { + nodeSig, err := k1util.Sign(p2pKey, lock.LockHash) + require.NoError(t, err) + + lock.NodeSignatures = append(lock.NodeSignatures, nodeSig) + } + } + + return lock +} + +func TestHashRejectsShortWithdrawalCredentials(t *testing.T) { + lock, _, _ := newLockWithDepositsForT(t, v1_11, false) + + // A 20 byte withdrawal credential (just the address) would be left padded to 32 bytes + // and hash identically to 0x000...
credentials. + creds := lock.Validators[0].PartialDepositData[0].WithdrawalCredentials + lock.Validators[0].PartialDepositData[0].WithdrawalCredentials = creds[12:] + + _, err := lock.SetLockHash() + require.ErrorContains(t, err, "invalid fixed size bytes length") + + require.ErrorContains(t, lock.VerifyHashes(), "invalid fixed size bytes length") +} + +// TestVerifyHashesRejectsInvalidFieldLengths ensures fixed size fields with invalid lengths are rejected by VerifyHashes. +func TestVerifyHashesRejectsInvalidFieldLengths(t *testing.T) { + lock, _, _ := newLockWithDepositsForT(t, v1_11, false) + + b, err := json.Marshal(lock) + require.NoError(t, err) + + short := func(hex string, n int) string { return hex[:len(hex)-2*n] } + long := func(hex string) string { return hex + "00" } + + tests := []struct { + name string + mutate func(m map[string]any) + field string + }{ + { + name: "20 byte withdrawal credentials", + mutate: func(m map[string]any) { + dd := depositJSON(m, 0, 0) + dd["withdrawal_credentials"] = "0x" + dd["withdrawal_credentials"].(string)[2+24:] + }, + field: "deposit_data_withdrawal_credentials", + }, + { + name: "33 byte withdrawal credentials", + mutate: func(m map[string]any) { + dd := depositJSON(m, 0, 0) + dd["withdrawal_credentials"] = long(dd["withdrawal_credentials"].(string)) + }, + field: "deposit_data_withdrawal_credentials", + }, + { + name: "short deposit signature", + mutate: func(m map[string]any) { + dd := depositJSON(m, 0, 1) + dd["signature"] = short(dd["signature"].(string), 1) + }, + field: "deposit_data_signature", + }, + { + name: "short deposit pubkey", + mutate: func(m map[string]any) { + dd := depositJSON(m, 1, 0) + dd["pubkey"] = short(dd["pubkey"].(string), 1) + }, + field: "deposit_data_pubkey", + }, + { + name: "short distributed public key", + mutate: func(m map[string]any) { + dv := dvJSON(m, 0) + dv["distributed_public_key"] = short(dv["distributed_public_key"].(string), 1) + }, + field: "distributed_public_key", + }, + { + name: "long public share", + mutate: func(m map[string]any) { + shares := dvJSON(m, 0)["public_shares"].([]any) + shares[0] = long(shares[0].(string)) + }, + field: "public_shares", + }, + { + name: "short builder registration fee recipient", + mutate: func(m map[string]any) { + reg := dvJSON(m, 0)["builder_registration"].(map[string]any)["message"].(map[string]any) + reg["fee_recipient"] = short(reg["fee_recipient"].(string), 1) + }, + field: "builder_registration_fee_recipient", + }, + { + name: "short builder registration signature", + mutate: func(m map[string]any) { + reg := dvJSON(m, 1)["builder_registration"].(map[string]any) + reg["signature"] = short(reg["signature"].(string), 1) + }, + field: "builder_registration_signature", + }, + { + name: "short lock hash", + mutate: func(m map[string]any) { m["lock_hash"] = short(m["lock_hash"].(string), 1) }, + field: "lock_hash", + }, + { + name: "short fork version", + mutate: func(m map[string]any) { + def := m["cluster_definition"].(map[string]any) + def["fork_version"] = short(def["fork_version"].(string), 1) + }, + field: "fork_version", + }, + { + name: "short config hash", + mutate: func(m map[string]any) { + def := m["cluster_definition"].(map[string]any) + def["config_hash"] = short(def["config_hash"].(string), 1) + }, + field: "config_hash", + }, + { + name: "long definition hash", + mutate: func(m map[string]any) { + def := m["cluster_definition"].(map[string]any) + def["definition_hash"] = long(def["definition_hash"].(string)) + }, + field: "definition_hash", + }, + { + name: "short withdrawal address", + mutate: func(m map[string]any) { + vaddrs := m["cluster_definition"].(map[string]any)["validators"].([]any)[0].(map[string]any) + vaddrs["withdrawal_address"] = short(vaddrs["withdrawal_address"].(string), 1) + }, + field: "withdrawal_address", + }, + { + name: "long fee recipient address", + mutate: func(m map[string]any) { + vaddrs := m["cluster_definition"].(map[string]any)["validators"].([]any)[1].(map[string]any) + vaddrs["fee_recipient_address"] = long(vaddrs["fee_recipient_address"].(string)) + }, + field: "fee_recipient_address", + }, + { + name: "short operator address", + mutate: func(m map[string]any) { + op := m["cluster_definition"].(map[string]any)["operators"].([]any)[0].(map[string]any) + op["address"] = short(op["address"].(string), 1) + }, + field: "operator_address", + }, + { + name: "short operator config signature", + mutate: func(m map[string]any) { + op := m["cluster_definition"].(map[string]any)["operators"].([]any)[0].(map[string]any) + op["config_signature"] = short(op["config_signature"].(string), 1) + }, + field: "operator_config_signature", + }, + { + name: "short creator config signature", + mutate: func(m map[string]any) { + creator := m["cluster_definition"].(map[string]any)["creator"].(map[string]any) + creator["config_signature"] = short(creator["config_signature"].(string), 1) + }, + field: "creator_config_signature", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var m map[string]any + require.NoError(t, json.Unmarshal(b, &m)) + + tt.mutate(m) + + b2, err := json.Marshal(m) + require.NoError(t, err) + require.False(t, bytes.Equal(b, b2)) + + // Unmarshalling must succeed so that --no-verify can still load the lock. + var lock2 cluster.Lock + require.NoError(t, json.Unmarshal(b2, &lock2)) + + err = lock2.VerifyHashes() + require.Error(t, err) + require.True(t, z.ContainsField(err, z.Str("field", tt.field)), "missing field %s: %v", tt.field, err) + }) + } + + t.Run("definition", func(t *testing.T) { + var m map[string]any + require.NoError(t, json.Unmarshal(b, &m)) + + def := m["cluster_definition"].(map[string]any) + def["fork_version"] = "0x000010" + + b2, err := json.Marshal(def) + require.NoError(t, err) + + var def2 cluster.Definition + require.NoError(t, json.Unmarshal(b2, &def2)) + + err = def2.VerifyHashes() + require.ErrorContains(t, err, "invalid fixed size bytes length") + require.True(t, z.ContainsField(err, z.Str("field", "fork_version"))) + }) +} + +func dvJSON(m map[string]any, dvIdx int) map[string]any { + return m["distributed_validators"].([]any)[dvIdx].(map[string]any) +} + +func depositJSON(m map[string]any, dvIdx, ddIdx int) map[string]any { + return dvJSON(m, dvIdx)["partial_deposit_data"].([]any)[ddIdx].(map[string]any) +} + +func TestVerifyLockRejectsInvalidSignatureLengths(t *testing.T) { + lock, p2pKeys, shares := newLockWithDepositsForT(t, v1_11, false) + + short := lock + short.SignatureAggregate = lock.SignatureAggregate[:95] + require.ErrorContains(t, short.VerifySignatures(nil), "invalid lock aggregate signature length") + + short = lock + short.NodeSignatures = [][]byte{lock.NodeSignatures[0][:64], lock.NodeSignatures[1], lock.NodeSignatures[2], lock.NodeSignatures[3]} + require.ErrorContains(t, short.VerifySignatures(nil), "invalid node signature length") + + require.NoError(t, resignLockForT(t, lock, p2pKeys, shares).VerifySignatures(nil)) +} diff --git a/cluster/ssz.go b/cluster/ssz.go index adae8d70c4..d5c9509884 100644 --- a/cluster/ssz.go +++ b/cluster/ssz.go @@ -108,6 +108,10 @@ func hashDefinitionLegacy(d Definition, hh sszutils.HashWalker, configOnly bool) hh.PutBytes([]byte(d.DKGAlgorithm)) // Field (8) 'forkVersion' + if err := checkBytesN(d.ForkVersion, sszLenForkVersion, "fork_version"); err != nil { + return err + } + hh.PutBytes([]byte(to0xHex(d.ForkVersion))) // Field (9) 'addresses' @@ -136,10 +140,14 @@ func hashDefinitionLegacy(d Definition, hh sszutils.HashWalker, configOnly bool) } // Field (2 or 3) 'ConfigSignature' - hh.PutBytes(o.ConfigSignature) + if err := putBytesRaw(hh, o.ConfigSignature, sszLenK1Sig, "config_signature"); err != nil { + return err + } // Field (3 or 4) 'ENRSignature' - hh.PutBytes(o.ENRSignature) + if err := putBytesRaw(hh, o.ENRSignature, sszLenK1Sig, "enr_signature"); err != nil { + return err + } hh.Merkleize(subIdx) } @@ -220,7 +228,9 @@ func hashDefinitionV1x3or4(d Definition, hh sszutils.HashWalker, configOnly bool } // Field (9) 'ForkVersion' Bytes4 - hh.PutBytes(d.ForkVersion) + if err := putBytesRaw(hh, d.ForkVersion, sszLenForkVersion, "fork_version"); err != nil { + return err + } // Field (10) 'Operators' CompositeList[256] { @@ -245,10 +255,14 @@ func hashDefinitionV1x3or4(d Definition, hh sszutils.HashWalker, configOnly bool } // Field (2) 'ConfigSignature' Bytes65 - hh.PutBytes(o.ConfigSignature) + if err := putBytesRaw(hh, o.ConfigSignature, sszLenK1Sig, "config_signature"); err != nil { + return err + } // Field (3) 'ENRSignature' Bytes65 - hh.PutBytes(o.ENRSignature) + if err := putBytesRaw(hh, o.ENRSignature, sszLenK1Sig, "enr_signature"); err != nil { + return err + } } hh.Merkleize(operatorIdx) @@ -271,7 +285,9 @@ func hashDefinitionV1x3or4(d Definition, hh sszutils.HashWalker, configOnly bool if !configOnly { // Field (1) 'ConfigSignature' Bytes65 - hh.PutBytes(d.Creator.ConfigSignature) + if err := putBytesRaw(hh, d.Creator.ConfigSignature, sszLenK1Sig, "creator_config_signature"); err != nil { + return err + } } hh.Merkleize(creatorIdx) @@ -279,7 +295,9 @@ func hashDefinitionV1x3or4(d Definition, hh sszutils.HashWalker, configOnly bool if !configOnly { // Field (12) 'ConfigHash' Bytes32 - hh.PutBytes(d.ConfigHash) + if err := putBytesRaw(hh, d.ConfigHash, sszLenHash, "config_hash"); err != nil { + return err + } } hh.Merkleize(indx) @@ -326,7 +344,7 @@ func hashDefinitionV1x5to9(d Definition, hh sszutils.HashWalker, configOnly bool } // Field (7) 'ForkVersion' Bytes4 - if err := putBytesN(hh, d.ForkVersion, sszLenForkVersion); err != nil { + if err := putBytesN(hh, d.ForkVersion, sszLenForkVersion, "fork_version"); err != nil { return err } @@ -350,12 +368,12 @@ func hashDefinitionV1x5to9(d Definition, hh sszutils.HashWalker, configOnly bool } // Field (2) 'ConfigSignature' Bytes65 - if err := putBytesN(hh, o.ConfigSignature, sszLenK1Sig); err != nil { + if err := putBytesN(hh, o.ConfigSignature, sszLenK1Sig, "config_signature"); err != nil { return err } // Field (3) 'ENRSignature' Bytes65 - if err := putBytesN(hh, o.ENRSignature, sszLenK1Sig); err != nil { + if err := putBytesN(hh, o.ENRSignature, sszLenK1Sig, "enr_signature"); err != nil { return err } } @@ -377,7 +395,7 @@ func hashDefinitionV1x5to9(d Definition, hh sszutils.HashWalker, configOnly bool if !configOnly { // Field (1) 'ConfigSignature' Bytes65 - if err := putBytesN(hh, d.Creator.ConfigSignature, sszLenK1Sig); err != nil { + if err := putBytesN(hh, d.Creator.ConfigSignature, sszLenK1Sig, "creator_config_signature"); err != nil { return err } } @@ -418,7 +436,7 @@ func hashDefinitionV1x5to9(d Definition, hh sszutils.HashWalker, configOnly bool if !configOnly { // Field (last) 'ConfigHash' Bytes32 - if err := putBytesN(hh, d.ConfigHash, sszLenHash); err != nil { + if err := putBytesN(hh, d.ConfigHash, sszLenHash, "config_hash"); err != nil { return err } } @@ -526,7 +544,7 @@ func hashDefinitionV1x11(d Definition, hh sszutils.HashWalker, configOnly bool) } // Field (7) 'ForkVersion' Bytes4 - if err := putBytesN(hh, d.ForkVersion, sszLenForkVersion); err != nil { + if err := putBytesN(hh, d.ForkVersion, sszLenForkVersion, "fork_version"); err != nil { return err } @@ -630,7 +648,9 @@ func hashDefinitionV1x11(d Definition, hh sszutils.HashWalker, configOnly bool) // Field (15) 'ConfigHash' Bytes32 (only for full definition hash) if !configOnly { - hh.PutBytes(d.ConfigHash) + if err := putBytesRaw(hh, d.ConfigHash, sszLenHash, "config_hash"); err != nil { + return err + } } hh.Merkleize(indx) @@ -720,7 +740,7 @@ func hashValidatorPubsharesField(v DistValidator, hh sszutils.HashWalker) error num := uint64(len(v.PubShares)) for _, pubshare := range v.PubShares { - if err := putBytesN(hh, pubshare, sszLenPubKey); err != nil { + if err := putBytesN(hh, pubshare, sszLenPubKey, "public_shares"); err != nil { return err } } @@ -735,7 +755,9 @@ func hashValidatorV1x3Or4(v DistValidator, hh sszutils.HashWalker, _ string) err indx := hh.Index() // Field (0) 'PubKey' Bytes48 - hh.PutBytes(v.PubKey) + if err := putBytesRaw(hh, v.PubKey, sszLenPubKey, "distributed_public_key"); err != nil { + return err + } // Field (1) 'Pubshares' CompositeList[256] if err := hashValidatorPubsharesField(v, hh); err != nil { @@ -755,7 +777,7 @@ func hashValidatorV1x5to7(v DistValidator, hh sszutils.HashWalker, version strin indx := hh.Index() // Field (0) 'PubKey' Bytes48 - if err := putBytesN(hh, v.PubKey, sszLenPubKey); err != nil { + if err := putBytesN(hh, v.PubKey, sszLenPubKey, "distributed_public_key"); err != nil { return err } @@ -799,7 +821,7 @@ func hashValidatorV1x8OrLater(v DistValidator, hh sszutils.HashWalker, version s indx := hh.Index() // Field (0) 'PubKey' Bytes48 - if err := putBytesN(hh, v.PubKey, sszLenPubKey); err != nil { + if err := putBytesN(hh, v.PubKey, sszLenPubKey, "distributed_public_key"); err != nil { return err } @@ -878,6 +900,10 @@ func hashValidatorLegacy(v DistValidator, hh sszutils.HashWalker) error { indx := hh.Index() // Field (0) 'PubKey' + if err := checkBytesN(v.PubKey, sszLenPubKey, "distributed_public_key"); err != nil { + return err + } + hh.PutBytes([]byte(to0xHex(v.PubKey))) // Field (1) 'Pubshares' @@ -886,7 +912,9 @@ func hashValidatorLegacy(v DistValidator, hh sszutils.HashWalker) error { num := uint64(len(v.PubShares)) for _, pubshare := range v.PubShares { - hh.PutBytes(pubshare) + if err := putBytesRaw(hh, pubshare, sszLenPubKey, "public_shares"); err != nil { + return err + } } hh.MerkleizeWithMixin(subIndx, num, num) @@ -930,12 +958,12 @@ func getRegistrationHashFunc(version string) (func(BuilderRegistration, sszutils // Note: There is a bug in this function where we missed merkleize step of DepositData. func hashDepositDataV1x6(d DepositData, hh sszutils.HashWalker) error { // Field (0) 'PubKey' Bytes48 - if err := putBytesN(hh, d.PubKey, sszLenPubKey); err != nil { + if err := putBytesN(hh, d.PubKey, sszLenPubKey, "deposit_pubkey"); err != nil { return err } // Field (1) 'WithdrawalCredentials' Bytes32 - if err := putBytesN(hh, d.WithdrawalCredentials, sszLenWithdrawCreds); err != nil { + if err := putBytesN(hh, d.WithdrawalCredentials, sszLenWithdrawCreds, "withdrawal_credentials"); err != nil { return err } @@ -943,7 +971,7 @@ func hashDepositDataV1x6(d DepositData, hh sszutils.HashWalker) error { hh.PutUint64(uint64(d.Amount)) // Field (3) 'Signature' Bytes96 - return putBytesN(hh, d.Signature, sszLenBLSSig) + return putBytesN(hh, d.Signature, sszLenBLSSig, "deposit_signature") } // hashDepositDataV1x7OrLater hashes the latest deposit data. @@ -951,12 +979,12 @@ func hashDepositDataV1x7OrLater(d DepositData, hh sszutils.HashWalker) error { indx := hh.Index() // Field (0) 'PubKey' Bytes48 - if err := putBytesN(hh, d.PubKey, sszLenPubKey); err != nil { + if err := putBytesN(hh, d.PubKey, sszLenPubKey, "deposit_pubkey"); err != nil { return err } // Field (1) 'WithdrawalCredentials' Bytes32 - if err := putBytesN(hh, d.WithdrawalCredentials, sszLenWithdrawCreds); err != nil { + if err := putBytesN(hh, d.WithdrawalCredentials, sszLenWithdrawCreds, "withdrawal_credentials"); err != nil { return err } @@ -964,7 +992,7 @@ func hashDepositDataV1x7OrLater(d DepositData, hh sszutils.HashWalker) error { hh.PutUint64(uint64(d.Amount)) // Field (3) 'Signature' Bytes96 - if err := putBytesN(hh, d.Signature, sszLenBLSSig); err != nil { + if err := putBytesN(hh, d.Signature, sszLenBLSSig, "deposit_signature"); err != nil { return err } @@ -983,7 +1011,7 @@ func hashBuilderRegistration(b BuilderRegistration, hh sszutils.HashWalker) erro } // Field (1) 'Signature' Bytes96 - if err := putBytesN(hh, b.Signature, sszLenBLSSig); err != nil { + if err := putBytesN(hh, b.Signature, sszLenBLSSig, "registration_signature"); err != nil { return err } @@ -996,8 +1024,10 @@ func hashBuilderRegistration(b BuilderRegistration, hh sszutils.HashWalker) erro func hashRegistration(r Registration, hh sszutils.HashWalker) error { indx := hh.Index() - // Field (0) 'FeeRecipient' - hh.PutBytes(r.FeeRecipient) + // Field (0) 'FeeRecipient' Bytes20 + if err := putBytesRaw(hh, r.FeeRecipient, addressLen, "fee_recipient"); err != nil { + return err + } // Field (1) 'GasLimit' uint64 hh.PutUint64(uint64(r.GasLimit)) @@ -1006,7 +1036,7 @@ func hashRegistration(r Registration, hh sszutils.HashWalker) error { hh.PutUint64(uint64(r.Timestamp.Unix())) // Field (3) 'PubKey' Bytes48 - if err := putBytesN(hh, r.PubKey, sszLenPubKey); err != nil { + if err := putBytesN(hh, r.PubKey, sszLenPubKey, "registration_pubkey"); err != nil { return err } diff --git a/cluster/testdata/TestPutBytesN.golden b/cluster/testdata/TestPutBytesN.golden index 393877ade8..08a98047ca 100644 --- a/cluster/testdata/TestPutBytesN.golden +++ b/cluster/testdata/TestPutBytesN.golden @@ -1,9 +1,7 @@ { "4bytes_n4": "0102030400000000000000000000000000000000000000000000000000000000", "after_bytes20": "c092c674a087720f7136046e41d587e355a5f359b3940336f8e4e9dde2ffe236", - "after_uint64_1": "392b769bc14dd0593bbb9d28f2fa7c4ec6eb113ef750db4046b7136c5844c95d", "full_n4": "deadbeef00000000000000000000000000000000000000000000000000000000", "nil_n20": "0000000000000000000000000000000000000000000000000000000000000000", - "nil_n32": "0000000000000000000000000000000000000000000000000000000000000000", - "short_n32": "000000000000000000000000000000000000000000000000000000000000abcd" + "nil_n32": "0000000000000000000000000000000000000000000000000000000000000000" } \ No newline at end of file diff --git a/cluster/testdata/cluster_lock_v1_0_0.json b/cluster/testdata/cluster_lock_v1_0_0.json index d3d37c1543..dd8292fb98 100644 --- a/cluster/testdata/cluster_lock_v1_0_0.json +++ b/cluster/testdata/cluster_lock_v1_0_0.json @@ -45,6 +45,6 @@ ] } ], - "signature_aggregate": "sqS0R11jr76PtWmHx39YGFJvGBS+gjNQ6rE5NfMdhEg=", + "signature_aggregate": "sqS0R11jr76PtWmHx39YGFJvGBS+gjNQ6rE5NfMdhEgvgoLL4vlpbzFEwKpM7Vbb2WfcKJeAavO+2KY6yhbhi2hroNwgjP7OZb1woj2gAmtmEI+60IRDY/4J3Wp3PiG4", "lock_hash": "GGj7/vrt/Dx0HVC86ULGjJc39Z+olAsvsGUxbpsMmhE=" } \ No newline at end of file diff --git a/cluster/testdata/cluster_lock_v1_10_0.json b/cluster/testdata/cluster_lock_v1_10_0.json index 01b7814c6f..609acf8751 100644 --- a/cluster/testdata/cluster_lock_v1_10_0.json +++ b/cluster/testdata/cluster_lock_v1_10_0.json @@ -108,10 +108,10 @@ ] } ], - "signature_aggregate": "0x1c1fd3be8990434179d3af4491a369012db92d184fc39d1734ff5716428953bb", + "signature_aggregate": "0x1c1fd3be8990434179d3af4491a369012db92d184fc39d1734ff5716428953bb2f8282cbe2f9696f3144c0aa4ced56dbd967dc2897806af3bed8a63aca16e18b686ba0dc208cfece65bd70a23da0026b66108fbad0844363fe09dd6a773e21b8", "lock_hash": "0xb1613824b71f57f877be2cdf230e0baa8ab2cc886f57171157be5180961764f0", "node_signatures": [ - "0xfa3f85b8a62708caebbac880b5b89b93da53810164402104e648b6226a1b7802", - "0x1851f5d9ac0f313a89ddfc454c5f8f72ac89b38b19f53784c19e9beac03c875a" + "0xfa3f85b8a62708caebbac880b5b89b93da53810164402104e648b6226a1b7802236a37f8283efb27367f6ee35437869c4043725d5ea2c63b01af2fcbb387de40da", + "0x1851f5d9ac0f313a89ddfc454c5f8f72ac89b38b19f53784c19e9beac03c875aac6225423c14a994dda08f399b7888fcb6c84703dd101ac77cf000e49b2a33f748" ] } \ No newline at end of file diff --git a/cluster/testdata/cluster_lock_v1_11_0.json b/cluster/testdata/cluster_lock_v1_11_0.json index 659e032469..df9b75cb8e 100644 --- a/cluster/testdata/cluster_lock_v1_11_0.json +++ b/cluster/testdata/cluster_lock_v1_11_0.json @@ -108,10 +108,10 @@ ] } ], - "signature_aggregate": "0x1c1fd3be8990434179d3af4491a369012db92d184fc39d1734ff5716428953bb", + "signature_aggregate": "0x1c1fd3be8990434179d3af4491a369012db92d184fc39d1734ff5716428953bb2f8282cbe2f9696f3144c0aa4ced56dbd967dc2897806af3bed8a63aca16e18b686ba0dc208cfece65bd70a23da0026b66108fbad0844363fe09dd6a773e21b8", "lock_hash": "0xecb029e3f8fb17a280a8960718d21aef2506d861b8b1a0ca9a95d8ee15dae8e0", "node_signatures": [ - "0xfa3f85b8a62708caebbac880b5b89b93da53810164402104e648b6226a1b7802", - "0x1851f5d9ac0f313a89ddfc454c5f8f72ac89b38b19f53784c19e9beac03c875a" + "0xfa3f85b8a62708caebbac880b5b89b93da53810164402104e648b6226a1b7802236a37f8283efb27367f6ee35437869c4043725d5ea2c63b01af2fcbb387de40da", + "0x1851f5d9ac0f313a89ddfc454c5f8f72ac89b38b19f53784c19e9beac03c875aac6225423c14a994dda08f399b7888fcb6c84703dd101ac77cf000e49b2a33f748" ] } \ No newline at end of file diff --git a/cluster/testdata/cluster_lock_v1_1_0.json b/cluster/testdata/cluster_lock_v1_1_0.json index 31b96647fc..8543e45ea3 100644 --- a/cluster/testdata/cluster_lock_v1_1_0.json +++ b/cluster/testdata/cluster_lock_v1_1_0.json @@ -45,6 +45,6 @@ ] } ], - "signature_aggregate": "sqS0R11jr76PtWmHx39YGFJvGBS+gjNQ6rE5NfMdhEg=", + "signature_aggregate": "sqS0R11jr76PtWmHx39YGFJvGBS+gjNQ6rE5NfMdhEgvgoLL4vlpbzFEwKpM7Vbb2WfcKJeAavO+2KY6yhbhi2hroNwgjP7OZb1woj2gAmtmEI+60IRDY/4J3Wp3PiG4", "lock_hash": "vJv0y0sxA4IisRD0EORwTw9KzDL8AmoIZyo8WdTPrmM=" } \ No newline at end of file diff --git a/cluster/testdata/cluster_lock_v1_2_0.json b/cluster/testdata/cluster_lock_v1_2_0.json index b964e10fb9..2a71e23732 100644 --- a/cluster/testdata/cluster_lock_v1_2_0.json +++ b/cluster/testdata/cluster_lock_v1_2_0.json @@ -43,6 +43,6 @@ ] } ], - "signature_aggregate": "0xb2a4b4475d63afbe8fb56987c77f5818526f1814be823350eab13935f31d8448", + "signature_aggregate": "0xb2a4b4475d63afbe8fb56987c77f5818526f1814be823350eab13935f31d84482f8282cbe2f9696f3144c0aa4ced56dbd967dc2897806af3bed8a63aca16e18b686ba0dc208cfece65bd70a23da0026b66108fbad0844363fe09dd6a773e21b8", "lock_hash": "0xac571367423f87187880d7b83b0cbf003841540fe9cb8d55ced56616666d9bc0" } \ No newline at end of file diff --git a/cluster/testdata/cluster_lock_v1_3_0.json b/cluster/testdata/cluster_lock_v1_3_0.json index d34dc1b3de..c6a5eed574 100644 --- a/cluster/testdata/cluster_lock_v1_3_0.json +++ b/cluster/testdata/cluster_lock_v1_3_0.json @@ -43,6 +43,6 @@ ] } ], - "signature_aggregate": "0xb2a4b4475d63afbe8fb56987c77f5818526f1814be823350eab13935f31d8448", + "signature_aggregate": "0xb2a4b4475d63afbe8fb56987c77f5818526f1814be823350eab13935f31d84482f8282cbe2f9696f3144c0aa4ced56dbd967dc2897806af3bed8a63aca16e18b686ba0dc208cfece65bd70a23da0026b66108fbad0844363fe09dd6a773e21b8", "lock_hash": "0xafcdd37f6bca760fee75c9b425425f1ba3320bacbe9949f874981a0363325259" } \ No newline at end of file diff --git a/cluster/testdata/cluster_lock_v1_4_0.json b/cluster/testdata/cluster_lock_v1_4_0.json index 79e2892da5..ea1bcd6674 100644 --- a/cluster/testdata/cluster_lock_v1_4_0.json +++ b/cluster/testdata/cluster_lock_v1_4_0.json @@ -47,6 +47,6 @@ ] } ], - "signature_aggregate": "0xb2a4b4475d63afbe8fb56987c77f5818526f1814be823350eab13935f31d8448", + "signature_aggregate": "0xb2a4b4475d63afbe8fb56987c77f5818526f1814be823350eab13935f31d84482f8282cbe2f9696f3144c0aa4ced56dbd967dc2897806af3bed8a63aca16e18b686ba0dc208cfece65bd70a23da0026b66108fbad0844363fe09dd6a773e21b8", "lock_hash": "0x1ca04d20ca3a48100a4d4e2ee8c7081351879fe372691632f2cb1b7e02498a6e" } \ No newline at end of file diff --git a/cluster/testdata/cluster_lock_v1_5_0.json b/cluster/testdata/cluster_lock_v1_5_0.json index ae553a5410..550a866da5 100644 --- a/cluster/testdata/cluster_lock_v1_5_0.json +++ b/cluster/testdata/cluster_lock_v1_5_0.json @@ -55,6 +55,6 @@ ] } ], - "signature_aggregate": "0x1c1fd3be8990434179d3af4491a369012db92d184fc39d1734ff5716428953bb", + "signature_aggregate": "0x1c1fd3be8990434179d3af4491a369012db92d184fc39d1734ff5716428953bb2f8282cbe2f9696f3144c0aa4ced56dbd967dc2897806af3bed8a63aca16e18b686ba0dc208cfece65bd70a23da0026b66108fbad0844363fe09dd6a773e21b8", "lock_hash": "0xf437572afcce5138cab7cb652abbfafa1fc0c248b1098cb430a275503ceea978" } \ No newline at end of file diff --git a/cluster/testdata/cluster_lock_v1_6_0.json b/cluster/testdata/cluster_lock_v1_6_0.json index 5bdbb84011..965916a19e 100644 --- a/cluster/testdata/cluster_lock_v1_6_0.json +++ b/cluster/testdata/cluster_lock_v1_6_0.json @@ -67,6 +67,6 @@ } } ], - "signature_aggregate": "0x1c1fd3be8990434179d3af4491a369012db92d184fc39d1734ff5716428953bb", + "signature_aggregate": "0x1c1fd3be8990434179d3af4491a369012db92d184fc39d1734ff5716428953bb2f8282cbe2f9696f3144c0aa4ced56dbd967dc2897806af3bed8a63aca16e18b686ba0dc208cfece65bd70a23da0026b66108fbad0844363fe09dd6a773e21b8", "lock_hash": "0x5931cc96a335b2dc6d781db06bc9a1e3e41550f2ff3b10dc5c757f225960de86" } \ No newline at end of file diff --git a/cluster/testdata/cluster_lock_v1_7_0.json b/cluster/testdata/cluster_lock_v1_7_0.json index 402b14cf05..821c38a723 100644 --- a/cluster/testdata/cluster_lock_v1_7_0.json +++ b/cluster/testdata/cluster_lock_v1_7_0.json @@ -85,10 +85,10 @@ } } ], - "signature_aggregate": "0x1c1fd3be8990434179d3af4491a369012db92d184fc39d1734ff5716428953bb", + "signature_aggregate": "0x1c1fd3be8990434179d3af4491a369012db92d184fc39d1734ff5716428953bb2f8282cbe2f9696f3144c0aa4ced56dbd967dc2897806af3bed8a63aca16e18b686ba0dc208cfece65bd70a23da0026b66108fbad0844363fe09dd6a773e21b8", "lock_hash": "0xbb0ce2dc50aa923842795e07b4f2a0a23c41177f1135855798ec5570a088926d", "node_signatures": [ - "0xfa3f85b8a62708caebbac880b5b89b93da53810164402104e648b6226a1b7802", - "0x1851f5d9ac0f313a89ddfc454c5f8f72ac89b38b19f53784c19e9beac03c875a" + "0xfa3f85b8a62708caebbac880b5b89b93da53810164402104e648b6226a1b7802236a37f8283efb27367f6ee35437869c4043725d5ea2c63b01af2fcbb387de40da", + "0x1851f5d9ac0f313a89ddfc454c5f8f72ac89b38b19f53784c19e9beac03c875aac6225423c14a994dda08f399b7888fcb6c84703dd101ac77cf000e49b2a33f748" ] } \ No newline at end of file diff --git a/cluster/testdata/cluster_lock_v1_8_0.json b/cluster/testdata/cluster_lock_v1_8_0.json index da5e50e99b..505b04be17 100644 --- a/cluster/testdata/cluster_lock_v1_8_0.json +++ b/cluster/testdata/cluster_lock_v1_8_0.json @@ -105,10 +105,10 @@ ] } ], - "signature_aggregate": "0x1c1fd3be8990434179d3af4491a369012db92d184fc39d1734ff5716428953bb", + "signature_aggregate": "0x1c1fd3be8990434179d3af4491a369012db92d184fc39d1734ff5716428953bb2f8282cbe2f9696f3144c0aa4ced56dbd967dc2897806af3bed8a63aca16e18b686ba0dc208cfece65bd70a23da0026b66108fbad0844363fe09dd6a773e21b8", "lock_hash": "0xefbdb47840715c7a3bb0fa795089afd713a863c8f523bfcb46e0ff8950b01b0d", "node_signatures": [ - "0xfa3f85b8a62708caebbac880b5b89b93da53810164402104e648b6226a1b7802", - "0x1851f5d9ac0f313a89ddfc454c5f8f72ac89b38b19f53784c19e9beac03c875a" + "0xfa3f85b8a62708caebbac880b5b89b93da53810164402104e648b6226a1b7802236a37f8283efb27367f6ee35437869c4043725d5ea2c63b01af2fcbb387de40da", + "0x1851f5d9ac0f313a89ddfc454c5f8f72ac89b38b19f53784c19e9beac03c875aac6225423c14a994dda08f399b7888fcb6c84703dd101ac77cf000e49b2a33f748" ] } \ No newline at end of file diff --git a/cluster/testdata/cluster_lock_v1_9_0.json b/cluster/testdata/cluster_lock_v1_9_0.json index 06c82e7bf2..3e05bdee08 100644 --- a/cluster/testdata/cluster_lock_v1_9_0.json +++ b/cluster/testdata/cluster_lock_v1_9_0.json @@ -106,10 +106,10 @@ ] } ], - "signature_aggregate": "0x1c1fd3be8990434179d3af4491a369012db92d184fc39d1734ff5716428953bb", + "signature_aggregate": "0x1c1fd3be8990434179d3af4491a369012db92d184fc39d1734ff5716428953bb2f8282cbe2f9696f3144c0aa4ced56dbd967dc2897806af3bed8a63aca16e18b686ba0dc208cfece65bd70a23da0026b66108fbad0844363fe09dd6a773e21b8", "lock_hash": "0x6a2e65973460f2392c797ccee621cdce72dc4eb3536d3afa7a5d0947e2d2adc0", "node_signatures": [ - "0xfa3f85b8a62708caebbac880b5b89b93da53810164402104e648b6226a1b7802", - "0x1851f5d9ac0f313a89ddfc454c5f8f72ac89b38b19f53784c19e9beac03c875a" + "0xfa3f85b8a62708caebbac880b5b89b93da53810164402104e648b6226a1b7802236a37f8283efb27367f6ee35437869c4043725d5ea2c63b01af2fcbb387de40da", + "0x1851f5d9ac0f313a89ddfc454c5f8f72ac89b38b19f53784c19e9beac03c875aac6225423c14a994dda08f399b7888fcb6c84703dd101ac77cf000e49b2a33f748" ] } \ No newline at end of file diff --git a/cmd/exit.go b/cmd/exit.go index 428b9334e3..19ce853e81 100644 --- a/cmd/exit.go +++ b/cmd/exit.go @@ -11,8 +11,10 @@ import ( "github.com/spf13/cobra" "github.com/obolnetwork/charon/app/errors" + "github.com/obolnetwork/charon/app/eth1wrap" "github.com/obolnetwork/charon/app/eth2wrap" "github.com/obolnetwork/charon/app/log" + "github.com/obolnetwork/charon/cluster" "github.com/obolnetwork/charon/eth2util" "github.com/obolnetwork/charon/eth2util/signing" "github.com/obolnetwork/charon/tbls" @@ -39,6 +41,7 @@ type exitConfig struct { Log log.Config All bool AllowIncompleteKeystores bool + NoVerify bool testnetConfig eth2util.Network BeaconNodeHeaders []string FallbackBeaconNodeAddrs []string @@ -82,6 +85,7 @@ const ( fallbackBeaconNodeAddrs executionClientRPCEndpoint allowIncompleteKeystores + noVerify ) func (ef exitFlag) String() string { @@ -132,6 +136,8 @@ func (ef exitFlag) String() string { return "execution-client-rpc-endpoint" case allowIncompleteKeystores: return "allow-incomplete-keystores" + case noVerify: + return "no-verify" default: return "unknown" } @@ -202,6 +208,8 @@ func bindExitFlags(cmd *cobra.Command, config *exitConfig, flags []exitCLIFlag) cmd.Flags().StringVar(&config.ExecutionEngineAddr, executionClientRPCEndpoint.String(), "", maybeRequired("The address of the execution engine JSON-RPC API, used to verify ERC-1271 (e.g. Safe multisig) cluster signatures.")) case allowIncompleteKeystores: cmd.Flags().BoolVar(&config.AllowIncompleteKeystores, allowIncompleteKeystores.String(), false, "Allow the validator keys directory to contain key shares for only a subset of the cluster's validators. The command operates on the validators whose key shares are present.") + case noVerify: + cmd.Flags().BoolVar(&config.NoVerify, noVerify.String(), false, "Disables cluster lock file verification. Exit messages are still verified by the Obol API and the beacon chain.") } if f.required { @@ -265,3 +273,16 @@ func sigDataForExit(ctx context.Context, exit eth2p0.VoluntaryExit, eth2Cl eth2w return sigData, nil } + +// loadExitLock loads the cluster lock, verifying it unless --no-verify is set. +// Exits are safe to sign and broadcast without lock verification: the Obol API verifies partial +// exits against its own copy of the lock, and the beacon chain verifies the aggregate signature +// against the validator's public key. +func loadExitLock(ctx context.Context, config exitConfig) (*cluster.Lock, error) { + eth1Cl := eth1wrap.NewDefaultEthClientRunner(config.ExecutionEngineAddr) + if err := eth1wrap.RunAndWait(ctx, eth1Cl); err != nil { + return nil, err + } + + return cluster.LoadClusterLock(ctx, config.LockFilePath, config.NoVerify, eth1Cl) +} diff --git a/cmd/exit_broadcast.go b/cmd/exit_broadcast.go index 19208d9d0c..7b8e738c23 100644 --- a/cmd/exit_broadcast.go +++ b/cmd/exit_broadcast.go @@ -57,6 +57,7 @@ func newBcastFullExitCmd(runFunc func(context.Context, exitConfig) error) *cobra {publishAddress, false}, {privateKeyPath, false}, {lockFilePath, false}, + {noVerify, false}, {executionClientRPCEndpoint, false}, {validatorKeysDir, false}, {exitEpoch, false}, @@ -126,7 +127,7 @@ func runBcastFullExit(ctx context.Context, config exitConfig) error { return errors.Wrap(err, "load identity key") } - cl, err := cluster.LoadClusterLockAndVerify(ctx, config.LockFilePath, config.ExecutionEngineAddr) + cl, err := loadExitLock(ctx, config) if err != nil { return err } diff --git a/cmd/exit_fetch.go b/cmd/exit_fetch.go index 634c2e9435..f71b68702d 100644 --- a/cmd/exit_fetch.go +++ b/cmd/exit_fetch.go @@ -48,6 +48,7 @@ func newFetchExitCmd(runFunc func(context.Context, exitConfig) error) *cobra.Com {publishAddress, false}, {privateKeyPath, false}, {lockFilePath, false}, + {noVerify, false}, {executionClientRPCEndpoint, false}, {validatorPubkey, false}, {all, false}, @@ -108,7 +109,7 @@ func runFetchExit(ctx context.Context, config exitConfig) error { return errors.Wrap(err, "load identity key", z.Str("private_key_path", config.PrivateKeyPath)) } - cl, err := cluster.LoadClusterLockAndVerify(ctx, config.LockFilePath, config.ExecutionEngineAddr) + cl, err := loadExitLock(ctx, config) if err != nil { return err } diff --git a/cmd/exit_internal_test.go b/cmd/exit_internal_test.go new file mode 100644 index 0000000000..d4f7d46ff5 --- /dev/null +++ b/cmd/exit_internal_test.go @@ -0,0 +1,34 @@ +// Copyright © 2022-2026 Obol Labs Inc. Licensed under the terms of a Business Source License 1.1 + +package cmd + +import ( + "bytes" + "encoding/json" + "math/rand" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/obolnetwork/charon/cluster" +) + +func TestLoadExitLockNoVerify(t *testing.T) { + lock, _, _ := cluster.NewForT(t, 1, 3, 4, 0, rand.New(rand.NewSource(0))) + lock.SignatureAggregate = bytes.Repeat([]byte{0x01}, 96) // Right length, invalid signature. + + b, err := json.Marshal(lock) + require.NoError(t, err) + + lockPath := filepath.Join(t.TempDir(), "cluster-lock.json") + require.NoError(t, os.WriteFile(lockPath, b, 0o600)) + + _, err = loadExitLock(t.Context(), exitConfig{LockFilePath: lockPath}) + require.ErrorContains(t, err, "verify lock signature aggregate") + + got, err := loadExitLock(t.Context(), exitConfig{LockFilePath: lockPath, NoVerify: true}) + require.NoError(t, err) + require.Equal(t, lock.LockHash, got.LockHash) +} diff --git a/cmd/exit_sign.go b/cmd/exit_sign.go index 93c53b120d..0971d8a18a 100644 --- a/cmd/exit_sign.go +++ b/cmd/exit_sign.go @@ -20,7 +20,6 @@ import ( "github.com/obolnetwork/charon/app/log" "github.com/obolnetwork/charon/app/obolapi" "github.com/obolnetwork/charon/app/z" - "github.com/obolnetwork/charon/cluster" "github.com/obolnetwork/charon/core" "github.com/obolnetwork/charon/eth2util" "github.com/obolnetwork/charon/eth2util/keystore" @@ -51,6 +50,7 @@ func newSignPartialExitCmd(runFunc func(context.Context, exitConfig) error) *cob {publishAddress, false}, {privateKeyPath, false}, {lockFilePath, false}, + {noVerify, false}, {executionClientRPCEndpoint, false}, {validatorKeysDir, false}, {exitEpoch, false}, @@ -112,7 +112,7 @@ func runSignPartialExit(ctx context.Context, config exitConfig) error { return errors.Wrap(err, "load identity key", z.Str("private_key_path", config.PrivateKeyPath)) } - cl, err := cluster.LoadClusterLockAndVerify(ctx, config.LockFilePath, config.ExecutionEngineAddr) + cl, err := loadExitLock(ctx, config) if err != nil { return err } diff --git a/eth2util/deposit/deposit.go b/eth2util/deposit/deposit.go index a6210e3db0..5ecba4e518 100644 --- a/eth2util/deposit/deposit.go +++ b/eth2util/deposit/deposit.go @@ -184,11 +184,6 @@ func getDepositDomain(forkVersion eth2p0.Version) (eth2p0.Domain, error) { // GetMessageSigningRoot returns the deposit message signing root created by the provided parameters. func GetMessageSigningRoot(msg eth2p0.DepositMessage, network string) ([32]byte, error) { - msgRoot, err := msg.HashTreeRoot() - if err != nil { - return [32]byte{}, errors.Wrap(err, "deposit message root") - } - fv, err := eth2util.NetworkToForkVersionBytes(network) if err != nil { return [32]byte{}, err @@ -197,6 +192,17 @@ func GetMessageSigningRoot(msg eth2p0.DepositMessage, network string) ([32]byte, var forkVersion eth2p0.Version copy(forkVersion[:], fv) + return GetMessageSigningRootByForkVersion(msg, forkVersion) +} + +// GetMessageSigningRootByForkVersion returns the deposit message signing root for the provided genesis fork version. +// Unlike GetMessageSigningRoot, it also supports networks unknown to charon. +func GetMessageSigningRootByForkVersion(msg eth2p0.DepositMessage, forkVersion eth2p0.Version) ([32]byte, error) { + msgRoot, err := msg.HashTreeRoot() + if err != nil { + return [32]byte{}, errors.Wrap(err, "deposit message root") + } + domain, err := getDepositDomain(forkVersion) if err != nil { return [32]byte{}, err