Drop invalid TLS certs during initial handshake (ava-labs#1923)

chains/manager.go

@@ -41,6 +41,7 @@ import (
 	"github.com/ava-labs/avalanchego/snow/networking/sender"
 	"github.com/ava-labs/avalanchego/snow/networking/timeout"
 	"github.com/ava-labs/avalanchego/snow/validators"
+	"github.com/ava-labs/avalanchego/staking"
 	"github.com/ava-labs/avalanchego/subnets"
 	"github.com/ava-labs/avalanchego/trace"
 	"github.com/ava-labs/avalanchego/utils/buffer"
@@ -168,7 +169,7 @@ type ChainConfig struct {
 
 type ManagerConfig struct {
 	SybilProtectionEnabled bool
-	StakingCert            tls.Certificate // needed to sign snowman++ blocks
+	StakingTLSCert         tls.Certificate // needed to sign snowman++ blocks
 	StakingBLSKey          *bls.SecretKey
 	TracingEnabled         bool
 	// Must not be used unless [TracingEnabled] is true as this may be nil.
@@ -234,6 +235,9 @@ type manager struct {
 	ids.Aliaser
 	ManagerConfig
 
+	stakingSigner crypto.Signer
+	stakingCert   *staking.Certificate
+
 	// Those notified when a chain is created
 	registrants []Registrant
 
@@ -262,6 +266,8 @@ func New(config *ManagerConfig) Manager {
 	return &manager{
 		Aliaser:                ids.NewAliaser(),
 		ManagerConfig:          *config,
+		stakingSigner:          config.StakingTLSCert.PrivateKey.(crypto.Signer),
+		stakingCert:            staking.CertificateFromX509(config.StakingTLSCert.Leaf),
 		subnets:                make(map[ids.ID]subnets.Subnet),
 		chains:                 make(map[ids.ID]handler.Handler),
 		chainsQueue:            buffer.NewUnboundedBlockingDeque[ChainParameters](initialQueueSize),
@@ -772,8 +778,8 @@ func (m *manager) createAvalancheChain(
 		m.ApricotPhase4Time,
 		m.ApricotPhase4MinPChainHeight,
 		minBlockDelay,
-		m.StakingCert.PrivateKey.(crypto.Signer),
-		m.StakingCert.Leaf,
+		m.stakingSigner,
+		m.stakingCert,
 	)
 
 	if m.MeterVMEnabled {
@@ -1114,8 +1120,8 @@ func (m *manager) createSnowmanChain(
 		m.ApricotPhase4Time,
 		m.ApricotPhase4MinPChainHeight,
 		minBlockDelay,
-		m.StakingCert.PrivateKey.(crypto.Signer),
-		m.StakingCert.Leaf,
+		m.stakingSigner,
+		m.stakingCert,
 	)
 
 	if m.MeterVMEnabled {

ids/node_id.go

@@ -5,10 +5,10 @@ package ids
 
 import (
 	"bytes"
-	"crypto/x509"
 	"errors"
 	"fmt"
 
+	"github.com/ava-labs/avalanchego/staking"
 	"github.com/ava-labs/avalanchego/utils"
 	"github.com/ava-labs/avalanchego/utils/hashing"
 )
@@ -76,7 +76,7 @@ func ToNodeID(bytes []byte) (NodeID, error) {
 	return NodeID(nodeID), err
 }
 
-func NodeIDFromCert(cert *x509.Certificate) NodeID {
+func NodeIDFromCert(cert *staking.Certificate) NodeID {
 	return hashing.ComputeHash160Array(
 		hashing.ComputeHash256(cert.Raw),
 	)

network/certs_test.go

@@ -34,6 +34,8 @@ func getTLS(t *testing.T, index int) (ids.NodeID, *tls.Certificate, *tls.Config)
 		tlsConfigs = append(tlsConfigs, tlsConfig)
 	}
 
-	cert := tlsCerts[index]
-	return ids.NodeIDFromCert(cert.Leaf), cert, tlsConfigs[index]
+	tlsCert := tlsCerts[index]
+	cert := staking.CertificateFromX509(tlsCert.Leaf)
+	nodeID := ids.NodeIDFromCert(cert)
+	return nodeID, tlsCert, tlsConfigs[index]
 }

network/network_test.go

@@ -26,6 +26,7 @@ import (
 	"github.com/ava-labs/avalanchego/snow/networking/tracker"
 	"github.com/ava-labs/avalanchego/snow/uptime"
 	"github.com/ava-labs/avalanchego/snow/validators"
+	"github.com/ava-labs/avalanchego/staking"
 	"github.com/ava-labs/avalanchego/subnets"
 	"github.com/ava-labs/avalanchego/utils/constants"
 	"github.com/ava-labs/avalanchego/utils/ips"
@@ -407,7 +408,7 @@ func TestTrackVerifiesSignatures(t *testing.T) {
 	require.NoError(validators.Add(network.config.Validators, constants.PrimaryNetworkID, nodeID, nil, ids.Empty, 1))
 
 	_, err := network.Track(ids.EmptyNodeID, []*ips.ClaimedIPPort{{
-		Cert: tlsCert.Leaf,
+		Cert: staking.CertificateFromX509(tlsCert.Leaf),
 		IPPort: ips.IPPort{
 			IP:   net.IPv4(123, 132, 123, 123),
 			Port: 10000,
@@ -589,7 +590,7 @@ func TestDialDeletesNonValidators(t *testing.T) {
 	for i, net := range networks {
 		if i != 0 {
 			peerAcks, err := net.Track(config.MyNodeID, []*ips.ClaimedIPPort{{
-				Cert:      config.TLSConfig.Certificates[0].Leaf,
+				Cert:      staking.CertificateFromX509(config.TLSConfig.Certificates[0].Leaf),
 				IPPort:    ip.IPPort,
 				Timestamp: ip.Timestamp,
 				Signature: ip.Signature,

network/peer/ip.go

@@ -6,7 +6,6 @@ package peer
 import (
 	"crypto"
 	"crypto/rand"
-	"crypto/x509"
 
 	"github.com/ava-labs/avalanchego/staking"
 	"github.com/ava-labs/avalanchego/utils/hashing"
@@ -50,7 +49,7 @@ type SignedIP struct {
 	Signature []byte
 }
 
-func (ip *SignedIP) Verify(cert *x509.Certificate) error {
+func (ip *SignedIP) Verify(cert *staking.Certificate) error {
 	return staking.CheckSignature(
 		cert,
 		ip.UnsignedIP.bytes(),

network/peer/peer.go

@@ -6,7 +6,6 @@ package peer
 import (
 	"bufio"
 	"context"
-	"crypto/x509"
 	"errors"
 	"io"
 	"math"
@@ -20,6 +19,7 @@ import (
 	"github.com/ava-labs/avalanchego/ids"
 	"github.com/ava-labs/avalanchego/message"
 	"github.com/ava-labs/avalanchego/proto/pb/p2p"
+	"github.com/ava-labs/avalanchego/staking"
 	"github.com/ava-labs/avalanchego/utils"
 	"github.com/ava-labs/avalanchego/utils/constants"
 	"github.com/ava-labs/avalanchego/utils/ips"
@@ -43,7 +43,7 @@ type Peer interface {
 
 	// Cert returns the certificate that the remote peer is using to
 	// authenticate their messages.
-	Cert() *x509.Certificate
+	Cert() *staking.Certificate
 
 	// LastSent returns the last time a message was sent to the peer.
 	LastSent() time.Time
@@ -112,7 +112,7 @@ type peer struct {
 
 	// [cert] is this peer's certificate, specifically the leaf of the
 	// certificate chain they provided.
-	cert *x509.Certificate
+	cert *staking.Certificate
 
 	// node ID of this peer.
 	id ids.NodeID
@@ -176,7 +176,7 @@ type peer struct {
 func Start(
 	config *Config,
 	conn net.Conn,
-	cert *x509.Certificate,
+	cert *staking.Certificate,
 	id ids.NodeID,
 	messageQueue MessageQueue,
 ) Peer {
@@ -207,7 +207,7 @@ func (p *peer) ID() ids.NodeID {
 	return p.id
 }
 
-func (p *peer) Cert() *x509.Certificate {
+func (p *peer) Cert() *staking.Certificate {
 	return p.cert
 }
 
@@ -1009,7 +1009,7 @@ func (p *peer) handlePeerList(msg *p2p.PeerList) {
 	// the peers this peer told us about
 	discoveredIPs := make([]*ips.ClaimedIPPort, len(msg.ClaimedIpPorts))
 	for i, claimedIPPort := range msg.ClaimedIpPorts {
-		tlsCert, err := x509.ParseCertificate(claimedIPPort.X509Certificate)
+		tlsCert, err := staking.ParseCertificate(claimedIPPort.X509Certificate)
 		if err != nil {
 			p.Log.Debug("message with invalid field",
 				zap.Stringer("nodeID", p.id),

network/peer/peer_test.go

@@ -6,7 +6,6 @@ package peer
 import (
 	"context"
 	"crypto"
-	"crypto/x509"
 	"net"
 	"testing"
 	"time"
@@ -41,7 +40,7 @@ type testPeer struct {
 type rawTestPeer struct {
 	config         *Config
 	conn           net.Conn
-	cert           *x509.Certificate
+	cert           *staking.Certificate
 	nodeID         ids.NodeID
 	inboundMsgChan <-chan message.InboundMessage
 }
@@ -69,12 +68,14 @@ func makeRawTestPeers(t *testing.T, trackedSubnets set.Set[ids.ID]) (*rawTestPee
 
 	tlsCert0, err := staking.NewTLSCert()
 	require.NoError(err)
+	cert0 := staking.CertificateFromX509(tlsCert0.Leaf)
 
 	tlsCert1, err := staking.NewTLSCert()
 	require.NoError(err)
+	cert1 := staking.CertificateFromX509(tlsCert1.Leaf)
 
-	nodeID0 := ids.NodeIDFromCert(tlsCert0.Leaf)
-	nodeID1 := ids.NodeIDFromCert(tlsCert1.Leaf)
+	nodeID0 := ids.NodeIDFromCert(cert0)
+	nodeID1 := ids.NodeIDFromCert(cert1)
 
 	mc := newMessageCreator(t)
 
@@ -134,14 +135,14 @@ func makeRawTestPeers(t *testing.T, trackedSubnets set.Set[ids.ID]) (*rawTestPee
 	peer0 := &rawTestPeer{
 		config:         &peerConfig0,
 		conn:           conn0,
-		cert:           tlsCert0.Leaf,
+		cert:           cert0,
 		nodeID:         nodeID0,
 		inboundMsgChan: inboundMsgChan0,
 	}
 	peer1 := &rawTestPeer{
 		config:         &peerConfig1,
 		conn:           conn1,
-		cert:           tlsCert1.Leaf,
+		cert:           cert1,
 		nodeID:         nodeID1,
 		inboundMsgChan: inboundMsgChan1,
 	}

network/peer/upgrader.go

@@ -5,11 +5,11 @@ package peer
 
 import (
 	"crypto/tls"
-	"crypto/x509"
 	"errors"
 	"net"
 
 	"github.com/ava-labs/avalanchego/ids"
+	"github.com/ava-labs/avalanchego/staking"
 )
 
 var (
@@ -21,7 +21,7 @@ var (
 
 type Upgrader interface {
 	// Must be thread safe
-	Upgrade(net.Conn) (ids.NodeID, net.Conn, *x509.Certificate, error)
+	Upgrade(net.Conn) (ids.NodeID, net.Conn, *staking.Certificate, error)
 }
 
 type tlsServerUpgrader struct {
@@ -34,7 +34,7 @@ func NewTLSServerUpgrader(config *tls.Config) Upgrader {
 	}
 }
 
-func (t tlsServerUpgrader) Upgrade(conn net.Conn) (ids.NodeID, net.Conn, *x509.Certificate, error) {
+func (t tlsServerUpgrader) Upgrade(conn net.Conn) (ids.NodeID, net.Conn, *staking.Certificate, error) {
 	return connToIDAndCert(tls.Server(conn, t.config))
 }
 
@@ -48,11 +48,11 @@ func NewTLSClientUpgrader(config *tls.Config) Upgrader {
 	}
 }
 
-func (t tlsClientUpgrader) Upgrade(conn net.Conn) (ids.NodeID, net.Conn, *x509.Certificate, error) {
+func (t tlsClientUpgrader) Upgrade(conn net.Conn) (ids.NodeID, net.Conn, *staking.Certificate, error) {
 	return connToIDAndCert(tls.Client(conn, t.config))
 }
 
-func connToIDAndCert(conn *tls.Conn) (ids.NodeID, net.Conn, *x509.Certificate, error) {
+func connToIDAndCert(conn *tls.Conn) (ids.NodeID, net.Conn, *staking.Certificate, error) {
 	if err := conn.Handshake(); err != nil {
 		return ids.NodeID{}, nil, nil, err
 	}
@@ -61,6 +61,24 @@ func connToIDAndCert(conn *tls.Conn) (ids.NodeID, net.Conn, *x509.Certificate, e
 	if len(state.PeerCertificates) == 0 {
 		return ids.NodeID{}, nil, nil, errNoCert
 	}
-	peerCert := state.PeerCertificates[0]
-	return ids.NodeIDFromCert(peerCert), conn, peerCert, nil
+
+	tlsCert := state.PeerCertificates[0]
+	// Invariant: ParseCertificate is used rather than CertificateFromX509 to
+	// ensure that signature verification can assume the certificate was
+	// parseable according the staking package's parser.
+	peerCert, err := staking.ParseCertificate(tlsCert.Raw)
+	if err != nil {
+		return ids.NodeID{}, nil, nil, err
+	}
+
+	// We validate the certificate here to attempt to make the validity of the
+	// peer certificate as clear as possible. Specifically, a node running a
+	// prior version using an invalid certificate should not be able to report
+	// healthy.
+	if err := staking.ValidateCertificate(peerCert); err != nil {
+		return ids.NodeID{}, nil, nil, err
+	}
+
+	nodeID := ids.NodeIDFromCert(peerCert)
+	return nodeID, conn, peerCert, nil
 }

node/node.go

@@ -57,6 +57,7 @@ import (
 	"github.com/ava-labs/avalanchego/snow/networking/tracker"
 	"github.com/ava-labs/avalanchego/snow/uptime"
 	"github.com/ava-labs/avalanchego/snow/validators"
+	"github.com/ava-labs/avalanchego/staking"
 	"github.com/ava-labs/avalanchego/trace"
 	"github.com/ava-labs/avalanchego/utils"
 	"github.com/ava-labs/avalanchego/utils/constants"
@@ -805,7 +806,7 @@ func (n *Node) initChainManager(avaxAssetID ids.ID) error {
 
 	n.chainManager = chains.New(&chains.ManagerConfig{
 		SybilProtectionEnabled:                  n.Config.SybilProtectionEnabled,
-		StakingCert:                             n.Config.StakingTLSCert,
+		StakingTLSCert:                          n.Config.StakingTLSCert,
 		StakingBLSKey:                           n.Config.StakingSigningKey,
 		Log:                                     n.Log,
 		LogFactory:                              n.LogFactory,
@@ -1344,16 +1345,23 @@ func (n *Node) Initialize(
 	logger logging.Logger,
 	logFactory logging.Factory,
 ) error {
+	tlsCert := config.StakingTLSCert.Leaf
+	stakingCert := staking.CertificateFromX509(tlsCert)
+	if err := staking.ValidateCertificate(stakingCert); err != nil {
+		return fmt.Errorf("invalid staking certificate: %w", err)
+	}
+
 	n.Log = logger
 	n.Config = config
-	n.ID = ids.NodeIDFromCert(n.Config.StakingTLSCert.Leaf)
+	n.ID = ids.NodeIDFromCert(stakingCert)
 	n.LogFactory = logFactory
 	n.DoneShuttingDown.Add(1)
 
 	pop := signer.NewProofOfPossession(n.Config.StakingSigningKey)
 	n.Log.Info("initializing node",
 		zap.Stringer("version", version.CurrentApp),
 		zap.Stringer("nodeID", n.ID),
+		zap.Stringer("stakingKeyType", tlsCert.PublicKeyAlgorithm),
 		zap.Reflect("nodePOP", pop),
 		zap.Reflect("providedFlags", n.Config.ProvidedFlags),
 		zap.Reflect("config", n.Config),

staking/asn1.go

@@ -0,0 +1,27 @@
+// Copyright (C) 2019-2023, Ava Labs, Inc. All rights reserved.
+// See the file LICENSE for licensing terms.
+
+package staking
+
+import (
+	"crypto"
+	"crypto/x509"
+	"fmt"
+
+	// Explicitly import for the crypto.RegisterHash init side-effects.
+	//
+	// Ref: https://github.com/golang/go/blob/go1.19.12/src/crypto/x509/x509.go#L30-L34
+	_ "crypto/sha256"
+)
+
+// Ref: https://github.com/golang/go/blob/go1.19.12/src/crypto/x509/x509.go#L326-L350
+var signatureAlgorithmVerificationDetails = map[x509.SignatureAlgorithm]x509.PublicKeyAlgorithm{
+	x509.SHA256WithRSA:   x509.RSA,
+	x509.ECDSAWithSHA256: x509.ECDSA,
+}
+
+func init() {
+	if !crypto.SHA256.Available() {
+		panic(fmt.Sprintf("required hash %q is not available", crypto.SHA256))
+	}
+}