Skip to content

Commit

Permalink
Remove Pre-Durango TLS certificate parsing logic (ava-labs#2831)
Browse files Browse the repository at this point in the history
Co-authored-by: Stephen Buttolph <[email protected]>
  • Loading branch information
dhrubabasu and StephenButtolph authored Mar 11, 2024
1 parent d003d29 commit 2196015
Show file tree
Hide file tree
Showing 29 changed files with 144 additions and 343 deletions.
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

0 comments on commit 2196015

Please sign in to comment.