diff --git a/network/network.go b/network/network.go index 1e13ed45b9c2..0c5d3dd9f858 100644 --- a/network/network.go +++ b/network/network.go @@ -625,7 +625,8 @@ func (n *network) track(ip *ips.ClaimedIPPort) error { }, Signature: ip.Signature, } - if err := signedIP.Verify(ip.Cert); err != nil { + maxTimestamp := n.peerConfig.Clock.Time().Add(n.peerConfig.MaxClockDifference) + if err := signedIP.Verify(ip.Cert, maxTimestamp); err != nil { return err } diff --git a/network/peer/ip.go b/network/peer/ip.go index 0112374b9b80..590003c850d8 100644 --- a/network/peer/ip.go +++ b/network/peer/ip.go @@ -6,6 +6,9 @@ package peer import ( "crypto" "crypto/rand" + "errors" + "fmt" + "time" "github.com/ava-labs/avalanchego/staking" "github.com/ava-labs/avalanchego/utils/hashing" @@ -13,6 +16,11 @@ import ( "github.com/ava-labs/avalanchego/utils/wrappers" ) +var ( + errTimestampTooFarInFuture = errors.New("timestamp too far in the future") + errInvalidSignature = errors.New("invalid signature") +) + // UnsignedIP is used for a validator to claim an IP. The [Timestamp] is used to // ensure that the most updated IP claim is tracked by peers for a given // validator. @@ -49,10 +57,24 @@ type SignedIP struct { Signature []byte } -func (ip *SignedIP) Verify(cert *staking.Certificate) error { - return staking.CheckSignature( +// Returns nil if: +// * [ip.Timestamp] is not after [maxTimestamp]. +// * [ip.Signature] is a valid signature over [ip.UnsignedIP] from [cert]. +func (ip *SignedIP) Verify( + cert *staking.Certificate, + maxTimestamp time.Time, +) error { + maxUnixTimestamp := uint64(maxTimestamp.Unix()) + if ip.Timestamp > maxUnixTimestamp { + return fmt.Errorf("%w: timestamp %d > maxTimestamp %d", errTimestampTooFarInFuture, ip.Timestamp, maxUnixTimestamp) + } + + if err := staking.CheckSignature( cert, ip.UnsignedIP.bytes(), ip.Signature, - ) + ); err != nil { + return fmt.Errorf("%w: %w", errInvalidSignature, err) + } + return nil } diff --git a/network/peer/ip_test.go b/network/peer/ip_test.go new file mode 100644 index 000000000000..9c9b0072cefa --- /dev/null +++ b/network/peer/ip_test.go @@ -0,0 +1,108 @@ +// Copyright (C) 2019-2024, Ava Labs, Inc. All rights reserved. +// See the file LICENSE for licensing terms. + +package peer + +import ( + "crypto" + "net" + "testing" + "time" + + "github.com/stretchr/testify/require" + + "github.com/ava-labs/avalanchego/staking" + "github.com/ava-labs/avalanchego/utils/ips" +) + +func TestSignedIpVerify(t *testing.T) { + tlsCert1, err := staking.NewTLSCert() + require.NoError(t, err) + cert1 := staking.CertificateFromX509(tlsCert1.Leaf) + require.NoError(t, staking.ValidateCertificate(cert1)) + + tlsCert2, err := staking.NewTLSCert() + require.NoError(t, err) + cert2 := staking.CertificateFromX509(tlsCert2.Leaf) + require.NoError(t, staking.ValidateCertificate(cert2)) + + now := time.Now() + + type test struct { + name string + signer crypto.Signer + expectedCert *staking.Certificate + ip UnsignedIP + maxTimestamp time.Time + expectedErr error + } + + tests := []test{ + { + name: "valid (before max time)", + signer: tlsCert1.PrivateKey.(crypto.Signer), + expectedCert: cert1, + ip: UnsignedIP{ + IPPort: ips.IPPort{ + IP: net.IPv4(1, 2, 3, 4), + Port: 1, + }, + Timestamp: uint64(now.Unix()) - 1, + }, + maxTimestamp: now, + expectedErr: nil, + }, + { + name: "valid (at max time)", + signer: tlsCert1.PrivateKey.(crypto.Signer), + expectedCert: cert1, + ip: UnsignedIP{ + IPPort: ips.IPPort{ + IP: net.IPv4(1, 2, 3, 4), + Port: 1, + }, + Timestamp: uint64(now.Unix()), + }, + maxTimestamp: now, + expectedErr: nil, + }, + { + name: "timestamp too far ahead", + signer: tlsCert1.PrivateKey.(crypto.Signer), + expectedCert: cert1, + ip: UnsignedIP{ + IPPort: ips.IPPort{ + IP: net.IPv4(1, 2, 3, 4), + Port: 1, + }, + Timestamp: uint64(now.Unix()) + 1, + }, + maxTimestamp: now, + expectedErr: errTimestampTooFarInFuture, + }, + { + name: "sig from wrong cert", + signer: tlsCert1.PrivateKey.(crypto.Signer), + expectedCert: cert2, // note this isn't cert1 + ip: UnsignedIP{ + IPPort: ips.IPPort{ + IP: net.IPv4(1, 2, 3, 4), + Port: 1, + }, + Timestamp: uint64(now.Unix()), + }, + maxTimestamp: now, + expectedErr: errInvalidSignature, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + signedIP, err := tt.ip.Sign(tt.signer) + require.NoError(t, err) + + err = signedIP.Verify(tt.expectedCert, tt.maxTimestamp) + require.ErrorIs(t, err, tt.expectedErr) + }) + } +} diff --git a/network/peer/peer.go b/network/peer/peer.go index 255f13821f23..f5cebb613e00 100644 --- a/network/peer/peer.go +++ b/network/peer/peer.go @@ -906,8 +906,9 @@ func (p *peer) handleHandshake(msg *p2p.Handshake) { return } - myTime := p.Clock.Unix() - clockDifference := math.Abs(float64(msg.MyTime) - float64(myTime)) + myTime := p.Clock.Time() + myTimeUnix := uint64(myTime.Unix()) + clockDifference := math.Abs(float64(msg.MyTime) - float64(myTimeUnix)) p.Metrics.ClockSkew.Observe(clockDifference) @@ -916,13 +917,13 @@ func (p *peer) handleHandshake(msg *p2p.Handshake) { p.Log.Warn("beacon reports out of sync time", zap.Stringer("nodeID", p.id), zap.Uint64("peerTime", msg.MyTime), - zap.Uint64("myTime", myTime), + zap.Uint64("myTime", myTimeUnix), ) } else { p.Log.Debug("peer reports out of sync time", zap.Stringer("nodeID", p.id), zap.Uint64("peerTime", msg.MyTime), - zap.Uint64("myTime", myTime), + zap.Uint64("myTime", myTimeUnix), ) } p.StartClose() @@ -974,18 +975,6 @@ func (p *peer) handleHandshake(msg *p2p.Handshake) { return } - // Note that it is expected that the [ipSigningTime] can be in the past. We - // are just verifying that the claimed signing time isn't too far in the - // future here. - if float64(msg.IpSigningTime)-float64(myTime) > p.MaxClockDifference.Seconds() { - p.Log.Debug("peer attempting to connect with version timestamp too far in the future", - zap.Stringer("nodeID", p.id), - zap.Uint64("ipSigningTime", msg.IpSigningTime), - ) - p.StartClose() - return - } - // handle subnet IDs for _, subnetIDBytes := range msg.TrackedSubnets { subnetID, err := ids.ToID(subnetIDBytes) @@ -1089,11 +1078,24 @@ func (p *peer) handleHandshake(msg *p2p.Handshake) { }, Signature: msg.Sig, } - if err := p.ip.Verify(p.cert); err != nil { - p.Log.Debug("signature verification failed", - zap.Stringer("nodeID", p.id), - zap.Error(err), - ) + maxTimestamp := myTime.Add(p.MaxClockDifference) + if err := p.ip.Verify(p.cert, maxTimestamp); err != nil { + if _, ok := p.Beacons.GetValidator(constants.PrimaryNetworkID, p.id); ok { + p.Log.Warn("beacon has invalid signature or is out of sync", + zap.Stringer("nodeID", p.id), + zap.Uint64("peerTime", msg.MyTime), + zap.Uint64("myTime", myTimeUnix), + zap.Error(err), + ) + } else { + p.Log.Debug("peer has invalid signature or is out of sync", + zap.Stringer("nodeID", p.id), + zap.Uint64("peerTime", msg.MyTime), + zap.Uint64("myTime", myTimeUnix), + zap.Error(err), + ) + } + p.StartClose() return }