GitLab API Client support for client certificates

4 files changed, 113 insertions, 25 deletions

diff --git a/client/client_test.go b/client/client_test.go
index 45d9819..fec51c5 100644
--- a/client/client_test.go
+++ b/client/client_test.go
@@ -49,7 +49,9 @@ func TestClients(t *testing.T) {
 		{
 			desc:   "Https client",
 			caFile: path.Join(testhelper.TestRoot, "certs/valid/server.crt"),
-			server: testserver.StartHttpsServer,
+			server: func(t *testing.T, handlers []testserver.TestRequestHandler) (string, func()) {
+				return testserver.StartHttpsServer(t, handlers, "")
+			},
 		},
 	}
 
diff --git a/client/httpclient.go b/client/httpclient.go
index 6635f1b..05c3032 100644
--- a/client/httpclient.go
+++ b/client/httpclient.go
@@ -4,6 +4,7 @@ import (
 	"context"
 	"crypto/tls"
 	"crypto/x509"
+	"errors"
 	"io/ioutil"
 	"net"
 	"net/http"
@@ -11,6 +12,7 @@ import (
 	"strings"
 	"time"
 
+	log "github.com/sirupsen/logrus"
 	"gitlab.com/gitlab-org/labkit/correlation"
 )
 
@@ -27,18 +29,59 @@ type HttpClient struct {
 	Host string
 }
 
+type httpClientCfg struct {
+	keyPath, certPath string
+	caFile, caPath    string
+}
+
+func (hcc httpClientCfg) HaveCertAndKey() bool { return hcc.keyPath != "" && hcc.certPath != "" }
+
+// HTTPClientOpt provides options for configuring an HttpClient
+type HTTPClientOpt func(*httpClientCfg)
+
+// WithClientCert will configure the HttpClient to provide client certificates
+// when connecting to a server.
+func WithClientCert(certPath, keyPath string) HTTPClientOpt {
+	return func(hcc *httpClientCfg) {
+		hcc.keyPath = keyPath
+		hcc.certPath = certPath
+	}
+}
+
+// Deprecated: use NewHTTPClientWithOpts - https://gitlab.com/gitlab-org/gitlab-shell/-/issues/484
 func NewHTTPClient(gitlabURL, gitlabRelativeURLRoot, caFile, caPath string, selfSignedCert bool, readTimeoutSeconds uint64) *HttpClient {
+	c, err := NewHTTPClientWithOpts(gitlabURL, gitlabRelativeURLRoot, caFile, caPath, selfSignedCert, readTimeoutSeconds, nil)
+	if err != nil {
+		log.WithError(err).Error("new http client with opts")
+	}
+	return c
+}
+
+// NewHTTPClientWithOpts builds an HTTP client using the provided options
+func NewHTTPClientWithOpts(gitlabURL, gitlabRelativeURLRoot, caFile, caPath string, selfSignedCert bool, readTimeoutSeconds uint64, opts []HTTPClientOpt) (*HttpClient, error) {
+	hcc := &httpClientCfg{
+		caFile: caFile,
+		caPath: caPath,
+	}
+
+	for _, opt := range opts {
+		opt(hcc)
+	}
 
 	var transport *http.Transport
 	var host string
+	var err error
 	if strings.HasPrefix(gitlabURL, unixSocketProtocol) {
 		transport, host = buildSocketTransport(gitlabURL, gitlabRelativeURLRoot)
 	} else if strings.HasPrefix(gitlabURL, httpProtocol) {
 		transport, host = buildHttpTransport(gitlabURL)
 	} else if strings.HasPrefix(gitlabURL, httpsProtocol) {
-		transport, host = buildHttpsTransport(caFile, caPath, selfSignedCert, gitlabURL)
+		transport, host, err = buildHttpsTransport(*hcc, selfSignedCert, gitlabURL)
+		if err != nil {
+			return nil, err
+		}
 	} else {
-		return nil
+		return nil, errors.New("unknown GitLab URL prefix")
 	}
 
 	c := &http.Client{
@@ -48,7 +91,7 @@ func NewHTTPClient(gitlabURL, gitlabRelativeURLRoot, caFile, caPath string, self
 
 	client := &HttpClient{Client: c, Host: host}
 
-	return client
+	return client, nil
 }
 
 func buildSocketTransport(gitlabURL, gitlabRelativeURLRoot string) (*http.Transport, string) {
@@ -70,36 +113,46 @@ func buildSocketTransport(gitlabURL, gitlabRelativeURLRoot string) (*http.Transp
 	return transport, host
 }
 
-func buildHttpsTransport(caFile, caPath string, selfSignedCert bool, gitlabURL string) (*http.Transport, string) {
+func buildHttpsTransport(hcc httpClientCfg, selfSignedCert bool, gitlabURL string) (*http.Transport, string, error) {
 	certPool, err := x509.SystemCertPool()
 
 	if err != nil {
 		certPool = x509.NewCertPool()
 	}
 
-	if caFile != "" {
-		addCertToPool(certPool, caFile)
+	if hcc.caFile != "" {
+		addCertToPool(certPool, hcc.caFile)
 	}
 
-	if caPath != "" {
-		fis, _ := ioutil.ReadDir(caPath)
+	if hcc.caPath != "" {
+		fis, _ := ioutil.ReadDir(hcc.caPath)
 		for _, fi := range fis {
 			if fi.IsDir() {
 				continue
 			}
 
-			addCertToPool(certPool, filepath.Join(caPath, fi.Name()))
+			addCertToPool(certPool, filepath.Join(hcc.caPath, fi.Name()))
 		}
 	}
+	tlsConfig := &tls.Config{
+		RootCAs:            certPool,
+		InsecureSkipVerify: selfSignedCert,
+	}
+
+	if hcc.HaveCertAndKey() {
+		cert, err := tls.LoadX509KeyPair(hcc.certPath, hcc.keyPath)
+		if err != nil {
+			return nil, "", err
+		}
+		tlsConfig.Certificates = []tls.Certificate{cert}
+		tlsConfig.BuildNameToCertificate()
+	}
 
 	transport := &http.Transport{
-		TLSClientConfig: &tls.Config{
-			RootCAs:            certPool,
-			InsecureSkipVerify: selfSignedCert,
-		},
+		TLSClientConfig: tlsConfig,
 	}
 
-	return transport, gitlabURL
+	return transport, gitlabURL, err
 }
 
 func addCertToPool(certPool *x509.CertPool, fileName string) {
diff --git a/client/httpsclient_test.go b/client/httpsclient_test.go
index d76890b..dadd095 100644
--- a/client/httpsclient_test.go
+++ b/client/httpsclient_test.go
@@ -13,11 +13,13 @@ import (
 	"gitlab.com/gitlab-org/gitlab-shell/internal/testhelper"
 )
 
+//go:generate openssl req -newkey rsa:4096 -new -nodes -x509 -days 3650 -out ../internal/testhelper/testdata/testroot/certs/client/server.crt -keyout ../internal/testhelper/testdata/testroot/certs/client/key.pem -subj "/C=US/ST=California/L=San Francisco/O=GitLab/OU=GitLab-Shell/CN=localhost"
 func TestSuccessfulRequests(t *testing.T) {
 	testCases := []struct {
-		desc           string
-		caFile, caPath string
-		selfSigned     bool
+		desc                                        string
+		caFile, caPath                              string
+		selfSigned                                  bool
+		clientCAPath, clientCertPath, clientKeyPath string // used for TLS client certs
 	}{
 		{
 			desc:   "Valid CaFile",
@@ -36,11 +38,20 @@ func TestSuccessfulRequests(t *testing.T) {
 			caFile:     path.Join(testhelper.TestRoot, "certs/valid/server.crt"),
 			selfSigned: true,
 		},
+		{
+			desc:   "Client certs with CA",
+			caFile: path.Join(testhelper.TestRoot, "certs/valid/server.crt"),
+			// Run the command "go generate httpsclient_test.go" to
+			// regenerate the following test fixtures:
+			clientCAPath:   path.Join(testhelper.TestRoot, "certs/client/server.crt"),
+			clientCertPath: path.Join(testhelper.TestRoot, "certs/client/server.crt"),
+			clientKeyPath:  path.Join(testhelper.TestRoot, "certs/client/key.pem"),
+		},
 	}
 
 	for _, tc := range testCases {
 		t.Run(tc.desc, func(t *testing.T) {
-			client, cleanup := setupWithRequests(t, tc.caFile, tc.caPath, tc.selfSigned)
+			client, cleanup := setupWithRequests(t, tc.caFile, tc.caPath, tc.clientCAPath, tc.clientCertPath, tc.clientKeyPath, tc.selfSigned)
 			defer cleanup()
 
 			response, err := client.Get(context.Background(), "/hello")
@@ -77,7 +88,7 @@ func TestFailedRequests(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.desc, func(t *testing.T) {
-			client, cleanup := setupWithRequests(t, tc.caFile, tc.caPath, false)
+			client, cleanup := setupWithRequests(t, tc.caFile, tc.caPath, "", "", "", false)
 			defer cleanup()
 
 			_, err := client.Get(context.Background(), "/hello")
@@ -88,7 +99,7 @@ func TestFailedRequests(t *testing.T) {
 	}
 }
 
-func setupWithRequests(t *testing.T, caFile, caPath string, selfSigned bool) (*GitlabNetClient, func()) {
+func setupWithRequests(t *testing.T, caFile, caPath, clientCAPath, clientCertPath, clientKeyPath string, selfSigned bool) (*GitlabNetClient, func()) {
 	testDirCleanup, err := testhelper.PrepareTestRootDir()
 	require.NoError(t, err)
 	defer testDirCleanup()
@@ -104,9 +115,15 @@ func setupWithRequests(t *testing.T, caFile, caPath string, selfSigned bool) (*G
 		},
 	}
 
-	url, cleanup := testserver.StartHttpsServer(t, requests)
+	url, cleanup := testserver.StartHttpsServer(t, requests, clientCAPath)
 
-	httpClient := NewHTTPClient(url, "", caFile, caPath, selfSigned, 1)
+	var opts []HTTPClientOpt
+	if clientCertPath != "" && clientKeyPath != "" {
+		opts = append(opts, WithClientCert(clientCertPath, clientKeyPath))
+	}
+
+	httpClient, err := NewHTTPClientWithOpts(url, "", caFile, caPath, selfSigned, 1, opts)
+	require.NoError(t, err)
 
 	client, err := NewGitlabNetClient("", "", "", httpClient)
 	require.NoError(t, err)
diff --git a/client/testserver/testserver.go b/client/testserver/testserver.go
index 377e331..8130c7a 100644
--- a/client/testserver/testserver.go
+++ b/client/testserver/testserver.go
@@ -2,6 +2,7 @@ package testserver
 
 import (
 	"crypto/tls"
+	"crypto/x509"
 	"io/ioutil"
 	"log"
 	"net"
@@ -52,7 +53,7 @@ func StartHttpServer(t *testing.T, handlers []TestRequestHandler) (string, func(
 	return server.URL, server.Close
 }
 
-func StartHttpsServer(t *testing.T, handlers []TestRequestHandler) (string, func()) {
+func StartHttpsServer(t *testing.T, handlers []TestRequestHandler, clientCAPath string) (string, func()) {
 	crt := path.Join(testhelper.TestRoot, "certs/valid/server.crt")
 	key := path.Join(testhelper.TestRoot, "certs/valid/server.key")
 
@@ -60,7 +61,22 @@ func StartHttpsServer(t *testing.T, handlers []TestRequestHandler) (string, func
 	cer, err := tls.LoadX509KeyPair(crt, key)
 	require.NoError(t, err)
 
-	server.TLS = &tls.Config{Certificates: []tls.Certificate{cer}}
+	server.TLS = &tls.Config{
+		Certificates: []tls.Certificate{cer},
+	}
+	server.TLS.BuildNameToCertificate()
+
+	if clientCAPath != "" {
+		caCert, err := ioutil.ReadFile(clientCAPath)
+		require.NoError(t, err)
+
+		caCertPool := x509.NewCertPool()
+		caCertPool.AppendCertsFromPEM(caCert)
+
+		server.TLS.ClientCAs = caCertPool
+		server.TLS.ClientAuth = tls.RequireAndVerifyClientCert
+	}
+
 	server.StartTLS()
 
 	return server.URL, server.Close