Add a feature flag to disable resolving of TLS chain

In the past, the runner needed to resolve a full TLS certificate
chain, including the self-signed root, in order for Git clones to work
over HTTPS. Go 1.9 changed the behavior to present a partial
certificate chain if a trusted intermediate certificate were placed in
the system certificate directory
(https://github.com/golang/go/issues/24685).
!1581
worked around that change by restoring the Go 1.8 behavior of
presenting the full chain in `CI_SERVER_TLS_CA_FILE`.

libcurl v7.68 has since fixed the behavior to trust a certificate
authority that is not self-signed
(https://github.com/curl/curl/commit/94f1f771586913addf5c68f9219e176036c50115).
As a result, the need to resolve the full chain is no longer
necessary. As long as there is a trusted certificate authority in the
chain, the TLS connection can proceed.

Go 1.18 modified `Certificate.Verify` to use the macOS and
Windows-specific platform APIs. As a result, a root certificate signed
with a SHA-1 certificate will be rejected, which prevents the runner
from generating `CI_SERVER_TLS_CA_FILE`. This may cause Git clones to
fail.

This commit adds a feature flag, `FF_RESOLVE_FULL_TLS_CHAIN`, that is
enabled by default. This flag makes it possible to disable this
resolving of the full certificate chain. On most platforms, this can
be disabled safely, assuming Git and other clients are compiled with
an updated libcurl version.

Relates to #29373

docs/configuration/feature-flags.md

@@ -63,6 +63,7 @@ The flags are defined in `./helpers/featureflags/flags.go` file.
 | `FF_KUBERNETES_HONOR_ENTRYPOINT` | `false` | **{dotted-circle}** No |  | When enabled, the Docker entrypoint of an image will be honored if `FF_USE_LEGACY_KUBERNETES_EXECUTION_STRATEGY` is not set to true |
 | `FF_POSIXLY_CORRECT_ESCAPES` | `false` | **{dotted-circle}** No |  | When enabled, [POSIX shell escapes](https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_02) are used rather than [`bash`-style ANSI-C quoting](https://www.gnu.org/software/bash/manual/html_node/Quoting.html). This should be enabled if the job environment uses a POSIX-compliant shell. |
 | `FF_USE_IMPROVED_URL_MASKING` | `false` | **{dotted-circle}** No |  | When enabled, any sensitive URL parameters are masked no matter where they appear in the trace log output. When this is disabled, sensitive URL parameters are only masked in select places and can [occasionally be revealed](https://gitlab.com/gitlab-org/gitlab-runner/-/issues/4625). This feature flag can only be configured via Runner's config and not from a job. |
+| `FF_RESOLVE_FULL_TLS_CHAIN` | `true` | **{dotted-circle}** No |  | When enabled, the runner resolves a full TLS chain all the way down to a self-signed root certificate for `CI_SERVER_TLS_CA_FILE`. This was previously [required to make Git HTTPS clones to work](tls-self-signed.md#git-cloning) for a Git client built with libcurl prior to v7.68.0 and OpenSSL. However, the process to resolve certificates may fail on some operating systems, such as macOS, that reject root certificates signed with older signature algorithms. If certificate resolution fails, you may need to disable this feature. This feature flag can only be disabled in the [`[runners.feature_flags]` configuration](#enable-feature-flag-in-runner-configuration). |
 
 <!-- feature_flags_list_end -->
 

helpers/featureflags/flags.go

@@ -25,6 +25,7 @@ const (
 	KubernetesHonorEntrypoint            string = "FF_KUBERNETES_HONOR_ENTRYPOINT"
 	PosixlyCorrectEscapes                string = "FF_POSIXLY_CORRECT_ESCAPES"
 	UseImprovedURLMasking                string = "FF_USE_IMPROVED_URL_MASKING"
+	ResolveFullTLSChain                  string = "FF_RESOLVE_FULL_TLS_CHAIN"
 )
 
 type FeatureFlag struct {
@@ -196,6 +197,21 @@ var flags = []FeatureFlag{
 			"[occasionally be revealed](https://gitlab.com/gitlab-org/gitlab-runner/-/issues/4625). This feature " +
 			"flag can only be configured via Runner's config and not from a job.",
 	},
+	{
+		Name:         ResolveFullTLSChain,
+		DefaultValue: true,
+		Deprecated:   false,
+		Description: "When enabled, the runner resolves a full TLS " +
+			"chain all the way down to a self-signed root certificate " +
+			"for `CI_SERVER_TLS_CA_FILE`. This was previously " +
+			"[required to make Git HTTPS clones to work](tls-self-signed.md#git-cloning) " +
+			"for a Git client built with libcurl prior to v7.68.0 and OpenSSL. " +
+			"However, the process to resolve certificates may fail on " +
+			"some operating systems, such as macOS, that reject root certificates " +
+			"signed with older signature algorithms. " +
+			"If certificate resolution fails, you may need to disable this feature. " +
+			"This feature flag can only be disabled in the [`[runners.feature_flags]` configuration](#enable-feature-flag-in-runner-configuration).",
+	},
 }
 
 func GetAll() []FeatureFlag {

helpers/tls/ca_chain/builder.go

@@ -25,7 +25,7 @@ type Builder interface {
 	BuildChainFromTLSConnectionState(TLS *tls.ConnectionState) error
 }
 
-func NewBuilder(logger logrus.FieldLogger) Builder {
+func NewBuilder(logger logrus.FieldLogger, resolveFullChain bool) Builder {
 	logger = logger.
 		WithField("context", "certificate-chain-build")
 
@@ -36,14 +36,16 @@ func NewBuilder(logger logrus.FieldLogger) Builder {
 			newURLResolver(logger),
 			newVerifyResolver(logger),
 		),
-		encodePEM: pem.Encode,
-		logger:    logger,
+		encodePEM:        pem.Encode,
+		logger:           logger,
+		resolveFullChain: resolveFullChain,
 	}
 }
 
 type defaultBuilder struct {
 	certificates     []*x509.Certificate
 	seenCertificates map[string]bool
+	resolveFullChain bool
 
 	resolver  resolver
 	encodePEM pemEncoder
@@ -54,8 +56,10 @@ type defaultBuilder struct {
 func (b *defaultBuilder) BuildChainFromTLSConnectionState(tls *tls.ConnectionState) error {
 	for _, verifiedChain := range tls.VerifiedChains {
 		b.logger.
-			WithField("chain-leaf", fmt.Sprintf("%v", verifiedChain)).
-			Debug("Processing chain")
+			WithFields(logrus.Fields{
+				"chain-leaf":         fmt.Sprintf("%v", verifiedChain),
+				"resolve-full-chain": b.resolveFullChain,
+			}).Debug("Processing chain")
 		err := b.fetchCertificatesFromVerifiedChain(verifiedChain)
 		if err != nil {
 			return fmt.Errorf("error while fetching certificates into the CA Chain: %w", err)
@@ -72,9 +76,11 @@ func (b *defaultBuilder) fetchCertificatesFromVerifiedChain(verifiedChain []*x50
 		return nil
 	}
 
-	verifiedChain, err = b.resolver.Resolve(verifiedChain)
-	if err != nil {
-		return fmt.Errorf("couldn't resolve certificates chain from the leaf certificate: %w", err)
+	if b.resolveFullChain {
+		verifiedChain, err = b.resolver.Resolve(verifiedChain)
+		if err != nil {
+			return fmt.Errorf("couldn't resolve certificates chain from the leaf certificate: %w", err)
+		}
 	}
 
 	for _, certificate := range verifiedChain {

helpers/tls/ca_chain/builder_test.go

@@ -131,15 +131,18 @@ func TestDefaultBuilder_BuildChainFromTLSConnectionState(t *testing.T) {
 	tests := map[string]struct {
 		chains              [][]*x509.Certificate
 		setupResolverMock   func(t *testing.T) (resolver, func())
+		resolveFullChain    bool
 		expectedError       string
 		expectedChainLength int
 	}{
 		"no chains": {
 			chains:              [][]*x509.Certificate{},
+			resolveFullChain:    true,
 			expectedChainLength: 0,
 		},
 		"empty chain": {
 			chains:              [][]*x509.Certificate{{}},
+			resolveFullChain:    true,
 			expectedChainLength: 0,
 		},
 		"error on chain resolving": {
@@ -157,6 +160,7 @@ func TestDefaultBuilder_BuildChainFromTLSConnectionState(t *testing.T) {
 
 				return mock, cleanup
 			},
+			resolveFullChain: true,
 			expectedError: "error while fetching certificates into the CA Chain: couldn't resolve certificates " +
 				"chain from the leaf certificate: test-error",
 			expectedChainLength: 0,
@@ -176,15 +180,21 @@ func TestDefaultBuilder_BuildChainFromTLSConnectionState(t *testing.T) {
 
 				return mock, cleanup
 			},
+			resolveFullChain:    true,
 			expectedChainLength: 2,
 		},
+		"certificates chain with resolve disabled": {
+			chains:              [][]*x509.Certificate{{testCertificate}},
+			resolveFullChain:    false,
+			expectedChainLength: 1,
+		},
 	}
 
 	for tn, tc := range tests {
 		t.Run(tn, func(t *testing.T) {
 			var err error
 
-			builder := NewBuilder(logrus.StandardLogger()).(*defaultBuilder)
+			builder := NewBuilder(logrus.StandardLogger(), tc.resolveFullChain).(*defaultBuilder)
 
 			if tc.setupResolverMock != nil {
 				resolverMock, cleanup := tc.setupResolverMock(t)
@@ -214,7 +224,7 @@ func TestDefaultBuilder_addCertificate(t *testing.T) {
 	testCertificate, err := x509.ParseCertificate(block.Bytes)
 	require.NoError(t, err)
 
-	b := NewBuilder(logrus.StandardLogger()).(*defaultBuilder)
+	b := NewBuilder(logrus.StandardLogger(), true).(*defaultBuilder)
 	b.addCertificate(testCertificate)
 	b.addCertificate(testCertificate)
 
@@ -267,7 +277,7 @@ func TestDefaultBuilder_String(t *testing.T) {
 			logger := logrus.New()
 			logger.Out = out
 
-			b := NewBuilder(logger).(*defaultBuilder)
+			b := NewBuilder(logger, true).(*defaultBuilder)
 			b.encodePEM = tc.encodePEMMock
 
 			b.addCertificate(testCertificate)

network/client.go

@@ -461,13 +461,13 @@ func getMessageFromJSONOrXMLResponse(res *http.Response) string {
 	return res.Status
 }
 
-func (n *client) getResponseTLSData(tls *tls.ConnectionState) (ResponseTLSData, error) {
+func (n *client) getResponseTLSData(tls *tls.ConnectionState, resolveFullChain bool) (ResponseTLSData, error) {
 	TLSData := ResponseTLSData{
 		CertFile: n.certFile,
 		KeyFile:  n.keyFile,
 	}
 
-	caChain, err := n.buildCAChain(tls)
+	caChain, err := n.buildCAChain(tls, resolveFullChain)
 	if err != nil {
 		return TLSData, fmt.Errorf("couldn't build CA Chain: %w", err)
 	}
@@ -477,7 +477,7 @@ func (n *client) getResponseTLSData(tls *tls.ConnectionState) (ResponseTLSData,
 	return TLSData, nil
 }
 
-func (n *client) buildCAChain(tls *tls.ConnectionState) (string, error) {
+func (n *client) buildCAChain(tls *tls.ConnectionState, resolveFullChain bool) (string, error) {
 	if len(n.caData) != 0 {
 		return string(n.caData), nil
 	}
@@ -486,7 +486,7 @@ func (n *client) buildCAChain(tls *tls.ConnectionState) (string, error) {
 		return "", nil
 	}
 
-	builder := ca_chain.NewBuilder(logrus.StandardLogger())
+	builder := ca_chain.NewBuilder(logrus.StandardLogger(), resolveFullChain)
 	err := builder.BuildChainFromTLSConnectionState(tls)
 	if err != nil {
 		return "", fmt.Errorf("error while fetching certificates from TLS ConnectionState: %w", err)

network/client_test.go

@@ -326,7 +326,7 @@ func TestClientTLSCAFile(t *testing.T) {
 	)
 	assert.Equal(t, http.StatusOK, statusCode, statusText)
 
-	tlsData, err := c.getResponseTLSData(resp.TLS)
+	tlsData, err := c.getResponseTLSData(resp.TLS, true)
 	assert.NoError(t, err)
 	assert.NotEmpty(t, tlsData.CAChain)
 }
@@ -360,7 +360,7 @@ func TestClientCertificateInPredefinedDirectory(t *testing.T) {
 	)
 	assert.Equal(t, http.StatusOK, statusCode, statusText)
 
-	tlsData, err := c.getResponseTLSData(resp.TLS)
+	tlsData, err := c.getResponseTLSData(resp.TLS, true)
 	assert.NoError(t, err)
 	assert.NotEmpty(t, tlsData.CAChain)
 }
@@ -443,7 +443,7 @@ func TestClientTLSAuth(t *testing.T) {
 	)
 	assert.Equal(t, http.StatusOK, statusCode, statusText)
 
-	tlsData, err := c.getResponseTLSData(resp.TLS)
+	tlsData, err := c.getResponseTLSData(resp.TLS, true)
 	assert.NoError(t, err)
 	assert.NotEmpty(t, tlsData.CAChain)
 	assert.Equal(t, cert.Name(), tlsData.CertFile)
@@ -489,7 +489,7 @@ func TestClientTLSAuthCertificatesInPredefinedDirectory(t *testing.T) {
 	)
 	assert.Equal(t, http.StatusOK, statusCode, statusText)
 
-	tlsData, err := c.getResponseTLSData(resp.TLS)
+	tlsData, err := c.getResponseTLSData(resp.TLS, true)
 	assert.NoError(t, err)
 	assert.NotEmpty(t, tlsData.CAChain)
 	assert.NotEmpty(t, tlsData.CertFile)

network/gitlab.go

@@ -19,6 +19,7 @@ import (
 
 	"gitlab.com/gitlab-org/gitlab-runner/common"
 	"gitlab.com/gitlab-org/gitlab-runner/helpers"
+	"gitlab.com/gitlab-org/gitlab-runner/helpers/featureflags"
 )
 
 const clientError = -100
@@ -215,6 +216,7 @@ func (n *GitLabClient) doJSONWithPAT(
 
 func (n *GitLabClient) getResponseTLSData(
 	credentials requestCredentials,
+	resolveFullChain bool,
 	response *http.Response,
 ) (ResponseTLSData, error) {
 	c, err := n.getClient(credentials)
@@ -222,7 +224,7 @@ func (n *GitLabClient) getResponseTLSData(
 		return ResponseTLSData{}, fmt.Errorf("couldn't get client: %w", err)
 	}
 
-	return c.getResponseTLSData(response.TLS)
+	return c.getResponseTLSData(response.TLS, resolveFullChain)
 }
 
 func (n *GitLabClient) RegisterRunner(
@@ -434,7 +436,8 @@ func (n *GitLabClient) RequestJob(
 			"repo_url": response.RepoCleanURL(),
 		}).Println("Checking for jobs...", "received")
 
-		tlsData, err := n.getResponseTLSData(&config.RunnerCredentials, httpResponse)
+		resolveFullChain := config.IsFeatureFlagOn(featureflags.ResolveFullTLSChain)
+		tlsData, err := n.getResponseTLSData(&config.RunnerCredentials, resolveFullChain, httpResponse)
 		if err != nil {
 			config.Log().
 				WithError(err).Errorln("Error on fetching TLS Data from API response...", "error")