Commit f11f51f9 authored by mergify[bot]'s avatar mergify[bot] Committed by GitHub

Merge pull request #4864 from ethereum-optimism/ajsutton/fix-signing-hash

fix(op-node): Include chainID in p2p signing hash
parents c9cd1215 9caa2f14
......@@ -459,7 +459,7 @@ func (cfg SystemConfig) Start() (*System, error) {
c.P2P = p
if c.Driver.SequencerEnabled {
c.P2PSigner = &p2p.PreparedSigner{Signer: p2p.NewLocalSigner(cfg.Secrets.SequencerP2P)}
c.P2PSigner = &p2p.PreparedSigner{Signer: p2p.NewLegacyLocalSigner(cfg.Secrets.SequencerP2P)}
}
}
......
......@@ -8,6 +8,7 @@ require (
github.com/decred/dcrd/dcrec/secp256k1/v4 v4.1.0
github.com/ethereum-optimism/optimism/op-bindings v0.10.14
github.com/ethereum-optimism/optimism/op-chain-ops v0.10.14
github.com/ethereum-optimism/optimism/op-e2e v0.10.14
github.com/ethereum-optimism/optimism/op-service v0.10.14
github.com/ethereum/go-ethereum v1.10.26
github.com/golang/snappy v0.0.4
......@@ -49,6 +50,7 @@ require (
github.com/docker/go-units v0.5.0 // indirect
github.com/edsrzf/mmap-go v1.1.0 // indirect
github.com/elastic/gosigar v0.14.2 // indirect
github.com/ethereum-optimism/go-ethereum-hdwallet v0.1.3 // indirect
github.com/fjl/memsize v0.0.1 // indirect
github.com/flynn/noise v1.0.0 // indirect
github.com/francoispqt/gojay v1.2.13 // indirect
......@@ -136,6 +138,7 @@ require (
github.com/syndtr/goleveldb v1.0.1-0.20220614013038-64ee5596c38a // indirect
github.com/tklauser/go-sysconf v0.3.10 // indirect
github.com/tklauser/numcpus v0.5.0 // indirect
github.com/tyler-smith/go-bip39 v1.1.0 // indirect
github.com/urfave/cli/v2 v2.17.2-0.20221006022127-8f469abc00aa // indirect
github.com/whyrusleeping/timecache v0.0.0-20160911033111-cfcb2f1abfee // indirect
github.com/xrash/smetrics v0.0.0-20201216005158-039620a65673 // indirect
......@@ -147,7 +150,7 @@ require (
golang.org/x/exp v0.0.0-20220916125017-b168a2c6b86b // indirect
golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4 // indirect
golang.org/x/net v0.0.0-20220920183852-bf014ff85ad5 // indirect
golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4 // indirect
golang.org/x/sync v0.1.0 // indirect
golang.org/x/sys v0.0.0-20221013171732-95e765b1cc43 // indirect
golang.org/x/tools v0.1.12 // indirect
google.golang.org/protobuf v1.28.1 // indirect
......
......@@ -143,12 +143,16 @@ github.com/envoyproxy/go-control-plane v0.9.0/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymF
github.com/envoyproxy/go-control-plane v0.9.1-0.20191026205805-5f8ba28d4473/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymFceY/DCBVvsKhRF0iEA4=
github.com/envoyproxy/go-control-plane v0.9.4/go.mod h1:6rpuAdCZL397s3pYoYcLgu1mIlRU8Am5FuJP05cCM98=
github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7+kN2VEUnK/pcBlmesArF7c=
github.com/ethereum-optimism/go-ethereum-hdwallet v0.1.3 h1:RWHKLhCrQThMfch+QJ1Z8veEq5ZO3DfIhZ7xgRP9WTc=
github.com/ethereum-optimism/go-ethereum-hdwallet v0.1.3/go.mod h1:QziizLAiF0KqyLdNJYD7O5cpDlaFMNZzlxYNcWsJUxs=
github.com/ethereum-optimism/op-geth v0.0.0-20221216190603-60b51d600468 h1:7KgjBYDji5AKi42eRYI+n8Gs+ZJVilSASL3WBu82c3M=
github.com/ethereum-optimism/op-geth v0.0.0-20221216190603-60b51d600468/go.mod h1:p0Yox74PhYlq1HvijrCBCD9A3cI7rXco7hT6KrQr+rY=
github.com/ethereum-optimism/optimism/op-bindings v0.10.14 h1:SMMnMdNb1QIhJDyvk7QMUv+crAP4UHHoSYBOASBDIjM=
github.com/ethereum-optimism/optimism/op-bindings v0.10.14/go.mod h1:9ZSUq/rjlzp3uYyBN4sZmhTc3oZgDVqJ4wrUja7vj6c=
github.com/ethereum-optimism/optimism/op-chain-ops v0.10.14 h1:QuO2E5jmMFlKwLVbXRSKV8oRgdOSqD0BCWdEpy7q34k=
github.com/ethereum-optimism/optimism/op-chain-ops v0.10.14/go.mod h1:j6j2eztwgX4NzLBq+vDVpZH5b1JXLXStMJWuM8zKR30=
github.com/ethereum-optimism/optimism/op-e2e v0.10.14 h1:EUAwz7A/qyn5oXwqk2FM4JctKANImpC4/eoPCvLXyi4=
github.com/ethereum-optimism/optimism/op-e2e v0.10.14/go.mod h1:quWW1XDffu4jS5EQ+kswoVlFCHRmLzLqqloYwnILDs4=
github.com/ethereum-optimism/optimism/op-service v0.10.14 h1:MC+rVwtPfX1aPAKA3855DQaCnhjjp4uFcSr8PY7HmaE=
github.com/ethereum-optimism/optimism/op-service v0.10.14/go.mod h1:8ay6Bs3YHaX+FbJRUGSbxBnXRtEbKXNHMhtJqATrBmY=
github.com/fjl/memsize v0.0.1 h1:+zhkb+dhUgx0/e+M8sF0QqiouvMQUiKR+QYvdxIOKcQ=
......@@ -613,6 +617,7 @@ github.com/tklauser/numcpus v0.4.0/go.mod h1:1+UI3pD8NW14VMwdgJNJ1ESk2UnwhAnz5hM
github.com/tklauser/numcpus v0.5.0 h1:ooe7gN0fg6myJ0EKoTAf5hebTZrH52px3New/D9iJ+A=
github.com/tklauser/numcpus v0.5.0/go.mod h1:OGzpTxpcIMNGYQdit2BYL1pvk/dSOaJWjKoflh+RQjo=
github.com/tyler-smith/go-bip39 v1.1.0 h1:5eUemwrMargf3BSLRRCalXT93Ns6pQJIjYQN2nyfOP8=
github.com/tyler-smith/go-bip39 v1.1.0/go.mod h1:gUYDtqQw1JS3ZJ8UWVcGTGqqr6YIN3CWg+kkNaLt55U=
github.com/urfave/cli v1.22.2/go.mod h1:Gos4lmkARVdJ6EkW0WaNv/tZAAMe9V7XWyB60NtXRu0=
github.com/urfave/cli v1.22.9 h1:cv3/KhXGBGjEXLC4bH0sLuJ9BewaAbpk5oyMOveu4pw=
github.com/urfave/cli v1.22.9/go.mod h1:Gos4lmkARVdJ6EkW0WaNv/tZAAMe9V7XWyB60NtXRu0=
......@@ -774,8 +779,8 @@ golang.org/x/sync v0.0.0-20200625203802-6e8e738ad208/go.mod h1:RxMgew5VJxzue5/jJ
golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.0.0-20201207232520-09787c993a3a/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.0.0-20210220032951-036812b2e83c/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4 h1:uVc8UZUe6tr40fFVnUP5Oj+veunVezqYl9z7DYw9xzw=
golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.1.0 h1:wsuoTGHzEhffawBOhz5CYhcrV4IdKZbEyZjBMuTp12o=
golang.org/x/sync v0.1.0/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sys v0.0.0-20180810173357-98c5dad5d1a0/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20180830151530-49385e6e1522/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20180905080454-ebe1bf3edb33/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
......@@ -864,7 +869,7 @@ golang.org/x/time v0.0.0-20180412165947-fbb02b2291d2/go.mod h1:tRJNPiyCQ0inRvYxb
golang.org/x/time v0.0.0-20181108054448-85acf8d2951c/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
golang.org/x/time v0.0.0-20190308202827-9d24e82272b4/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
golang.org/x/time v0.0.0-20191024005414-555d28b269f0/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
golang.org/x/time v0.0.0-20220224211638-0e9765cccd65 h1:M73Iuj3xbbb9Uk1DYhzydthsj6oOd6l9bpuFcNoUvTs=
golang.org/x/time v0.0.0-20220722155302-e5dcc9cfc0b9 h1:ftMN5LMiBFjbzleLqtoBZk7KdJwhuybIU+FckUHgoyQ=
golang.org/x/tools v0.0.0-20180828015842-6cd1fcedba52/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
golang.org/x/tools v0.0.0-20181030000716-a0a13e073c7b/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
......
......@@ -24,7 +24,7 @@ func LoadSignerSetup(ctx *cli.Context) (p2p.SignerSetup, error) {
return nil, fmt.Errorf("failed to read batch submitter key: %w", err)
}
return &p2p.PreparedSigner{Signer: p2p.NewLocalSigner(priv)}, nil
return &p2p.PreparedSigner{Signer: p2p.NewLegacyLocalSigner(priv)}, nil
}
// TODO: create remote signer
......
......@@ -268,30 +268,9 @@ func BuildBlocksValidator(log log.Logger, cfg *rollup.Config, runCfg GossipRunti
signatureBytes, payloadBytes := data[:65], data[65:]
// [REJECT] if the signature by the sequencer is not valid
signingHash, err := BlockSigningHash(cfg, payloadBytes)
if err != nil {
log.Warn("failed to compute block signing hash", "err", err, "peer", id)
return pubsub.ValidationReject
}
pub, err := crypto.SigToPub(signingHash[:], signatureBytes)
if err != nil {
log.Warn("invalid block signature", "err", err, "peer", id)
return pubsub.ValidationReject
}
addr := crypto.PubkeyToAddress(*pub)
// In the future we may load & validate block metadata before checking the signature.
// And then check the signer based on the metadata, to support e.g. multiple p2p signers at the same time.
// For now we only have one signer at a time and thus check the address directly.
// This means we may drop old payloads upon key rotation,
// but this can be recovered from like any other missed unsafe payload.
if expected := runCfg.P2PSequencerAddress(); expected == (common.Address{}) {
log.Warn("no configured p2p sequencer address, ignoring gossiped block", "peer", id, "addr", addr)
return pubsub.ValidationIgnore
} else if addr != expected {
log.Warn("unexpected block author", "err", err, "peer", id, "addr", addr, "expected", expected)
return pubsub.ValidationReject
result := verifyBlockSignature(log, cfg, runCfg, id, signatureBytes, payloadBytes)
if result != pubsub.ValidationAccept {
return result
}
// [REJECT] if the block encoding is not valid
......@@ -348,6 +327,43 @@ func BuildBlocksValidator(log log.Logger, cfg *rollup.Config, runCfg GossipRunti
}
}
func verifyBlockSignature(log log.Logger, cfg *rollup.Config, runCfg GossipRuntimeConfig, id peer.ID, signatureBytes []byte, payloadBytes []byte) pubsub.ValidationResult {
result := verifyBlockSignatureWithHasher(log, cfg, runCfg, id, signatureBytes, payloadBytes, BlockSigningHash)
if result != pubsub.ValidationAccept {
return verifyBlockSignatureWithHasher(log, cfg, runCfg, id, signatureBytes, payloadBytes, LegacyBlockSigningHash)
}
return result
}
func verifyBlockSignatureWithHasher(log log.Logger, cfg *rollup.Config, runCfg GossipRuntimeConfig, id peer.ID, signatureBytes []byte, payloadBytes []byte, hasher func(cfg *rollup.Config, payloadBytes []byte) (common.Hash, error)) pubsub.ValidationResult {
signingHash, err := hasher(cfg, payloadBytes)
if err != nil {
log.Warn("failed to compute block signing hash", "err", err, "peer", id)
return pubsub.ValidationReject
}
pub, err := crypto.SigToPub(signingHash[:], signatureBytes)
if err != nil {
log.Warn("invalid block signature", "err", err, "peer", id)
return pubsub.ValidationReject
}
addr := crypto.PubkeyToAddress(*pub)
// In the future we may load & validate block metadata before checking the signature.
// And then check the signer based on the metadata, to support e.g. multiple p2p signers at the same time.
// For now we only have one signer at a time and thus check the address directly.
// This means we may drop old payloads upon key rotation,
// but this can be recovered from like any other missed unsafe payload.
if expected := runCfg.P2PSequencerAddress(); expected == (common.Address{}) {
log.Warn("no configured p2p sequencer address, ignoring gossiped block", "peer", id, "addr", addr)
return pubsub.ValidationIgnore
} else if addr != expected {
log.Warn("unexpected block author", "err", err, "peer", id, "addr", addr, "expected", expected)
return pubsub.ValidationReject
}
return pubsub.ValidationAccept
}
type GossipIn interface {
OnUnsafeL2Payload(ctx context.Context, from peer.ID, msg *eth.ExecutionPayload) error
}
......
......@@ -2,8 +2,16 @@ package p2p
import (
"context"
"crypto/ecdsa"
"math/big"
"testing"
"github.com/ethereum-optimism/optimism/op-e2e/e2eutils"
"github.com/ethereum-optimism/optimism/op-node/rollup"
"github.com/ethereum-optimism/optimism/op-node/testutils"
"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/crypto"
"github.com/ethereum/go-ethereum/log"
pubsub "github.com/libp2p/go-libp2p-pubsub"
"github.com/libp2p/go-libp2p/core/peer"
......@@ -32,3 +40,65 @@ func TestGuardGossipValidator(t *testing.T) {
require.Equal(t, pubsub.ValidationAccept, val(context.Background(), "alice", nil))
require.Equal(t, pubsub.ValidationIgnore, val(context.Background(), "bob", nil))
}
func TestVerifyBlockSignature(t *testing.T) {
// Should accept signatures over both the legacy and updated signature hashes
tests := []struct {
name string
newSigner func(priv *ecdsa.PrivateKey) *LocalSigner
}{
{
name: "Legacy",
newSigner: NewLegacyLocalSigner,
},
{
name: "Updated",
newSigner: NewLocalSigner,
},
}
logger := testlog.Logger(t, log.LvlCrit)
cfg := &rollup.Config{
L2ChainID: big.NewInt(100),
}
peerId := peer.ID("foo")
secrets, err := e2eutils.DefaultMnemonicConfig.Secrets()
require.NoError(t, err)
msg := []byte("any msg")
for _, test := range tests {
t.Run("Valid "+test.name, func(t *testing.T) {
runCfg := &testutils.MockRuntimeConfig{P2PSeqAddress: crypto.PubkeyToAddress(secrets.SequencerP2P.PublicKey)}
signer := &PreparedSigner{Signer: test.newSigner(secrets.SequencerP2P)}
sig, err := signer.Sign(context.Background(), SigningDomainBlocksV1, cfg.L2ChainID, msg)
require.NoError(t, err)
result := verifyBlockSignature(logger, cfg, runCfg, peerId, sig[:65], msg)
require.Equal(t, pubsub.ValidationAccept, result)
})
t.Run("WrongSigner "+test.name, func(t *testing.T) {
runCfg := &testutils.MockRuntimeConfig{P2PSeqAddress: common.HexToAddress("0x1234")}
signer := &PreparedSigner{Signer: test.newSigner(secrets.SequencerP2P)}
sig, err := signer.Sign(context.Background(), SigningDomainBlocksV1, cfg.L2ChainID, msg)
require.NoError(t, err)
result := verifyBlockSignature(logger, cfg, runCfg, peerId, sig[:65], msg)
require.Equal(t, pubsub.ValidationReject, result)
})
t.Run("InvalidSignature "+test.name, func(t *testing.T) {
runCfg := &testutils.MockRuntimeConfig{P2PSeqAddress: crypto.PubkeyToAddress(secrets.SequencerP2P.PublicKey)}
sig := make([]byte, 65)
result := verifyBlockSignature(logger, cfg, runCfg, peerId, sig, msg)
require.Equal(t, pubsub.ValidationReject, result)
})
t.Run("NoSequencer "+test.name, func(t *testing.T) {
runCfg := &testutils.MockRuntimeConfig{}
signer := &PreparedSigner{Signer: test.newSigner(secrets.SequencerP2P)}
sig, err := signer.Sign(context.Background(), SigningDomainBlocksV1, cfg.L2ChainID, msg)
require.NoError(t, err)
result := verifyBlockSignature(logger, cfg, runCfg, peerId, sig[:65], msg)
require.Equal(t, pubsub.ValidationIgnore, result)
})
}
}
......@@ -20,7 +20,7 @@ type Signer interface {
io.Closer
}
func SigningHash(domain [32]byte, chainID *big.Int, payloadBytes []byte) (common.Hash, error) {
func LegacySigningHash(domain [32]byte, chainID *big.Int, payloadBytes []byte) (common.Hash, error) {
var msgInput [32 + 32 + 32]byte
// domain: first 32 bytes
copy(msgInput[:32], domain[:])
......@@ -35,24 +35,48 @@ func SigningHash(domain [32]byte, chainID *big.Int, payloadBytes []byte) (common
return crypto.Keccak256Hash(msgInput[:]), nil
}
func SigningHash(domain [32]byte, chainID *big.Int, payloadBytes []byte) (common.Hash, error) {
var msgInput [32 + 32 + 32]byte
// domain: first 32 bytes
copy(msgInput[:32], domain[:])
// chain_id: second 32 bytes
if chainID.BitLen() > 256 {
return common.Hash{}, errors.New("chain_id is too large")
}
chainID.FillBytes(msgInput[32:64])
// payload_hash: third 32 bytes, hash of encoded payload
copy(msgInput[64:], crypto.Keccak256(payloadBytes))
return crypto.Keccak256Hash(msgInput[:]), nil
}
func BlockSigningHash(cfg *rollup.Config, payloadBytes []byte) (common.Hash, error) {
return SigningHash(SigningDomainBlocksV1, cfg.L2ChainID, payloadBytes)
}
func LegacyBlockSigningHash(cfg *rollup.Config, payloadBytes []byte) (common.Hash, error) {
return LegacySigningHash(SigningDomainBlocksV1, cfg.L2ChainID, payloadBytes)
}
// LocalSigner is suitable for testing
type LocalSigner struct {
priv *ecdsa.PrivateKey
priv *ecdsa.PrivateKey
hasher func(domain [32]byte, chainID *big.Int, payloadBytes []byte) (common.Hash, error)
}
func NewLegacyLocalSigner(priv *ecdsa.PrivateKey) *LocalSigner {
return &LocalSigner{priv: priv, hasher: LegacySigningHash}
}
func NewLocalSigner(priv *ecdsa.PrivateKey) *LocalSigner {
return &LocalSigner{priv: priv}
return &LocalSigner{priv: priv, hasher: SigningHash}
}
func (s *LocalSigner) Sign(ctx context.Context, domain [32]byte, chainID *big.Int, encodedMsg []byte) (sig *[65]byte, err error) {
if s.priv == nil {
return nil, errors.New("signer is closed")
}
signingHash, err := SigningHash(domain, chainID, encodedMsg)
signingHash, err := s.hasher(domain, chainID, encodedMsg)
if err != nil {
return nil, err
}
......
package p2p
import (
"math/big"
"testing"
"github.com/ethereum-optimism/optimism/op-node/rollup"
"github.com/stretchr/testify/require"
)
func TestSigningHash_DifferentDomain(t *testing.T) {
cfg := &rollup.Config{
L2ChainID: big.NewInt(100),
}
payloadBytes := []byte("arbitraryData")
hash, err := SigningHash(SigningDomainBlocksV1, cfg.L2ChainID, payloadBytes)
require.NoError(t, err, "creating first signing hash")
hash2, err := SigningHash([32]byte{3}, cfg.L2ChainID, payloadBytes)
require.NoError(t, err, "creating second signing hash")
require.NotEqual(t, hash, hash2, "signing hash should be different when domain is different")
}
func TestSigningHash_DifferentChainID(t *testing.T) {
cfg1 := &rollup.Config{
L2ChainID: big.NewInt(100),
}
cfg2 := &rollup.Config{
L2ChainID: big.NewInt(101),
}
payloadBytes := []byte("arbitraryData")
hash, err := SigningHash(SigningDomainBlocksV1, cfg1.L2ChainID, payloadBytes)
require.NoError(t, err, "creating first signing hash")
hash2, err := SigningHash(SigningDomainBlocksV1, cfg2.L2ChainID, payloadBytes)
require.NoError(t, err, "creating second signing hash")
require.NotEqual(t, hash, hash2, "signing hash should be different when chain ID is different")
}
func TestSigningHash_DifferentMessage(t *testing.T) {
cfg := &rollup.Config{
L2ChainID: big.NewInt(100),
}
hash, err := SigningHash(SigningDomainBlocksV1, cfg.L2ChainID, []byte("msg1"))
require.NoError(t, err, "creating first signing hash")
hash2, err := SigningHash(SigningDomainBlocksV1, cfg.L2ChainID, []byte("msg2"))
require.NoError(t, err, "creating second signing hash")
require.NotEqual(t, hash, hash2, "signing hash should be different when message is different")
}
func TestSigningHash_LimitChainID(t *testing.T) {
// ChainID with bitlen 257
chainID := big.NewInt(1)
chainID = chainID.SetBit(chainID, 256, 1)
cfg := &rollup.Config{
L2ChainID: chainID,
}
_, err := SigningHash(SigningDomainBlocksV1, cfg.L2ChainID, []byte("arbitraryData"))
require.ErrorContains(t, err, "chain_id is too large")
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment