trust: extend TLSCryptoManager with support for verifying correctness…

… of extKeyUsage extensions (scionproto#4222)

This PR includes the following changes:
- Adds extKeyUsage verification for TLS HS [1] [2].
- Replaces `VerifyPeerCertificate` with `VerifyClientCertificate` and `VerifyServerCertificate`.
- Extends `X509KeyPairLoader` with two separate functions to load a client and a server certificate.
- Adds `VerifyConnection` callback implementation, available as of Go1.16.

private/trust/tls_handshake.go

@@ -18,17 +18,23 @@ import (
 	"context"
 	"crypto/tls"
 	"crypto/x509"
+	"strings"
 	"time"
 
+	"github.com/scionproto/scion/pkg/addr"
 	"github.com/scionproto/scion/pkg/private/serrors"
 	"github.com/scionproto/scion/pkg/scrypto/cppki"
 )
 
 const defaultTimeout = 5 * time.Second
 
-// X509KeyPairLoader provides a certificate to be presented during TLS handshake.
 type X509KeyPairLoader interface {
-	LoadX509KeyPair() (*tls.Certificate, error)
+	// LoadServerKeyPair provides a certificate to be presented by the server
+	// during TLS handshake.
+	LoadServerKeyPair(ctx context.Context) (*tls.Certificate, error)
+	// LoadClientKeyPair provides a certificate to be presented by the client
+	// during TLS handshake.
+	LoadClientKeyPair(ctx context.Context) (*tls.Certificate, error)
 }
 
 // TLSCryptoManager implements callbacks which will be called during TLS handshake.
@@ -48,28 +54,73 @@ func NewTLSCryptoManager(loader X509KeyPairLoader, db DB) *TLSCryptoManager {
 }
 
 // GetCertificate retrieves a certificate to be presented during TLS handshake.
-func (m *TLSCryptoManager) GetCertificate(_ *tls.ClientHelloInfo) (*tls.Certificate, error) {
-	c, err := m.Loader.LoadX509KeyPair()
+func (m *TLSCryptoManager) GetCertificate(hello *tls.ClientHelloInfo) (*tls.Certificate, error) {
+	c, err := m.Loader.LoadServerKeyPair(hello.Context())
 	if err != nil {
 		return nil, serrors.WrapStr("loading server key pair", err)
 	}
 	return c, nil
 }
 
 // GetClientCertificate retrieves a client certificate to be presented during TLS handshake.
-func (m *TLSCryptoManager) GetClientCertificate(_ *tls.CertificateRequestInfo) (*tls.Certificate,
-	error) {
-	c, err := m.Loader.LoadX509KeyPair()
+func (m *TLSCryptoManager) GetClientCertificate(
+	reqInfo *tls.CertificateRequestInfo,
+) (*tls.Certificate, error) {
+
+	c, err := m.Loader.LoadClientKeyPair(reqInfo.Context())
 	if err != nil {
 		return nil, serrors.WrapStr("loading client key pair", err)
 	}
 	return c, nil
 }
 
-// VerifyPeerCertificate verifies the certificate presented by the peer during TLS handshake,
+// VerifyServerCertificate verifies the certificate presented by the server
+// using the CP-PKI.
+func (m *TLSCryptoManager) VerifyServerCertificate(
+	rawCerts [][]byte,
+	_ [][]*x509.Certificate,
+) error {
+
+	return m.verifyPeerCertificate(rawCerts, x509.ExtKeyUsageServerAuth)
+}
+
+// VerifyClientCertificate verifies the certificate presented by the client
+// using the CP-PKI.
+func (m *TLSCryptoManager) VerifyClientCertificate(
+	rawCerts [][]byte,
+	_ [][]*x509.Certificate,
+) error {
+
+	return m.verifyPeerCertificate(rawCerts, x509.ExtKeyUsageClientAuth)
+}
+
+// VerifyConnection callback is intended to be used by the client to verify
+// that the certificate presented by the server matches the server name
+// the client is trying to connect to.
+func (m *TLSCryptoManager) VerifyConnection(cs tls.ConnectionState) error {
+	serverNameIA := strings.Split(cs.ServerName, ",")[0]
+	serverIA, err := addr.ParseIA(serverNameIA)
+	if err != nil {
+		return serrors.WrapStr("extracting IA from server name", err)
+	}
+	certIA, err := cppki.ExtractIA(cs.PeerCertificates[0].Subject)
+	if err != nil {
+		return serrors.WrapStr("extracting IA from peer cert", err)
+	}
+	if !serverIA.Equal(certIA) {
+		return serrors.New("extracted IA from cert and server IA do not match",
+			"peer IA", certIA, "server IA", serverIA)
+	}
+	return nil
+}
+
+// verifyPeerCertificate verifies the certificate presented by the peer during TLS handshake,
 // based on the TRC.
-func (m *TLSCryptoManager) VerifyPeerCertificate(rawCerts [][]byte,
-	_ [][]*x509.Certificate) error {
+func (m *TLSCryptoManager) verifyPeerCertificate(
+	rawCerts [][]byte,
+	extKeyUsage x509.ExtKeyUsage,
+) error {
+
 	chain := make([]*x509.Certificate, len(rawCerts))
 	for i, asn1Data := range rawCerts {
 		cert, err := x509.ParseCertificate(asn1Data)
@@ -78,6 +129,9 @@ func (m *TLSCryptoManager) VerifyPeerCertificate(rawCerts [][]byte,
 		}
 		chain[i] = cert
 	}
+	if err := verifyExtendedKeyUsage(chain[0], extKeyUsage); err != nil {
+		return err
+	}
 	ia, err := cppki.ExtractIA(chain[0].Subject)
 	if err != nil {
 		return serrors.WrapStr("extracting ISD-AS from peer certificate", err)
@@ -106,3 +160,14 @@ func verifyChain(chain []*x509.Certificate, trcs []cppki.SignedTRC) error {
 	}
 	return errs.ToError()
 }
+
+// verifyExtendedKeyUsage return an error if the certifcate extended key usages do not
+// include any requested extended key usage.
+func verifyExtendedKeyUsage(cert *x509.Certificate, expectedKeyUsage x509.ExtKeyUsage) error {
+	for _, certExtKeyUsage := range cert.ExtKeyUsage {
+		if expectedKeyUsage == certExtKeyUsage {
+			return nil
+		}
+	}
+	return serrors.New("Invalid certificate key usages")
+}

private/trust/tls_handshake_test.go

@@ -32,7 +32,7 @@ import (
 	"github.com/scionproto/scion/private/trust/mock_trust"
 )
 
-func TestTLSCryptoManagerVerifyPeerCertificate(t *testing.T) {
+func TestTLSCryptoManagerVerifyServerCertificate(t *testing.T) {
 	dir := genCrypto(t)
 
 	trc := xtest.LoadTRC(t, filepath.Join(dir, "trcs/ISD1-B1-S1.trc"))
@@ -66,31 +66,73 @@ func TestTLSCryptoManagerVerifyPeerCertificate(t *testing.T) {
 				Timeout: 5 * time.Second,
 			}
 			rawChain := loadRawChain(t, crt111File)
-			err := mgr.VerifyPeerCertificate(rawChain, nil)
+			err := mgr.VerifyServerCertificate(rawChain, nil)
+			tc.assertErr(t, err)
+		})
+	}
+}
+
+func TestTLSCryptoManagerVerifyClientCertificate(t *testing.T) {
+	dir := genCrypto(t)
+
+	trc := xtest.LoadTRC(t, filepath.Join(dir, "trcs/ISD1-B1-S1.trc"))
+	crt111File := filepath.Join(dir, "certs/ISD1-ASff00_0_111.pem")
+
+	out, _ := exec.Command("tree", dir).CombinedOutput()
+	t.Log(string(out))
+
+	testCases := map[string]struct {
+		db        func(ctrl *gomock.Controller) trust.DB
+		assertErr assert.ErrorAssertionFunc
+	}{
+		"valid": {
+			db: func(ctrl *gomock.Controller) trust.DB {
+				db := mock_trust.NewMockDB(ctrl)
+				db.EXPECT().SignedTRC(gomock.Any(), gomock.Any()).Return(trc, nil)
+				return db
+			},
+			assertErr: assert.NoError,
+		},
+	}
+	for name, tc := range testCases {
+		name, tc := name, tc
+		t.Run(name, func(t *testing.T) {
+			ctrl := gomock.NewController(t)
+			defer ctrl.Finish()
+
+			db := tc.db(ctrl)
+			mgr := trust.TLSCryptoManager{
+				DB:      db,
+				Timeout: 5 * time.Second,
+			}
+			rawChain := loadRawChain(t, crt111File)
+			err := mgr.VerifyClientCertificate(rawChain, nil)
 			tc.assertErr(t, err)
 		})
 	}
 }
 
 func TestHandshake(t *testing.T) {
+	dir := genCrypto(t)
 	ctrl := gomock.NewController(t)
 	defer ctrl.Finish()
 
-	dir := genCrypto(t)
-
 	trc := xtest.LoadTRC(t, filepath.Join(dir, "trcs/ISD1-B1-S1.trc"))
 	crt111File := filepath.Join(dir, "certs/ISD1-ASff00_0_111.pem")
 	key111File := filepath.Join(dir, "ISD1/ASff00_0_111/crypto/as/cp-as.key")
-
 	tlsCert, err := tls.LoadX509KeyPair(crt111File, key111File)
 	require.NoError(t, err)
 	chain, err := cppki.ReadPEMCerts(crt111File)
 	require.NoError(t, err)
 
+	out, _ := exec.Command("tree", dir).CombinedOutput()
+	t.Log(string(out))
+
 	db := mock_trust.NewMockDB(ctrl)
 	db.EXPECT().SignedTRC(gomock.Any(), gomock.Any()).MaxTimes(2).Return(trc, nil)
 	loader := mock_trust.NewMockX509KeyPairLoader(ctrl)
-	loader.EXPECT().LoadX509KeyPair().MaxTimes(2).Return(&tlsCert, nil)
+	loader.EXPECT().LoadServerKeyPair(gomock.Any()).Return(&tlsCert, nil)
+	loader.EXPECT().LoadClientKeyPair(gomock.Any()).Return(&tlsCert, nil)
 
 	mgr := trust.NewTLSCryptoManager(loader, db)
 	clientConn, serverConn := net.Pipe()
@@ -100,12 +142,12 @@ func TestHandshake(t *testing.T) {
 	client := tls.Client(clientConn, &tls.Config{
 		InsecureSkipVerify:    true,
 		GetClientCertificate:  mgr.GetClientCertificate,
-		VerifyPeerCertificate: mgr.VerifyPeerCertificate,
+		VerifyPeerCertificate: mgr.VerifyServerCertificate,
 	})
 	server := tls.Server(serverConn, &tls.Config{
 		InsecureSkipVerify:    true,
 		GetCertificate:        mgr.GetCertificate,
-		VerifyPeerCertificate: mgr.VerifyPeerCertificate,
+		VerifyPeerCertificate: mgr.VerifyClientCertificate,
 		ClientAuth:            tls.RequireAnyClientCert,
 	})