Commit 5e865c2e authored by Stan Hu's avatar Stan Hu

Add support for propagation correlation IDs from trusted CIDRs

When Gitaly makes internal API calls back to Workhorse in Git hooks,
Workhorse previously would generate new correlation IDs, making it hard
to trace the entire call flow.

In https://gitlab.com/gitlab-org/labkit/-/merge_requests/123, we added
the ability to propagate correlation IDs from trusted CIDR blocks.

To use this feature, we add two configuraton parameters:

* `trusted_cidrs_for_x_forwarded_for`
* `trusted_cidrs_for_propagation`

If propagation of correlation ID is enabled,
`trusted_cidrs_for_x_forwarded_for` tells LabKit what remote IPs can be
trusted to use the `X-Forwarded-For` HTTP header to resolve the actual
client IP. Note that this parameter is not yet used in Workhorse's
remote IP resolution, but it should be.

`trusted_cidrs_for_propagation` allows Workhorse to restrict propagation
to certain IP ranges. We will want to add the Gitaly servers to this
list.

Relates to https://gitlab.com/gitlab-org/gitlab/-/issues/324836

Changelog: added
parent d26b12d6
# alt_document_root = '/home/git/public/assets' # alt_document_root = '/home/git/public/assets'
# shutdown_timeout = "60s" # shutdown_timeout = "60s"
# trusted_cidrs_for_x_forwarded_for = []
# trusted_cidrs_for_propagation = []
[redis] [redis]
URL = "unix:/home/git/gitlab/redis/redis.socket" URL = "unix:/home/git/gitlab/redis/redis.socket"
......
...@@ -30,6 +30,9 @@ func TestConfigFile(t *testing.T) { ...@@ -30,6 +30,9 @@ func TestConfigFile(t *testing.T) {
data := ` data := `
shutdown_timeout = "60s" 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] [redis]
password = "redis password" password = "redis password"
[object_storage] [object_storage]
...@@ -51,6 +54,8 @@ max_scaler_procs = 123 ...@@ -51,6 +54,8 @@ max_scaler_procs = 123
require.Equal(t, "redis password", cfg.Redis.Password) require.Equal(t, "redis password", cfg.Redis.Password)
require.Equal(t, "test provider", cfg.ObjectStorageCredentials.Provider) require.Equal(t, "test provider", cfg.ObjectStorageCredentials.Provider)
require.Equal(t, uint32(123), cfg.ImageResizerConfig.MaxScalerProcs, "image resizer max_scaler_procs") 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) require.Equal(t, 60*time.Second, cfg.ShutdownTimeout.Duration)
} }
......
...@@ -7,7 +7,7 @@ require ( ...@@ -7,7 +7,7 @@ require (
github.com/BurntSushi/toml v0.3.1 github.com/BurntSushi/toml v0.3.1
github.com/FZambia/sentinel v1.0.0 github.com/FZambia/sentinel v1.0.0
github.com/alecthomas/chroma v0.7.3 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/certifi/gocertifi v0.0.0-20200922220541-2c3bb06c6054 // indirect
github.com/dgrijalva/jwt-go v3.2.0+incompatible github.com/dgrijalva/jwt-go v3.2.0+incompatible
github.com/disintegration/imaging v1.6.2 github.com/disintegration/imaging v1.6.2
...@@ -29,7 +29,7 @@ require ( ...@@ -29,7 +29,7 @@ require (
github.com/smartystreets/goconvey v1.6.4 github.com/smartystreets/goconvey v1.6.4
github.com/stretchr/testify v1.7.0 github.com/stretchr/testify v1.7.0
gitlab.com/gitlab-org/gitaly/v14 v14.0.0-rc1 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 gocloud.dev v0.21.1-0.20201223184910-5094f54ed8bb
golang.org/x/exp v0.0.0-20200331195152-e8c3332aa8e5 // indirect golang.org/x/exp v0.0.0-20200331195152-e8c3332aa8e5 // indirect
golang.org/x/image v0.0.0-20191009234506-e7c1f5e7dbb8 golang.org/x/image v0.0.0-20191009234506-e7c1f5e7dbb8
...@@ -37,5 +37,6 @@ require ( ...@@ -37,5 +37,6 @@ require (
golang.org/x/net v0.0.0-20210316092652-d523dce5a7f4 golang.org/x/net v0.0.0-20210316092652-d523dce5a7f4
golang.org/x/tools v0.1.0 golang.org/x/tools v0.1.0
google.golang.org/grpc v1.37.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 honnef.co/go/tools v0.1.3
) )
This diff is collapsed.
...@@ -104,6 +104,8 @@ type Config struct { ...@@ -104,6 +104,8 @@ type Config struct {
ImageResizerConfig ImageResizerConfig `toml:"image_resizer"` ImageResizerConfig ImageResizerConfig `toml:"image_resizer"`
AltDocumentRoot string `toml:"alt_document_root"` AltDocumentRoot string `toml:"alt_document_root"`
ShutdownTimeout TomlDuration `toml:"shutdown_timeout"` ShutdownTimeout TomlDuration `toml:"shutdown_timeout"`
TrustedCIDRsForXForwardedFor []string `toml:"trusted_cidrs_for_x_forwarded_for"`
TrustedCIDRsForPropagation []string `toml:"trusted_cidrs_for_propagation"`
} }
var DefaultImageResizerConfig = ImageResizerConfig{ var DefaultImageResizerConfig = ImageResizerConfig{
......
...@@ -87,6 +87,12 @@ func newUpstream(cfg config.Config, accessLogger *logrus.Logger, routesCallback ...@@ -87,6 +87,12 @@ func newUpstream(cfg config.Config, accessLogger *logrus.Logger, routesCallback
if cfg.PropagateCorrelationID { if cfg.PropagateCorrelationID {
correlationOpts = append(correlationOpts, correlation.WithPropagation()) 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...) handler := correlation.InjectCorrelationID(&up, correlationOpts...)
// TODO: move to LabKit https://gitlab.com/gitlab-org/gitlab/-/issues/324823 // TODO: move to LabKit https://gitlab.com/gitlab-org/gitlab/-/issues/324823
......
...@@ -152,6 +152,8 @@ func buildConfig(arg0 string, args []string) (*bootConfig, *config.Config, error ...@@ -152,6 +152,8 @@ func buildConfig(arg0 string, args []string) (*bootConfig, *config.Config, error
cfg.ImageResizerConfig = cfgFromFile.ImageResizerConfig cfg.ImageResizerConfig = cfgFromFile.ImageResizerConfig
cfg.AltDocumentRoot = cfgFromFile.AltDocumentRoot cfg.AltDocumentRoot = cfgFromFile.AltDocumentRoot
cfg.ShutdownTimeout = cfgFromFile.ShutdownTimeout cfg.ShutdownTimeout = cfgFromFile.ShutdownTimeout
cfg.TrustedCIDRsForXForwardedFor = cfgFromFile.TrustedCIDRsForXForwardedFor
cfg.TrustedCIDRsForPropagation = cfgFromFile.TrustedCIDRsForPropagation
return boot, cfg, nil return boot, cfg, nil
} }
......
...@@ -613,14 +613,51 @@ func TestPropagateCorrelationIdHeader(t *testing.T) { ...@@ -613,14 +613,51 @@ func TestPropagateCorrelationIdHeader(t *testing.T) {
testCases := []struct { testCases := []struct {
desc string desc string
propagateCorrelationID bool propagateCorrelationID bool
xffHeader string
trustedCIDRsForPropagation []string
trustedCIDRsForXForwardedFor []string
propagationExpected bool
}{ }{
{ {
desc: "propagateCorrelatedId is true", desc: "propagateCorrelatedId is true",
propagateCorrelationID: true, propagateCorrelationID: true,
propagationExpected: true,
}, },
{ {
desc: "propagateCorrelatedId is false", desc: "propagateCorrelatedId is false",
propagateCorrelationID: 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) { ...@@ -628,19 +665,27 @@ func TestPropagateCorrelationIdHeader(t *testing.T) {
t.Run(tc.desc, func(t *testing.T) { t.Run(tc.desc, func(t *testing.T) {
upstreamConfig := newUpstreamConfig(ts.URL) upstreamConfig := newUpstreamConfig(ts.URL)
upstreamConfig.PropagateCorrelationID = tc.propagateCorrelationID upstreamConfig.PropagateCorrelationID = tc.propagateCorrelationID
upstreamConfig.TrustedCIDRsForPropagation = tc.trustedCIDRsForPropagation
upstreamConfig.TrustedCIDRsForXForwardedFor = tc.trustedCIDRsForXForwardedFor
ws := startWorkhorseServerWithConfig(upstreamConfig) ws := startWorkhorseServerWithConfig(upstreamConfig)
defer ws.Close() defer ws.Close()
resource := "/api/v3/projects/123/repository/not/special" resource := "/api/v3/projects/123/repository/not/special"
propagatedRequestId := "Propagated-RequestId-12345678" 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"] requestIds := resp.Header["X-Request-Id"]
require.Equal(t, 200, resp.StatusCode, "GET %q: status code", resource) 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) 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) require.Contains(t, requestIds, propagatedRequestId, "GET %q: Has X-Request-Id %s present", resource, propagatedRequestId)
} else { } else {
require.NotContains(t, requestIds, propagatedRequestId, "GET %q: X-Request-Id not propagated") require.NotContains(t, requestIds, propagatedRequestId, "GET %q: X-Request-Id not propagated")
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment