Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove Pre-Durango TLS certificate parsing logic #2831

Merged
merged 11 commits into from
Mar 11, 2024
17 changes: 6 additions & 11 deletions chains/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ package chains
import (
"context"
"crypto"
"crypto/tls"
"errors"
"fmt"
"os"
Expand Down Expand Up @@ -173,7 +172,8 @@ type ChainConfig struct {

type ManagerConfig struct {
SybilProtectionEnabled bool
StakingTLSCert tls.Certificate // needed to sign snowman++ blocks
StakingTLSSigner crypto.Signer
StakingTLSCert *staking.Certificate
StakingBLSKey *bls.SecretKey
TracingEnabled bool
// Must not be used unless [TracingEnabled] is true as this may be nil.
Expand Down Expand Up @@ -239,9 +239,6 @@ type manager struct {
ids.Aliaser
ManagerConfig

stakingSigner crypto.Signer
stakingCert *staking.Certificate

// Those notified when a chain is created
registrants []Registrant

Expand All @@ -268,8 +265,6 @@ func New(config *ManagerConfig) Manager {
return &manager{
Aliaser: ids.NewAliaser(),
ManagerConfig: *config,
stakingSigner: config.StakingTLSCert.PrivateKey.(crypto.Signer),
stakingCert: staking.CertificateFromX509(config.StakingTLSCert.Leaf),
chains: make(map[ids.ID]handler.Handler),
chainsQueue: buffer.NewUnboundedBlockingDeque[ChainParameters](initialQueueSize),
unblockChainCreatorCh: make(chan struct{}),
Expand Down Expand Up @@ -725,8 +720,8 @@ func (m *manager) createAvalancheChain(
MinimumPChainHeight: m.ApricotPhase4MinPChainHeight,
MinBlkDelay: minBlockDelay,
NumHistoricalBlocks: numHistoricalBlocks,
StakingLeafSigner: m.stakingSigner,
StakingCertLeaf: m.stakingCert,
StakingLeafSigner: m.StakingTLSSigner,
StakingCertLeaf: m.StakingTLSCert,
},
)

Expand Down Expand Up @@ -1062,8 +1057,8 @@ func (m *manager) createSnowmanChain(
MinimumPChainHeight: m.ApricotPhase4MinPChainHeight,
MinBlkDelay: minBlockDelay,
NumHistoricalBlocks: numHistoricalBlocks,
StakingLeafSigner: m.stakingSigner,
StakingCertLeaf: m.stakingCert,
StakingLeafSigner: m.StakingTLSSigner,
StakingCertLeaf: m.StakingTLSCert,
},
)

Expand Down
3 changes: 1 addition & 2 deletions indexer/examples/p-chain/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"time"

"github.com/ava-labs/avalanchego/indexer"
"github.com/ava-labs/avalanchego/version"
"github.com/ava-labs/avalanchego/wallet/subnet/primary"

platformvmblock "github.com/ava-labs/avalanchego/vms/platformvm/block"
Expand All @@ -34,7 +33,7 @@ func main() {
}

platformvmBlockBytes := container.Bytes
proposerVMBlock, err := proposervmblock.Parse(container.Bytes, version.DefaultUpgradeTime)
proposerVMBlock, err := proposervmblock.Parse(container.Bytes)
if err == nil {
platformvmBlockBytes = proposerVMBlock.Block()
}
Expand Down
3 changes: 1 addition & 2 deletions indexer/examples/x-chain-blocks/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"time"

"github.com/ava-labs/avalanchego/indexer"
"github.com/ava-labs/avalanchego/version"
"github.com/ava-labs/avalanchego/vms/proposervm/block"
"github.com/ava-labs/avalanchego/wallet/chain/x"
"github.com/ava-labs/avalanchego/wallet/subnet/primary"
Expand All @@ -32,7 +31,7 @@ func main() {
continue
}

proposerVMBlock, err := block.Parse(container.Bytes, version.DefaultUpgradeTime)
proposerVMBlock, err := block.Parse(container.Bytes)
if err != nil {
log.Fatalf("failed to parse proposervm block: %s\n", err)
}
Expand Down
16 changes: 13 additions & 3 deletions network/certs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,17 @@ func init() {
cert1, cert2, cert3,
}

stakingCert1, err := staking.ParseCertificate(cert1.Leaf.Raw)
if err != nil {
panic(err)
}
stakingCert2, err := staking.ParseCertificate(cert2.Leaf.Raw)
if err != nil {
panic(err)
}

ip = ips.NewClaimedIPPort(
staking.CertificateFromX509(cert1.Leaf),
stakingCert1,
ips.IPPort{
IP: net.IPv4(127, 0, 0, 1),
Port: 9651,
Expand All @@ -68,7 +77,7 @@ func init() {
nil, // signature
)
otherIP = ips.NewClaimedIPPort(
staking.CertificateFromX509(cert2.Leaf),
stakingCert2,
ips.IPPort{
IP: net.IPv4(127, 0, 0, 1),
Port: 9651,
Expand All @@ -94,7 +103,8 @@ func getTLS(t *testing.T, index int) (ids.NodeID, *tls.Certificate, *tls.Config)
}

tlsCert := tlsCerts[index]
cert := staking.CertificateFromX509(tlsCert.Leaf)
cert, err := staking.ParseCertificate(tlsCert.Leaf.Raw)
require.NoError(t, err)
nodeID := ids.NodeIDFromCert(cert)
return nodeID, tlsCert, tlsConfigs[index]
}
10 changes: 2 additions & 8 deletions network/network.go
Original file line number Diff line number Diff line change
Expand Up @@ -272,12 +272,6 @@ func NewNetwork(
IPSigner: peer.NewIPSigner(config.MyIPPort, config.TLSKey, config.BLSKey),
}

// Invariant: We delay the activation of durango during the TLS handshake to
// avoid gossiping any TLS certs that anyone else in the network may
// consider invalid. Recall that if a peer gossips an invalid cert, the
// connection is terminated.
durangoTime := version.GetDurangoTime(config.NetworkID)
durangoTimeWithClockSkew := durangoTime.Add(config.MaxClockDifference)
onCloseCtx, cancel := context.WithCancel(context.Background())
n := &network{
config: config,
Expand All @@ -288,8 +282,8 @@ func NewNetwork(
inboundConnUpgradeThrottler: throttling.NewInboundConnUpgradeThrottler(log, config.ThrottlerConfig.InboundConnUpgradeThrottlerConfig),
listener: listener,
dialer: dialer,
serverUpgrader: peer.NewTLSServerUpgrader(config.TLSConfig, metrics.tlsConnRejected, durangoTimeWithClockSkew),
clientUpgrader: peer.NewTLSClientUpgrader(config.TLSConfig, metrics.tlsConnRejected, durangoTimeWithClockSkew),
serverUpgrader: peer.NewTLSServerUpgrader(config.TLSConfig, metrics.tlsConnRejected),
clientUpgrader: peer.NewTLSClientUpgrader(config.TLSConfig, metrics.tlsConnRejected),

onCloseCtx: onCloseCtx,
onCloseCtxCancel: cancel,
Expand Down
17 changes: 11 additions & 6 deletions network/network_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -403,9 +403,12 @@ func TestTrackVerifiesSignatures(t *testing.T) {
nodeID, tlsCert, _ := getTLS(t, 1)
require.NoError(network.config.Validators.AddStaker(constants.PrimaryNetworkID, nodeID, nil, ids.Empty, 1))

err := network.Track([]*ips.ClaimedIPPort{
stakingCert, err := staking.ParseCertificate(tlsCert.Leaf.Raw)
require.NoError(err)

err = network.Track([]*ips.ClaimedIPPort{
ips.NewClaimedIPPort(
staking.CertificateFromX509(tlsCert.Leaf),
stakingCert,
ips.IPPort{
IP: net.IPv4(123, 132, 123, 123),
Port: 10000,
Expand Down Expand Up @@ -558,15 +561,17 @@ func TestDialDeletesNonValidators(t *testing.T) {
wg.Add(len(networks))
for i, net := range networks {
if i != 0 {
err := net.Track([]*ips.ClaimedIPPort{
stakingCert, err := staking.ParseCertificate(config.TLSConfig.Certificates[0].Leaf.Raw)
require.NoError(err)

require.NoError(net.Track([]*ips.ClaimedIPPort{
ips.NewClaimedIPPort(
staking.CertificateFromX509(config.TLSConfig.Certificates[0].Leaf),
stakingCert,
ip.IPPort,
ip.Timestamp,
ip.TLSSignature,
),
})
require.NoError(err)
}))
}

go func(net Network) {
Expand Down
8 changes: 4 additions & 4 deletions network/peer/ip_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,16 @@ import (
func TestSignedIpVerify(t *testing.T) {
tlsCert1, err := staking.NewTLSCert()
require.NoError(t, err)
cert1 := staking.CertificateFromX509(tlsCert1.Leaf)
require.NoError(t, staking.ValidateCertificate(cert1))
cert1, err := staking.ParseCertificate(tlsCert1.Leaf.Raw)
require.NoError(t, err)
tlsKey1 := tlsCert1.PrivateKey.(crypto.Signer)
blsKey1, err := bls.NewSecretKey()
require.NoError(t, err)

tlsCert2, err := staking.NewTLSCert()
require.NoError(t, err)
cert2 := staking.CertificateFromX509(tlsCert2.Leaf)
require.NoError(t, staking.ValidateCertificate(cert2))
cert2, err := staking.ParseCertificate(tlsCert2.Leaf.Raw)
require.NoError(t, err)

now := time.Now()

Expand Down
15 changes: 1 addition & 14 deletions network/peer/peer.go
Original file line number Diff line number Diff line change
Expand Up @@ -1207,22 +1207,9 @@ func (p *peer) handlePeerList(msg *p2p.PeerList) {
close(p.onFinishHandshake)
}

// Invariant: We do not account for clock skew here, as the sender of the
// certificate is expected to account for clock skew during the activation
// of Durango.
durangoTime := version.GetDurangoTime(p.NetworkID)
beforeDurango := time.Now().Before(durangoTime)
discoveredIPs := make([]*ips.ClaimedIPPort, len(msg.ClaimedIpPorts)) // the peers this peer told us about
for i, claimedIPPort := range msg.ClaimedIpPorts {
var (
tlsCert *staking.Certificate
err error
)
if beforeDurango {
tlsCert, err = staking.ParseCertificate(claimedIPPort.X509Certificate)
} else {
tlsCert, err = staking.ParseCertificatePermissive(claimedIPPort.X509Certificate)
}
tlsCert, err := staking.ParseCertificate(claimedIPPort.X509Certificate)
if err != nil {
p.Log.Debug("message with invalid field",
zap.Stringer("nodeID", p.id),
Expand Down
6 changes: 4 additions & 2 deletions network/peer/peer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,13 @@ func makeRawTestPeers(t *testing.T, trackedSubnets set.Set[ids.ID]) (*rawTestPee

tlsCert0, err := staking.NewTLSCert()
require.NoError(err)
cert0 := staking.CertificateFromX509(tlsCert0.Leaf)
cert0, err := staking.ParseCertificate(tlsCert0.Leaf.Raw)
require.NoError(err)

tlsCert1, err := staking.NewTLSCert()
require.NoError(err)
cert1 := staking.CertificateFromX509(tlsCert1.Leaf)
cert1, err := staking.ParseCertificate(tlsCert1.Leaf.Raw)
require.NoError(err)

nodeID0 := ids.NodeIDFromCert(cert0)
nodeID1 := ids.NodeIDFromCert(cert1)
Expand Down
1 change: 0 additions & 1 deletion network/peer/test_peer.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ func StartTestPeer(
clientUpgrader := NewTLSClientUpgrader(
tlsConfg,
prometheus.NewCounter(prometheus.CounterOpts{}),
version.GetDurangoTime(networkID),
)

peerID, conn, cert, err := clientUpgrader.Upgrade(conn)
Expand Down
30 changes: 6 additions & 24 deletions network/peer/upgrader.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"crypto/tls"
"errors"
"net"
"time"

"github.com/prometheus/client_golang/prometheus"

Expand All @@ -30,40 +29,36 @@ type Upgrader interface {
type tlsServerUpgrader struct {
config *tls.Config
invalidCerts prometheus.Counter
durangoTime time.Time
}

func NewTLSServerUpgrader(config *tls.Config, invalidCerts prometheus.Counter, durangoTime time.Time) Upgrader {
func NewTLSServerUpgrader(config *tls.Config, invalidCerts prometheus.Counter) Upgrader {
return &tlsServerUpgrader{
config: config,
invalidCerts: invalidCerts,
durangoTime: durangoTime,
}
}

func (t *tlsServerUpgrader) Upgrade(conn net.Conn) (ids.NodeID, net.Conn, *staking.Certificate, error) {
return connToIDAndCert(tls.Server(conn, t.config), t.invalidCerts, t.durangoTime)
return connToIDAndCert(tls.Server(conn, t.config), t.invalidCerts)
}

type tlsClientUpgrader struct {
config *tls.Config
invalidCerts prometheus.Counter
durangoTime time.Time
}

func NewTLSClientUpgrader(config *tls.Config, invalidCerts prometheus.Counter, durangoTime time.Time) Upgrader {
func NewTLSClientUpgrader(config *tls.Config, invalidCerts prometheus.Counter) Upgrader {
return &tlsClientUpgrader{
config: config,
invalidCerts: invalidCerts,
durangoTime: durangoTime,
}
}

func (t *tlsClientUpgrader) Upgrade(conn net.Conn) (ids.NodeID, net.Conn, *staking.Certificate, error) {
return connToIDAndCert(tls.Client(conn, t.config), t.invalidCerts, t.durangoTime)
return connToIDAndCert(tls.Client(conn, t.config), t.invalidCerts)
}

func connToIDAndCert(conn *tls.Conn, invalidCerts prometheus.Counter, durangoTime time.Time) (ids.NodeID, net.Conn, *staking.Certificate, error) {
func connToIDAndCert(conn *tls.Conn, invalidCerts prometheus.Counter) (ids.NodeID, net.Conn, *staking.Certificate, error) {
if err := conn.Handshake(); err != nil {
return ids.EmptyNodeID, nil, nil, err
}
Expand All @@ -74,20 +69,7 @@ func connToIDAndCert(conn *tls.Conn, invalidCerts prometheus.Counter, durangoTim
}

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.
//
// TODO: Remove pre-Durango parsing after v1.11.x has activated.
var (
peerCert *staking.Certificate
err error
)
if time.Now().Before(durangoTime) {
peerCert, err = staking.ParseCertificate(tlsCert.Raw)
} else {
peerCert, err = staking.ParseCertificatePermissive(tlsCert.Raw)
}
peerCert, err := staking.ParseCertificate(tlsCert.Raw)
if err != nil {
invalidCerts.Inc()
return ids.EmptyNodeID, nil, nil, err
Expand Down
21 changes: 13 additions & 8 deletions node/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,16 +110,18 @@ func New(
logger logging.Logger,
) (*Node, error) {
tlsCert := config.StakingTLSCert.Leaf
stakingCert := staking.CertificateFromX509(tlsCert)
if err := staking.ValidateCertificate(stakingCert); err != nil {
stakingCert, err := staking.ParseCertificate(tlsCert.Raw)
if err != nil {
return nil, fmt.Errorf("invalid staking certificate: %w", err)
}

n := &Node{
Log: logger,
LogFactory: logFactory,
ID: ids.NodeIDFromCert(stakingCert),
Config: config,
Log: logger,
LogFactory: logFactory,
StakingTLSSigner: config.StakingTLSCert.PrivateKey.(crypto.Signer),
StakingTLSCert: stakingCert,
ID: ids.NodeIDFromCert(stakingCert),
Config: config,
}

n.DoneShuttingDown.Add(1)
Expand All @@ -134,7 +136,6 @@ func New(
zap.Reflect("config", n.Config),
)

var err error
n.VMFactoryLog, err = logFactory.Make("vm-factory")
if err != nil {
return nil, fmt.Errorf("problem creating vm logger: %w", err)
Expand Down Expand Up @@ -263,6 +264,9 @@ type Node struct {
// (in consensus, for example)
ID ids.NodeID

StakingTLSSigner crypto.Signer
StakingTLSCert *staking.Certificate

// Storage for this node
DB database.Database

Expand Down Expand Up @@ -1054,7 +1058,8 @@ func (n *Node) initChainManager(avaxAssetID ids.ID) error {
n.chainManager = chains.New(
&chains.ManagerConfig{
SybilProtectionEnabled: n.Config.SybilProtectionEnabled,
StakingTLSCert: n.Config.StakingTLSCert,
StakingTLSSigner: n.StakingTLSSigner,
StakingTLSCert: n.StakingTLSCert,
StakingBLSKey: n.Config.StakingSigningKey,
Log: n.Log,
LogFactory: n.LogFactory,
Expand Down
Loading
Loading