Commit 2b4161c1 authored by feistel's avatar feistel
Browse files

fix: validate client cert paths exist on disk before proceeding

parent 6ea629933069
......@@ -5,6 +5,7 @@
"crypto/tls"
"crypto/x509"
"errors"
"fmt"
"io/ioutil"
"net"
"net/http"
......@@ -8,6 +9,7 @@
"io/ioutil"
"net"
"net/http"
"os"
"path/filepath"
"strings"
"time"
......@@ -25,6 +27,10 @@
defaultReadTimeoutSeconds = 300
)
var (
ErrCafileNotFound = errors.New("cafile not found")
)
type HttpClient struct {
*http.Client
Host string
......@@ -60,15 +66,6 @@
// 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
......@@ -77,6 +74,19 @@
} else if strings.HasPrefix(gitlabURL, httpProtocol) {
transport, host = buildHttpTransport(gitlabURL)
} else if strings.HasPrefix(gitlabURL, httpsProtocol) {
hcc := &httpClientCfg{
caFile: caFile,
caPath: caPath,
}
for _, opt := range opts {
opt(hcc)
}
if _, err := os.Stat(caFile); err != nil {
return nil, fmt.Errorf("cannot find cafile '%s': %w", caFile, ErrCafileNotFound)
}
transport, host, err = buildHttpsTransport(*hcc, selfSignedCert, gitlabURL)
if err != nil {
return nil, err
......
......@@ -28,10 +28,7 @@
{
desc: "Valid CaPath",
caPath: path.Join(testhelper.TestRoot, "certs/valid"),
},
{
desc: "Self signed cert option enabled",
selfSigned: true,
caFile: path.Join(testhelper.TestRoot, "certs/valid/server.crt"),
},
{
desc: "Invalid cert with self signed cert option enabled",
......@@ -51,7 +48,8 @@
for _, tc := range testCases {
t.Run(tc.desc, func(t *testing.T) {
client := setupWithRequests(t, tc.caFile, tc.caPath, tc.clientCAPath, tc.clientCertPath, tc.clientKeyPath, tc.selfSigned)
client, err := setupWithRequests(t, tc.caFile, tc.caPath, tc.clientCAPath, tc.clientCertPath, tc.clientKeyPath, tc.selfSigned)
require.NoError(t, err)
response, err := client.Get(context.Background(), "/hello")
require.NoError(t, err)
......@@ -68,8 +66,9 @@
func TestFailedRequests(t *testing.T) {
testCases := []struct {
desc string
caFile string
caPath string
desc string
caFile string
caPath string
expectedError string
}{
{
......@@ -74,7 +73,8 @@
}{
{
desc: "Invalid CaFile",
caFile: path.Join(testhelper.TestRoot, "certs/invalid/server.crt"),
desc: "Invalid CaFile",
caFile: path.Join(testhelper.TestRoot, "certs/invalid/server.crt"),
expectedError: "Internal API unreachable",
},
{
desc: "Invalid CaPath",
......@@ -87,5 +87,11 @@
for _, tc := range testCases {
t.Run(tc.desc, func(t *testing.T) {
client := setupWithRequests(t, tc.caFile, tc.caPath, "", "", "", false)
client, err := setupWithRequests(t, tc.caFile, tc.caPath, "", "", "", false)
if tc.caFile == "" {
require.Error(t, err)
require.ErrorIs(t, err, ErrCafileNotFound)
} else {
_, err = client.Get(context.Background(), "/hello")
require.Error(t, err)
......@@ -91,9 +97,7 @@
_, err := client.Get(context.Background(), "/hello")
require.Error(t, err)
require.Equal(t, err.Error(), "Internal API unreachable")
require.Equal(t, err.Error(), tc.expectedError)
}
})
}
}
......@@ -96,8 +100,8 @@
})
}
}
func setupWithRequests(t *testing.T, caFile, caPath, clientCAPath, clientCertPath, clientKeyPath string, selfSigned bool) *GitlabNetClient {
func setupWithRequests(t *testing.T, caFile, caPath, clientCAPath, clientCertPath, clientKeyPath string, selfSigned bool) (*GitlabNetClient, error) {
testhelper.PrepareTestRootDir(t)
requests := []testserver.TestRequestHandler{
......@@ -119,6 +123,8 @@
}
httpClient, err := NewHTTPClientWithOpts(url, "", caFile, caPath, selfSigned, 1, opts)
require.NoError(t, err)
if err != nil {
return nil, err
}
client, err := NewGitlabNetClient("", "", "", httpClient)
......@@ -123,4 +129,3 @@
client, err := NewGitlabNetClient("", "", "", httpClient)
require.NoError(t, err)
......@@ -126,3 +131,3 @@
return client
return client, err
}
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment