Merge branch 'sh-trusted-cidrs-workhorse' into 'master'

Add support for propagation correlation IDs from trusted CIDRs

See merge request gitlab-org/gitlab!66715

workhorse/config.toml.example

 # alt_document_root = '/home/git/public/assets'
 # shutdown_timeout = "60s"
+# trusted_cidrs_for_x_forwarded_for = []
+# trusted_cidrs_for_propagation = []
 
 [redis]
 URL = "unix:/home/git/gitlab/redis/redis.socket"

workhorse/config_test.go

@@ -30,6 +30,9 @@ func TestConfigFile(t *testing.T) {
 
 	data := `
 shutdown_timeout = "60s"
+trusted_cidrs_for_x_forwarded_for = ["127.0.0.1/8", "192.168.0.1/8"]
+trusted_cidrs_for_propagation = ["10.0.0.1/8"]
+
 [redis]
 password = "redis password"
 [object_storage]
@@ -51,6 +54,8 @@ max_scaler_procs = 123
 	require.Equal(t, "redis password", cfg.Redis.Password)
 	require.Equal(t, "test provider", cfg.ObjectStorageCredentials.Provider)
 	require.Equal(t, uint32(123), cfg.ImageResizerConfig.MaxScalerProcs, "image resizer max_scaler_procs")
+	require.Equal(t, []string{"127.0.0.1/8", "192.168.0.1/8"}, cfg.TrustedCIDRsForXForwardedFor)
+	require.Equal(t, []string{"10.0.0.1/8"}, cfg.TrustedCIDRsForPropagation)
 	require.Equal(t, 60*time.Second, cfg.ShutdownTimeout.Duration)
 }
 

workhorse/go.mod

@@ -7,7 +7,7 @@ require (
 	github.com/BurntSushi/toml v0.3.1
 	github.com/FZambia/sentinel v1.0.0
 	github.com/alecthomas/chroma v0.7.3
-	github.com/aws/aws-sdk-go v1.36.1
+	github.com/aws/aws-sdk-go v1.37.0
 	github.com/certifi/gocertifi v0.0.0-20200922220541-2c3bb06c6054 // indirect
 	github.com/dgrijalva/jwt-go v3.2.0+incompatible
 	github.com/disintegration/imaging v1.6.2
@@ -29,7 +29,7 @@ require (
 	github.com/smartystreets/goconvey v1.6.4
 	github.com/stretchr/testify v1.7.0
 	gitlab.com/gitlab-org/gitaly/v14 v14.0.0-rc1
-	gitlab.com/gitlab-org/labkit v1.4.0
+	gitlab.com/gitlab-org/labkit v1.6.0
 	gocloud.dev v0.21.1-0.20201223184910-5094f54ed8bb
 	golang.org/x/exp v0.0.0-20200331195152-e8c3332aa8e5 // indirect
 	golang.org/x/image v0.0.0-20191009234506-e7c1f5e7dbb8
@@ -37,5 +37,6 @@ require (
 	golang.org/x/net v0.0.0-20210316092652-d523dce5a7f4
 	golang.org/x/tools v0.1.0
 	google.golang.org/grpc v1.37.0
+	gopkg.in/DataDog/dd-trace-go.v1 v1.31.0 // indirect
 	honnef.co/go/tools v0.1.3
 )

workhorse/internal/config/config.go

@@ -85,25 +85,27 @@ type ImageResizerConfig struct {
 }
 
 type Config struct {
-	Redis                    *RedisConfig             `toml:"redis"`
-	Backend                  *url.URL                 `toml:"-"`
-	CableBackend             *url.URL                 `toml:"-"`
-	Version                  string                   `toml:"-"`
-	DocumentRoot             string                   `toml:"-"`
-	DevelopmentMode          bool                     `toml:"-"`
-	Socket                   string                   `toml:"-"`
-	CableSocket              string                   `toml:"-"`
-	ProxyHeadersTimeout      time.Duration            `toml:"-"`
-	APILimit                 uint                     `toml:"-"`
-	APIQueueLimit            uint                     `toml:"-"`
-	APIQueueTimeout          time.Duration            `toml:"-"`
-	APICILongPollingDuration time.Duration            `toml:"-"`
-	ObjectStorageConfig      ObjectStorageConfig      `toml:"-"`
-	ObjectStorageCredentials ObjectStorageCredentials `toml:"object_storage"`
-	PropagateCorrelationID   bool                     `toml:"-"`
-	ImageResizerConfig       ImageResizerConfig       `toml:"image_resizer"`
-	AltDocumentRoot          string                   `toml:"alt_document_root"`
-	ShutdownTimeout          TomlDuration             `toml:"shutdown_timeout"`
+	Redis                        *RedisConfig             `toml:"redis"`
+	Backend                      *url.URL                 `toml:"-"`
+	CableBackend                 *url.URL                 `toml:"-"`
+	Version                      string                   `toml:"-"`
+	DocumentRoot                 string                   `toml:"-"`
+	DevelopmentMode              bool                     `toml:"-"`
+	Socket                       string                   `toml:"-"`
+	CableSocket                  string                   `toml:"-"`
+	ProxyHeadersTimeout          time.Duration            `toml:"-"`
+	APILimit                     uint                     `toml:"-"`
+	APIQueueLimit                uint                     `toml:"-"`
+	APIQueueTimeout              time.Duration            `toml:"-"`
+	APICILongPollingDuration     time.Duration            `toml:"-"`
+	ObjectStorageConfig          ObjectStorageConfig      `toml:"-"`
+	ObjectStorageCredentials     ObjectStorageCredentials `toml:"object_storage"`
+	PropagateCorrelationID       bool                     `toml:"-"`
+	ImageResizerConfig           ImageResizerConfig       `toml:"image_resizer"`
+	AltDocumentRoot              string                   `toml:"alt_document_root"`
+	ShutdownTimeout              TomlDuration             `toml:"shutdown_timeout"`
+	TrustedCIDRsForXForwardedFor []string                 `toml:"trusted_cidrs_for_x_forwarded_for"`
+	TrustedCIDRsForPropagation   []string                 `toml:"trusted_cidrs_for_propagation"`
 }
 
 var DefaultImageResizerConfig = ImageResizerConfig{

workhorse/internal/upstream/upstream.go

@@ -87,6 +87,12 @@ func newUpstream(cfg config.Config, accessLogger *logrus.Logger, routesCallback
 	if cfg.PropagateCorrelationID {
 		correlationOpts = append(correlationOpts, correlation.WithPropagation())
 	}
+	if cfg.TrustedCIDRsForPropagation != nil {
+		correlationOpts = append(correlationOpts, correlation.WithCIDRsTrustedForPropagation(cfg.TrustedCIDRsForPropagation))
+	}
+	if cfg.TrustedCIDRsForXForwardedFor != nil {
+		correlationOpts = append(correlationOpts, correlation.WithCIDRsTrustedForXForwardedFor(cfg.TrustedCIDRsForXForwardedFor))
+	}
 
 	handler := correlation.InjectCorrelationID(&up, correlationOpts...)
 	// TODO: move to LabKit https://gitlab.com/gitlab-org/gitlab/-/issues/324823

workhorse/main.go

@@ -152,6 +152,8 @@ func buildConfig(arg0 string, args []string) (*bootConfig, *config.Config, error
 	cfg.ImageResizerConfig = cfgFromFile.ImageResizerConfig
 	cfg.AltDocumentRoot = cfgFromFile.AltDocumentRoot
 	cfg.ShutdownTimeout = cfgFromFile.ShutdownTimeout
+	cfg.TrustedCIDRsForXForwardedFor = cfgFromFile.TrustedCIDRsForXForwardedFor
+	cfg.TrustedCIDRsForPropagation = cfgFromFile.TrustedCIDRsForPropagation
 
 	return boot, cfg, nil
 }

workhorse/main_test.go

@@ -611,16 +611,53 @@ func TestPropagateCorrelationIdHeader(t *testing.T) {
 	defer ts.Close()
 
 	testCases := []struct {
-		desc                   string
-		propagateCorrelationID bool
+		desc                         string
+		propagateCorrelationID       bool
+		xffHeader                    string
+		trustedCIDRsForPropagation   []string
+		trustedCIDRsForXForwardedFor []string
+		propagationExpected          bool
 	}{
 		{
 			desc:                   "propagateCorrelatedId is true",
 			propagateCorrelationID: true,
+			propagationExpected:    true,
 		},
 		{
 			desc:                   "propagateCorrelatedId is false",
 			propagateCorrelationID: false,
+			propagationExpected:    false,
+		},
+		{
+			desc:                   "propagation with trusted propagation CIDR",
+			propagateCorrelationID: true,
+			// Assumes HTTP connection's RemoteAddr will be 127.0.0.1:x
+			trustedCIDRsForPropagation: []string{"127.0.0.1/8"},
+			propagationExpected:        true,
+		},
+		{
+			desc:                   "propagation with trusted propagation and X-Forwarded-For CIDRs",
+			propagateCorrelationID: true,
+			// Assumes HTTP connection's RemoteAddr will be 127.0.0.1:x
+			xffHeader:                    "1.2.3.4, 127.0.0.1",
+			trustedCIDRsForPropagation:   []string{"1.2.3.4/32"},
+			trustedCIDRsForXForwardedFor: []string{"127.0.0.1/32", "192.168.0.1/32"},
+			propagationExpected:          true,
+		},
+		{
+			desc:                       "propagation not active with invalid propagation CIDR",
+			propagateCorrelationID:     true,
+			trustedCIDRsForPropagation: []string{"asdf"},
+			propagationExpected:        false,
+		},
+		{
+			desc:                   "propagation with invalid X-Forwarded-For CIDR",
+			propagateCorrelationID: true,
+			// Assumes HTTP connection's RemoteAddr will be 127.0.0.1:x
+			xffHeader:                    "1.2.3.4, 127.0.0.1",
+			trustedCIDRsForPropagation:   []string{"1.2.3.4/32"},
+			trustedCIDRsForXForwardedFor: []string{"bad"},
+			propagationExpected:          false,
 		},
 	}
 
@@ -628,19 +665,27 @@ func TestPropagateCorrelationIdHeader(t *testing.T) {
 		t.Run(tc.desc, func(t *testing.T) {
 			upstreamConfig := newUpstreamConfig(ts.URL)
 			upstreamConfig.PropagateCorrelationID = tc.propagateCorrelationID
+			upstreamConfig.TrustedCIDRsForPropagation = tc.trustedCIDRsForPropagation
+			upstreamConfig.TrustedCIDRsForXForwardedFor = tc.trustedCIDRsForXForwardedFor
 
 			ws := startWorkhorseServerWithConfig(upstreamConfig)
 			defer ws.Close()
 
 			resource := "/api/v3/projects/123/repository/not/special"
 			propagatedRequestId := "Propagated-RequestId-12345678"
-			resp, _ := httpGet(t, ws.URL+resource, map[string]string{"X-Request-Id": propagatedRequestId})
+			headers := map[string]string{"X-Request-Id": propagatedRequestId}
+
+			if tc.xffHeader != "" {
+				headers["X-Forwarded-For"] = tc.xffHeader
+			}
+
+			resp, _ := httpGet(t, ws.URL+resource, headers)
 			requestIds := resp.Header["X-Request-Id"]
 
 			require.Equal(t, 200, resp.StatusCode, "GET %q: status code", resource)
 			require.Equal(t, 1, len(requestIds), "GET %q: One X-Request-Id present", resource)
 
-			if tc.propagateCorrelationID {
+			if tc.propagationExpected {
 				require.Contains(t, requestIds, propagatedRequestId, "GET %q: Has X-Request-Id %s present", resource, propagatedRequestId)
 			} else {
 				require.NotContains(t, requestIds, propagatedRequestId, "GET %q: X-Request-Id not propagated")