From 35bce889e9cd28d169d4ce5938b0ccc5fb951faa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ois=C3=ADn=20Kyne?= Date: Wed, 4 Nov 2026 01:32:35 +0000 Subject: [PATCH] cluster: reject fixed size fields of the wrong length putBytesN zero-padded short values, so for example 20 byte withdrawal credentials hashed the same as their padded 32 byte form and passed lock verification. Fixed size definition and lock fields must now be empty or exactly the right length, checked by VerifyHashes and when hashing. Lock aggregate and node signature lengths are checked by VerifySignatures. Unmarshalling stays lenient so --no-verify can still load such a lock. Adds --no-verify to exit sign, fetch and broadcast so operators can still exit with a lock that fails verification; exit messages are still verified by the Obol API and the beacon chain. category: bug ticket: none --- cluster/cluster_test.go | 43 ++- cluster/definition.go | 64 ++++ cluster/helpers.go | 38 ++- cluster/helpers_internal_test.go | 38 ++- cluster/lock.go | 69 +++++ cluster/lock_test.go | 338 +++++++++++++++++++++ cluster/ssz.go | 88 ++++-- cluster/testdata/TestPutBytesN.golden | 4 +- cluster/testdata/cluster_lock_v1_0_0.json | 2 +- cluster/testdata/cluster_lock_v1_10_0.json | 6 +- cluster/testdata/cluster_lock_v1_11_0.json | 6 +- cluster/testdata/cluster_lock_v1_1_0.json | 2 +- cluster/testdata/cluster_lock_v1_2_0.json | 2 +- cluster/testdata/cluster_lock_v1_3_0.json | 2 +- cluster/testdata/cluster_lock_v1_4_0.json | 2 +- cluster/testdata/cluster_lock_v1_5_0.json | 2 +- cluster/testdata/cluster_lock_v1_6_0.json | 2 +- cluster/testdata/cluster_lock_v1_7_0.json | 6 +- cluster/testdata/cluster_lock_v1_8_0.json | 6 +- cluster/testdata/cluster_lock_v1_9_0.json | 6 +- cmd/exit.go | 21 ++ cmd/exit_broadcast.go | 3 +- cmd/exit_fetch.go | 3 +- cmd/exit_internal_test.go | 34 +++ cmd/exit_sign.go | 4 +- eth2util/deposit/deposit.go | 16 +- 26 files changed, 725 insertions(+), 82 deletions(-) create mode 100644 cmd/exit_internal_test.go 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