diff options
-rw-r--r-- | client/client_test.go | 4 | ||||
-rw-r--r-- | client/httpclient.go | 81 | ||||
-rw-r--r-- | client/httpsclient_test.go | 33 | ||||
-rw-r--r-- | client/testserver/testserver.go | 20 | ||||
-rw-r--r-- | internal/testhelper/testdata/testroot/certs/client/key.pem | 52 | ||||
-rw-r--r-- | internal/testhelper/testdata/testroot/certs/client/server.crt | 34 |
6 files changed, 199 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 diff --git a/internal/testhelper/testdata/testroot/certs/client/key.pem b/internal/testhelper/testdata/testroot/certs/client/key.pem new file mode 100644 index 0000000..c634ead --- /dev/null +++ b/internal/testhelper/testdata/testroot/certs/client/key.pem @@ -0,0 +1,52 @@ +-----BEGIN PRIVATE KEY----- +MIIJQgIBADANBgkqhkiG9w0BAQEFAASCCSwwggkoAgEAAoICAQDRt8ajfb9sVLAe +W05cKQ7t0wNbcer1EaZDLmNrlqRLFk3SdJarBf6ninI214K6Uyv3ijBLlnactqrc +5NU9+igRY8qkKpdiU4AMDYOVUSB4JL0z0YcO6zariBDQx2dO5S/D1WgZZtDtvMWZ +YqWWqToX8Lt3L/9Ek//i1m0ohJxnkMVA/LxdgCXfj93Dq79/QCB74TWybiL9QELU +KPtFmNXMTffGg8QsRVkRYrwCCXqfx6J6X8fOFlCDNBSjODRBLgWUKhOOytv90NLn +Hp6CGK5BHbBnA6Y0ghrUCNyLDGlc0x8AKII1HWKJ+PXtRbPmYNm3i5JVTYx04vf9 +J1v51mPYh5A/WHteaCBVetEut2UUnyRdCgce7Tv4ilZKQITR8NRd495hx57nmqVA +NST//IVwyGoOa8cSAjOlUZ1Q+3ZniBqZE1S0lqhPHykl/5m9VXhFuMq8i67z83aK +1SiIfbYdxgjdlq20Mfc1gRCJJDmDsYpYziKw1scL3gKWY5F6Yj8OkQLzXhdLC1dw +PtmjMKY6E4BxEov6NsopN77simGuUHogsruyhtuDcWWRZRrlrk6s2e8fkfqvtecV +TrEnxeWg1o0oEm04PnH9kA6YmoOF+J0w4UrOuN0EB7f/89j8TCWZRugeibig7qop +b94Awchk4zreYOgjbL+pEZKw6QvfPQIDAQABAoICAGW5e8uf2jNE3OzMozTG4avw +V8eKeUqIVhpuLOFp/6VAW11DGjY4wS4pVH9Ph+SzJTd8OzLe+AfJ/xUIlnrqlXbh +7dA1rJqQICM4huPtpw8/2tqAvr84zprjdCyhHHZDayjVohn4Kk227C4bkHCFA13L +clM839g25b70/ZvSvz7pFRURwpij6TsIwKwB6fBifZ85PV+gVq569i+M9Vzr5oCk +LRSIo6ZJuQta1hEy4d0Q67nqLbPEVSdfIseNIqOfHCujQTtZIN575WEgFAjMyfFh +4kgFmCAOH89LwRZdXdodugLMo2P6Ler47Ok7jyinP9PtCn0AEao80cdkyRNlr6Xe +OqQ3Grv9yj8mX0lAKqJrhWNtd/20XCqOFeTjEm02cOnWcE7rEpKk0y70Hryq81/U +8x4qYW0KwLcQTi5o8mAYU6d/1lmzeZujfcA7ywlNjmTJDClq3d0Gfh7QrilTmypY +xuqQ/zJ1izNoYdEt0or3LcKy+DTmg8rjPSBsmGa4cLY3FAht5N5vcvqoDj1MTy4G +6ylKls7LAoZfezYW267/bA9Dpyubh/iS9It2M8KW2adf/e0yFesaMGUnoR8lhXXe +q3b9CdXXGM5XaQDb1FFrGT3In17IAwf93BEOJ6SSd4153MgnFvP7iY3DyxdQt3zN +VjU1p/E9cHCk2h/wnT4tAoIBAQDtVisBYTx4eg+OsLHeBQzzovf33WGa+Xcp01pT +4zBM3A0IJsYHABPxe0x3q+14HBmtNQJZYDjzw6YU4HvsJBeNirrKMz21sCQJH4uz +9H+y3F0BC5RETanVS6gAcgEVsljPwx/qAiw4F/K8rn63doC3mGIh/p9wJPLtLWUb +P7Vkiw6sZDzJLy3+02H1dGZhL8naEvYdhU+gu7kpAVhWOmxc97a9oPBXcYJWq6hJ +0VUgpBV+gNYETAJaMqiU3zGsMzcBOK++cBiiJhEOY29Rl6HuOz/Q8W7om7uQk6L0 +Bgg+JJDFW2+3dtfO04pSEQLcRiMUFvNX3FkKG7H2tbUwDJKLAoIBAQDiNZ1NJhtU +D8sbjFBNxHrHmrCkc8lRT4D22echcOp5S6vW4EowA9bQfRHnk7jEmCeZMsiGyddb +Ep9v+j9cF+1WtzN3p5m76BPJpIW9VkNjbUVTKfY8lB0O0TAqm4juIpjW62oh4HPU +TJDgo7WQbmIgP3ezjO2t+mNscqASPHdswovXbU+4UmM4lk2j4YSn3qis/wW0vMKN +vCf3V3HlnxCTqTVM76oPfrdObyiI66Sd5kNNPzMHYT6/fqrhJjb0ckqeUxiADHcy +VCR9a/2XDytfHrs+/95t520yS63f88m8Gl5lpsp6CM8zVKJPibDvQTEf4W90HF1b +/89oqoQk7nZXAoIBAEHRtsWAMOP8fdoFmJ5I6kma9YfQ5mOzMV/xFEjVZay7DgYn +sp14YQ+EMTWzAX1g1aIaZFdi/whjRujdRKC9daa0RY8T3NZJTgUVsYmrkcqJoGVM +z8aNfz7+502QUEqzFjwwEea0yYyY36GCBvRcMeA4q2ZgFdlk9dXe0/5VkbmbcutO +NSlaIzhbaPxIVqg3N5R507VmJioeRYBgth3bv/ecXxqByoWFni7pFhe6rRALUUau +9itk5PYcvHHk4AKwhV2aWerHbZ1yTyKdYt7O3YKS/eS1QBvULJUwzG0+SwTo4RlK +fVX06G6cbezKeO+bp9jHcJ76JdtOyPDxfZkgs3cCggEAFC6CXTq0H3jVPxzyoS2R +YrOLZPCrmmSEdgGU3Gftk2rL5vzVwZjmFm3CJi4IwwlsJv/f4h6p5wcvUFc8ReQg +mab4oYlDbv9SnJ/gCrdihcFe+P96Z4czXHoPWQ3NVqmhhzMzodgbnWpDVrdkYIFo +ocXn0Q4WunnnWuqTG21nnj1xKoQnI6O+FHNcc+2P30Y/OEf8Y1af6PNLgYa8s6bQ +XMww5C9RtdYxVn8WV7jmU+wSPxcPX24uofkUF8hICOEVhTCWs/3ouIXHR6VV159T +2EWuoP1FA/ssw9r6pUtjyTN1Do6l6+NTURoQ7RW0wnPHhTegsPRC5A1bnNPxvDXG +OwKCAQEAuYbtsQJSQK6MG3/aYvV4GxMZ+5lL9zy7f00+Os7R8GyR7T3aw4mvIty8 +LeVVzExETLMuJZ57w0aKgmkLLNaJ3zzs76fFTGpdwM0R6v1Kj3LO6yckvBdbXnJS +icBODG8Kh7UpWu0hdWO/7M2Vmscz+zcAaEufjA8O18PMu7XNwFDOQniF5W/Rv6Ym +AOI/IItrkH6CpkaDqmJzKI6QfWxfal6UfLgQJLEFLwRXBWu/dv2XoNz2l6yC75Fn +BJXf+dZuPckwnIAUtosJfTU+Lecih/yKnfVxUzDX6ZKs4Ll7A4FsITY0ORnPoAJv +bmebasEKpSH4ftWF03etDPQQiA4o3A== +-----END PRIVATE KEY----- diff --git a/internal/testhelper/testdata/testroot/certs/client/server.crt b/internal/testhelper/testdata/testroot/certs/client/server.crt new file mode 100644 index 0000000..8289f7f --- /dev/null +++ b/internal/testhelper/testdata/testroot/certs/client/server.crt @@ -0,0 +1,34 @@ +-----BEGIN CERTIFICATE----- +MIIFzTCCA7WgAwIBAgIUGAR3YWGMIbkGVY1XZeAsYKbOJkgwDQYJKoZIhvcNAQEL +BQAwdjELMAkGA1UEBhMCVVMxEzARBgNVBAgMCkNhbGlmb3JuaWExFjAUBgNVBAcM +DVNhbiBGcmFuY2lzY28xDzANBgNVBAoMBkdpdExhYjEVMBMGA1UECwwMR2l0TGFi +LVNoZWxsMRIwEAYDVQQDDAlsb2NhbGhvc3QwHhcNMjAxMTE1MjIzMjM1WhcNMzAx +MTEzMjIzMjM1WjB2MQswCQYDVQQGEwJVUzETMBEGA1UECAwKQ2FsaWZvcm5pYTEW +MBQGA1UEBwwNU2FuIEZyYW5jaXNjbzEPMA0GA1UECgwGR2l0TGFiMRUwEwYDVQQL +DAxHaXRMYWItU2hlbGwxEjAQBgNVBAMMCWxvY2FsaG9zdDCCAiIwDQYJKoZIhvcN +AQEBBQADggIPADCCAgoCggIBANG3xqN9v2xUsB5bTlwpDu3TA1tx6vURpkMuY2uW +pEsWTdJ0lqsF/qeKcjbXgrpTK/eKMEuWdpy2qtzk1T36KBFjyqQql2JTgAwNg5VR +IHgkvTPRhw7rNquIENDHZ07lL8PVaBlm0O28xZlipZapOhfwu3cv/0ST/+LWbSiE +nGeQxUD8vF2AJd+P3cOrv39AIHvhNbJuIv1AQtQo+0WY1cxN98aDxCxFWRFivAIJ +ep/Honpfx84WUIM0FKM4NEEuBZQqE47K2/3Q0ucenoIYrkEdsGcDpjSCGtQI3IsM +aVzTHwAogjUdYon49e1Fs+Zg2beLklVNjHTi9/0nW/nWY9iHkD9Ye15oIFV60S63 +ZRSfJF0KBx7tO/iKVkpAhNHw1F3j3mHHnueapUA1JP/8hXDIag5rxxICM6VRnVD7 +dmeIGpkTVLSWqE8fKSX/mb1VeEW4yryLrvPzdorVKIh9th3GCN2WrbQx9zWBEIkk +OYOxiljOIrDWxwveApZjkXpiPw6RAvNeF0sLV3A+2aMwpjoTgHESi/o2yik3vuyK +Ya5QeiCyu7KG24NxZZFlGuWuTqzZ7x+R+q+15xVOsSfF5aDWjSgSbTg+cf2QDpia +g4X4nTDhSs643QQHt//z2PxMJZlG6B6JuKDuqilv3gDByGTjOt5g6CNsv6kRkrDp +C989AgMBAAGjUzBRMB0GA1UdDgQWBBS6KR9YtGVCh3Rqa5tBGp8AMy7fYzAfBgNV +HSMEGDAWgBS6KR9YtGVCh3Rqa5tBGp8AMy7fYzAPBgNVHRMBAf8EBTADAQH/MA0G +CSqGSIb3DQEBCwUAA4ICAQDIomc8qF4bbyB9V2HJv3Dk3MmQ7r96D++C8vvi0NTt +r7zdwuLFsv2MZ1tu5noHWo1ardDLpvC92a/JZMpmzzdS+cysrbMIjDVbNkYl3pV6 +Uxt/OrdkFlcZCyzSaSZmB8hKdcoRuGuI2PrhoXp15SVal7UTNdg6GkIgWjEh+nGx +kXfjNtus/LJNyssBkeCXBr+sxBvGmwJSRodK3kJgBk33opouFyKNBkkm7qSHTQ8L +IuYDtgGItDCWupLHrXpvjQUSzYPmshesgbbHm3tJopnziFqDI+LcO5dMiwByBBW4 +W7I9YvWwd0ZBo9+QfREGypr4lmFfKThjUiC92Lzn2xJk9PYU5uDtPz3RABzsxA6W +9yZcVNUWUiGw/NhWemPezjSBrsAvFxlnKb5ORMHhkKuqAW5dctRstONdzMMYR61T +PRPI1zZCRoxtdSu6WTVhJxQfC0PvnZGXoLk4uuacu2USezpFeRNTD9bZd5H2Bmla +RcqcvsgraOfmH9q73c6pjzWyQexBm4+RXJcl0pmJtDF8zNjn9kzxWE3fRzptBpBS +JXPVbnwG/4tf99FsFn2X//iYP1bxjvIbm0TcTXMuiQWrOeOdrf2QQU+KiTkLXvHy +1TvrVUgELzmd0sjt+tfMTXpsBm+kaGl9jnIee5PTj5yUNPfyHvv9RwBHwRaenIJD +bQ== +-----END CERTIFICATE----- |