GetClientCertificate cannot return nil cert in TLS 1.3

Author: everesio

tls/client/client.go

@@ -20,14 +20,22 @@ func NewTLSClientConfigFunc(logger *slog.Logger, src source.ClientCertsSource, o
 	if err != nil {
 		return nil, err
 	}
+	var getClientCertificateFunc func(info *tls.CertificateRequestInfo) (*tls.Certificate, error)
+	if store.LoadClientCerts().Certificate != nil {
+		// Set function only when client certificate is available.
+		// TLS 1.3 checks if GetClientCertificate function is nil, if it is not nil,
+		// it assumes client certificate is available which call cause the panic if nil is returned.
+		// nolint:unparam
+		getClientCertificateFunc = func(_ *tls.CertificateRequestInfo) (*tls.Certificate, error) {
+			return store.LoadClientCerts().Certificate, nil
+		}
+	}
 	return func() *tls.Config {
 		cs := store.LoadClientCerts()
 		x := &tls.Config{
-			RootCAs:            cs.RootCAs,
-			InsecureSkipVerify: cs.InsecureSkipVerify,
-			GetClientCertificate: func(info *tls.CertificateRequestInfo) (*tls.Certificate, error) {
-				return store.LoadClientCerts().Certificate, nil
-			},
+			RootCAs:              cs.RootCAs,
+			InsecureSkipVerify:   cs.InsecureSkipVerify,
+			GetClientCertificate: getClientCertificateFunc,
 		}
 		for _, opt := range opts {
 			opt(x)

tls/client/config/config_test.go

@@ -31,3 +31,37 @@ func TestGetClientTLSConfig(t *testing.T) {
 	require.NoError(t, err)
 	require.NotNil(t, clientCert)
 }
+
+func TestGetClientTLSConfigNoConfig(t *testing.T) {
+	bundle := testutil.NewCertsBundle()
+	defer bundle.Close()
+	tlsConfigFunc, err := GetTLSClientConfigFunc(slog.Default(), &config.TLSClientConfig{
+		Enable:  true,
+		Refresh: 0,
+		File:    config.TLSClientFiles{},
+	})
+	require.NoError(t, err)
+	tlsConfig := tlsConfigFunc()
+	require.Nil(t, tlsConfig.RootCAs)
+	require.Nil(t, tlsConfig.GetClientCertificate)
+}
+
+func TestGetClientTLSConfigSkipVerify(t *testing.T) {
+	bundle := testutil.NewCertsBundle()
+	defer bundle.Close()
+	tlsConfigFunc, err := GetTLSClientConfigFunc(slog.Default(), &config.TLSClientConfig{
+		Enable:  true,
+		Refresh: 0,
+		File: config.TLSClientFiles{
+			Key:  bundle.ClientKey.Name(),
+			Cert: bundle.ClientCert.Name(),
+		},
+	})
+	require.NoError(t, err)
+	tlsConfig := tlsConfigFunc()
+	require.Nil(t, tlsConfig.RootCAs)
+
+	clientCert, err := tlsConfig.GetClientCertificate(nil)
+	require.NoError(t, err)
+	require.NotNil(t, clientCert)
+}

tls/server/config/config_test.go

@@ -29,6 +29,10 @@ func TestGetServerTLSConfig(t *testing.T) {
 	})
 	require.NoError(t, err)
 	require.NotNil(t, tlsConfig.ClientCAs)
+	systemPool, err := x509.SystemCertPool()
+	require.NoError(t, err)
+	// not system pool
+	require.False(t, tlsConfig.ClientCAs.Equal(systemPool))
 	require.Equal(t, tlsConfig.ClientAuth, tls.RequireAndVerifyClientCert)
 	require.NotEmpty(t, tlsConfig.Certificates)
 	// clientCRL verification

tls/server/source/pems.go

@@ -40,10 +40,7 @@ func (s ServerPEMs) ClientCAs() (*x509.CertPool, error) {
 	if len(s.ClientAuthPEMBlock) == 0 {
 		return nil, nil
 	}
-	certPool, err := x509.SystemCertPool()
-	if err != nil {
-		certPool = x509.NewCertPool()
-	}
+	certPool := x509.NewCertPool()
 	if !certPool.AppendCertsFromPEM(s.ClientAuthPEMBlock) {
 		return nil, errors.New("server PEMs: building client CAs failed")
 	}