Further restrict public keys in TLS handshake

This commit further restricts the public keys allowed in the TLS certificates of clients:

- No ed25519
- For ECDSA, only P256 is supported

Signed-off-by: Yacov Manevich <[email protected]>

network/peer/tls_config.go

@@ -4,18 +4,24 @@
 package peer
 
 import (
+	"crypto/ecdsa"
+	"crypto/elliptic"
 	"crypto/rsa"
 	"crypto/tls"
 	"errors"
 	"io"
 
+	"golang.org/x/crypto/ed25519"
+
 	"github.com/ava-labs/avalanchego/staking"
 )
 
 var (
-	ErrNoCertsSent    = errors.New("no certificates sent by peer")
-	ErrEmptyCert      = errors.New("certificate sent by peer is empty")
-	ErrEmptyPublicKey = errors.New("no public key sent by peer")
+	ErrNoCertsSent       = errors.New("no certificates sent by peer")
+	ErrEmptyCert         = errors.New("certificate sent by peer is empty")
+	ErrEmptyPublicKey    = errors.New("no public key sent by peer")
+	ErrCurveMismatch     = errors.New("only P256 is allowed for ECDSA")
+	ErrForbidden25519Key = errors.New("ed25519 is not allowed in this version")
 )
 
 // TLSConfig returns the TLS config that will allow secure connections to other
@@ -36,13 +42,12 @@ func TLSConfig(cert tls.Certificate, keyLogWriter io.Writer) *tls.Config {
 		InsecureSkipVerify: true, //#nosec G402
 		MinVersion:         tls.VersionTLS13,
 		KeyLogWriter:       keyLogWriter,
-		VerifyConnection:   ValidateRSACertificate,
+		VerifyConnection:   ValidateCertificate,
 	}
 }
 
-// ValidateRSACertificate validates TLS certificates
-// with RSA public keys in the leaf of the certificate chain of the given connection state.
-func ValidateRSACertificate(cs tls.ConnectionState) error {
+// ValidateCertificate validates TLS certificates according their public keys on the leaf certificate in the certification chain.
+func ValidateCertificate(cs tls.ConnectionState) error {
 	if len(cs.PeerCertificates) == 0 {
 		return ErrNoCertsSent
 	}
@@ -53,9 +58,19 @@ func ValidateRSACertificate(cs tls.ConnectionState) error {
 
 	pk := cs.PeerCertificates[0].PublicKey
 
-	switch rsaKey := pk.(type) {
+	switch key := pk.(type) {
+	case ed25519.PublicKey:
+		return ErrForbidden25519Key
+	case *ecdsa.PublicKey:
+		if key == nil {
+			return ErrEmptyPublicKey
+		}
+		if key.Curve != elliptic.P256() {
+			return ErrCurveMismatch
+		}
+		return nil
 	case *rsa.PublicKey:
-		return staking.ValidateRSAPublicKeyIsWellFormed(rsaKey)
+		return staking.ValidateRSAPublicKeyIsWellFormed(key)
 	case nil:
 		return ErrEmptyPublicKey
 	default:

network/peer/tls_config_test.go

@@ -13,12 +13,13 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/require"
+	"golang.org/x/crypto/ed25519"
 
 	"github.com/ava-labs/avalanchego/network/peer"
 	"github.com/ava-labs/avalanchego/staking"
 )
 
-func TestValidateRSACertificate(t *testing.T) {
+func TestValidateCertificate(t *testing.T) {
 	for _, testCase := range []struct {
 		description string
 		input       func(t *testing.T) tls.ConnectionState
@@ -86,9 +87,45 @@ func TestValidateRSACertificate(t *testing.T) {
 				return tls.ConnectionState{PeerCertificates: []*x509.Certificate{ecCert}}
 			},
 		},
+		{
+			description: "EC cert with empty key",
+			expectedErr: peer.ErrEmptyPublicKey,
+			input: func(t *testing.T) tls.ConnectionState {
+				ecKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+				require.NoError(t, err)
+
+				basicCert := basicCert()
+				certBytes, err := x509.CreateCertificate(rand.Reader, basicCert, basicCert, &ecKey.PublicKey, ecKey)
+				require.NoError(t, err)
+
+				ecCert, err := x509.ParseCertificate(certBytes)
+				require.NoError(t, err)
+
+				ecCert.PublicKey = nil
+
+				return tls.ConnectionState{PeerCertificates: []*x509.Certificate{ecCert}}
+			},
+		},
+		{
+			description: "EC cert with ed25519 key",
+			expectedErr: peer.ErrForbidden25519Key,
+			input: func(t *testing.T) tls.ConnectionState {
+				pub, priv, err := ed25519.GenerateKey(rand.Reader)
+				require.NoError(t, err)
+
+				basicCert := basicCert()
+				certBytes, err := x509.CreateCertificate(rand.Reader, basicCert, basicCert, pub, priv)
+				require.NoError(t, err)
+
+				ecCert, err := x509.ParseCertificate(certBytes)
+				require.NoError(t, err)
+
+				return tls.ConnectionState{PeerCertificates: []*x509.Certificate{ecCert}}
+			},
+		},
 	} {
 		t.Run(testCase.description, func(t *testing.T) {
-			require.Equal(t, testCase.expectedErr, peer.ValidateRSACertificate(testCase.input(t)))
+			require.Equal(t, testCase.expectedErr, peer.ValidateCertificate(testCase.input(t)))
 		})
 	}
 }