From d9cfee687bdf9fcbf45345d7905f55e34115db45 Mon Sep 17 00:00:00 2001 From: ngala Date: Thu, 11 Apr 2024 17:10:24 -0400 Subject: [PATCH 01/11] Support mutual TLS when calling the GitLab API Related: https://gitlab.com/gitlab-org/gitlab-pages/-/issues/548 Changelog: added --- app.go | 3 +- internal/artifact/artifact.go | 11 +- internal/artifact/artifact_test.go | 6 +- internal/auth/auth.go | 9 +- internal/config/config.go | 32 ++++++ internal/config/flags.go | 3 + internal/config/validate.go | 1 + internal/httptransport/transport.go | 3 +- internal/source/gitlab/client/client.go | 13 ++- internal/source/gitlab/client/client_test.go | 8 +- test/gitlabstub/cmd/server/main.go | 114 ++++++++++++++----- test/gitlabstub/option.go | 21 +++- 12 files changed, 178 insertions(+), 46 deletions(-) diff --git a/app.go b/app.go index bda50e890..e86120b87 100644 --- a/app.go +++ b/app.go @@ -384,7 +384,7 @@ func runApp(config *cfg.Config) error { return fmt.Errorf("failed to initialize logging: %w", err) } - a.Artifact = artifact.New(config.ArtifactsServer.URL, config.ArtifactsServer.TimeoutSeconds, config.General.Domain) + a.Artifact = artifact.New(config.ArtifactsServer.URL, config.ArtifactsServer.TimeoutSeconds, config.General.Domain, config.GitLab.ClientCertificates) if err := a.setAuth(config); err != nil { return err @@ -423,6 +423,7 @@ func (a *theApp) setAuth(config *cfg.Config) error { AuthTimeout: config.Authentication.Timeout, CookieSessionTimeout: config.Authentication.CookieSessionTimeout, AllowNamespaceInPath: config.General.NamespaceInPath, + ClientCerts: config.GitLab.ClientCertificates, }) if err != nil { return fmt.Errorf("could not initialize auth package: %w", err) diff --git a/internal/artifact/artifact.go b/internal/artifact/artifact.go index de79bc8d8..7ad74cb37 100644 --- a/internal/artifact/artifact.go +++ b/internal/artifact/artifact.go @@ -2,6 +2,7 @@ package artifact import ( "context" + "crypto/tls" "errors" "fmt" "io" @@ -47,13 +48,19 @@ type Artifact struct { // New when provided the arguments defined herein, returns a pointer to an // Artifact that is used to proxy requests. -func New(server string, timeoutSeconds int, pagesDomain string) *Artifact { +func New(server string, timeoutSeconds int, pagesDomain string, clientCerts []tls.Certificate) *Artifact { + httpTransport := httptransport.DefaultTransport.Clone() + + if len(clientCerts) != 0 { + httpTransport.TLSClientConfig.Certificates = clientCerts + } + return &Artifact{ server: strings.TrimRight(server, "/"), suffix: "." + strings.ToLower(pagesDomain), client: &http.Client{ Timeout: time.Second * time.Duration(timeoutSeconds), - Transport: httptransport.DefaultTransport, + Transport: httpTransport, }, } } diff --git a/internal/artifact/artifact_test.go b/internal/artifact/artifact_test.go index 8f40b842b..9f2ad7532 100644 --- a/internal/artifact/artifact_test.go +++ b/internal/artifact/artifact_test.go @@ -107,7 +107,7 @@ func TestTryMakeRequest(t *testing.T) { r.RemoteAddr = c.RemoteAddr - art := artifact.New(testServer.URL, 1, "gitlab-example.io") + art := artifact.New(testServer.URL, 1, "gitlab-example.io", nil) result := httptest.NewRecorder() @@ -301,7 +301,7 @@ func TestBuildURL(t *testing.T) { for _, c := range cases { t.Run(c.Description, func(t *testing.T) { - a := artifact.New(c.RawServer, 1, c.PagesDomain) + a := artifact.New(c.RawServer, 1, c.PagesDomain, nil) u, ok := a.BuildURL(c.Host, c.Path) msg := c.Description + " - generated URL: " @@ -332,7 +332,7 @@ func TestContextCanceled(t *testing.T) { r = r.WithContext(ctx) // cancel context explicitly cancel() - art := artifact.New(testServer.URL, 1, "gitlab-example.io") + art := artifact.New(testServer.URL, 1, "gitlab-example.io", nil) require.True(t, art.TryMakeRequest(result, r, "", func(resp *http.Response) bool { return false })) require.Equal(t, http.StatusNotFound, result.Code) diff --git a/internal/auth/auth.go b/internal/auth/auth.go index 7c21cd6b9..f33faac0f 100644 --- a/internal/auth/auth.go +++ b/internal/auth/auth.go @@ -3,6 +3,7 @@ package auth import ( "context" "crypto/sha256" + "crypto/tls" "encoding/base64" "encoding/json" "errors" @@ -710,6 +711,7 @@ type Options struct { AuthTimeout time.Duration CookieSessionTimeout time.Duration AllowNamespaceInPath bool + ClientCerts []tls.Certificate } // New when authentication supported this will be used to create authentication handler @@ -719,6 +721,11 @@ func New(options *Options) (*Auth, error) { if err != nil { return nil, err } + httpTransport := httptransport.DefaultTransport.Clone() + + if len(options.ClientCerts) != 0 { + httpTransport.TLSClientConfig.Certificates = options.ClientCerts + } return &Auth{ pagesDomain: options.PagesDomain, @@ -729,7 +736,7 @@ func New(options *Options) (*Auth, error) { publicGitlabServer: strings.TrimRight(options.PublicGitlabServer, "/"), apiClient: &http.Client{ Timeout: options.AuthTimeout, - Transport: httptransport.DefaultTransport, + Transport: httpTransport, }, store: sessions.NewCookieStore(keys[0], keys[1]), authSecret: options.StoreSecret, diff --git a/internal/config/config.go b/internal/config/config.go index ba2270168..375e2e73a 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -119,6 +119,7 @@ type GitLab struct { JWTTokenExpiration time.Duration Cache Cache EnableDisk bool + ClientCertificates []tls.Certificate } // Log groups settings related to configuring logging @@ -422,6 +423,13 @@ func loadConfig() (*Config, error) { } } + certs, err := loadClientCerts(clientCerts.Split()) + if err != nil { + return nil, err + } + + config.GitLab.ClientCertificates = certs + customHeaders, err := parseHeaderString(header.Split()) if err != nil { return nil, fmt.Errorf("unable to parse header string: %w", err) @@ -449,6 +457,30 @@ func loadConfig() (*Config, error) { return config, nil } +func loadClientCerts(certs []string) ([]tls.Certificate, error) { + c := make([]tls.Certificate, 0, len(certs)) + + for i, pair := range certs { + sep := strings.Index(pair, ":") + + if sep == -1 { + return nil, fmt.Errorf("malformed client certs at position %d: %w", i, errMalformedClientCert) + } + + cert := pair[:sep] + key := pair[sep+1:] + + tlsCert, err := tls.LoadX509KeyPair(cert, key) + if err != nil { + return nil, err + } + + c = append(c, tlsCert) + } + + return c, nil +} + func logFields(config *Config) map[string]any { return map[string]any{ "artifacts-server": config.ArtifactsServer.URL, diff --git a/internal/config/flags.go b/internal/config/flags.go index 069173c3b..5e1ce28ac 100644 --- a/internal/config/flags.go +++ b/internal/config/flags.go @@ -107,6 +107,8 @@ var ( header = MultiStringFlag{separator: ";;"} + clientCerts = MultiStringFlag{separator: ","} + namespaceInPath = flag.Bool("namespace-in-path", false, "Enable Namespace in path") // flags that won't be logged to the output on Pages boot @@ -126,6 +128,7 @@ func initFlags() { flag.Var(&listenHTTPSProxyv2, "listen-https-proxyv2", "The address(es) or unix socket paths to listen on for HTTPS PROXYv2 requests (https://www.haproxy.org/download/1.8/doc/proxy-protocol.txt)") flag.Var(&header, "header", "The additional http header(s) that should be send to the client") flag.Var(&tlsClientAuthDomains, "tls-client-auth-domains", "The domain(s) that require client certificate authentication") + flag.Var(&clientCerts, "client-cert-key-pairs", "File paths to client certificate and key PEM files to use for mutual TLS") // read from -config=/path/to/gitlab-pages-config flag.String(flag.DefaultConfigFlagname, "", "path to config file") diff --git a/internal/config/validate.go b/internal/config/validate.go index bb247287c..d572870c5 100644 --- a/internal/config/validate.go +++ b/internal/config/validate.go @@ -18,6 +18,7 @@ var ( errArtifactsServerUnsupportedScheme = errors.New("artifacts-server scheme must be either http:// or https://") errArtifactsServerInvalidTimeout = errors.New("artifacts-server-timeout must be greater than or equal to 1") errEmptyListener = errors.New("listener must not be empty") + errMalformedClientCert = errors.New("client certificates must be defined as /path/to/cert:/path/to/key") ) // Validate values populated in Config diff --git a/internal/httptransport/transport.go b/internal/httptransport/transport.go index 374e39d43..c726e77f2 100644 --- a/internal/httptransport/transport.go +++ b/internal/httptransport/transport.go @@ -41,7 +41,8 @@ func NewTransport() *http.Transport { DialTLS: func(network, addr string) (net.Conn, error) { return tls.Dial(network, addr, &tls.Config{RootCAs: pool(), MinVersion: tls.VersionTLS12}) }, - Proxy: http.ProxyFromEnvironment, + TLSClientConfig: &tls.Config{MinVersion: tls.VersionTLS12}, // set MinVersion to fix gosec: G402 + Proxy: http.ProxyFromEnvironment, // overrides the DefaultMaxIdleConnsPerHost = 2 MaxIdleConns: 100, MaxIdleConnsPerHost: 100, diff --git a/internal/source/gitlab/client/client.go b/internal/source/gitlab/client/client.go index a876c7344..79fd9738f 100644 --- a/internal/source/gitlab/client/client.go +++ b/internal/source/gitlab/client/client.go @@ -2,6 +2,7 @@ package client import ( "context" + "crypto/tls" "errors" "fmt" "io" @@ -44,7 +45,7 @@ type Client struct { // NewClient initializes and returns new Client baseUrl is // appConfig.InternalGitLabServer secretKey is appConfig.GitLabAPISecretKey -func NewClient(baseURL string, secretKey []byte, connectionTimeout, jwtTokenExpiry time.Duration) (*Client, error) { +func NewClient(baseURL string, secretKey []byte, connectionTimeout, jwtTokenExpiry time.Duration, clientCerts []tls.Certificate) (*Client, error) { if len(baseURL) == 0 || len(secretKey) == 0 { return nil, errors.New("GitLab API URL or API secret has not been provided") } @@ -62,6 +63,12 @@ func NewClient(baseURL string, secretKey []byte, connectionTimeout, jwtTokenExpi return nil, errors.New("GitLab JWT token expiry has not been provided") } + httpTransport := httptransport.DefaultTransport.Clone() + + if len(clientCerts) != 0 { + httpTransport.TLSClientConfig.Certificates = clientCerts + } + return &Client{ secretKey: secretKey, baseURL: parsedURL, @@ -69,7 +76,7 @@ func NewClient(baseURL string, secretKey []byte, connectionTimeout, jwtTokenExpi Timeout: connectionTimeout, Transport: httptransport.NewMeteredRoundTripper( correlation.NewInstrumentedRoundTripper( - httptransport.DefaultTransport, + httpTransport, correlation.WithClientName(transportClientName), ), transportClientName, @@ -85,7 +92,7 @@ func NewClient(baseURL string, secretKey []byte, connectionTimeout, jwtTokenExpi // NewFromConfig creates a new client from Config struct func NewFromConfig(cfg *config.GitLab) (*Client, error) { - return NewClient(cfg.InternalServer, cfg.APISecretKey, cfg.ClientHTTPTimeout, cfg.JWTTokenExpiration) + return NewClient(cfg.InternalServer, cfg.APISecretKey, cfg.ClientHTTPTimeout, cfg.JWTTokenExpiration, cfg.ClientCertificates) } // Resolve returns a VirtualDomain configuration wrapped into a Lookup for a diff --git a/internal/source/gitlab/client/client_test.go b/internal/source/gitlab/client/client_test.go index ca7b21fb3..724b14123 100644 --- a/internal/source/gitlab/client/client_test.go +++ b/internal/source/gitlab/client/client_test.go @@ -59,7 +59,7 @@ func TestConnectionReuse(t *testing.T) { } func TestNewValidBaseURL(t *testing.T) { - _, err := NewClient("https://gitlab.com", secretKey(t), defaultClientConnTimeout, defaultJWTTokenExpiry) + _, err := NewClient("https://gitlab.com", secretKey(t), defaultClientConnTimeout, defaultJWTTokenExpiry, nil) require.NoError(t, err) } @@ -129,7 +129,7 @@ func TestNewInvalidConfiguration(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := NewClient(tt.args.baseURL, tt.args.secretKey, tt.args.connectionTimeout, tt.args.jwtTokenExpiry) + got, err := NewClient(tt.args.baseURL, tt.args.secretKey, tt.args.connectionTimeout, tt.args.jwtTokenExpiry, nil) require.Nil(t, got) require.Error(t, err) require.Contains(t, err.Error(), tt.wantErrMsg) @@ -263,7 +263,7 @@ func secretKey(t *testing.T) []byte { func defaultClient(t *testing.T, url string) *Client { t.Helper() - client, err := NewClient(url, secretKey(t), defaultClientConnTimeout, defaultJWTTokenExpiry) + client, err := NewClient(url, secretKey(t), defaultClientConnTimeout, defaultJWTTokenExpiry, nil) require.NoError(t, err) return client @@ -324,7 +324,7 @@ func Test_endpoint(t *testing.T) { for name, tt := range tests { t.Run(name, func(t *testing.T) { - gc, err := NewClient(tt.basePath, []byte("secret"), defaultClientConnTimeout, defaultJWTTokenExpiry) + gc, err := NewClient(tt.basePath, []byte("secret"), defaultClientConnTimeout, defaultJWTTokenExpiry, nil) require.NoError(t, err) got, err := gc.endpoint(tt.urlPath, tt.params) diff --git a/test/gitlabstub/cmd/server/main.go b/test/gitlabstub/cmd/server/main.go index 9820b7220..1bfc92c78 100644 --- a/test/gitlabstub/cmd/server/main.go +++ b/test/gitlabstub/cmd/server/main.go @@ -3,12 +3,17 @@ package main import ( "context" "crypto/tls" + "crypto/x509" + "encoding/pem" "errors" "flag" + "fmt" "log" "net/http" + "net/http/httptest" "os" "os/signal" + "path/filepath" "syscall" "time" @@ -16,40 +21,24 @@ import ( ) var ( - pagesRoot = flag.String("pages-root", "shared/pages", "The directory where pages are stored") - keyFile = flag.String("key-file", "", "Path to file certificate") - certFile = flag.String("cert-file", "", "Path to file certificate") + pagesRoot = flag.String("pages-root", "shared/pages", "The directory where pages are stored") + keyFile = flag.String("key-file", "", "Path to file certificate") + certFile = flag.String("cert-file", "", "Path to file certificate") + caCertPath = flag.String("ca-crt", "", "CA certificate for mutual TLS") ) func main() { flag.Parse() - var opts []gitlabstub.Option - - if *keyFile != "" && *certFile != "" { - log.Printf("Loading key pair: (%s) - (%s)", *certFile, *keyFile) - cert, err := tls.LoadX509KeyPair(*certFile, *keyFile) - if err != nil { - log.Fatalf("error loading certificate: %v", err) - } - - opts = append(opts, gitlabstub.WithCertificate(cert)) + if err := run(); err != nil { + log.Fatal(err) } +} - if err := os.Chdir(*pagesRoot); err != nil { - log.Fatalf("error chdir in %s: %v", *pagesRoot, err) - } - - wd, err := os.Getwd() - if err != nil { - log.Fatalf("error getting current dir: %v", err) - } - - opts = append(opts, gitlabstub.WithPagesRoot(wd)) - - server, err := gitlabstub.NewUnstartedServer(opts...) +func run() error { + server, err := createServer() if err != nil { - log.Fatalf("error starting the server: %v", err) + return fmt.Errorf("error starting the server: %w", err) } if server.TLS != nil { @@ -69,6 +58,77 @@ func main() { defer cancel() if err := server.Config.Shutdown(ctx); err != nil && !errors.Is(err, http.ErrServerClosed) { - log.Fatalf("error shutting down %v", err) + return fmt.Errorf("error shutting down %w", err) + } + + return nil +} + +func createServer() (*httptest.Server, error) { + var opts []gitlabstub.Option + + if cert, err := loadCertificate(*certFile, *keyFile); err != nil { + return nil, err + } else if cert != nil { + opts = append(opts, gitlabstub.WithCertificate(*cert)) + } + + if wd, err := filepath.Abs(*pagesRoot); err != nil { + return nil, err + } else if *pagesRoot != "" { + opts = append(opts, gitlabstub.WithPagesRoot(wd)) + } + + if caCert, err := loadCACertificate(*caCertPath); err != nil { + return nil, err + } else if caCert != nil { + opts = append(opts, gitlabstub.WithMutualTLS(caCert)) + } + + server, err := gitlabstub.NewUnstartedServer(opts...) + if err != nil { + return nil, fmt.Errorf("error starting the server: %w", err) + } + + return server, err +} + +func loadCertificate(cert string, key string) (*tls.Certificate, error) { + if cert == "" && key == "" { + log.Printf("No certificate provided, TLS is disabled") + return nil, nil + } + + if cert != "" && key != "" { + log.Printf("Loading key pair: (%s) - (%s)", *certFile, *keyFile) + cert, err := tls.LoadX509KeyPair(*certFile, *keyFile) + if err != nil { + return nil, fmt.Errorf("error loading certificate: %w", err) + } + + return &cert, nil + } + + return nil, fmt.Errorf("missing certificate or key: cert(%s) key(%s)", cert, key) +} +func loadCACertificate(certPath string) (*x509.Certificate, error) { + if certPath == "" { + return nil, nil } + + log.Printf("Reading CA file: %s", certPath) + + caCertFile, err := os.ReadFile(certPath) + if err != nil { + return nil, fmt.Errorf("error reading CA file: %w", err) + } + + block, _ := pem.Decode(caCertFile) + + caCert, err := x509.ParseCertificate(block.Bytes) + if err != nil { + return nil, fmt.Errorf("error parsing CA certificate: %w", err) + } + + return caCert, nil } diff --git a/test/gitlabstub/option.go b/test/gitlabstub/option.go index 366aeb5db..3057fe00b 100644 --- a/test/gitlabstub/option.go +++ b/test/gitlabstub/option.go @@ -2,6 +2,7 @@ package gitlabstub import ( "crypto/tls" + "crypto/x509" "net/http" "time" ) @@ -40,10 +41,22 @@ func WithDelay(delay time.Duration) Option { } func WithCertificate(cert tls.Certificate) Option { - return func(c *config) { - if c.tlsConfig == nil { - c.tlsConfig = defaultTLSConfig() + return func(sc *config) { + if sc.tlsConfig == nil { + sc.tlsConfig = defaultTLSConfig() } - c.tlsConfig.Certificates = append(c.tlsConfig.Certificates, cert) + sc.tlsConfig.Certificates = append(sc.tlsConfig.Certificates, cert) + } +} + +func WithMutualTLS(caCert *x509.Certificate) Option { + return func(sc *config) { + if sc.tlsConfig == nil { + sc.tlsConfig = defaultTLSConfig() + } + + sc.tlsConfig.ClientAuth = tls.RequireAndVerifyClientCert + sc.tlsConfig.ClientCAs = x509.NewCertPool() + sc.tlsConfig.ClientCAs.AddCert(caCert) } } -- GitLab From 0779fe3d3b28db5862b6cd4c98ffca5eb993f430 Mon Sep 17 00:00:00 2001 From: ngala Date: Thu, 18 Apr 2024 12:33:37 +0530 Subject: [PATCH 02/11] Add unit and acceptance tests for GitLab API mutual TLS --- internal/source/gitlab/client/client_test.go | 32 +++++++ .../gitlab/client/testdata/certs/client.crt | 23 +++++ .../gitlab/client/testdata/certs/client.key | 5 ++ test/acceptance/gitlab_api_mutual_tls_test.go | 90 +++++++++++++++++++ test/gitlabstub/option.go | 1 + 5 files changed, 151 insertions(+) create mode 100644 internal/source/gitlab/client/testdata/certs/client.crt create mode 100644 internal/source/gitlab/client/testdata/certs/client.key create mode 100644 test/acceptance/gitlab_api_mutual_tls_test.go diff --git a/internal/source/gitlab/client/client_test.go b/internal/source/gitlab/client/client_test.go index 724b14123..8a579f5cc 100644 --- a/internal/source/gitlab/client/client_test.go +++ b/internal/source/gitlab/client/client_test.go @@ -2,6 +2,7 @@ package client import ( "context" + "crypto/tls" "encoding/base64" "fmt" "net/http" @@ -233,6 +234,37 @@ func TestGetVirtualDomainAuthenticatedRequest(t *testing.T) { require.Equal(t, "mygroup/myproject/public/", lookupPath.Source.Path) } +func TestMutualTLSClientAuthentication(t *testing.T) { + mux := http.NewServeMux() + + mux.HandleFunc("/api/v4/internal/pages", func(w http.ResponseWriter, r *http.Request) { + // Ensure that client certificate is present + if r.TLS == nil || len(r.TLS.PeerCertificates) == 0 { + w.WriteHeader(http.StatusForbidden) + return + } + + w.WriteHeader(http.StatusOK) + }) + server := httptest.NewTLSServer(mux) + defer server.Close() + + clientCert, err := tls.LoadX509KeyPair("testdata/certs/client.crt", "testdata/certs/client.key") + require.NoError(t, err) + + client, err := NewClient(server.URL, secretKey(t), defaultClientConnTimeout, defaultJWTTokenExpiry, []tls.Certificate{clientCert}) + require.NoError(t, err) + + params := url.Values{} + params.Set("host", "group.gitlab.io") + + resp, err := client.get(context.Background(), "/api/v4/internal/pages", params) + require.NoError(t, err) + defer resp.Body.Close() + + require.Equal(t, http.StatusOK, resp.StatusCode) +} + func validateToken(t *testing.T, tokenString string) { t.Helper() token, err := jwt.Parse(tokenString, func(token *jwt.Token) (interface{}, error) { diff --git a/internal/source/gitlab/client/testdata/certs/client.crt b/internal/source/gitlab/client/testdata/certs/client.crt new file mode 100644 index 000000000..069624ff2 --- /dev/null +++ b/internal/source/gitlab/client/testdata/certs/client.crt @@ -0,0 +1,23 @@ +-----BEGIN CERTIFICATE----- +MIIDxDCCAawCFByFBHU3RaAIgFvtrNvhHMhgqGCUMA0GCSqGSIb3DQEBCwUAMHIx +CzAJBgNVBAYTAlVTMQ0wCwYDVQQIDARUZXN0MRUwEwYDVQQHDAxEZWZhdWx0IENp +dHkxHDAaBgNVBAoME0RlZmF1bHQgQ29tcGFueSBMdGQxDTALBgNVBAsMBFRlc3Qx +EDAOBgNVBAMMB1Rlc3QgQ0EwHhcNMjMwOTIwMTEzNDM3WhcNMzMwOTE3MTEzNDM3 +WjCBlTELMAkGA1UEBhMCVVMxDTALBgNVBAgMBFRlc3QxFTATBgNVBAcMDERlZmF1 +bHQgQ2l0eTEcMBoGA1UECgwTRGVmYXVsdCBDb21wYW55IEx0ZDENMAsGA1UECwwE +VGVzdDESMBAGA1UEAwwJVGVzdCBVc2VyMR8wHQYJKoZIhvcNAQkBFhB0ZXN0QGV4 +YW1wbGUub3JnMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAE5S6e7bNszpyhc279 +oRgW8TMytl0IGaPIhMoDF4oUXO+/sOK/cCz59II7nkwkk6COrvbh70ft8e9cdsqt +a5UsvzANBgkqhkiG9w0BAQsFAAOCAgEAFGDKm6825SRYPTi7HG6uK1S1CXbPNnSy +hv9umTa1iRLqip9eacNx6uijdRQr2IOOBZfuRAjut9IYlH/PfrNYPVcc/uYH/gIP +LZdVjMxqpNxqQYUup3Wq8mVu8Gnfbm2ymvCPHnUhM4xH++grmUQW5bq7BozL/oea +AfflJeS3AGmqPBNBDvjubn7NCirzKEkrt7COAXnpXmFmiErT9gWkeE1Xtn51/W3b +27lu+5aBzDUtlppa5eT1GPc6fu5+HLtHod+nDn/7Ys/HUWw/5iA6DRhGXnLlz2W7 +C+m1yDtSVH+axhAXbhnzvZUpB6jXLLce99a5ff84fp/nJ96hR5lHekIkfNj0wtrQ +WqvQ9HuU1Q3dbWOpC9xtOvQBINAnsZGrM8XwLzG39WdK2G9mLNQqHqKedCWW9UhE +lsOZrYihuXOmTBCIW+SGDw2z4unsFdNMXXwJVv8REDJzGmq5JXai40nnnemS7JNQ +ztgO1gPnznZQFsbHgvSSJOoi5sz/o2VeJJ+zYzPSc7Hzbm/MPlcOdXo2uLO6JrIN +3Kk9Gdv/6tQ+gVUq3qNS/1cmiVTyNiiSOpLM1qLPVL/n5HFYPTbI3OQxRomkMb5j +ab5Oj4+trDQQ1ysm6i8UEqQoCr/izj+2muIGp7u94UrcRmJkULwpjkRsQD/uX5yd +hNo/s+Sx0hA= +-----END CERTIFICATE----- diff --git a/internal/source/gitlab/client/testdata/certs/client.key b/internal/source/gitlab/client/testdata/certs/client.key new file mode 100644 index 000000000..17a6bff88 --- /dev/null +++ b/internal/source/gitlab/client/testdata/certs/client.key @@ -0,0 +1,5 @@ +-----BEGIN EC PRIVATE KEY----- +MHcCAQEEIFWzobEUkcIgOUR81lz2KSmoPKyNI5UraogKlWOESRbHoAoGCCqGSM49 +AwEHoUQDQgAE5S6e7bNszpyhc279oRgW8TMytl0IGaPIhMoDF4oUXO+/sOK/cCz5 +9II7nkwkk6COrvbh70ft8e9cdsqta5Usvw== +-----END EC PRIVATE KEY----- diff --git a/test/acceptance/gitlab_api_mutual_tls_test.go b/test/acceptance/gitlab_api_mutual_tls_test.go new file mode 100644 index 000000000..5e4059d78 --- /dev/null +++ b/test/acceptance/gitlab_api_mutual_tls_test.go @@ -0,0 +1,90 @@ +package acceptance_test + +import ( + "crypto/x509" + _ "embed" + "encoding/pem" + "fmt" + "net/http" + "os" + "testing" + + "github.com/stretchr/testify/require" + + "gitlab.com/gitlab-org/gitlab-pages/test/gitlabstub" +) + +const ( + clientCert = "../../test/acceptance/testdata/client.crt" + clientKey = "../../test/acceptance/testdata/client.key" +) + +func TestGitLabAPIMutualTLS(t *testing.T) { + tests := []struct { + name string + host string + path string + status int + expectError bool + }{ + { + name: "basic request works with GitLab API mutual TLS", + host: "group.gitlab-example.com", + path: "/-/private/-/jobs/1/artifacts/200.html", + status: http.StatusOK, + expectError: false, + }, + } + + caCert, err := loadCACertificate(clientCert) + require.NoError(t, err) + + configFile := defaultConfigFileWith(t, "client-cert-key-pairs"+"="+clientCert+":"+clientKey) + + RunPagesProcess(t, + withListeners([]ListenSpec{httpsListener}), + withArguments([]string{ + "-config=" + configFile, + }), + withPublicServer, + withExtraArgument("auth-redirect-uri", "https://projects.gitlab-example.com/auth"), + withExtraArgument("artifacts-server-timeout", "1"), + withStubOptions(gitlabstub.WithMutualTLS(caCert)), + ) + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + rsp, err := GetPageFromListener(t, httpsListener, tt.host, tt.path) + + require.NoError(t, err) + if tt.expectError { + require.Error(t, err) + } else { + require.NoError(t, err) + rsp.Body.Close() + } + require.Equal(t, tt.status, rsp.StatusCode) + }) + } +} + +func loadCACertificate(certPath string) (*x509.Certificate, error) { + if certPath == "" { + return nil, nil + } + + caCertFile, err := os.ReadFile(certPath) + if err != nil { + return nil, fmt.Errorf("error reading CA file: %w", err) + } + + block, _ := pem.Decode(caCertFile) + + caCert, err := x509.ParseCertificate(block.Bytes) + if err != nil { + return nil, fmt.Errorf("error parsing CA certificate: %w", err) + } + + return caCert, nil +} diff --git a/test/gitlabstub/option.go b/test/gitlabstub/option.go index 3057fe00b..0fbc9a1a9 100644 --- a/test/gitlabstub/option.go +++ b/test/gitlabstub/option.go @@ -58,5 +58,6 @@ func WithMutualTLS(caCert *x509.Certificate) Option { sc.tlsConfig.ClientAuth = tls.RequireAndVerifyClientCert sc.tlsConfig.ClientCAs = x509.NewCertPool() sc.tlsConfig.ClientCAs.AddCert(caCert) + sc.tlsConfig.InsecureSkipVerify = true } } -- GitLab From f30257f7d4e90cb58ef8f18e06f8baea604dd914 Mon Sep 17 00:00:00 2001 From: ngala Date: Fri, 26 Apr 2024 00:04:33 +0530 Subject: [PATCH 03/11] Add CA cert in the client --- app.go | 4 +- internal/artifact/artifact.go | 10 ++--- internal/artifact/artifact_test.go | 7 ++-- internal/auth/auth.go | 10 ++--- internal/config/config.go | 9 ++++- internal/config/flags.go | 2 + internal/httptransport/transport.go | 42 ++++++++++++++++++++ internal/source/gitlab/client/client.go | 11 ++--- internal/source/gitlab/client/client_test.go | 13 +++--- 9 files changed, 74 insertions(+), 34 deletions(-) diff --git a/app.go b/app.go index e86120b87..3fc3ddc02 100644 --- a/app.go +++ b/app.go @@ -384,7 +384,7 @@ func runApp(config *cfg.Config) error { return fmt.Errorf("failed to initialize logging: %w", err) } - a.Artifact = artifact.New(config.ArtifactsServer.URL, config.ArtifactsServer.TimeoutSeconds, config.General.Domain, config.GitLab.ClientCertificates) + a.Artifact = artifact.New(config.ArtifactsServer.URL, config.ArtifactsServer.TimeoutSeconds, config.General.Domain, config.GitLab.HTTPClientCfg) if err := a.setAuth(config); err != nil { return err @@ -423,7 +423,7 @@ func (a *theApp) setAuth(config *cfg.Config) error { AuthTimeout: config.Authentication.Timeout, CookieSessionTimeout: config.Authentication.CookieSessionTimeout, AllowNamespaceInPath: config.General.NamespaceInPath, - ClientCerts: config.GitLab.ClientCertificates, + CertCfg: config.GitLab.HTTPClientCfg, }) if err != nil { return fmt.Errorf("could not initialize auth package: %w", err) diff --git a/internal/artifact/artifact.go b/internal/artifact/artifact.go index 7ad74cb37..5f58be62b 100644 --- a/internal/artifact/artifact.go +++ b/internal/artifact/artifact.go @@ -2,7 +2,6 @@ package artifact import ( "context" - "crypto/tls" "errors" "fmt" "io" @@ -14,6 +13,7 @@ import ( "strings" "time" + "gitlab.com/gitlab-org/gitlab-pages/internal/config" "gitlab.com/gitlab-org/gitlab-pages/internal/errortracking" "gitlab.com/gitlab-org/gitlab-pages/internal/httperrors" "gitlab.com/gitlab-org/gitlab-pages/internal/httptransport" @@ -48,12 +48,8 @@ type Artifact struct { // New when provided the arguments defined herein, returns a pointer to an // Artifact that is used to proxy requests. -func New(server string, timeoutSeconds int, pagesDomain string, clientCerts []tls.Certificate) *Artifact { - httpTransport := httptransport.DefaultTransport.Clone() - - if len(clientCerts) != 0 { - httpTransport.TLSClientConfig.Certificates = clientCerts - } +func New(server string, timeoutSeconds int, pagesDomain string, httpClientCfg config.HTTPClientCfg) *Artifact { + httpTransport := httptransport.NewTransportWithClientCert(httpClientCfg) return &Artifact{ server: strings.TrimRight(server, "/"), diff --git a/internal/artifact/artifact_test.go b/internal/artifact/artifact_test.go index 9f2ad7532..167bd5978 100644 --- a/internal/artifact/artifact_test.go +++ b/internal/artifact/artifact_test.go @@ -10,6 +10,7 @@ import ( "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitlab-pages/internal/artifact" + "gitlab.com/gitlab-org/gitlab-pages/internal/config" ) func TestTryMakeRequest(t *testing.T) { @@ -107,7 +108,7 @@ func TestTryMakeRequest(t *testing.T) { r.RemoteAddr = c.RemoteAddr - art := artifact.New(testServer.URL, 1, "gitlab-example.io", nil) + art := artifact.New(testServer.URL, 1, "gitlab-example.io", config.HTTPClientCfg{}) result := httptest.NewRecorder() @@ -301,7 +302,7 @@ func TestBuildURL(t *testing.T) { for _, c := range cases { t.Run(c.Description, func(t *testing.T) { - a := artifact.New(c.RawServer, 1, c.PagesDomain, nil) + a := artifact.New(c.RawServer, 1, c.PagesDomain, config.HTTPClientCfg{}) u, ok := a.BuildURL(c.Host, c.Path) msg := c.Description + " - generated URL: " @@ -332,7 +333,7 @@ func TestContextCanceled(t *testing.T) { r = r.WithContext(ctx) // cancel context explicitly cancel() - art := artifact.New(testServer.URL, 1, "gitlab-example.io", nil) + art := artifact.New(testServer.URL, 1, "gitlab-example.io", config.HTTPClientCfg{}) require.True(t, art.TryMakeRequest(result, r, "", func(resp *http.Response) bool { return false })) require.Equal(t, http.StatusNotFound, result.Code) diff --git a/internal/auth/auth.go b/internal/auth/auth.go index f33faac0f..559f365b3 100644 --- a/internal/auth/auth.go +++ b/internal/auth/auth.go @@ -3,7 +3,6 @@ package auth import ( "context" "crypto/sha256" - "crypto/tls" "encoding/base64" "encoding/json" "errors" @@ -22,6 +21,7 @@ import ( "golang.org/x/crypto/hkdf" "gitlab.com/gitlab-org/gitlab-pages/internal" + "gitlab.com/gitlab-org/gitlab-pages/internal/config" "gitlab.com/gitlab-org/gitlab-pages/internal/errortracking" "gitlab.com/gitlab-org/gitlab-pages/internal/feature" "gitlab.com/gitlab-org/gitlab-pages/internal/httperrors" @@ -711,7 +711,7 @@ type Options struct { AuthTimeout time.Duration CookieSessionTimeout time.Duration AllowNamespaceInPath bool - ClientCerts []tls.Certificate + CertCfg config.HTTPClientCfg } // New when authentication supported this will be used to create authentication handler @@ -721,11 +721,7 @@ func New(options *Options) (*Auth, error) { if err != nil { return nil, err } - httpTransport := httptransport.DefaultTransport.Clone() - - if len(options.ClientCerts) != 0 { - httpTransport.TLSClientConfig.Certificates = options.ClientCerts - } + httpTransport := httptransport.NewTransportWithClientCert(options.CertCfg) return &Auth{ pagesDomain: options.PagesDomain, diff --git a/internal/config/config.go b/internal/config/config.go index 375e2e73a..d1e480be1 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -119,7 +119,12 @@ type GitLab struct { JWTTokenExpiration time.Duration Cache Cache EnableDisk bool - ClientCertificates []tls.Certificate + HTTPClientCfg HTTPClientCfg +} + +type HTTPClientCfg struct { + CAFiles []string + Certs []tls.Certificate } // Log groups settings related to configuring logging @@ -428,7 +433,7 @@ func loadConfig() (*Config, error) { return nil, err } - config.GitLab.ClientCertificates = certs + config.GitLab.HTTPClientCfg = HTTPClientCfg{Certs: certs, CAFiles: caCerts.Split()} customHeaders, err := parseHeaderString(header.Split()) if err != nil { diff --git a/internal/config/flags.go b/internal/config/flags.go index 5e1ce28ac..3068c602f 100644 --- a/internal/config/flags.go +++ b/internal/config/flags.go @@ -108,6 +108,7 @@ var ( header = MultiStringFlag{separator: ";;"} clientCerts = MultiStringFlag{separator: ","} + caCerts = MultiStringFlag{separator: ","} namespaceInPath = flag.Bool("namespace-in-path", false, "Enable Namespace in path") @@ -129,6 +130,7 @@ func initFlags() { flag.Var(&header, "header", "The additional http header(s) that should be send to the client") flag.Var(&tlsClientAuthDomains, "tls-client-auth-domains", "The domain(s) that require client certificate authentication") flag.Var(&clientCerts, "client-cert-key-pairs", "File paths to client certificate and key PEM files to use for mutual TLS") + flag.Var(&caCerts, "ca-certs", "File paths to CA certificates used to sign client certificates for mutual TLS") // read from -config=/path/to/gitlab-pages-config flag.String(flag.DefaultConfigFlagname, "", "path to config file") diff --git a/internal/httptransport/transport.go b/internal/httptransport/transport.go index c726e77f2..dd117fb7d 100644 --- a/internal/httptransport/transport.go +++ b/internal/httptransport/transport.go @@ -5,10 +5,13 @@ import ( "crypto/x509" "net" "net/http" + "os" "sync" "time" "gitlab.com/gitlab-org/labkit/log" + + "gitlab.com/gitlab-org/gitlab-pages/internal/config" ) const ( @@ -54,6 +57,45 @@ func NewTransport() *http.Transport { } } +// NewTransportWithClientCert creates a new http.Transport with the provided client certificate configuration. +// It sets up the TLS configuration with the provided CA files and client certificates. +// The transport is configured with default connection values such as timeouts and max idle connections. +func NewTransportWithClientCert(hcc config.HTTPClientCfg) *http.Transport { + certPool := pool() + + for _, caFile := range hcc.CAFiles { + cert, err := os.ReadFile(caFile) + if err == nil { + certPool.AppendCertsFromPEM(cert) + } + } + + tlsConfig := &tls.Config{ + RootCAs: certPool, + MinVersion: tls.VersionTLS12, // set MinVersion to fix gosec: G402 + } + + if hcc.Certs != nil { + tlsConfig.Certificates = hcc.Certs + } + + return &http.Transport{ + DialTLS: func(network, addr string) (net.Conn, error) { + return tls.Dial(network, addr, &tls.Config{RootCAs: certPool, MinVersion: tls.VersionTLS12}) + }, + TLSClientConfig: tlsConfig, + Proxy: http.ProxyFromEnvironment, + // overrides the DefaultMaxIdleConnsPerHost = 2 + MaxIdleConns: 100, + MaxIdleConnsPerHost: 100, + IdleConnTimeout: 90 * time.Second, + // Set more timeouts https://gitlab.com/gitlab-org/gitlab-pages/-/issues/495 + TLSHandshakeTimeout: 10 * time.Second, + ResponseHeaderTimeout: 15 * time.Second, + ExpectContinueTimeout: 15 * time.Second, + } +} + // This is here because macOS does not support the SSL_CERT_FILE and // SSL_CERT_DIR environment variables. We have arranged things to read // SSL_CERT_FILE and SSL_CERT_DIR as late as possible to avoid conflicts diff --git a/internal/source/gitlab/client/client.go b/internal/source/gitlab/client/client.go index 79fd9738f..2fa22830b 100644 --- a/internal/source/gitlab/client/client.go +++ b/internal/source/gitlab/client/client.go @@ -2,7 +2,6 @@ package client import ( "context" - "crypto/tls" "errors" "fmt" "io" @@ -45,7 +44,7 @@ type Client struct { // NewClient initializes and returns new Client baseUrl is // appConfig.InternalGitLabServer secretKey is appConfig.GitLabAPISecretKey -func NewClient(baseURL string, secretKey []byte, connectionTimeout, jwtTokenExpiry time.Duration, clientCerts []tls.Certificate) (*Client, error) { +func NewClient(baseURL string, secretKey []byte, connectionTimeout, jwtTokenExpiry time.Duration, httpClientCfg config.HTTPClientCfg) (*Client, error) { if len(baseURL) == 0 || len(secretKey) == 0 { return nil, errors.New("GitLab API URL or API secret has not been provided") } @@ -63,11 +62,7 @@ func NewClient(baseURL string, secretKey []byte, connectionTimeout, jwtTokenExpi return nil, errors.New("GitLab JWT token expiry has not been provided") } - httpTransport := httptransport.DefaultTransport.Clone() - - if len(clientCerts) != 0 { - httpTransport.TLSClientConfig.Certificates = clientCerts - } + httpTransport := httptransport.NewTransportWithClientCert(httpClientCfg) return &Client{ secretKey: secretKey, @@ -92,7 +87,7 @@ func NewClient(baseURL string, secretKey []byte, connectionTimeout, jwtTokenExpi // NewFromConfig creates a new client from Config struct func NewFromConfig(cfg *config.GitLab) (*Client, error) { - return NewClient(cfg.InternalServer, cfg.APISecretKey, cfg.ClientHTTPTimeout, cfg.JWTTokenExpiration, cfg.ClientCertificates) + return NewClient(cfg.InternalServer, cfg.APISecretKey, cfg.ClientHTTPTimeout, cfg.JWTTokenExpiration, cfg.HTTPClientCfg) } // Resolve returns a VirtualDomain configuration wrapped into a Lookup for a diff --git a/internal/source/gitlab/client/client_test.go b/internal/source/gitlab/client/client_test.go index 8a579f5cc..31f5b7af0 100644 --- a/internal/source/gitlab/client/client_test.go +++ b/internal/source/gitlab/client/client_test.go @@ -15,6 +15,7 @@ import ( "github.com/golang-jwt/jwt/v5" "github.com/stretchr/testify/require" + "gitlab.com/gitlab-org/gitlab-pages/internal/config" "gitlab.com/gitlab-org/gitlab-pages/internal/domain" "gitlab.com/gitlab-org/gitlab-pages/internal/fixture" ) @@ -60,7 +61,7 @@ func TestConnectionReuse(t *testing.T) { } func TestNewValidBaseURL(t *testing.T) { - _, err := NewClient("https://gitlab.com", secretKey(t), defaultClientConnTimeout, defaultJWTTokenExpiry, nil) + _, err := NewClient("https://gitlab.com", secretKey(t), defaultClientConnTimeout, defaultJWTTokenExpiry, config.HTTPClientCfg{}) require.NoError(t, err) } @@ -130,7 +131,7 @@ func TestNewInvalidConfiguration(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := NewClient(tt.args.baseURL, tt.args.secretKey, tt.args.connectionTimeout, tt.args.jwtTokenExpiry, nil) + got, err := NewClient(tt.args.baseURL, tt.args.secretKey, tt.args.connectionTimeout, tt.args.jwtTokenExpiry, config.HTTPClientCfg{}) require.Nil(t, got) require.Error(t, err) require.Contains(t, err.Error(), tt.wantErrMsg) @@ -252,7 +253,9 @@ func TestMutualTLSClientAuthentication(t *testing.T) { clientCert, err := tls.LoadX509KeyPair("testdata/certs/client.crt", "testdata/certs/client.key") require.NoError(t, err) - client, err := NewClient(server.URL, secretKey(t), defaultClientConnTimeout, defaultJWTTokenExpiry, []tls.Certificate{clientCert}) + hcc := config.HTTPClientCfg{Certs: []tls.Certificate{clientCert}, CAFiles: []string{"testdata/certs/client.crt"}} + + client, err := NewClient(server.URL, secretKey(t), defaultClientConnTimeout, defaultJWTTokenExpiry, hcc) require.NoError(t, err) params := url.Values{} @@ -295,7 +298,7 @@ func secretKey(t *testing.T) []byte { func defaultClient(t *testing.T, url string) *Client { t.Helper() - client, err := NewClient(url, secretKey(t), defaultClientConnTimeout, defaultJWTTokenExpiry, nil) + client, err := NewClient(url, secretKey(t), defaultClientConnTimeout, defaultJWTTokenExpiry, config.HTTPClientCfg{}) require.NoError(t, err) return client @@ -356,7 +359,7 @@ func Test_endpoint(t *testing.T) { for name, tt := range tests { t.Run(name, func(t *testing.T) { - gc, err := NewClient(tt.basePath, []byte("secret"), defaultClientConnTimeout, defaultJWTTokenExpiry, nil) + gc, err := NewClient(tt.basePath, []byte("secret"), defaultClientConnTimeout, defaultJWTTokenExpiry, config.HTTPClientCfg{}) require.NoError(t, err) got, err := gc.endpoint(tt.urlPath, tt.params) -- GitLab From 2e5bd628984eced3ea16a0c919b3d025dfabeff1 Mon Sep 17 00:00:00 2001 From: ngala Date: Sun, 28 Apr 2024 15:19:41 +0530 Subject: [PATCH 04/11] Fixed TLSConfig for HTTPTransport --- internal/httptransport/transport.go | 2 +- internal/source/gitlab/client/client_test.go | 33 +++++++++----- .../gitlab/client/testdata/certs/ca.crt | 22 +++++++++ .../gitlab/client/testdata/certs/client.crt | 44 +++++++++--------- .../gitlab/client/testdata/certs/client.key | 33 +++++++++++--- internal/testhelpers/testhelpers.go | 45 +++++++++++++++++++ 6 files changed, 140 insertions(+), 39 deletions(-) create mode 100644 internal/source/gitlab/client/testdata/certs/ca.crt diff --git a/internal/httptransport/transport.go b/internal/httptransport/transport.go index dd117fb7d..3b60c7946 100644 --- a/internal/httptransport/transport.go +++ b/internal/httptransport/transport.go @@ -81,7 +81,7 @@ func NewTransportWithClientCert(hcc config.HTTPClientCfg) *http.Transport { return &http.Transport{ DialTLS: func(network, addr string) (net.Conn, error) { - return tls.Dial(network, addr, &tls.Config{RootCAs: certPool, MinVersion: tls.VersionTLS12}) + return tls.Dial(network, addr, tlsConfig) }, TLSClientConfig: tlsConfig, Proxy: http.ProxyFromEnvironment, diff --git a/internal/source/gitlab/client/client_test.go b/internal/source/gitlab/client/client_test.go index 31f5b7af0..1e050d581 100644 --- a/internal/source/gitlab/client/client_test.go +++ b/internal/source/gitlab/client/client_test.go @@ -18,6 +18,7 @@ import ( "gitlab.com/gitlab-org/gitlab-pages/internal/config" "gitlab.com/gitlab-org/gitlab-pages/internal/domain" "gitlab.com/gitlab-org/gitlab-pages/internal/fixture" + "gitlab.com/gitlab-org/gitlab-pages/internal/testhelpers" ) const ( @@ -236,10 +237,9 @@ func TestGetVirtualDomainAuthenticatedRequest(t *testing.T) { } func TestMutualTLSClientAuthentication(t *testing.T) { - mux := http.NewServeMux() - - mux.HandleFunc("/api/v4/internal/pages", func(w http.ResponseWriter, r *http.Request) { - // Ensure that client certificate is present + gitlabMux := http.NewServeMux() + gitlabMux.HandleFunc("/api/v4/internal/pages", func(w http.ResponseWriter, r *http.Request) { + // Ensure that pagesClient certificate is present if r.TLS == nil || len(r.TLS.PeerCertificates) == 0 { w.WriteHeader(http.StatusForbidden) return @@ -247,21 +247,32 @@ func TestMutualTLSClientAuthentication(t *testing.T) { w.WriteHeader(http.StatusOK) }) - server := httptest.NewTLSServer(mux) - defer server.Close() - clientCert, err := tls.LoadX509KeyPair("testdata/certs/client.crt", "testdata/certs/client.key") - require.NoError(t, err) + certPath := "testdata/certs/client.crt" + keyPath := "testdata/certs/client.key" + caCertPath := "testdata/certs/ca.crt" + + tlsCfg := &tls.Config{ + ClientCAs: testhelpers.CertPool(t, caCertPath), + ClientAuth: tls.RequireAndVerifyClientCert, + Certificates: []tls.Certificate{testhelpers.Cert(t, certPath, keyPath)}, + MinVersion: tls.VersionTLS12, + } + + server := httptest.NewUnstartedServer(gitlabMux) + server.TLS = tlsCfg + server.StartTLS() + defer server.Close() - hcc := config.HTTPClientCfg{Certs: []tls.Certificate{clientCert}, CAFiles: []string{"testdata/certs/client.crt"}} + hcc := config.HTTPClientCfg{Certs: []tls.Certificate{testhelpers.Cert(t, certPath, keyPath)}, CAFiles: []string{caCertPath}} - client, err := NewClient(server.URL, secretKey(t), defaultClientConnTimeout, defaultJWTTokenExpiry, hcc) + pagesClient, err := NewClient(server.URL, secretKey(t), defaultClientConnTimeout, defaultJWTTokenExpiry, hcc) require.NoError(t, err) params := url.Values{} params.Set("host", "group.gitlab.io") - resp, err := client.get(context.Background(), "/api/v4/internal/pages", params) + resp, err := pagesClient.get(context.Background(), "/api/v4/internal/pages", params) require.NoError(t, err) defer resp.Body.Close() diff --git a/internal/source/gitlab/client/testdata/certs/ca.crt b/internal/source/gitlab/client/testdata/certs/ca.crt new file mode 100644 index 000000000..3d619ef2d --- /dev/null +++ b/internal/source/gitlab/client/testdata/certs/ca.crt @@ -0,0 +1,22 @@ +-----BEGIN CERTIFICATE----- +MIIDjzCCAnegAwIBAgIUNiXaQMFLwUd4LGjXhfK5JFpI+DcwDQYJKoZIhvcNAQEL +BQAwVzELMAkGA1UEBhMCQ04xCzAJBgNVBAgMAkdEMQswCQYDVQQHDAJTWjEVMBMG +A1UECgwMR2l0TGFiLCBJbmMuMRcwFQYDVQQDDA5HaXRMYWIgUm9vdCBDQTAeFw0y +NDA0MjgwNTExMzJaFw0yNTA0MjgwNTExMzJaMFcxCzAJBgNVBAYTAkNOMQswCQYD +VQQIDAJHRDELMAkGA1UEBwwCU1oxFTATBgNVBAoMDEdpdExhYiwgSW5jLjEXMBUG +A1UEAwwOR2l0TGFiIFJvb3QgQ0EwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEK +AoIBAQCNERChbGl/+Phb6psr95gP/uFGyfkTt8nWuL0AlGpx7yyyGC9Pzm1FskpL +iNQB7xmtj/SN6k8EZddXBZcFrRgAiCPa4OO125QveDuBJ+2s9+P+WWhZXudyGnWO +lT5m/doM4HeAZ3ioePsGcjGPC4S+K9bmCsBOlpR/xp/Yc5V7upl6X3E8rK/PpfYn +Kjue+UX8TTseQ6OJSLt+FxwnHPMkcSD7tpuWjr+gAmYaloMvnsHP1pKW1MKrj7XU +FhoWyX4WjCR+zuDgBeBtxgaX8eLFKNwtbbaltc0XStS8qGoRJcV6wsmS7CG3UtFx +gjYBJorvzgmLZOWcc+OMete0rFFrAgMBAAGjUzBRMB0GA1UdDgQWBBRSnkD/LMde +F3Fez1dTGaqURUl61TAfBgNVHSMEGDAWgBRSnkD/LMdeF3Fez1dTGaqURUl61TAP +BgNVHRMBAf8EBTADAQH/MA0GCSqGSIb3DQEBCwUAA4IBAQBzA0yEL69ifRqD2QEv +s5r5jtKQtH04h+x4DEWwWkxrDH9NUPu8+vDWrEMARCvICOkS42CPqhGSRkEka7Ij +vixAo6gzh0mRkxR90Ztjf9fQ4hrKDl7NlcZ0aeeIStExJd5pf4JCv9uyM8p27flP +2+ZHXWx+PfKaRPA4C+WrcYMTTbEGRWJVKFl3rTCkIZ2sWGV8Gt+wzxYWutjB91RI +m3rnC0xvrBBo6AMlFmAcqOKs+pzThIECJhM5zwQ6VBfx9xgIF20JAUS78176NpiY +JWxUEjkdR4ofaBbYTSdef4M8vJI6nm2fCWucCXwB1qeOyU4xki/QU+Cfqqwuqvyt +WVib +-----END CERTIFICATE----- \ No newline at end of file diff --git a/internal/source/gitlab/client/testdata/certs/client.crt b/internal/source/gitlab/client/testdata/certs/client.crt index 069624ff2..3f545b4aa 100644 --- a/internal/source/gitlab/client/testdata/certs/client.crt +++ b/internal/source/gitlab/client/testdata/certs/client.crt @@ -1,23 +1,23 @@ -----BEGIN CERTIFICATE----- -MIIDxDCCAawCFByFBHU3RaAIgFvtrNvhHMhgqGCUMA0GCSqGSIb3DQEBCwUAMHIx -CzAJBgNVBAYTAlVTMQ0wCwYDVQQIDARUZXN0MRUwEwYDVQQHDAxEZWZhdWx0IENp -dHkxHDAaBgNVBAoME0RlZmF1bHQgQ29tcGFueSBMdGQxDTALBgNVBAsMBFRlc3Qx -EDAOBgNVBAMMB1Rlc3QgQ0EwHhcNMjMwOTIwMTEzNDM3WhcNMzMwOTE3MTEzNDM3 -WjCBlTELMAkGA1UEBhMCVVMxDTALBgNVBAgMBFRlc3QxFTATBgNVBAcMDERlZmF1 -bHQgQ2l0eTEcMBoGA1UECgwTRGVmYXVsdCBDb21wYW55IEx0ZDENMAsGA1UECwwE -VGVzdDESMBAGA1UEAwwJVGVzdCBVc2VyMR8wHQYJKoZIhvcNAQkBFhB0ZXN0QGV4 -YW1wbGUub3JnMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAE5S6e7bNszpyhc279 -oRgW8TMytl0IGaPIhMoDF4oUXO+/sOK/cCz59II7nkwkk6COrvbh70ft8e9cdsqt -a5UsvzANBgkqhkiG9w0BAQsFAAOCAgEAFGDKm6825SRYPTi7HG6uK1S1CXbPNnSy -hv9umTa1iRLqip9eacNx6uijdRQr2IOOBZfuRAjut9IYlH/PfrNYPVcc/uYH/gIP -LZdVjMxqpNxqQYUup3Wq8mVu8Gnfbm2ymvCPHnUhM4xH++grmUQW5bq7BozL/oea -AfflJeS3AGmqPBNBDvjubn7NCirzKEkrt7COAXnpXmFmiErT9gWkeE1Xtn51/W3b -27lu+5aBzDUtlppa5eT1GPc6fu5+HLtHod+nDn/7Ys/HUWw/5iA6DRhGXnLlz2W7 -C+m1yDtSVH+axhAXbhnzvZUpB6jXLLce99a5ff84fp/nJ96hR5lHekIkfNj0wtrQ -WqvQ9HuU1Q3dbWOpC9xtOvQBINAnsZGrM8XwLzG39WdK2G9mLNQqHqKedCWW9UhE -lsOZrYihuXOmTBCIW+SGDw2z4unsFdNMXXwJVv8REDJzGmq5JXai40nnnemS7JNQ -ztgO1gPnznZQFsbHgvSSJOoi5sz/o2VeJJ+zYzPSc7Hzbm/MPlcOdXo2uLO6JrIN -3Kk9Gdv/6tQ+gVUq3qNS/1cmiVTyNiiSOpLM1qLPVL/n5HFYPTbI3OQxRomkMb5j -ab5Oj4+trDQQ1ysm6i8UEqQoCr/izj+2muIGp7u94UrcRmJkULwpjkRsQD/uX5yd -hNo/s+Sx0hA= ------END CERTIFICATE----- +MIIDwzCCAqugAwIBAgIUJiLmI0GYBnuegJ+klIYNo0xwWFgwDQYJKoZIhvcNAQEL +BQAwVzELMAkGA1UEBhMCQ04xCzAJBgNVBAgMAkdEMQswCQYDVQQHDAJTWjEVMBMG +A1UECgwMR2l0TGFiLCBJbmMuMRcwFQYDVQQDDA5HaXRMYWIgUm9vdCBDQTAgFw0y +NDA0MjgwNTE0MTVaGA8yMDUxMDkxMzA1MTQxNVowgYgxCzAJBgNVBAYTAlVTMQsw +CQYDVQQIDAJDQTELMAkGA1UEBwwCU0YxEzARBgNVBAoMCkdpdExhYiBJbmMxFTAT +BgNVBAsMDEdpdExhYiBQYWdlczESMBAGA1UEAwwJMTI3LjAuMC4xMR8wHQYJKoZI +hvcNAQkBFhBuZ2FsYUBnaXRsYWIuY29tMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8A +MIIBCgKCAQEAvrRzIVkbZn3N30EAx6NLXbYX8mX49KbHKLQv2OFGElh21n7FHcSb +Eu3jc6U4HwFgtdeALtg6ooqtxWH1mI2BvkhzibAlqm1pJ/2U2YM84K916MxvmQHS +zw3kOlN1jA941vq2dYKiyNGYlOoxMxHXOSorB05eMtjAuj+Wl9B8t2ixW+IaS01O +Tw5jW6YM+XkwduN6cGDxXL4G/WO+XRfskKCNmFQCL5g0Bd3o7O1if8L1071VVcvF +teZXvCmbQrHoT+mRgvleONfE7KkllJ04HYZtNwPcVPepbxdGgi5ZKpKOYRmHyIlG +j915Q3Q4lYjCT9Osynv0lSbanswgcEdaLQIDAQABo1MwUTAPBgNVHREECDAGhwR/ +AAABMB0GA1UdDgQWBBRQLcraJY4zJvSvlfxnVihpnbyZJjAfBgNVHSMEGDAWgBRS +nkD/LMdeF3Fez1dTGaqURUl61TANBgkqhkiG9w0BAQsFAAOCAQEAL3qgjOxhuuKw +HEOJqDqRpTWxWIU2NjFnqegg6V+sU1KCgEdgSHtWZ8jIhVn201cZOsgfF0nUvYST +LUNsWYLga6l7VRUZqFlC8pkuV7I7QMmddBKGDelhsJbcLaAAbXhiQ36/UYF9kWeJ +Wff42L5GFJjQ1fCuVEYHpFFrUW6HEqLiDjPXHJS6kWKV/SSnAy9X0Uc0lurdk8Nd +uX/qkisH/lCqHAV/KQLuWbF7+DDf3YUPZjMOqagC6sUpV/geENMi/jcbTE7vpOuO +qQQU7VuKZJFdqfOoNHvnQiWIbAThVzXyh2nY0El/so1+5IDdVDdaacn3dw/VeHpN +r8jTtrEuYg== +-----END CERTIFICATE----- \ No newline at end of file diff --git a/internal/source/gitlab/client/testdata/certs/client.key b/internal/source/gitlab/client/testdata/certs/client.key index 17a6bff88..5fea77861 100644 --- a/internal/source/gitlab/client/testdata/certs/client.key +++ b/internal/source/gitlab/client/testdata/certs/client.key @@ -1,5 +1,28 @@ ------BEGIN EC PRIVATE KEY----- -MHcCAQEEIFWzobEUkcIgOUR81lz2KSmoPKyNI5UraogKlWOESRbHoAoGCCqGSM49 -AwEHoUQDQgAE5S6e7bNszpyhc279oRgW8TMytl0IGaPIhMoDF4oUXO+/sOK/cCz5 -9II7nkwkk6COrvbh70ft8e9cdsqta5Usvw== ------END EC PRIVATE KEY----- +-----BEGIN PRIVATE KEY----- +MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQC+tHMhWRtmfc3f +QQDHo0tdthfyZfj0pscotC/Y4UYSWHbWfsUdxJsS7eNzpTgfAWC114Au2Dqiiq3F +YfWYjYG+SHOJsCWqbWkn/ZTZgzzgr3XozG+ZAdLPDeQ6U3WMD3jW+rZ1gqLI0ZiU +6jEzEdc5KisHTl4y2MC6P5aX0Hy3aLFb4hpLTU5PDmNbpgz5eTB243pwYPFcvgb9 +Y75dF+yQoI2YVAIvmDQF3ejs7WJ/wvXTvVVVy8W15le8KZtCsehP6ZGC+V4418Ts +qSWUnTgdhm03A9xU96lvF0aCLlkqko5hGYfIiUaP3XlDdDiViMJP06zKe/SVJtqe +zCBwR1otAgMBAAECggEABVTVy860wY/uePmAFD9gRILqNs3GAM+cIsBYKW0xgHgg +/mHLO0f7ToirAq58Rqr1sVplzCl3CNiX/2bRLw4fB0WkQUBU6pJQzcbnlJB4MQ8x +XIGWnxrvcfgWniYmKPUScO7PugedwGp8+bDLEP1EhzsYD9tk2RwOgJvK0dIIzTcf +agvHIE33LSDBPy6tcoagxv8Z/M/jXVMdPsgN1xKWKoUSEq6X24IpriLXBQSkoX83 +ecp8hZyeIXuc6SNb8d7eEXCcg/JK6ZRLyRXGHaalCYdy594VrA8qeuZ1gtUwPBgq +zU4JyRY4K7IGxgo13cE0W+6Fvg/5Al6kdRxHrB3QsQKBgQD+yVQcPozRVL48f1g/ +EpTTJyEi/9piKcvWWQc+BrrEOGDrtnv+eJ/c4YuzrcFfwQhN18VIuWl0xbUBnKii +3DkOu9LOT4fozSeTDOG880mFgSDEjFQu+KbpxQh81HOG0GzUM8mLq4Gcr30WIMMM +xIVg3iHsxPHxHIkHI1WQo97eiQKBgQC/nPvist9YzJYCNvkQ6xi4F6E6mS4Ey2+V +a/yn9H8fwpHZA93yg6ovL20tL5Hfd9cAPjQ6JcjLRuefd5AYe1E/EbkzCv8LG4zL +GXoovVofPxAEMG2O7U2TeoNauCJ9CJurfIkko7lziQVKawx4sYq2QvkIP2zY+FnN +1atQqDiVhQKBgGERQsIf8nYt2uwhd/VPlvN7DNzQrNqJIedfs6ql1bG76PDkbQjd +28nDA/5ITEu2tvsxITA7szmRuQwMKxMg43wBgqanFhhTUKhtV/MsnO4H6/v1mnzq +rmyRbFJifkD2Vv/hWv+jL5YKJZWwlZ7foBDvj+0seyBoxqu5gnfAdsBBAoGAQVh6 +Fk/GF3R92/d/bSOf5Hg6hc9jgEMYpK6VFXouOFiUgJvu/xuj2D+mTfihGMK30d9k +1Ee6eIiPyTRvMcosZQPYUu33GISmuUTRAj/BElLhVWxmkI2hHSB012VgbZ+X5x2r +b5FeV2ZtJXnoYOi7U3j3kLaAmmXnymiJ6hHUajkCgYEApQb7xeBAMpwqc58VZ1s3 +aXlZJja3LPohcvNZsSJ+CupfOgSJAKGa83+jdbmHQaCTUmlLyS8/JiJ6x5RBa1t3 +fHqAtDfYXVU7V/HYJ1+dOOZTVdSK1xJTINiE6n8YXRtRKIJhxuA4Vo/hyh28WX/E +C613sVOjicenNabI1Qjo7aY= +-----END PRIVATE KEY----- \ No newline at end of file diff --git a/internal/testhelpers/testhelpers.go b/internal/testhelpers/testhelpers.go index 1bb537cdd..481a949c3 100644 --- a/internal/testhelpers/testhelpers.go +++ b/internal/testhelpers/testhelpers.go @@ -1,6 +1,8 @@ package testhelpers import ( + "crypto/tls" + "crypto/x509" "fmt" "io" "net/http" @@ -110,3 +112,46 @@ func Close(t *testing.T, c io.Closer) { require.NoError(t, c.Close()) }) } + +// Certificate is a generated certificate. +type Certificate struct { + CertPath string + KeyPath string +} + +// CertPool creates a new certificate pool containing the certificate. +func CertPool(tb testing.TB, certPath string) *x509.CertPool { + tb.Helper() + pem := MustReadFile(tb, certPath) + pool := x509.NewCertPool() + require.True(tb, pool.AppendCertsFromPEM(pem)) + return pool +} + +// Cert returns the parsed certificate. +func Cert(tb testing.TB, certPath, keyPath string) tls.Certificate { + tb.Helper() + cert, err := tls.LoadX509KeyPair(certPath, keyPath) + require.NoError(tb, err) + return cert +} + +// MustReadFile returns the content of a file or fails at once. +func MustReadFile(tb testing.TB, filename string) []byte { + tb.Helper() + + content, err := os.ReadFile(filename) + if err != nil { + tb.Fatal(err) + } + + return content +} + +// MustClose calls Close() on the Closer and fails the test in case it returns +// an error. This function is useful when closing via `defer`, as a simple +// `defer require.NoError(t, closer.Close())` would cause `closer.Close()` to +// be executed early already. +func MustClose(tb testing.TB, closer io.Closer) { + require.NoError(tb, closer.Close()) +} -- GitLab From 6fe9b96e813d90a07948e93b7cda842ed2d25db8 Mon Sep 17 00:00:00 2001 From: ngala Date: Mon, 29 Apr 2024 09:21:23 +0530 Subject: [PATCH 05/11] Fix gitlab_api_mutual_tls_test.go acceptance test --- internal/config/config.go | 1 + internal/httptransport/transport.go | 2 + internal/testhelpers/testhelpers.go | 14 ---- test/acceptance/gitlab_api_mutual_tls_test.go | 84 ++++++------------- test/acceptance/helpers_test.go | 39 +++++++++ .../testdata/mutualtls/invalid/ca.crt | 33 ++++++++ .../testdata/mutualtls/invalid/client.crt | 23 +++++ .../testdata/mutualtls/invalid/client.key | 5 ++ .../testdata/mutualtls/valid/ca.crt | 22 +++++ .../testdata/mutualtls/valid/client.crt | 23 +++++ .../testdata/mutualtls/valid/client.key | 28 +++++++ test/gitlabstub/option.go | 1 - 12 files changed, 203 insertions(+), 72 deletions(-) create mode 100644 test/acceptance/testdata/mutualtls/invalid/ca.crt create mode 100644 test/acceptance/testdata/mutualtls/invalid/client.crt create mode 100644 test/acceptance/testdata/mutualtls/invalid/client.key create mode 100644 test/acceptance/testdata/mutualtls/valid/ca.crt create mode 100644 test/acceptance/testdata/mutualtls/valid/client.crt create mode 100644 test/acceptance/testdata/mutualtls/valid/client.key diff --git a/internal/config/config.go b/internal/config/config.go index d1e480be1..32ae71116 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -559,6 +559,7 @@ func logFields(config *Config) map[string]any { "sentry-environment": config.Sentry.Environment, "version": config.General.ShowVersion, "namespace-in-path": *namespaceInPath, + "ca-certs": config.GitLab.HTTPClientCfg.CAFiles, } } diff --git a/internal/httptransport/transport.go b/internal/httptransport/transport.go index 3b60c7946..e93bfb326 100644 --- a/internal/httptransport/transport.go +++ b/internal/httptransport/transport.go @@ -64,6 +64,7 @@ func NewTransportWithClientCert(hcc config.HTTPClientCfg) *http.Transport { certPool := pool() for _, caFile := range hcc.CAFiles { + log.Info("Reading CA file: ", caFile) cert, err := os.ReadFile(caFile) if err == nil { certPool.AppendCertsFromPEM(cert) @@ -76,6 +77,7 @@ func NewTransportWithClientCert(hcc config.HTTPClientCfg) *http.Transport { } if hcc.Certs != nil { + log.Info("Setting mutual TLS certificates to access GitLab API") tlsConfig.Certificates = hcc.Certs } diff --git a/internal/testhelpers/testhelpers.go b/internal/testhelpers/testhelpers.go index 481a949c3..f6d5f062c 100644 --- a/internal/testhelpers/testhelpers.go +++ b/internal/testhelpers/testhelpers.go @@ -113,12 +113,6 @@ func Close(t *testing.T, c io.Closer) { }) } -// Certificate is a generated certificate. -type Certificate struct { - CertPath string - KeyPath string -} - // CertPool creates a new certificate pool containing the certificate. func CertPool(tb testing.TB, certPath string) *x509.CertPool { tb.Helper() @@ -147,11 +141,3 @@ func MustReadFile(tb testing.TB, filename string) []byte { return content } - -// MustClose calls Close() on the Closer and fails the test in case it returns -// an error. This function is useful when closing via `defer`, as a simple -// `defer require.NoError(t, closer.Close())` would cause `closer.Close()` to -// be executed early already. -func MustClose(tb testing.TB, closer io.Closer) { - require.NoError(tb, closer.Close()) -} diff --git a/test/acceptance/gitlab_api_mutual_tls_test.go b/test/acceptance/gitlab_api_mutual_tls_test.go index 5e4059d78..2d7810220 100644 --- a/test/acceptance/gitlab_api_mutual_tls_test.go +++ b/test/acceptance/gitlab_api_mutual_tls_test.go @@ -1,60 +1,50 @@ package acceptance_test import ( - "crypto/x509" _ "embed" - "encoding/pem" - "fmt" "net/http" - "os" "testing" "github.com/stretchr/testify/require" - - "gitlab.com/gitlab-org/gitlab-pages/test/gitlabstub" -) - -const ( - clientCert = "../../test/acceptance/testdata/client.crt" - clientKey = "../../test/acceptance/testdata/client.key" ) func TestGitLabAPIMutualTLS(t *testing.T) { tests := []struct { - name string - host string - path string - status int - expectError bool + name string + host string + path string + clientCertPath string + clientKeyPath string + caCertPath string + status int + expectError bool }{ { - name: "basic request works with GitLab API mutual TLS", - host: "group.gitlab-example.com", - path: "/-/private/-/jobs/1/artifacts/200.html", - status: http.StatusOK, - expectError: false, + name: "basic request works with GitLab API mutual TLS", + host: "group.gitlab-example.com", + path: "/index.html", + clientCertPath: "../../test/acceptance/testdata/mutualtls/valid/client.crt", + clientKeyPath: "../../test/acceptance/testdata/mutualtls/valid/client.key", + caCertPath: "../../test/acceptance/testdata/mutualtls/valid/ca.crt", + status: http.StatusOK, + expectError: false, + }, + { + name: "502 when invalid mutual TLS is used", + host: "group.gitlab-example.com", + path: "/index.html", + clientCertPath: "../../test/acceptance/testdata/mutualtls/invalid/client.crt", + clientKeyPath: "../../test/acceptance/testdata/mutualtls/invalid/client.key", + caCertPath: "../../test/acceptance/testdata/mutualtls/invalid/ca.crt", + status: http.StatusBadGateway, + expectError: false, }, } - caCert, err := loadCACertificate(clientCert) - require.NoError(t, err) - - configFile := defaultConfigFileWith(t, "client-cert-key-pairs"+"="+clientCert+":"+clientKey) - - RunPagesProcess(t, - withListeners([]ListenSpec{httpsListener}), - withArguments([]string{ - "-config=" + configFile, - }), - withPublicServer, - withExtraArgument("auth-redirect-uri", "https://projects.gitlab-example.com/auth"), - withExtraArgument("artifacts-server-timeout", "1"), - withStubOptions(gitlabstub.WithMutualTLS(caCert)), - ) - for _, tt := range tests { tt := tt t.Run(tt.name, func(t *testing.T) { + RunPagesProcessWithMutualTLS(t, httpsListener, tt.clientCertPath, tt.clientKeyPath, tt.caCertPath) rsp, err := GetPageFromListener(t, httpsListener, tt.host, tt.path) require.NoError(t, err) @@ -68,23 +58,3 @@ func TestGitLabAPIMutualTLS(t *testing.T) { }) } } - -func loadCACertificate(certPath string) (*x509.Certificate, error) { - if certPath == "" { - return nil, nil - } - - caCertFile, err := os.ReadFile(certPath) - if err != nil { - return nil, fmt.Errorf("error reading CA file: %w", err) - } - - block, _ := pem.Decode(caCertFile) - - caCert, err := x509.ParseCertificate(block.Bytes) - if err != nil { - return nil, fmt.Errorf("error parsing CA certificate: %w", err) - } - - return caCert, nil -} diff --git a/test/acceptance/helpers_test.go b/test/acceptance/helpers_test.go index d089c0913..8bd227af6 100644 --- a/test/acceptance/helpers_test.go +++ b/test/acceptance/helpers_test.go @@ -5,6 +5,7 @@ import ( "context" "crypto/tls" "crypto/x509" + "encoding/pem" "fmt" "io" "net" @@ -334,6 +335,44 @@ func RunPagesProcessWithSSLCertDir(t *testing.T, listeners []ListenSpec, sslCert ) } +func RunPagesProcessWithMutualTLS(t *testing.T, listener ListenSpec, clientCertPath, clientKeyPath, caCertPath string) { + clientCert, err := tls.LoadX509KeyPair(clientCertPath, clientKeyPath) + require.NoError(t, err) + + caCert, err := loadCACertificate(caCertPath) + require.NoError(t, err) + + configFile := newConfigFile(t, "client-cert-key-pairs="+clientCertPath+":"+clientKeyPath, "ca-certs="+caCertPath) + + RunPagesProcess(t, + withListeners([]ListenSpec{listener}), + withArguments([]string{ + "-config=" + configFile, + }), + withStubOptions(gitlabstub.WithCertificate(clientCert), gitlabstub.WithMutualTLS(caCert)), + ) +} + +func loadCACertificate(certPath string) (*x509.Certificate, error) { + if certPath == "" { + return nil, nil + } + + caCertFile, err := os.ReadFile(certPath) + if err != nil { + return nil, fmt.Errorf("error reading CA file: %w", err) + } + + block, _ := pem.Decode(caCertFile) + + caCert, err := x509.ParseCertificate(block.Bytes) + if err != nil { + return nil, fmt.Errorf("error parsing CA certificate: %w", err) + } + + return caCert, nil +} + func runPagesProcess(t *testing.T, wait bool, pagesBinary string, listeners []ListenSpec, promPort string, extraArgs ...string) (*LogCaptureBuffer, func()) { t.Helper() diff --git a/test/acceptance/testdata/mutualtls/invalid/ca.crt b/test/acceptance/testdata/mutualtls/invalid/ca.crt new file mode 100644 index 000000000..5ddf7b1e3 --- /dev/null +++ b/test/acceptance/testdata/mutualtls/invalid/ca.crt @@ -0,0 +1,33 @@ +-----BEGIN CERTIFICATE----- +MIIFxTCCA62gAwIBAgIURIchto1SmBcKMY+PSPzuXHzkZz0wDQYJKoZIhvcNAQEL +BQAwcjELMAkGA1UEBhMCVVMxDTALBgNVBAgMBFRlc3QxFTATBgNVBAcMDERlZmF1 +bHQgQ2l0eTEcMBoGA1UECgwTRGVmYXVsdCBDb21wYW55IEx0ZDENMAsGA1UECwwE +VGVzdDEQMA4GA1UEAwwHVGVzdCBDQTAeFw0yMzA5MjAxMTMyMDVaFw0zMzA5MTcx +MTMyMDVaMHIxCzAJBgNVBAYTAlVTMQ0wCwYDVQQIDARUZXN0MRUwEwYDVQQHDAxE +ZWZhdWx0IENpdHkxHDAaBgNVBAoME0RlZmF1bHQgQ29tcGFueSBMdGQxDTALBgNV +BAsMBFRlc3QxEDAOBgNVBAMMB1Rlc3QgQ0EwggIiMA0GCSqGSIb3DQEBAQUAA4IC +DwAwggIKAoICAQCbagpIeWGEj8JYjO1sH2bdZx7MZQ7rQc5qNCNw25d15tU/HavS +2PsUU3FhWaS7CN1VNQZqyJ+rkDRXPQWvaOq1Nd4jiRil34garMVWOoYT8xOVQVFp +Mbvp9H90O8MVPwMtOde+A4UklBVxX7uBiqHDe/Tky/fp1iWIuYwqhrHphTX26A9a +cAUM0ruR5MqsPRdmE/+vIBRwsPCV3oJikDfoaqOtOIUXiv4Mtvf1CWei9YarxW2P +R79GLDCRTHV36OXGQ6zXdCGflNcfmFWY3GXUP2uv6W7xICmWoiIqnlweV0Z2rxFq +acMa2/1IkxWbJ6CMqQX0exdBLc+M5JUUUE6OdQbz2X3z5J4VcXyzjcaa7Bh37Ulz +gvY/hvcRY0+X6H8rvuNiT4lgrgNFZY05mEEP/P55stklSpt6qQEPhFTMKHqH2NQS +dtgU9sA3kcHikih8c9kZ++M96GIZ0bM8kiMaKVszcY0Z5E1ff597egqZcOiJx1c5 +2lLUpTP+5Te+x56mi1lalB9uMsv37kg7Xgkw4nnp5z+jZqcAK5CT+JwdRz9KUCLR +WCX6NUBXT4ANgBkTywaAuPNW8Ph9hLoQg5lhtZ7pjdAzB7zIaub21MEHEMctCg68 +2HKGRigoDiwGYM9ihIpj9FaT6vGUnRUz9hG4t8MDicrbtEEDe3/PYf0VTwIDAQAB +o1MwUTAdBgNVHQ4EFgQU4l2t6V6DL4q3W3PCClYAC6BSH3swHwYDVR0jBBgwFoAU +4l2t6V6DL4q3W3PCClYAC6BSH3swDwYDVR0TAQH/BAUwAwEB/zANBgkqhkiG9w0B +AQsFAAOCAgEAAj8W2aDxpbHGmR80BK2zriEt0QPVzdh1e8svRHJiasRagkMovhe6 +miLaGInx/y3YXC151KKjEn6qA93aatma3beFVl9UTs5EDqRUy07d0d1KuGrwqwpn +apc/ghiaS9PwoufEnb8dAbbQr4aexpn8lybFwOfICPZF06GtBeAn6DtQ95XlhLin +oZsJKJsF2jmLuWU5qgzau/I3LbDg+cVRsoDBQIMDC9YqyMAlkwno2ktbx0nxfqmT +stYsXCZTwaJ6RM8JOTdo6SVM6czICiocgJI3iBgiSkw0alN6PZbaxPlwasmtw9fl +ly6O/yoLG4nRYQ2c8TDtX/Kn0iIufVhSann2gznhji3ZeisUkepwm5hq5aN2tCAz +AX8p/AFBtzcJJYR6AJtHdNG3QRdFOlaYDp5e26LL0qfDrl7ryP4juEGuCJjcckIS +Be6gS7pAhvIcRszOFA3kGI4QQob8sJP+9TYdoYDsUkm6/IY+zWUNTEyjaw8pw17Y +jsoP0oJfGYSgiDjul0ZHzUltEXjCfxp7AEP4D0/U+fVmEPD1yJP/tlX4wAHfwCcR +sw9xWMKpH2oRSWCULfCXqb5YiRYPVbJYQPdN4DXNC72+mVBrcOYE4DdbcEFjxklc +3eIIltvepQ76lpEX7bWHgzpAg3zvz+KL98CvMBPX5UNEagxFEY+0/vw= +-----END CERTIFICATE----- diff --git a/test/acceptance/testdata/mutualtls/invalid/client.crt b/test/acceptance/testdata/mutualtls/invalid/client.crt new file mode 100644 index 000000000..069624ff2 --- /dev/null +++ b/test/acceptance/testdata/mutualtls/invalid/client.crt @@ -0,0 +1,23 @@ +-----BEGIN CERTIFICATE----- +MIIDxDCCAawCFByFBHU3RaAIgFvtrNvhHMhgqGCUMA0GCSqGSIb3DQEBCwUAMHIx +CzAJBgNVBAYTAlVTMQ0wCwYDVQQIDARUZXN0MRUwEwYDVQQHDAxEZWZhdWx0IENp +dHkxHDAaBgNVBAoME0RlZmF1bHQgQ29tcGFueSBMdGQxDTALBgNVBAsMBFRlc3Qx +EDAOBgNVBAMMB1Rlc3QgQ0EwHhcNMjMwOTIwMTEzNDM3WhcNMzMwOTE3MTEzNDM3 +WjCBlTELMAkGA1UEBhMCVVMxDTALBgNVBAgMBFRlc3QxFTATBgNVBAcMDERlZmF1 +bHQgQ2l0eTEcMBoGA1UECgwTRGVmYXVsdCBDb21wYW55IEx0ZDENMAsGA1UECwwE +VGVzdDESMBAGA1UEAwwJVGVzdCBVc2VyMR8wHQYJKoZIhvcNAQkBFhB0ZXN0QGV4 +YW1wbGUub3JnMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAE5S6e7bNszpyhc279 +oRgW8TMytl0IGaPIhMoDF4oUXO+/sOK/cCz59II7nkwkk6COrvbh70ft8e9cdsqt +a5UsvzANBgkqhkiG9w0BAQsFAAOCAgEAFGDKm6825SRYPTi7HG6uK1S1CXbPNnSy +hv9umTa1iRLqip9eacNx6uijdRQr2IOOBZfuRAjut9IYlH/PfrNYPVcc/uYH/gIP +LZdVjMxqpNxqQYUup3Wq8mVu8Gnfbm2ymvCPHnUhM4xH++grmUQW5bq7BozL/oea +AfflJeS3AGmqPBNBDvjubn7NCirzKEkrt7COAXnpXmFmiErT9gWkeE1Xtn51/W3b +27lu+5aBzDUtlppa5eT1GPc6fu5+HLtHod+nDn/7Ys/HUWw/5iA6DRhGXnLlz2W7 +C+m1yDtSVH+axhAXbhnzvZUpB6jXLLce99a5ff84fp/nJ96hR5lHekIkfNj0wtrQ +WqvQ9HuU1Q3dbWOpC9xtOvQBINAnsZGrM8XwLzG39WdK2G9mLNQqHqKedCWW9UhE +lsOZrYihuXOmTBCIW+SGDw2z4unsFdNMXXwJVv8REDJzGmq5JXai40nnnemS7JNQ +ztgO1gPnznZQFsbHgvSSJOoi5sz/o2VeJJ+zYzPSc7Hzbm/MPlcOdXo2uLO6JrIN +3Kk9Gdv/6tQ+gVUq3qNS/1cmiVTyNiiSOpLM1qLPVL/n5HFYPTbI3OQxRomkMb5j +ab5Oj4+trDQQ1ysm6i8UEqQoCr/izj+2muIGp7u94UrcRmJkULwpjkRsQD/uX5yd +hNo/s+Sx0hA= +-----END CERTIFICATE----- diff --git a/test/acceptance/testdata/mutualtls/invalid/client.key b/test/acceptance/testdata/mutualtls/invalid/client.key new file mode 100644 index 000000000..17a6bff88 --- /dev/null +++ b/test/acceptance/testdata/mutualtls/invalid/client.key @@ -0,0 +1,5 @@ +-----BEGIN EC PRIVATE KEY----- +MHcCAQEEIFWzobEUkcIgOUR81lz2KSmoPKyNI5UraogKlWOESRbHoAoGCCqGSM49 +AwEHoUQDQgAE5S6e7bNszpyhc279oRgW8TMytl0IGaPIhMoDF4oUXO+/sOK/cCz5 +9II7nkwkk6COrvbh70ft8e9cdsqta5Usvw== +-----END EC PRIVATE KEY----- diff --git a/test/acceptance/testdata/mutualtls/valid/ca.crt b/test/acceptance/testdata/mutualtls/valid/ca.crt new file mode 100644 index 000000000..3d619ef2d --- /dev/null +++ b/test/acceptance/testdata/mutualtls/valid/ca.crt @@ -0,0 +1,22 @@ +-----BEGIN CERTIFICATE----- +MIIDjzCCAnegAwIBAgIUNiXaQMFLwUd4LGjXhfK5JFpI+DcwDQYJKoZIhvcNAQEL +BQAwVzELMAkGA1UEBhMCQ04xCzAJBgNVBAgMAkdEMQswCQYDVQQHDAJTWjEVMBMG +A1UECgwMR2l0TGFiLCBJbmMuMRcwFQYDVQQDDA5HaXRMYWIgUm9vdCBDQTAeFw0y +NDA0MjgwNTExMzJaFw0yNTA0MjgwNTExMzJaMFcxCzAJBgNVBAYTAkNOMQswCQYD +VQQIDAJHRDELMAkGA1UEBwwCU1oxFTATBgNVBAoMDEdpdExhYiwgSW5jLjEXMBUG +A1UEAwwOR2l0TGFiIFJvb3QgQ0EwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEK +AoIBAQCNERChbGl/+Phb6psr95gP/uFGyfkTt8nWuL0AlGpx7yyyGC9Pzm1FskpL +iNQB7xmtj/SN6k8EZddXBZcFrRgAiCPa4OO125QveDuBJ+2s9+P+WWhZXudyGnWO +lT5m/doM4HeAZ3ioePsGcjGPC4S+K9bmCsBOlpR/xp/Yc5V7upl6X3E8rK/PpfYn +Kjue+UX8TTseQ6OJSLt+FxwnHPMkcSD7tpuWjr+gAmYaloMvnsHP1pKW1MKrj7XU +FhoWyX4WjCR+zuDgBeBtxgaX8eLFKNwtbbaltc0XStS8qGoRJcV6wsmS7CG3UtFx +gjYBJorvzgmLZOWcc+OMete0rFFrAgMBAAGjUzBRMB0GA1UdDgQWBBRSnkD/LMde +F3Fez1dTGaqURUl61TAfBgNVHSMEGDAWgBRSnkD/LMdeF3Fez1dTGaqURUl61TAP +BgNVHRMBAf8EBTADAQH/MA0GCSqGSIb3DQEBCwUAA4IBAQBzA0yEL69ifRqD2QEv +s5r5jtKQtH04h+x4DEWwWkxrDH9NUPu8+vDWrEMARCvICOkS42CPqhGSRkEka7Ij +vixAo6gzh0mRkxR90Ztjf9fQ4hrKDl7NlcZ0aeeIStExJd5pf4JCv9uyM8p27flP +2+ZHXWx+PfKaRPA4C+WrcYMTTbEGRWJVKFl3rTCkIZ2sWGV8Gt+wzxYWutjB91RI +m3rnC0xvrBBo6AMlFmAcqOKs+pzThIECJhM5zwQ6VBfx9xgIF20JAUS78176NpiY +JWxUEjkdR4ofaBbYTSdef4M8vJI6nm2fCWucCXwB1qeOyU4xki/QU+Cfqqwuqvyt +WVib +-----END CERTIFICATE----- \ No newline at end of file diff --git a/test/acceptance/testdata/mutualtls/valid/client.crt b/test/acceptance/testdata/mutualtls/valid/client.crt new file mode 100644 index 000000000..3f545b4aa --- /dev/null +++ b/test/acceptance/testdata/mutualtls/valid/client.crt @@ -0,0 +1,23 @@ +-----BEGIN CERTIFICATE----- +MIIDwzCCAqugAwIBAgIUJiLmI0GYBnuegJ+klIYNo0xwWFgwDQYJKoZIhvcNAQEL +BQAwVzELMAkGA1UEBhMCQ04xCzAJBgNVBAgMAkdEMQswCQYDVQQHDAJTWjEVMBMG +A1UECgwMR2l0TGFiLCBJbmMuMRcwFQYDVQQDDA5HaXRMYWIgUm9vdCBDQTAgFw0y +NDA0MjgwNTE0MTVaGA8yMDUxMDkxMzA1MTQxNVowgYgxCzAJBgNVBAYTAlVTMQsw +CQYDVQQIDAJDQTELMAkGA1UEBwwCU0YxEzARBgNVBAoMCkdpdExhYiBJbmMxFTAT +BgNVBAsMDEdpdExhYiBQYWdlczESMBAGA1UEAwwJMTI3LjAuMC4xMR8wHQYJKoZI +hvcNAQkBFhBuZ2FsYUBnaXRsYWIuY29tMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8A +MIIBCgKCAQEAvrRzIVkbZn3N30EAx6NLXbYX8mX49KbHKLQv2OFGElh21n7FHcSb +Eu3jc6U4HwFgtdeALtg6ooqtxWH1mI2BvkhzibAlqm1pJ/2U2YM84K916MxvmQHS +zw3kOlN1jA941vq2dYKiyNGYlOoxMxHXOSorB05eMtjAuj+Wl9B8t2ixW+IaS01O +Tw5jW6YM+XkwduN6cGDxXL4G/WO+XRfskKCNmFQCL5g0Bd3o7O1if8L1071VVcvF +teZXvCmbQrHoT+mRgvleONfE7KkllJ04HYZtNwPcVPepbxdGgi5ZKpKOYRmHyIlG +j915Q3Q4lYjCT9Osynv0lSbanswgcEdaLQIDAQABo1MwUTAPBgNVHREECDAGhwR/ +AAABMB0GA1UdDgQWBBRQLcraJY4zJvSvlfxnVihpnbyZJjAfBgNVHSMEGDAWgBRS +nkD/LMdeF3Fez1dTGaqURUl61TANBgkqhkiG9w0BAQsFAAOCAQEAL3qgjOxhuuKw +HEOJqDqRpTWxWIU2NjFnqegg6V+sU1KCgEdgSHtWZ8jIhVn201cZOsgfF0nUvYST +LUNsWYLga6l7VRUZqFlC8pkuV7I7QMmddBKGDelhsJbcLaAAbXhiQ36/UYF9kWeJ +Wff42L5GFJjQ1fCuVEYHpFFrUW6HEqLiDjPXHJS6kWKV/SSnAy9X0Uc0lurdk8Nd +uX/qkisH/lCqHAV/KQLuWbF7+DDf3YUPZjMOqagC6sUpV/geENMi/jcbTE7vpOuO +qQQU7VuKZJFdqfOoNHvnQiWIbAThVzXyh2nY0El/so1+5IDdVDdaacn3dw/VeHpN +r8jTtrEuYg== +-----END CERTIFICATE----- \ No newline at end of file diff --git a/test/acceptance/testdata/mutualtls/valid/client.key b/test/acceptance/testdata/mutualtls/valid/client.key new file mode 100644 index 000000000..5fea77861 --- /dev/null +++ b/test/acceptance/testdata/mutualtls/valid/client.key @@ -0,0 +1,28 @@ +-----BEGIN PRIVATE KEY----- +MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQC+tHMhWRtmfc3f +QQDHo0tdthfyZfj0pscotC/Y4UYSWHbWfsUdxJsS7eNzpTgfAWC114Au2Dqiiq3F +YfWYjYG+SHOJsCWqbWkn/ZTZgzzgr3XozG+ZAdLPDeQ6U3WMD3jW+rZ1gqLI0ZiU +6jEzEdc5KisHTl4y2MC6P5aX0Hy3aLFb4hpLTU5PDmNbpgz5eTB243pwYPFcvgb9 +Y75dF+yQoI2YVAIvmDQF3ejs7WJ/wvXTvVVVy8W15le8KZtCsehP6ZGC+V4418Ts +qSWUnTgdhm03A9xU96lvF0aCLlkqko5hGYfIiUaP3XlDdDiViMJP06zKe/SVJtqe +zCBwR1otAgMBAAECggEABVTVy860wY/uePmAFD9gRILqNs3GAM+cIsBYKW0xgHgg +/mHLO0f7ToirAq58Rqr1sVplzCl3CNiX/2bRLw4fB0WkQUBU6pJQzcbnlJB4MQ8x +XIGWnxrvcfgWniYmKPUScO7PugedwGp8+bDLEP1EhzsYD9tk2RwOgJvK0dIIzTcf +agvHIE33LSDBPy6tcoagxv8Z/M/jXVMdPsgN1xKWKoUSEq6X24IpriLXBQSkoX83 +ecp8hZyeIXuc6SNb8d7eEXCcg/JK6ZRLyRXGHaalCYdy594VrA8qeuZ1gtUwPBgq +zU4JyRY4K7IGxgo13cE0W+6Fvg/5Al6kdRxHrB3QsQKBgQD+yVQcPozRVL48f1g/ +EpTTJyEi/9piKcvWWQc+BrrEOGDrtnv+eJ/c4YuzrcFfwQhN18VIuWl0xbUBnKii +3DkOu9LOT4fozSeTDOG880mFgSDEjFQu+KbpxQh81HOG0GzUM8mLq4Gcr30WIMMM +xIVg3iHsxPHxHIkHI1WQo97eiQKBgQC/nPvist9YzJYCNvkQ6xi4F6E6mS4Ey2+V +a/yn9H8fwpHZA93yg6ovL20tL5Hfd9cAPjQ6JcjLRuefd5AYe1E/EbkzCv8LG4zL +GXoovVofPxAEMG2O7U2TeoNauCJ9CJurfIkko7lziQVKawx4sYq2QvkIP2zY+FnN +1atQqDiVhQKBgGERQsIf8nYt2uwhd/VPlvN7DNzQrNqJIedfs6ql1bG76PDkbQjd +28nDA/5ITEu2tvsxITA7szmRuQwMKxMg43wBgqanFhhTUKhtV/MsnO4H6/v1mnzq +rmyRbFJifkD2Vv/hWv+jL5YKJZWwlZ7foBDvj+0seyBoxqu5gnfAdsBBAoGAQVh6 +Fk/GF3R92/d/bSOf5Hg6hc9jgEMYpK6VFXouOFiUgJvu/xuj2D+mTfihGMK30d9k +1Ee6eIiPyTRvMcosZQPYUu33GISmuUTRAj/BElLhVWxmkI2hHSB012VgbZ+X5x2r +b5FeV2ZtJXnoYOi7U3j3kLaAmmXnymiJ6hHUajkCgYEApQb7xeBAMpwqc58VZ1s3 +aXlZJja3LPohcvNZsSJ+CupfOgSJAKGa83+jdbmHQaCTUmlLyS8/JiJ6x5RBa1t3 +fHqAtDfYXVU7V/HYJ1+dOOZTVdSK1xJTINiE6n8YXRtRKIJhxuA4Vo/hyh28WX/E +C613sVOjicenNabI1Qjo7aY= +-----END PRIVATE KEY----- \ No newline at end of file diff --git a/test/gitlabstub/option.go b/test/gitlabstub/option.go index 0fbc9a1a9..3057fe00b 100644 --- a/test/gitlabstub/option.go +++ b/test/gitlabstub/option.go @@ -58,6 +58,5 @@ func WithMutualTLS(caCert *x509.Certificate) Option { sc.tlsConfig.ClientAuth = tls.RequireAndVerifyClientCert sc.tlsConfig.ClientCAs = x509.NewCertPool() sc.tlsConfig.ClientCAs.AddCert(caCert) - sc.tlsConfig.InsecureSkipVerify = true } } -- GitLab From 994e92f61481b7ce9997467bad1f3112745a4f66 Mon Sep 17 00:00:00 2001 From: ngala Date: Fri, 3 May 2024 17:50:22 +0530 Subject: [PATCH 06/11] Address review comments --- app.go | 4 ++-- internal/artifact/artifact.go | 4 ++-- internal/auth/auth.go | 4 ++-- internal/config/config.go | 17 ++++++++++++----- internal/httptransport/transport.go | 13 +++++++++---- internal/source/gitlab/client/client.go | 6 +++--- 6 files changed, 30 insertions(+), 18 deletions(-) diff --git a/app.go b/app.go index 3fc3ddc02..f4662a9ef 100644 --- a/app.go +++ b/app.go @@ -384,7 +384,7 @@ func runApp(config *cfg.Config) error { return fmt.Errorf("failed to initialize logging: %w", err) } - a.Artifact = artifact.New(config.ArtifactsServer.URL, config.ArtifactsServer.TimeoutSeconds, config.General.Domain, config.GitLab.HTTPClientCfg) + a.Artifact = artifact.New(config.ArtifactsServer.URL, config.ArtifactsServer.TimeoutSeconds, config.General.Domain, config.GitLab.ClientCfg) if err := a.setAuth(config); err != nil { return err @@ -423,7 +423,7 @@ func (a *theApp) setAuth(config *cfg.Config) error { AuthTimeout: config.Authentication.Timeout, CookieSessionTimeout: config.Authentication.CookieSessionTimeout, AllowNamespaceInPath: config.General.NamespaceInPath, - CertCfg: config.GitLab.HTTPClientCfg, + ClientCfg: config.GitLab.ClientCfg, }) if err != nil { return fmt.Errorf("could not initialize auth package: %w", err) diff --git a/internal/artifact/artifact.go b/internal/artifact/artifact.go index 5f58be62b..ec43c455b 100644 --- a/internal/artifact/artifact.go +++ b/internal/artifact/artifact.go @@ -48,8 +48,8 @@ type Artifact struct { // New when provided the arguments defined herein, returns a pointer to an // Artifact that is used to proxy requests. -func New(server string, timeoutSeconds int, pagesDomain string, httpClientCfg config.HTTPClientCfg) *Artifact { - httpTransport := httptransport.NewTransportWithClientCert(httpClientCfg) +func New(server string, timeoutSeconds int, pagesDomain string, clientCfg config.HTTPClientCfg) *Artifact { + httpTransport := httptransport.NewTransportWithClientCert(clientCfg) return &Artifact{ server: strings.TrimRight(server, "/"), diff --git a/internal/auth/auth.go b/internal/auth/auth.go index 559f365b3..210bba030 100644 --- a/internal/auth/auth.go +++ b/internal/auth/auth.go @@ -711,7 +711,7 @@ type Options struct { AuthTimeout time.Duration CookieSessionTimeout time.Duration AllowNamespaceInPath bool - CertCfg config.HTTPClientCfg + ClientCfg config.HTTPClientCfg } // New when authentication supported this will be used to create authentication handler @@ -721,7 +721,7 @@ func New(options *Options) (*Auth, error) { if err != nil { return nil, err } - httpTransport := httptransport.NewTransportWithClientCert(options.CertCfg) + httpTransport := httptransport.NewTransportWithClientCert(options.ClientCfg) return &Auth{ pagesDomain: options.PagesDomain, diff --git a/internal/config/config.go b/internal/config/config.go index 32ae71116..85e26f06b 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -119,12 +119,14 @@ type GitLab struct { JWTTokenExpiration time.Duration Cache Cache EnableDisk bool - HTTPClientCfg HTTPClientCfg + ClientCfg HTTPClientCfg } type HTTPClientCfg struct { - CAFiles []string - Certs []tls.Certificate + CAFiles []string + Certs []tls.Certificate + MinVersion uint16 + MaxVersion uint16 } // Log groups settings related to configuring logging @@ -433,7 +435,12 @@ func loadConfig() (*Config, error) { return nil, err } - config.GitLab.HTTPClientCfg = HTTPClientCfg{Certs: certs, CAFiles: caCerts.Split()} + config.GitLab.ClientCfg = HTTPClientCfg{ + Certs: certs, + CAFiles: caCerts.Split(), + MinVersion: allTLSVersions[*tlsMinVersion], + MaxVersion: allTLSVersions[*tlsMaxVersion], + } customHeaders, err := parseHeaderString(header.Split()) if err != nil { @@ -559,7 +566,7 @@ func logFields(config *Config) map[string]any { "sentry-environment": config.Sentry.Environment, "version": config.General.ShowVersion, "namespace-in-path": *namespaceInPath, - "ca-certs": config.GitLab.HTTPClientCfg.CAFiles, + "ca-certs": config.GitLab.ClientCfg.CAFiles, } } diff --git a/internal/httptransport/transport.go b/internal/httptransport/transport.go index e93bfb326..da38e0a92 100644 --- a/internal/httptransport/transport.go +++ b/internal/httptransport/transport.go @@ -60,14 +60,16 @@ func NewTransport() *http.Transport { // NewTransportWithClientCert creates a new http.Transport with the provided client certificate configuration. // It sets up the TLS configuration with the provided CA files and client certificates. // The transport is configured with default connection values such as timeouts and max idle connections. -func NewTransportWithClientCert(hcc config.HTTPClientCfg) *http.Transport { +func NewTransportWithClientCert(clientCfg config.HTTPClientCfg) *http.Transport { certPool := pool() - for _, caFile := range hcc.CAFiles { + for _, caFile := range clientCfg.CAFiles { log.Info("Reading CA file: ", caFile) cert, err := os.ReadFile(caFile) if err == nil { certPool.AppendCertsFromPEM(cert) + } else { + log.WithError(err).Error("Error reading CA file") } } @@ -76,9 +78,12 @@ func NewTransportWithClientCert(hcc config.HTTPClientCfg) *http.Transport { MinVersion: tls.VersionTLS12, // set MinVersion to fix gosec: G402 } - if hcc.Certs != nil { + tlsConfig.MinVersion = clientCfg.MinVersion + tlsConfig.MaxVersion = clientCfg.MaxVersion + + if clientCfg.Certs != nil { log.Info("Setting mutual TLS certificates to access GitLab API") - tlsConfig.Certificates = hcc.Certs + tlsConfig.Certificates = clientCfg.Certs } return &http.Transport{ diff --git a/internal/source/gitlab/client/client.go b/internal/source/gitlab/client/client.go index 2fa22830b..45c78e2d6 100644 --- a/internal/source/gitlab/client/client.go +++ b/internal/source/gitlab/client/client.go @@ -44,7 +44,7 @@ type Client struct { // NewClient initializes and returns new Client baseUrl is // appConfig.InternalGitLabServer secretKey is appConfig.GitLabAPISecretKey -func NewClient(baseURL string, secretKey []byte, connectionTimeout, jwtTokenExpiry time.Duration, httpClientCfg config.HTTPClientCfg) (*Client, error) { +func NewClient(baseURL string, secretKey []byte, connectionTimeout, jwtTokenExpiry time.Duration, clientCfg config.HTTPClientCfg) (*Client, error) { if len(baseURL) == 0 || len(secretKey) == 0 { return nil, errors.New("GitLab API URL or API secret has not been provided") } @@ -62,7 +62,7 @@ func NewClient(baseURL string, secretKey []byte, connectionTimeout, jwtTokenExpi return nil, errors.New("GitLab JWT token expiry has not been provided") } - httpTransport := httptransport.NewTransportWithClientCert(httpClientCfg) + httpTransport := httptransport.NewTransportWithClientCert(clientCfg) return &Client{ secretKey: secretKey, @@ -87,7 +87,7 @@ func NewClient(baseURL string, secretKey []byte, connectionTimeout, jwtTokenExpi // NewFromConfig creates a new client from Config struct func NewFromConfig(cfg *config.GitLab) (*Client, error) { - return NewClient(cfg.InternalServer, cfg.APISecretKey, cfg.ClientHTTPTimeout, cfg.JWTTokenExpiration, cfg.HTTPClientCfg) + return NewClient(cfg.InternalServer, cfg.APISecretKey, cfg.ClientHTTPTimeout, cfg.JWTTokenExpiration, cfg.ClientCfg) } // Resolve returns a VirtualDomain configuration wrapped into a Lookup for a -- GitLab From 0765c184c317149a4773605fbac2c60d8d35a0c7 Mon Sep 17 00:00:00 2001 From: ngala Date: Mon, 13 May 2024 12:20:12 +0530 Subject: [PATCH 07/11] update logs with extra fields --- internal/httptransport/transport.go | 4 +--- test/acceptance/gitlab_api_mutual_tls_test.go | 1 - test/gitlabstub/cmd/server/main.go | 4 ---- 3 files changed, 1 insertion(+), 8 deletions(-) diff --git a/internal/httptransport/transport.go b/internal/httptransport/transport.go index da38e0a92..04af87995 100644 --- a/internal/httptransport/transport.go +++ b/internal/httptransport/transport.go @@ -64,12 +64,11 @@ func NewTransportWithClientCert(clientCfg config.HTTPClientCfg) *http.Transport certPool := pool() for _, caFile := range clientCfg.CAFiles { - log.Info("Reading CA file: ", caFile) cert, err := os.ReadFile(caFile) if err == nil { certPool.AppendCertsFromPEM(cert) } else { - log.WithError(err).Error("Error reading CA file") + log.WithError(err).WithField("ca-file", caFile).Error("reading CA file") } } @@ -82,7 +81,6 @@ func NewTransportWithClientCert(clientCfg config.HTTPClientCfg) *http.Transport tlsConfig.MaxVersion = clientCfg.MaxVersion if clientCfg.Certs != nil { - log.Info("Setting mutual TLS certificates to access GitLab API") tlsConfig.Certificates = clientCfg.Certs } diff --git a/test/acceptance/gitlab_api_mutual_tls_test.go b/test/acceptance/gitlab_api_mutual_tls_test.go index 2d7810220..2ab8f36df 100644 --- a/test/acceptance/gitlab_api_mutual_tls_test.go +++ b/test/acceptance/gitlab_api_mutual_tls_test.go @@ -47,7 +47,6 @@ func TestGitLabAPIMutualTLS(t *testing.T) { RunPagesProcessWithMutualTLS(t, httpsListener, tt.clientCertPath, tt.clientKeyPath, tt.caCertPath) rsp, err := GetPageFromListener(t, httpsListener, tt.host, tt.path) - require.NoError(t, err) if tt.expectError { require.Error(t, err) } else { diff --git a/test/gitlabstub/cmd/server/main.go b/test/gitlabstub/cmd/server/main.go index 1bfc92c78..46cf665f3 100644 --- a/test/gitlabstub/cmd/server/main.go +++ b/test/gitlabstub/cmd/server/main.go @@ -95,12 +95,10 @@ func createServer() (*httptest.Server, error) { func loadCertificate(cert string, key string) (*tls.Certificate, error) { if cert == "" && key == "" { - log.Printf("No certificate provided, TLS is disabled") return nil, nil } if cert != "" && key != "" { - log.Printf("Loading key pair: (%s) - (%s)", *certFile, *keyFile) cert, err := tls.LoadX509KeyPair(*certFile, *keyFile) if err != nil { return nil, fmt.Errorf("error loading certificate: %w", err) @@ -116,8 +114,6 @@ func loadCACertificate(certPath string) (*x509.Certificate, error) { return nil, nil } - log.Printf("Reading CA file: %s", certPath) - caCertFile, err := os.ReadFile(certPath) if err != nil { return nil, fmt.Errorf("error reading CA file: %w", err) -- GitLab From 59025a6314c8a49ba50aa907724804918358da98 Mon Sep 17 00:00:00 2001 From: ngala Date: Mon, 13 May 2024 13:01:14 +0530 Subject: [PATCH 08/11] Move mtls certs to common folder --- internal/source/gitlab/client/client_test.go | 6 ++-- .../gitlab/client/testdata/certs/ca.crt | 22 --------------- .../gitlab/client/testdata/certs/client.crt | 23 --------------- .../gitlab/client/testdata/certs/client.key | 28 ------------------- test/acceptance/gitlab_api_mutual_tls_test.go | 12 ++++---- .../testdata/mutualtls/valid/ca.crt | 22 --------------- .../testdata/mutualtls/valid/client.crt | 23 --------------- .../testdata/mutualtls/valid/client.key | 28 ------------------- .../testdata/mutualtls/invalid/ca.crt | 0 .../testdata/mutualtls/invalid/client.crt | 0 .../testdata/mutualtls/invalid/client.key | 0 test/testdata/mutualtls/valid/ca.crt | 21 ++++++++++++++ test/testdata/mutualtls/valid/client.crt | 22 +++++++++++++++ test/testdata/mutualtls/valid/client.key | 28 +++++++++++++++++++ 14 files changed, 80 insertions(+), 155 deletions(-) delete mode 100644 internal/source/gitlab/client/testdata/certs/ca.crt delete mode 100644 internal/source/gitlab/client/testdata/certs/client.crt delete mode 100644 internal/source/gitlab/client/testdata/certs/client.key delete mode 100644 test/acceptance/testdata/mutualtls/valid/ca.crt delete mode 100644 test/acceptance/testdata/mutualtls/valid/client.crt delete mode 100644 test/acceptance/testdata/mutualtls/valid/client.key rename test/{acceptance => }/testdata/mutualtls/invalid/ca.crt (100%) rename test/{acceptance => }/testdata/mutualtls/invalid/client.crt (100%) rename test/{acceptance => }/testdata/mutualtls/invalid/client.key (100%) create mode 100644 test/testdata/mutualtls/valid/ca.crt create mode 100644 test/testdata/mutualtls/valid/client.crt create mode 100644 test/testdata/mutualtls/valid/client.key diff --git a/internal/source/gitlab/client/client_test.go b/internal/source/gitlab/client/client_test.go index 1e050d581..d14a02c01 100644 --- a/internal/source/gitlab/client/client_test.go +++ b/internal/source/gitlab/client/client_test.go @@ -248,9 +248,9 @@ func TestMutualTLSClientAuthentication(t *testing.T) { w.WriteHeader(http.StatusOK) }) - certPath := "testdata/certs/client.crt" - keyPath := "testdata/certs/client.key" - caCertPath := "testdata/certs/ca.crt" + certPath := "../../../../test/testdata/mutualtls/valid/client.crt" + keyPath := "../../../../test/testdata/mutualtls/valid/client.key" + caCertPath := "../../../../test/testdata/mutualtls/valid/ca.crt" tlsCfg := &tls.Config{ ClientCAs: testhelpers.CertPool(t, caCertPath), diff --git a/internal/source/gitlab/client/testdata/certs/ca.crt b/internal/source/gitlab/client/testdata/certs/ca.crt deleted file mode 100644 index 3d619ef2d..000000000 --- a/internal/source/gitlab/client/testdata/certs/ca.crt +++ /dev/null @@ -1,22 +0,0 @@ ------BEGIN CERTIFICATE----- -MIIDjzCCAnegAwIBAgIUNiXaQMFLwUd4LGjXhfK5JFpI+DcwDQYJKoZIhvcNAQEL -BQAwVzELMAkGA1UEBhMCQ04xCzAJBgNVBAgMAkdEMQswCQYDVQQHDAJTWjEVMBMG -A1UECgwMR2l0TGFiLCBJbmMuMRcwFQYDVQQDDA5HaXRMYWIgUm9vdCBDQTAeFw0y -NDA0MjgwNTExMzJaFw0yNTA0MjgwNTExMzJaMFcxCzAJBgNVBAYTAkNOMQswCQYD -VQQIDAJHRDELMAkGA1UEBwwCU1oxFTATBgNVBAoMDEdpdExhYiwgSW5jLjEXMBUG -A1UEAwwOR2l0TGFiIFJvb3QgQ0EwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEK -AoIBAQCNERChbGl/+Phb6psr95gP/uFGyfkTt8nWuL0AlGpx7yyyGC9Pzm1FskpL -iNQB7xmtj/SN6k8EZddXBZcFrRgAiCPa4OO125QveDuBJ+2s9+P+WWhZXudyGnWO -lT5m/doM4HeAZ3ioePsGcjGPC4S+K9bmCsBOlpR/xp/Yc5V7upl6X3E8rK/PpfYn -Kjue+UX8TTseQ6OJSLt+FxwnHPMkcSD7tpuWjr+gAmYaloMvnsHP1pKW1MKrj7XU -FhoWyX4WjCR+zuDgBeBtxgaX8eLFKNwtbbaltc0XStS8qGoRJcV6wsmS7CG3UtFx -gjYBJorvzgmLZOWcc+OMete0rFFrAgMBAAGjUzBRMB0GA1UdDgQWBBRSnkD/LMde -F3Fez1dTGaqURUl61TAfBgNVHSMEGDAWgBRSnkD/LMdeF3Fez1dTGaqURUl61TAP -BgNVHRMBAf8EBTADAQH/MA0GCSqGSIb3DQEBCwUAA4IBAQBzA0yEL69ifRqD2QEv -s5r5jtKQtH04h+x4DEWwWkxrDH9NUPu8+vDWrEMARCvICOkS42CPqhGSRkEka7Ij -vixAo6gzh0mRkxR90Ztjf9fQ4hrKDl7NlcZ0aeeIStExJd5pf4JCv9uyM8p27flP -2+ZHXWx+PfKaRPA4C+WrcYMTTbEGRWJVKFl3rTCkIZ2sWGV8Gt+wzxYWutjB91RI -m3rnC0xvrBBo6AMlFmAcqOKs+pzThIECJhM5zwQ6VBfx9xgIF20JAUS78176NpiY -JWxUEjkdR4ofaBbYTSdef4M8vJI6nm2fCWucCXwB1qeOyU4xki/QU+Cfqqwuqvyt -WVib ------END CERTIFICATE----- \ No newline at end of file diff --git a/internal/source/gitlab/client/testdata/certs/client.crt b/internal/source/gitlab/client/testdata/certs/client.crt deleted file mode 100644 index 3f545b4aa..000000000 --- a/internal/source/gitlab/client/testdata/certs/client.crt +++ /dev/null @@ -1,23 +0,0 @@ ------BEGIN CERTIFICATE----- -MIIDwzCCAqugAwIBAgIUJiLmI0GYBnuegJ+klIYNo0xwWFgwDQYJKoZIhvcNAQEL -BQAwVzELMAkGA1UEBhMCQ04xCzAJBgNVBAgMAkdEMQswCQYDVQQHDAJTWjEVMBMG -A1UECgwMR2l0TGFiLCBJbmMuMRcwFQYDVQQDDA5HaXRMYWIgUm9vdCBDQTAgFw0y -NDA0MjgwNTE0MTVaGA8yMDUxMDkxMzA1MTQxNVowgYgxCzAJBgNVBAYTAlVTMQsw -CQYDVQQIDAJDQTELMAkGA1UEBwwCU0YxEzARBgNVBAoMCkdpdExhYiBJbmMxFTAT -BgNVBAsMDEdpdExhYiBQYWdlczESMBAGA1UEAwwJMTI3LjAuMC4xMR8wHQYJKoZI -hvcNAQkBFhBuZ2FsYUBnaXRsYWIuY29tMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8A -MIIBCgKCAQEAvrRzIVkbZn3N30EAx6NLXbYX8mX49KbHKLQv2OFGElh21n7FHcSb -Eu3jc6U4HwFgtdeALtg6ooqtxWH1mI2BvkhzibAlqm1pJ/2U2YM84K916MxvmQHS -zw3kOlN1jA941vq2dYKiyNGYlOoxMxHXOSorB05eMtjAuj+Wl9B8t2ixW+IaS01O -Tw5jW6YM+XkwduN6cGDxXL4G/WO+XRfskKCNmFQCL5g0Bd3o7O1if8L1071VVcvF -teZXvCmbQrHoT+mRgvleONfE7KkllJ04HYZtNwPcVPepbxdGgi5ZKpKOYRmHyIlG -j915Q3Q4lYjCT9Osynv0lSbanswgcEdaLQIDAQABo1MwUTAPBgNVHREECDAGhwR/ -AAABMB0GA1UdDgQWBBRQLcraJY4zJvSvlfxnVihpnbyZJjAfBgNVHSMEGDAWgBRS -nkD/LMdeF3Fez1dTGaqURUl61TANBgkqhkiG9w0BAQsFAAOCAQEAL3qgjOxhuuKw -HEOJqDqRpTWxWIU2NjFnqegg6V+sU1KCgEdgSHtWZ8jIhVn201cZOsgfF0nUvYST -LUNsWYLga6l7VRUZqFlC8pkuV7I7QMmddBKGDelhsJbcLaAAbXhiQ36/UYF9kWeJ -Wff42L5GFJjQ1fCuVEYHpFFrUW6HEqLiDjPXHJS6kWKV/SSnAy9X0Uc0lurdk8Nd -uX/qkisH/lCqHAV/KQLuWbF7+DDf3YUPZjMOqagC6sUpV/geENMi/jcbTE7vpOuO -qQQU7VuKZJFdqfOoNHvnQiWIbAThVzXyh2nY0El/so1+5IDdVDdaacn3dw/VeHpN -r8jTtrEuYg== ------END CERTIFICATE----- \ No newline at end of file diff --git a/internal/source/gitlab/client/testdata/certs/client.key b/internal/source/gitlab/client/testdata/certs/client.key deleted file mode 100644 index 5fea77861..000000000 --- a/internal/source/gitlab/client/testdata/certs/client.key +++ /dev/null @@ -1,28 +0,0 @@ ------BEGIN PRIVATE KEY----- -MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQC+tHMhWRtmfc3f -QQDHo0tdthfyZfj0pscotC/Y4UYSWHbWfsUdxJsS7eNzpTgfAWC114Au2Dqiiq3F -YfWYjYG+SHOJsCWqbWkn/ZTZgzzgr3XozG+ZAdLPDeQ6U3WMD3jW+rZ1gqLI0ZiU -6jEzEdc5KisHTl4y2MC6P5aX0Hy3aLFb4hpLTU5PDmNbpgz5eTB243pwYPFcvgb9 -Y75dF+yQoI2YVAIvmDQF3ejs7WJ/wvXTvVVVy8W15le8KZtCsehP6ZGC+V4418Ts -qSWUnTgdhm03A9xU96lvF0aCLlkqko5hGYfIiUaP3XlDdDiViMJP06zKe/SVJtqe -zCBwR1otAgMBAAECggEABVTVy860wY/uePmAFD9gRILqNs3GAM+cIsBYKW0xgHgg -/mHLO0f7ToirAq58Rqr1sVplzCl3CNiX/2bRLw4fB0WkQUBU6pJQzcbnlJB4MQ8x -XIGWnxrvcfgWniYmKPUScO7PugedwGp8+bDLEP1EhzsYD9tk2RwOgJvK0dIIzTcf -agvHIE33LSDBPy6tcoagxv8Z/M/jXVMdPsgN1xKWKoUSEq6X24IpriLXBQSkoX83 -ecp8hZyeIXuc6SNb8d7eEXCcg/JK6ZRLyRXGHaalCYdy594VrA8qeuZ1gtUwPBgq -zU4JyRY4K7IGxgo13cE0W+6Fvg/5Al6kdRxHrB3QsQKBgQD+yVQcPozRVL48f1g/ -EpTTJyEi/9piKcvWWQc+BrrEOGDrtnv+eJ/c4YuzrcFfwQhN18VIuWl0xbUBnKii -3DkOu9LOT4fozSeTDOG880mFgSDEjFQu+KbpxQh81HOG0GzUM8mLq4Gcr30WIMMM -xIVg3iHsxPHxHIkHI1WQo97eiQKBgQC/nPvist9YzJYCNvkQ6xi4F6E6mS4Ey2+V -a/yn9H8fwpHZA93yg6ovL20tL5Hfd9cAPjQ6JcjLRuefd5AYe1E/EbkzCv8LG4zL -GXoovVofPxAEMG2O7U2TeoNauCJ9CJurfIkko7lziQVKawx4sYq2QvkIP2zY+FnN -1atQqDiVhQKBgGERQsIf8nYt2uwhd/VPlvN7DNzQrNqJIedfs6ql1bG76PDkbQjd -28nDA/5ITEu2tvsxITA7szmRuQwMKxMg43wBgqanFhhTUKhtV/MsnO4H6/v1mnzq -rmyRbFJifkD2Vv/hWv+jL5YKJZWwlZ7foBDvj+0seyBoxqu5gnfAdsBBAoGAQVh6 -Fk/GF3R92/d/bSOf5Hg6hc9jgEMYpK6VFXouOFiUgJvu/xuj2D+mTfihGMK30d9k -1Ee6eIiPyTRvMcosZQPYUu33GISmuUTRAj/BElLhVWxmkI2hHSB012VgbZ+X5x2r -b5FeV2ZtJXnoYOi7U3j3kLaAmmXnymiJ6hHUajkCgYEApQb7xeBAMpwqc58VZ1s3 -aXlZJja3LPohcvNZsSJ+CupfOgSJAKGa83+jdbmHQaCTUmlLyS8/JiJ6x5RBa1t3 -fHqAtDfYXVU7V/HYJ1+dOOZTVdSK1xJTINiE6n8YXRtRKIJhxuA4Vo/hyh28WX/E -C613sVOjicenNabI1Qjo7aY= ------END PRIVATE KEY----- \ No newline at end of file diff --git a/test/acceptance/gitlab_api_mutual_tls_test.go b/test/acceptance/gitlab_api_mutual_tls_test.go index 2ab8f36df..b74b3bcb8 100644 --- a/test/acceptance/gitlab_api_mutual_tls_test.go +++ b/test/acceptance/gitlab_api_mutual_tls_test.go @@ -23,9 +23,9 @@ func TestGitLabAPIMutualTLS(t *testing.T) { name: "basic request works with GitLab API mutual TLS", host: "group.gitlab-example.com", path: "/index.html", - clientCertPath: "../../test/acceptance/testdata/mutualtls/valid/client.crt", - clientKeyPath: "../../test/acceptance/testdata/mutualtls/valid/client.key", - caCertPath: "../../test/acceptance/testdata/mutualtls/valid/ca.crt", + clientCertPath: "../../test/testdata/mutualtls/valid/client.crt", + clientKeyPath: "../../test/testdata/mutualtls/valid/client.key", + caCertPath: "../../test/testdata/mutualtls/valid/ca.crt", status: http.StatusOK, expectError: false, }, @@ -33,9 +33,9 @@ func TestGitLabAPIMutualTLS(t *testing.T) { name: "502 when invalid mutual TLS is used", host: "group.gitlab-example.com", path: "/index.html", - clientCertPath: "../../test/acceptance/testdata/mutualtls/invalid/client.crt", - clientKeyPath: "../../test/acceptance/testdata/mutualtls/invalid/client.key", - caCertPath: "../../test/acceptance/testdata/mutualtls/invalid/ca.crt", + clientCertPath: "../../test/testdata/mutualtls/invalid/client.crt", + clientKeyPath: "../../test/testdata/mutualtls/invalid/client.key", + caCertPath: "../../test/testdata/mutualtls/invalid/ca.crt", status: http.StatusBadGateway, expectError: false, }, diff --git a/test/acceptance/testdata/mutualtls/valid/ca.crt b/test/acceptance/testdata/mutualtls/valid/ca.crt deleted file mode 100644 index 3d619ef2d..000000000 --- a/test/acceptance/testdata/mutualtls/valid/ca.crt +++ /dev/null @@ -1,22 +0,0 @@ ------BEGIN CERTIFICATE----- -MIIDjzCCAnegAwIBAgIUNiXaQMFLwUd4LGjXhfK5JFpI+DcwDQYJKoZIhvcNAQEL -BQAwVzELMAkGA1UEBhMCQ04xCzAJBgNVBAgMAkdEMQswCQYDVQQHDAJTWjEVMBMG -A1UECgwMR2l0TGFiLCBJbmMuMRcwFQYDVQQDDA5HaXRMYWIgUm9vdCBDQTAeFw0y -NDA0MjgwNTExMzJaFw0yNTA0MjgwNTExMzJaMFcxCzAJBgNVBAYTAkNOMQswCQYD -VQQIDAJHRDELMAkGA1UEBwwCU1oxFTATBgNVBAoMDEdpdExhYiwgSW5jLjEXMBUG -A1UEAwwOR2l0TGFiIFJvb3QgQ0EwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEK -AoIBAQCNERChbGl/+Phb6psr95gP/uFGyfkTt8nWuL0AlGpx7yyyGC9Pzm1FskpL -iNQB7xmtj/SN6k8EZddXBZcFrRgAiCPa4OO125QveDuBJ+2s9+P+WWhZXudyGnWO -lT5m/doM4HeAZ3ioePsGcjGPC4S+K9bmCsBOlpR/xp/Yc5V7upl6X3E8rK/PpfYn -Kjue+UX8TTseQ6OJSLt+FxwnHPMkcSD7tpuWjr+gAmYaloMvnsHP1pKW1MKrj7XU -FhoWyX4WjCR+zuDgBeBtxgaX8eLFKNwtbbaltc0XStS8qGoRJcV6wsmS7CG3UtFx -gjYBJorvzgmLZOWcc+OMete0rFFrAgMBAAGjUzBRMB0GA1UdDgQWBBRSnkD/LMde -F3Fez1dTGaqURUl61TAfBgNVHSMEGDAWgBRSnkD/LMdeF3Fez1dTGaqURUl61TAP -BgNVHRMBAf8EBTADAQH/MA0GCSqGSIb3DQEBCwUAA4IBAQBzA0yEL69ifRqD2QEv -s5r5jtKQtH04h+x4DEWwWkxrDH9NUPu8+vDWrEMARCvICOkS42CPqhGSRkEka7Ij -vixAo6gzh0mRkxR90Ztjf9fQ4hrKDl7NlcZ0aeeIStExJd5pf4JCv9uyM8p27flP -2+ZHXWx+PfKaRPA4C+WrcYMTTbEGRWJVKFl3rTCkIZ2sWGV8Gt+wzxYWutjB91RI -m3rnC0xvrBBo6AMlFmAcqOKs+pzThIECJhM5zwQ6VBfx9xgIF20JAUS78176NpiY -JWxUEjkdR4ofaBbYTSdef4M8vJI6nm2fCWucCXwB1qeOyU4xki/QU+Cfqqwuqvyt -WVib ------END CERTIFICATE----- \ No newline at end of file diff --git a/test/acceptance/testdata/mutualtls/valid/client.crt b/test/acceptance/testdata/mutualtls/valid/client.crt deleted file mode 100644 index 3f545b4aa..000000000 --- a/test/acceptance/testdata/mutualtls/valid/client.crt +++ /dev/null @@ -1,23 +0,0 @@ ------BEGIN CERTIFICATE----- -MIIDwzCCAqugAwIBAgIUJiLmI0GYBnuegJ+klIYNo0xwWFgwDQYJKoZIhvcNAQEL -BQAwVzELMAkGA1UEBhMCQ04xCzAJBgNVBAgMAkdEMQswCQYDVQQHDAJTWjEVMBMG -A1UECgwMR2l0TGFiLCBJbmMuMRcwFQYDVQQDDA5HaXRMYWIgUm9vdCBDQTAgFw0y -NDA0MjgwNTE0MTVaGA8yMDUxMDkxMzA1MTQxNVowgYgxCzAJBgNVBAYTAlVTMQsw -CQYDVQQIDAJDQTELMAkGA1UEBwwCU0YxEzARBgNVBAoMCkdpdExhYiBJbmMxFTAT -BgNVBAsMDEdpdExhYiBQYWdlczESMBAGA1UEAwwJMTI3LjAuMC4xMR8wHQYJKoZI -hvcNAQkBFhBuZ2FsYUBnaXRsYWIuY29tMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8A -MIIBCgKCAQEAvrRzIVkbZn3N30EAx6NLXbYX8mX49KbHKLQv2OFGElh21n7FHcSb -Eu3jc6U4HwFgtdeALtg6ooqtxWH1mI2BvkhzibAlqm1pJ/2U2YM84K916MxvmQHS -zw3kOlN1jA941vq2dYKiyNGYlOoxMxHXOSorB05eMtjAuj+Wl9B8t2ixW+IaS01O -Tw5jW6YM+XkwduN6cGDxXL4G/WO+XRfskKCNmFQCL5g0Bd3o7O1if8L1071VVcvF -teZXvCmbQrHoT+mRgvleONfE7KkllJ04HYZtNwPcVPepbxdGgi5ZKpKOYRmHyIlG -j915Q3Q4lYjCT9Osynv0lSbanswgcEdaLQIDAQABo1MwUTAPBgNVHREECDAGhwR/ -AAABMB0GA1UdDgQWBBRQLcraJY4zJvSvlfxnVihpnbyZJjAfBgNVHSMEGDAWgBRS -nkD/LMdeF3Fez1dTGaqURUl61TANBgkqhkiG9w0BAQsFAAOCAQEAL3qgjOxhuuKw -HEOJqDqRpTWxWIU2NjFnqegg6V+sU1KCgEdgSHtWZ8jIhVn201cZOsgfF0nUvYST -LUNsWYLga6l7VRUZqFlC8pkuV7I7QMmddBKGDelhsJbcLaAAbXhiQ36/UYF9kWeJ -Wff42L5GFJjQ1fCuVEYHpFFrUW6HEqLiDjPXHJS6kWKV/SSnAy9X0Uc0lurdk8Nd -uX/qkisH/lCqHAV/KQLuWbF7+DDf3YUPZjMOqagC6sUpV/geENMi/jcbTE7vpOuO -qQQU7VuKZJFdqfOoNHvnQiWIbAThVzXyh2nY0El/so1+5IDdVDdaacn3dw/VeHpN -r8jTtrEuYg== ------END CERTIFICATE----- \ No newline at end of file diff --git a/test/acceptance/testdata/mutualtls/valid/client.key b/test/acceptance/testdata/mutualtls/valid/client.key deleted file mode 100644 index 5fea77861..000000000 --- a/test/acceptance/testdata/mutualtls/valid/client.key +++ /dev/null @@ -1,28 +0,0 @@ ------BEGIN PRIVATE KEY----- -MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQC+tHMhWRtmfc3f -QQDHo0tdthfyZfj0pscotC/Y4UYSWHbWfsUdxJsS7eNzpTgfAWC114Au2Dqiiq3F -YfWYjYG+SHOJsCWqbWkn/ZTZgzzgr3XozG+ZAdLPDeQ6U3WMD3jW+rZ1gqLI0ZiU -6jEzEdc5KisHTl4y2MC6P5aX0Hy3aLFb4hpLTU5PDmNbpgz5eTB243pwYPFcvgb9 -Y75dF+yQoI2YVAIvmDQF3ejs7WJ/wvXTvVVVy8W15le8KZtCsehP6ZGC+V4418Ts -qSWUnTgdhm03A9xU96lvF0aCLlkqko5hGYfIiUaP3XlDdDiViMJP06zKe/SVJtqe -zCBwR1otAgMBAAECggEABVTVy860wY/uePmAFD9gRILqNs3GAM+cIsBYKW0xgHgg -/mHLO0f7ToirAq58Rqr1sVplzCl3CNiX/2bRLw4fB0WkQUBU6pJQzcbnlJB4MQ8x -XIGWnxrvcfgWniYmKPUScO7PugedwGp8+bDLEP1EhzsYD9tk2RwOgJvK0dIIzTcf -agvHIE33LSDBPy6tcoagxv8Z/M/jXVMdPsgN1xKWKoUSEq6X24IpriLXBQSkoX83 -ecp8hZyeIXuc6SNb8d7eEXCcg/JK6ZRLyRXGHaalCYdy594VrA8qeuZ1gtUwPBgq -zU4JyRY4K7IGxgo13cE0W+6Fvg/5Al6kdRxHrB3QsQKBgQD+yVQcPozRVL48f1g/ -EpTTJyEi/9piKcvWWQc+BrrEOGDrtnv+eJ/c4YuzrcFfwQhN18VIuWl0xbUBnKii -3DkOu9LOT4fozSeTDOG880mFgSDEjFQu+KbpxQh81HOG0GzUM8mLq4Gcr30WIMMM -xIVg3iHsxPHxHIkHI1WQo97eiQKBgQC/nPvist9YzJYCNvkQ6xi4F6E6mS4Ey2+V -a/yn9H8fwpHZA93yg6ovL20tL5Hfd9cAPjQ6JcjLRuefd5AYe1E/EbkzCv8LG4zL -GXoovVofPxAEMG2O7U2TeoNauCJ9CJurfIkko7lziQVKawx4sYq2QvkIP2zY+FnN -1atQqDiVhQKBgGERQsIf8nYt2uwhd/VPlvN7DNzQrNqJIedfs6ql1bG76PDkbQjd -28nDA/5ITEu2tvsxITA7szmRuQwMKxMg43wBgqanFhhTUKhtV/MsnO4H6/v1mnzq -rmyRbFJifkD2Vv/hWv+jL5YKJZWwlZ7foBDvj+0seyBoxqu5gnfAdsBBAoGAQVh6 -Fk/GF3R92/d/bSOf5Hg6hc9jgEMYpK6VFXouOFiUgJvu/xuj2D+mTfihGMK30d9k -1Ee6eIiPyTRvMcosZQPYUu33GISmuUTRAj/BElLhVWxmkI2hHSB012VgbZ+X5x2r -b5FeV2ZtJXnoYOi7U3j3kLaAmmXnymiJ6hHUajkCgYEApQb7xeBAMpwqc58VZ1s3 -aXlZJja3LPohcvNZsSJ+CupfOgSJAKGa83+jdbmHQaCTUmlLyS8/JiJ6x5RBa1t3 -fHqAtDfYXVU7V/HYJ1+dOOZTVdSK1xJTINiE6n8YXRtRKIJhxuA4Vo/hyh28WX/E -C613sVOjicenNabI1Qjo7aY= ------END PRIVATE KEY----- \ No newline at end of file diff --git a/test/acceptance/testdata/mutualtls/invalid/ca.crt b/test/testdata/mutualtls/invalid/ca.crt similarity index 100% rename from test/acceptance/testdata/mutualtls/invalid/ca.crt rename to test/testdata/mutualtls/invalid/ca.crt diff --git a/test/acceptance/testdata/mutualtls/invalid/client.crt b/test/testdata/mutualtls/invalid/client.crt similarity index 100% rename from test/acceptance/testdata/mutualtls/invalid/client.crt rename to test/testdata/mutualtls/invalid/client.crt diff --git a/test/acceptance/testdata/mutualtls/invalid/client.key b/test/testdata/mutualtls/invalid/client.key similarity index 100% rename from test/acceptance/testdata/mutualtls/invalid/client.key rename to test/testdata/mutualtls/invalid/client.key diff --git a/test/testdata/mutualtls/valid/ca.crt b/test/testdata/mutualtls/valid/ca.crt new file mode 100644 index 000000000..a2e52b722 --- /dev/null +++ b/test/testdata/mutualtls/valid/ca.crt @@ -0,0 +1,21 @@ +-----BEGIN CERTIFICATE----- +MIIDhzCCAm+gAwIBAgIUfCgs0bsZePJLm7rGA4p2DXWpVNMwDQYJKoZIhvcNAQEL +BQAwUjELMAkGA1UEBhMCQ04xCzAJBgNVBAgMAkdEMQswCQYDVQQHDAJTWjEVMBMG +A1UECgwMR2l0TGFiLCBJbmMuMRIwEAYDVQQDDAlsb2NhbGhvc3QwIBcNMjQwNTEz +MDc1MzAxWhgPMjA1MTA5MjgwNzUzMDFaMFIxCzAJBgNVBAYTAkNOMQswCQYDVQQI +DAJHRDELMAkGA1UEBwwCU1oxFTATBgNVBAoMDEdpdExhYiwgSW5jLjESMBAGA1UE +AwwJbG9jYWxob3N0MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAoJ4T +OzEIw70l2EQkrpEmvUpWy0JNkotW9fsrWdgg+nbC+vj/s/7rd38crWU3ygYYlfrz +CeWg4ogwu5dv+HdvIsNKkORurYMlME5FMNUdRN9kzdoNwxHfiA1pVmnFg9O5uLAk +8+r8qgPSPIjDFoAOPl+WTaseQVXeZZHNbrNaSXuWFxdYlbSl5Ek8isWWxYSho+dI +zXoeOmhmh503IDpIfdE0bkeX7SJWhYAs8wIkGpX4Fkaeyq5SKyNv4IHibDyugtv8 +Ye9sYFLKvZytiry6y7kO8m/CDFyMpVBh3/lG5wayxUBDRWxyr8RTn3YwlvKY/iM3 +bUy4/6HPceKbzCUUbwIDAQABo1MwUTAdBgNVHQ4EFgQUtVgxsOKDyxnLpuuhg5+H +O7UGit4wHwYDVR0jBBgwFoAUtVgxsOKDyxnLpuuhg5+HO7UGit4wDwYDVR0TAQH/ +BAUwAwEB/zANBgkqhkiG9w0BAQsFAAOCAQEAb4V74iywZNhn76D9moBPR+6nNprf +HhX4uVwza1bKsIIDpTqgchAmsuo5b7RYs29Qi9nLXDKh3IxTnf0l63x6c+At/aWC +XTcC1x9+XZKuF/J+eoQQMPQdo4o24F/1Lu1cJnWbc9hHlxiuqrB2t3eSc5gffQRL +jJKGz+geTClxuMV23D6BugbJksom+JCVhWp5MRQFI4y+sDpVPgxm6AdgVAXHbYr/ +uFTiZ63PILrodaHXCJKYIR50p48fyiMqhOa2sqVAQ5qIh1C05K1JVpUlVDnZRT7E +Y+Dx9ZI8NlXpHxfiFKxcz5aCPjqZEelqAuroQiCZ8b+rErsqcV57z5ArOw== +-----END CERTIFICATE----- diff --git a/test/testdata/mutualtls/valid/client.crt b/test/testdata/mutualtls/valid/client.crt new file mode 100644 index 000000000..7f017fd5a --- /dev/null +++ b/test/testdata/mutualtls/valid/client.crt @@ -0,0 +1,22 @@ +-----BEGIN CERTIFICATE----- +MIIDkDCCAnigAwIBAgIUel32w9wyRXUoVIzTWaGVZHqMAoIwDQYJKoZIhvcNAQEL +BQAwUjELMAkGA1UEBhMCQ04xCzAJBgNVBAgMAkdEMQswCQYDVQQHDAJTWjEVMBMG +A1UECgwMR2l0TGFiLCBJbmMuMRIwEAYDVQQDDAlsb2NhbGhvc3QwHhcNMjQwNTEz +MDc1NDU1WhcNMjUwNTEzMDc1NDU1WjBSMQswCQYDVQQGEwJDTjELMAkGA1UECAwC +R0QxCzAJBgNVBAcMAlNaMRUwEwYDVQQKDAxHaXRMYWIsIEluYy4xEjAQBgNVBAMM +CWxvY2FsaG9zdDCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAJn1wZjc +wWLUqXaf+/L919l+wh0pS8BwLlfioUwn3HsshtgOe+Pw/gJY/E0jgBjgQH3yFe4Z +ViupK13LGbzT1hKx2nnZXnoOss4tw68xo960RSIbEDD/utwEK7sUne4xDbMo1v4q +ddPVeop3CjCQGwE97E6EA0N3TEvdauc53wE0sh4JoYzKANUZ+qufvbUtH7ICbVq7 +/qvAFaWL9zVXIPmXUE+1X6OpT3IUIN3nQ4fAoEnO1E4FZ06apVtX0DCBP/6L44iO +0WDch9aJHQci++vTY9aQB7G02ZMCloMTixhBxWusaN6HaXiDcNwxAZvYo5UceyLP +nEdtDM4f1e179yECAwEAAaNeMFwwGgYDVR0RBBMwEYIJbG9jYWxob3N0hwR/AAAB +MB0GA1UdDgQWBBRDoBadWPhUIkHhSdBhwzs5Y1+NzjAfBgNVHSMEGDAWgBS1WDGw +4oPLGcum66GDn4c7tQaK3jANBgkqhkiG9w0BAQsFAAOCAQEAVpPoH0f55LXTMvBN +AD3jM0xGHvEgzaBr4IxX4S5NebOU0FBn1ww2UXlSKDrotI+UOpEWFZAfpgvcKHBz +LlrOnppOlXQFNQr3cTg0HhHtCnzDmViu5XG3zBqCn0c/Atf4o5jlAhkPAj4M0aWi +I6oF/Qb69oKEg5/tBd5fU7A+VHzsXVSQYmGMlAEs7WGGfEQ7r6iJd+eXz6Lnwu8r +rOisxhA32yVtCjwH1pyAFwDQnKzbuNL14UWbUvf2WFj8/4KDFKkdCyElIVmU8yMJ +zHO3rZHnvKMyk9petIvptmqQH4RF1gmG2uSeJK23M9vVVH5tfCf8n6rYTM9pVglo +yDZi8A== +-----END CERTIFICATE----- diff --git a/test/testdata/mutualtls/valid/client.key b/test/testdata/mutualtls/valid/client.key new file mode 100644 index 000000000..97e855e26 --- /dev/null +++ b/test/testdata/mutualtls/valid/client.key @@ -0,0 +1,28 @@ +-----BEGIN PRIVATE KEY----- +MIIEvgIBADANBgkqhkiG9w0BAQEFAASCBKgwggSkAgEAAoIBAQCZ9cGY3MFi1Kl2 +n/vy/dfZfsIdKUvAcC5X4qFMJ9x7LIbYDnvj8P4CWPxNI4AY4EB98hXuGVYrqStd +yxm809YSsdp52V56DrLOLcOvMaPetEUiGxAw/7rcBCu7FJ3uMQ2zKNb+KnXT1XqK +dwowkBsBPexOhANDd0xL3WrnOd8BNLIeCaGMygDVGfqrn721LR+yAm1au/6rwBWl +i/c1VyD5l1BPtV+jqU9yFCDd50OHwKBJztROBWdOmqVbV9AwgT/+i+OIjtFg3IfW +iR0HIvvr02PWkAextNmTApaDE4sYQcVrrGjeh2l4g3DcMQGb2KOVHHsiz5xHbQzO +H9Xte/chAgMBAAECggEAAS3Z500M2RrWV798FegTnV7kZzCasSpaxyxdPmCmxmBj +Vv0YvfiURW8qMtVfr4ZrerDI3LY1IVKjb6Lfe1am/Xp/Dm8CU3kl2EDkTlpDuO// +VeEdrmENrLjbyeFrn+L1ydEOq8smSbKemeK1Vq+1LpJWFeLacIuVxMCjxzS7rGjD +G6YiQ7EJJMJ0DiDo7OdUYInNal7/VdYY+yPoTb+W43tfF8jhqqdimTMGKUuuXeWL +e8G5fNUgmkc7QCatl2GF4wBRUQCjx9UIp080GchQ1dQPjtxLm6/6qdKr3IEYlHne +OkMCRjOS5+98AwKqq7U7G1LKzhFMkquhlKwS0t7GcQKBgQDVRWyGlpJoSePsUZYX +BHyJ6NyLsqh7RKF3Yz4HdJJQe9vi8/LXJFU7En+zWKb7JWfMYwt4tESNEGQ3Oj2y +Ama71QKm+XvRhYz/hfkVglQRQeUDBZMiYvOxzZVyUfX4BxnXFqFdVSgbc7ESfo1T +guSOtvkfDNsQVJ5R9jzYltDmKQKBgQC4zkoY4hZbZnsiVEZnaYk2Addmj++esKSO +PcVbrO57s8qwu3Ln8VbtGW28SXGW2VFH3igCcO8Ai2YlrYc6lcEfNBmsGG0XKb1e +ki0gY/7X433i4BvlnSwAN1Xs49Kusz/Mml+m0q8HivoYETYu3Nxfu0tKTkMbXyFm +Jg6vgBn4OQKBgQDGRBoWLNjC9x5azaYYk+UrWD3f6SFUJ4NsN+isiaSUCfFrVZqG +g5JwrkvlcR8bD7Ulf1ZkykGIWpqv9Qbx++WB7Q7gJ8MCD4P68JOVeWmp+XZrjr0w +FIm03Ah5FNTz1bYiDTnKSKZWjwEozlmYL3FHc7a5NPxafDAKxj3epKZjsQKBgGap +AfRss6q2dTSOyEVuFPDReQzabGwlCGST3+ybVieVqsUefChoorc3ZwQvcFAyDLr1 +qBgjEEGnLmlDylk7E3r4AELflspFP5MndLYHlmvrTeUYRab59pVwJ+VecYzmukw4 +fWY4p05zX5a7CPRjcHAlpR9z9kdgQzdxcLsBWGvRAoGBAIWZ8JFxf8FKtbqb6Xls +tvLDb8bJ+ofdbzZYFTKhGjFeFWopE5mZwqFaMvqOEAWfm1mEl0YATqylX0AWeUTg +2x4/+IYrJdQcSvmki1eesKt9WhqsRkfuoakuHmaXtjgf9kkqTsdUNSHxZLhoFxDY +3nfEsRGo+Ch0fze+GMJHv0vu +-----END PRIVATE KEY----- -- GitLab From c32a127eae35e54d085927582298620eef60f5dc Mon Sep 17 00:00:00 2001 From: ngala Date: Mon, 13 May 2024 14:07:35 +0530 Subject: [PATCH 09/11] Add steps to generate certificate --- test/testdata/mutualtls/valid/README.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 test/testdata/mutualtls/valid/README.md diff --git a/test/testdata/mutualtls/valid/README.md b/test/testdata/mutualtls/valid/README.md new file mode 100644 index 000000000..1ad56fc50 --- /dev/null +++ b/test/testdata/mutualtls/valid/README.md @@ -0,0 +1,15 @@ +## Steps to generate certificates in terminal for mutual TLS: + +### Generate CA certificate: +``` +openssl genrsa -out ca.key 2048 +openssl req -new -x509 -days 9999 -key ca.key -subj "/C=CN/ST=GD/L=SZ/O=GitLab, Inc./CN=localhost" -out ca.crt +``` + +### Generate client certificate and sign using above generated CA certificate: +``` +echo "subjectAltName = DNS:localhost,IP:127.0.0.1" > test.txt + +openssl req -newkey rsa:2048 -nodes -keyout client.key -subj "/C=CN/ST=GD/L=SZ/O=GitLab, Inc./CN=localhost" -out client.csr +openssl x509 -req -extfile test.txt -days 365 -in client.csr -CA ca.crt -CAkey ca.key -CAcreateserial -out client.crt +``` \ No newline at end of file -- GitLab From b8156e7c1ebe5e6b9870bfe11b9dbf077be00736 Mon Sep 17 00:00:00 2001 From: ngala Date: Mon, 13 May 2024 15:36:53 +0530 Subject: [PATCH 10/11] Add mTLS acceptance test for auth --- test/acceptance/auth_test.go | 50 +++++++++++++++++++ test/acceptance/gitlab_api_mutual_tls_test.go | 2 +- test/acceptance/helpers_test.go | 8 ++- 3 files changed, 57 insertions(+), 3 deletions(-) diff --git a/test/acceptance/auth_test.go b/test/acceptance/auth_test.go index 022afcbd1..c6e8f871e 100644 --- a/test/acceptance/auth_test.go +++ b/test/acceptance/auth_test.go @@ -635,6 +635,56 @@ func TestHijackedCode(t *testing.T) { require.Equal(t, impersonatingRes.StatusCode, http.StatusInternalServerError, "should fail to decode code") } +func TestWhenAuthIsEnabledPrivateWillRedirectToAuthorizeWithValidMTLS(t *testing.T) { + clientCertPath := "../../test/testdata/mutualtls/valid/client.crt" + clientKeyPath := "../../test/testdata/mutualtls/valid/client.key" + caCertPath := "../../test/testdata/mutualtls/valid/ca.crt" + + RunPagesProcessWithMutualTLS(t, httpsListener, defaultAuthConfig(t), clientCertPath, clientKeyPath, caCertPath) + + rsp, err := GetRedirectPage(t, httpsListener, "group.auth.gitlab-example.com", "private.project/") + + require.NoError(t, err) + testhelpers.Close(t, rsp.Body) + + require.Equal(t, http.StatusFound, rsp.StatusCode) + require.Len(t, rsp.Header["Location"], 1) + url, err := url.Parse(rsp.Header.Get("Location")) + require.NoError(t, err) + rsp, err = GetRedirectPage(t, httpsListener, url.Host, url.Path+"?"+url.RawQuery) + require.NoError(t, err) + testhelpers.Close(t, rsp.Body) + + require.Equal(t, http.StatusFound, rsp.StatusCode) + require.Len(t, rsp.Header["Location"], 1) + + url, err = url.Parse(rsp.Header.Get("Location")) + require.NoError(t, err) + + require.Equal(t, "https", url.Scheme) + require.Equal(t, "public-gitlab-auth.com", url.Host) + require.Equal(t, "/oauth/authorize", url.Path) + require.Equal(t, "clientID", url.Query().Get("client_id")) + require.Equal(t, "https://projects.gitlab-example.com/auth", url.Query().Get("redirect_uri")) + require.NotEmpty(t, url.Query().Get("scope")) + require.NotEmpty(t, url.Query().Get("state")) +} + +func TestWhenAuthIsEnabledPrivateWillRedirectToAuthorizeWithInvalidMTLS(t *testing.T) { + clientCertPath := "../../test/testdata/mutualtls/invalid/client.crt" + clientKeyPath := "../../test/testdata/mutualtls/invalid/client.key" + caCertPath := "../../test/testdata/mutualtls/invalid/ca.crt" + + RunPagesProcessWithMutualTLS(t, httpsListener, defaultAuthConfig(t), clientCertPath, clientKeyPath, caCertPath) + + rsp, err := GetRedirectPage(t, httpsListener, "group.auth.gitlab-example.com", "private.project/") + + require.NoError(t, err) + testhelpers.Close(t, rsp.Body) + + require.Equal(t, http.StatusBadGateway, rsp.StatusCode) +} + func getValidCookieAndState(t *testing.T, domain string) (string, string) { t.Helper() diff --git a/test/acceptance/gitlab_api_mutual_tls_test.go b/test/acceptance/gitlab_api_mutual_tls_test.go index b74b3bcb8..151e5e830 100644 --- a/test/acceptance/gitlab_api_mutual_tls_test.go +++ b/test/acceptance/gitlab_api_mutual_tls_test.go @@ -44,7 +44,7 @@ func TestGitLabAPIMutualTLS(t *testing.T) { for _, tt := range tests { tt := tt t.Run(tt.name, func(t *testing.T) { - RunPagesProcessWithMutualTLS(t, httpsListener, tt.clientCertPath, tt.clientKeyPath, tt.caCertPath) + RunPagesProcessWithMutualTLS(t, httpsListener, "", tt.clientCertPath, tt.clientKeyPath, tt.caCertPath) rsp, err := GetPageFromListener(t, httpsListener, tt.host, tt.path) if tt.expectError { diff --git a/test/acceptance/helpers_test.go b/test/acceptance/helpers_test.go index 8bd227af6..f3b02f1c0 100644 --- a/test/acceptance/helpers_test.go +++ b/test/acceptance/helpers_test.go @@ -335,20 +335,24 @@ func RunPagesProcessWithSSLCertDir(t *testing.T, listeners []ListenSpec, sslCert ) } -func RunPagesProcessWithMutualTLS(t *testing.T, listener ListenSpec, clientCertPath, clientKeyPath, caCertPath string) { +func RunPagesProcessWithMutualTLS(t *testing.T, listener ListenSpec, configFile string, clientCertPath, clientKeyPath, caCertPath string) { clientCert, err := tls.LoadX509KeyPair(clientCertPath, clientKeyPath) require.NoError(t, err) caCert, err := loadCACertificate(caCertPath) require.NoError(t, err) - configFile := newConfigFile(t, "client-cert-key-pairs="+clientCertPath+":"+clientKeyPath, "ca-certs="+caCertPath) + if configFile == "" { + configFile = newConfigFile(t) + } RunPagesProcess(t, withListeners([]ListenSpec{listener}), withArguments([]string{ "-config=" + configFile, }), + withExtraArgument("client-cert-key-pairs", clientCertPath+":"+clientKeyPath), + withExtraArgument("ca-certs", caCertPath), withStubOptions(gitlabstub.WithCertificate(clientCert), gitlabstub.WithMutualTLS(caCert)), ) } -- GitLab From 56f9e8f4d02a33b79c6044e0e95717c8e6ec0bd4 Mon Sep 17 00:00:00 2001 From: ngala Date: Mon, 13 May 2024 18:24:05 +0530 Subject: [PATCH 11/11] Add mTLS acceptance test for artifacts --- internal/httptransport/transport.go | 3 +- test/acceptance/artifacts_test.go | 158 ++++++++++++++++++++++++++++ 2 files changed, 159 insertions(+), 2 deletions(-) diff --git a/internal/httptransport/transport.go b/internal/httptransport/transport.go index 04af87995..2abffba19 100644 --- a/internal/httptransport/transport.go +++ b/internal/httptransport/transport.go @@ -44,8 +44,7 @@ func NewTransport() *http.Transport { DialTLS: func(network, addr string) (net.Conn, error) { return tls.Dial(network, addr, &tls.Config{RootCAs: pool(), MinVersion: tls.VersionTLS12}) }, - TLSClientConfig: &tls.Config{MinVersion: tls.VersionTLS12}, // set MinVersion to fix gosec: G402 - Proxy: http.ProxyFromEnvironment, + Proxy: http.ProxyFromEnvironment, // overrides the DefaultMaxIdleConnsPerHost = 2 MaxIdleConns: 100, MaxIdleConnsPerHost: 100, diff --git a/test/acceptance/artifacts_test.go b/test/acceptance/artifacts_test.go index c88f48bf4..6a58c8009 100644 --- a/test/acceptance/artifacts_test.go +++ b/test/acceptance/artifacts_test.go @@ -266,3 +266,161 @@ func TestPrivateArtifactProxyRequest(t *testing.T) { }) } } + +type testCase struct { + name string + host string + path string + status int + content string + length int64 + cacheControl string + contentType string +} + +func testArtifactProxyRequestWithMTLS(t *testing.T, content string, tests []testCase, clientCertPath, clientKeyPath, caCertPath string) { + t.Helper() + + testServer := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.RawPath { + case "/api/v4/projects/group%2Fproject/jobs/1/artifacts/200.html", + "/api/v4/projects/group%2Fsubgroup%2Fproject/jobs/1/artifacts/200.html": + w.Header().Set("Content-Type", "text/html; charset=utf-8") + fmt.Fprint(w, content) + default: + t.Logf("Unexpected r.URL.RawPath: %q", r.URL.RawPath) + w.Header().Set("Content-Type", "text/html; charset=utf-8") + w.WriteHeader(http.StatusNotFound) + fmt.Fprint(w, content) + } + })) + + keyFile, certFile := CreateHTTPSFixtureFiles(t) + serverCert, err := tls.LoadX509KeyPair(certFile, keyFile) + require.NoError(t, err) + + tlsCfg := &tls.Config{ + ClientCAs: testhelpers.CertPool(t, caCertPath), + ClientAuth: tls.RequireAndVerifyClientCert, + Certificates: []tls.Certificate{serverCert, testhelpers.Cert(t, clientCertPath, clientKeyPath)}, + MinVersion: tls.VersionTLS12, + } + + testServer.TLS = tlsCfg + testServer.StartTLS() + + t.Cleanup(func() { + testServer.Close() + }) + + // Ensure the IP address is used in the URL, as we're relying on IP SANs to + // validate + artifactServerURL := testServer.URL + "/api/v4" + t.Log("Artifact server URL", artifactServerURL) + + args := []string{"-artifacts-server=" + artifactServerURL, "-artifacts-server-timeout=1"} + + t.Setenv("SSL_CERT_FILE", certFile) + + clientCert, err := tls.LoadX509KeyPair(clientCertPath, clientKeyPath) + require.NoError(t, err) + + caCert, err := loadCACertificate(caCertPath) + require.NoError(t, err) + + RunPagesProcess(t, + withListeners([]ListenSpec{httpListener}), + withArguments(args), + withExtraArgument("client-cert-key-pairs", clientCertPath+":"+clientKeyPath), + withExtraArgument("ca-certs", caCertPath), + withStubOptions(gitlabstub.WithCertificate(clientCert), gitlabstub.WithMutualTLS(caCert)), + ) + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + resp, err := GetPageFromListener(t, httpListener, tt.host, tt.path) + require.NoError(t, err) + testhelpers.Close(t, resp.Body) + + require.Equal(t, tt.status, resp.StatusCode) + require.Equal(t, tt.contentType, resp.Header.Get("Content-Type")) + + if tt.status == http.StatusOK { + body, err := io.ReadAll(resp.Body) + require.NoError(t, err) + require.Equal(t, tt.content, string(body)) + require.Equal(t, tt.length, resp.ContentLength) + require.Equal(t, tt.cacheControl, resp.Header.Get("Cache-Control")) + } + }) + } +} + +func TestArtifactProxyRequestWithValidMTLS(t *testing.T) { + content := "Title of the document" + contentLength := int64(len(content)) + + clientCertPath := "../../test/testdata/mutualtls/valid/client.crt" + clientKeyPath := "../../test/testdata/mutualtls/valid/client.key" + caCertPath := "../../test/testdata/mutualtls/valid/ca.crt" + + tests := []testCase{ + { + name: "basic proxied request", + host: "group.gitlab-example.com", + path: "/-/project/-/jobs/1/artifacts/200.html", + status: http.StatusOK, + content: content, + length: contentLength, + cacheControl: "max-age=3600", + contentType: "text/html; charset=utf-8", + }, + { + name: "basic proxied request for subgroup", + host: "group.gitlab-example.com", + path: "/-/subgroup/project/-/jobs/1/artifacts/200.html", + status: http.StatusOK, + content: content, + length: contentLength, + cacheControl: "max-age=3600", + contentType: "text/html; charset=utf-8", + }, + } + + testArtifactProxyRequestWithMTLS(t, content, tests, clientCertPath, clientKeyPath, caCertPath) +} + +func TestArtifactProxyRequestWithInvalidMTLS(t *testing.T) { + content := "Title of the document" + + clientCertPath := "../../test/testdata/mutualtls/invalid/client.crt" + clientKeyPath := "../../test/testdata/mutualtls/invalid/client.key" + caCertPath := "../../test/testdata/mutualtls/invalid/ca.crt" + + tests := []testCase{ + { + name: "basic proxied request", + host: "group.gitlab-example.com", + path: "/-/project/-/jobs/1/artifacts/200.html", + status: http.StatusBadGateway, + content: "", + length: 0, + cacheControl: "", + contentType: "text/html; charset=utf-8", + }, + { + name: "basic proxied request for subgroup", + host: "group.gitlab-example.com", + path: "/-/subgroup/project/-/jobs/1/artifacts/200.html", + status: http.StatusBadGateway, + content: "", + length: 0, + cacheControl: "", + contentType: "text/html; charset=utf-8", + }, + } + testArtifactProxyRequestWithMTLS(t, content, tests, clientCertPath, clientKeyPath, caCertPath) +} -- GitLab