diff options
author | Jacob Vosmaer <jacob@gitlab.com> | 2019-01-23 20:19:55 +0300 |
---|---|---|
committer | Jacob Vosmaer <jacob@gitlab.com> | 2019-01-23 20:19:55 +0300 |
commit | d3c498cef29942de95931e9bbf2aac302ba73293 (patch) | |
tree | bbc21861bf722771c16b981dcaf3e88e281db2f7 | |
parent | 8aacfc36317eaa286d2f04990de4c4acf218aa52 (diff) | |
parent | fa872c1518a7865562d5840cfc33242e2e86e58c (diff) |
Merge branch '2780-disable-git-v2-1-12' into '1-12-stable'
[Gitaly v1.12] Disable git protocol v2 temporarily
See merge request gitlab/gitaly!12
-rw-r--r-- | changelogs/unreleased/2780-disable-git-v2.yml | 5 | ||||
-rw-r--r-- | internal/config/config.go | 11 | ||||
-rw-r--r-- | internal/git/protocol.go | 10 | ||||
-rw-r--r-- | internal/git/protocol_test.go | 58 | ||||
-rw-r--r-- | internal/service/smarthttp/.gitignore | 1 | ||||
-rw-r--r-- | internal/service/smarthttp/inforefs_test.go | 9 | ||||
-rw-r--r-- | internal/service/smarthttp/receive_pack_test.go | 6 | ||||
l--------- | internal/service/smarthttp/testdata/env_git | 1 | ||||
-rw-r--r-- | internal/service/smarthttp/upload_pack_test.go | 7 | ||||
-rw-r--r-- | internal/service/ssh/.gitignore | 1 | ||||
-rw-r--r-- | internal/service/ssh/receive_pack_test.go | 6 | ||||
l--------- | internal/service/ssh/testdata/env_git | 1 | ||||
-rw-r--r-- | internal/service/ssh/testhelper_test.go | 2 | ||||
-rw-r--r-- | internal/service/ssh/upload_pack_test.go | 7 | ||||
-rw-r--r-- | internal/testhelper/git_protocol.go | 26 |
15 files changed, 124 insertions, 27 deletions
diff --git a/changelogs/unreleased/2780-disable-git-v2.yml b/changelogs/unreleased/2780-disable-git-v2.yml new file mode 100644 index 000000000..1f236be8b --- /dev/null +++ b/changelogs/unreleased/2780-disable-git-v2.yml @@ -0,0 +1,5 @@ +--- +title: Disable git protocol v2 temporarily +merge_request: +author: +type: security diff --git a/internal/config/config.go b/internal/config/config.go index 2145c2857..14bebbf3f 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -49,6 +49,17 @@ type GitlabShell struct { // Git contains the settings for the Git executable type Git struct { BinPath string `toml:"bin_path"` + + // ProtocolV2Enabled can be set to true to enable the newer Git protocol + // version. This should not be enabled until GitLab *either* stops + // using transfer.hideRefs for security purposes, *or* Git protocol v2 + // respects this setting: + // + // https://public-inbox.org/git/20181213155817.27666-1-avarab@gmail.com/T/ + // + // This is not user-configurable. Once a new Git version has been released, + // we can add code to enable it if the detected git binary is new enough + ProtocolV2Enabled bool } // Storage contains a single storage-shard diff --git a/internal/git/protocol.go b/internal/git/protocol.go index 88262ec56..922238d50 100644 --- a/internal/git/protocol.go +++ b/internal/git/protocol.go @@ -7,6 +7,8 @@ import ( grpc_ctxtags "github.com/grpc-ecosystem/go-grpc-middleware/tags" "github.com/prometheus/client_golang/prometheus" + + "gitlab.com/gitlab-org/gitaly/internal/config" ) const ( @@ -39,9 +41,13 @@ func AddGitProtocolEnv(ctx context.Context, req RequestWithGitProtocol, env []st service, method := methodFromContext(ctx) if req.GetGitProtocol() == ProtocolV2 { - env = append(env, fmt.Sprintf("GIT_PROTOCOL=%s", ProtocolV2)) + if config.Config.Git.ProtocolV2Enabled { + env = append(env, fmt.Sprintf("GIT_PROTOCOL=%s", ProtocolV2)) - gitProtocolRequests.WithLabelValues(service, method, "v2").Inc() + gitProtocolRequests.WithLabelValues(service, method, "v2").Inc() + } else { + gitProtocolRequests.WithLabelValues(service, method, "v2-rejected").Inc() + } } else { gitProtocolRequests.WithLabelValues(service, method, "v0").Inc() } diff --git a/internal/git/protocol_test.go b/internal/git/protocol_test.go new file mode 100644 index 000000000..d3d430b95 --- /dev/null +++ b/internal/git/protocol_test.go @@ -0,0 +1,58 @@ +package git + +import ( + "context" + "testing" + + "github.com/stretchr/testify/require" + + "gitlab.com/gitlab-org/gitaly/internal/config" +) + +type fakeProtocolMessage struct { + protocol string +} + +func (f fakeProtocolMessage) GetGitProtocol() string { + return f.protocol +} + +func setGitProtocolV2Support(value bool) func() { + orig := config.Config.Git.ProtocolV2Enabled + config.Config.Git.ProtocolV2Enabled = value + + return func() { + config.Config.Git.ProtocolV2Enabled = orig + } +} + +func TestAddGitProtocolEnvRespectsConfigEnabled(t *testing.T) { + restore := setGitProtocolV2Support(true) + defer restore() + + env := []string{"fake=value"} + msg := fakeProtocolMessage{protocol: "version=2"} + value := AddGitProtocolEnv(context.Background(), msg, env) + + require.Equal(t, value, append(env, "GIT_PROTOCOL=version=2")) +} + +func TestAddGitProtocolEnvWhenV2NotRequested(t *testing.T) { + restore := setGitProtocolV2Support(true) + defer restore() + + env := []string{"fake=value"} + msg := fakeProtocolMessage{protocol: ""} + value := AddGitProtocolEnv(context.Background(), msg, env) + require.Equal(t, value, env) +} + +func TestAddGitProtocolEnvRespectsConfigDisabled(t *testing.T) { + restore := setGitProtocolV2Support(false) + defer restore() + + env := []string{"fake=value"} + msg := fakeProtocolMessage{protocol: "GIT_PROTOCOL=version=2"} + value := AddGitProtocolEnv(context.Background(), msg, env) + require.Equal(t, value, env) +} diff --git a/internal/service/smarthttp/.gitignore b/internal/service/smarthttp/.gitignore new file mode 100644 index 000000000..3d3722967 --- /dev/null +++ b/internal/service/smarthttp/.gitignore @@ -0,0 +1 @@ +/testdata/scratch diff --git a/internal/service/smarthttp/inforefs_test.go b/internal/service/smarthttp/inforefs_test.go index e2ed888d4..a1e08cbef 100644 --- a/internal/service/smarthttp/inforefs_test.go +++ b/internal/service/smarthttp/inforefs_test.go @@ -9,7 +9,6 @@ import ( "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly-proto/go/gitalypb" - "gitlab.com/gitlab-org/gitaly/internal/config" "gitlab.com/gitlab-org/gitaly/internal/git" "gitlab.com/gitlab-org/gitaly/internal/testhelper" "gitlab.com/gitlab-org/gitaly/streamio" @@ -54,10 +53,8 @@ func TestSuccessfulInfoRefsUploadPackWithGitConfigOptions(t *testing.T) { } func TestSuccessfulInfoRefsUploadPackWithGitProtocol(t *testing.T) { - defer func(old string) { - config.Config.Git.BinPath = old - }(config.Config.Git.BinPath) - config.Config.Git.BinPath = "../../testhelper/env_git" + restore := testhelper.EnableGitProtocolV2Support() + defer restore() server, serverSocketPath := runSmartHTTPServer(t) defer server.Stop() @@ -148,7 +145,7 @@ func TestFailureRepoNotFoundInfoRefsReceivePack(t *testing.T) { client, conn := newSmartHTTPClient(t, serverSocketPath) defer conn.Close() - repo := &gitalypb.Repository{StorageName: "default", RelativePath: "testdata/data/another_repo"} + repo := &gitalypb.Repository{StorageName: "default", RelativePath: "testdata/scratch/another_repo"} rpcRequest := &gitalypb.InfoRefsRequest{Repository: repo} ctx, cancel := context.WithCancel(context.Background()) diff --git a/internal/service/smarthttp/receive_pack_test.go b/internal/service/smarthttp/receive_pack_test.go index 3b906ec7f..c08b95584 100644 --- a/internal/service/smarthttp/receive_pack_test.go +++ b/internal/service/smarthttp/receive_pack_test.go @@ -77,10 +77,8 @@ func TestSuccessfulReceivePackRequestWithGitOpts(t *testing.T) { } func TestSuccessfulReceivePackRequestWithGitProtocol(t *testing.T) { - defer func(old string) { - config.Config.Git.BinPath = old - }(config.Config.Git.BinPath) - config.Config.Git.BinPath = "../../testhelper/env_git" + restore := testhelper.EnableGitProtocolV2Support() + defer restore() server, serverSocketPath := runSmartHTTPServer(t) defer server.Stop() diff --git a/internal/service/smarthttp/testdata/env_git b/internal/service/smarthttp/testdata/env_git new file mode 120000 index 000000000..a01a70012 --- /dev/null +++ b/internal/service/smarthttp/testdata/env_git @@ -0,0 +1 @@ +../../../testhelper/env_git
\ No newline at end of file diff --git a/internal/service/smarthttp/upload_pack_test.go b/internal/service/smarthttp/upload_pack_test.go index 3c44fcd45..e8dcc8808 100644 --- a/internal/service/smarthttp/upload_pack_test.go +++ b/internal/service/smarthttp/upload_pack_test.go @@ -14,7 +14,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly-proto/go/gitalypb" - "gitlab.com/gitlab-org/gitaly/internal/config" "gitlab.com/gitlab-org/gitaly/internal/git" "gitlab.com/gitlab-org/gitaly/internal/git/pktline" "gitlab.com/gitlab-org/gitaly/internal/helper" @@ -149,10 +148,8 @@ func TestUploadPackRequestWithGitConfigOptions(t *testing.T) { } func TestUploadPackRequestWithGitProtocol(t *testing.T) { - defer func(old string) { - config.Config.Git.BinPath = old - }(config.Config.Git.BinPath) - config.Config.Git.BinPath = "../../testhelper/env_git" + restore := testhelper.EnableGitProtocolV2Support() + defer restore() server, serverSocketPath := runSmartHTTPServer(t) defer server.Stop() diff --git a/internal/service/ssh/.gitignore b/internal/service/ssh/.gitignore new file mode 100644 index 000000000..3d3722967 --- /dev/null +++ b/internal/service/ssh/.gitignore @@ -0,0 +1 @@ +/testdata/scratch diff --git a/internal/service/ssh/receive_pack_test.go b/internal/service/ssh/receive_pack_test.go index 350f84bc2..46f5fe679 100644 --- a/internal/service/ssh/receive_pack_test.go +++ b/internal/service/ssh/receive_pack_test.go @@ -94,10 +94,8 @@ func TestReceivePackPushSuccess(t *testing.T) { } func TestReceivePackPushSuccessWithGitProtocol(t *testing.T) { - defer func(old string) { - config.Config.Git.BinPath = old - }(config.Config.Git.BinPath) - config.Config.Git.BinPath = "../../testhelper/env_git" + restore := testhelper.EnableGitProtocolV2Support() + defer restore() server, serverSocketPath := runSSHServer(t) defer server.Stop() diff --git a/internal/service/ssh/testdata/env_git b/internal/service/ssh/testdata/env_git new file mode 120000 index 000000000..a01a70012 --- /dev/null +++ b/internal/service/ssh/testdata/env_git @@ -0,0 +1 @@ +../../../testhelper/env_git
\ No newline at end of file diff --git a/internal/service/ssh/testhelper_test.go b/internal/service/ssh/testhelper_test.go index b22fd7c33..1945bf428 100644 --- a/internal/service/ssh/testhelper_test.go +++ b/internal/service/ssh/testhelper_test.go @@ -14,7 +14,7 @@ import ( ) const ( - testPath = "testdata" + testPath = "testdata/scratch" testRepoRoot = testPath + "/data" ) diff --git a/internal/service/ssh/upload_pack_test.go b/internal/service/ssh/upload_pack_test.go index a52cc99ae..9252a1fa1 100644 --- a/internal/service/ssh/upload_pack_test.go +++ b/internal/service/ssh/upload_pack_test.go @@ -12,7 +12,6 @@ import ( "github.com/golang/protobuf/jsonpb" "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly-proto/go/gitalypb" - "gitlab.com/gitlab-org/gitaly/internal/config" "gitlab.com/gitlab-org/gitaly/internal/git" "gitlab.com/gitlab-org/gitaly/internal/testhelper" "golang.org/x/net/context" @@ -98,10 +97,8 @@ func TestUploadPackCloneSuccess(t *testing.T) { } func TestUploadPackCloneSuccessWithGitProtocol(t *testing.T) { - defer func(old string) { - config.Config.Git.BinPath = old - }(config.Config.Git.BinPath) - config.Config.Git.BinPath = "../../testhelper/env_git" + restore := testhelper.EnableGitProtocolV2Support() + defer restore() server, serverSocketPath := runSSHServer(t) defer server.Stop() diff --git a/internal/testhelper/git_protocol.go b/internal/testhelper/git_protocol.go new file mode 100644 index 000000000..5619739cb --- /dev/null +++ b/internal/testhelper/git_protocol.go @@ -0,0 +1,26 @@ +package testhelper + +import ( + "gitlab.com/gitlab-org/gitaly/internal/config" +) + +// EnableGitProtocolV2Support ensures that Git protocol v2 support is enabled, +// and replaces the git binary in config with an `env_git` wrapper that allows +// the protocol to be tested. It returns a function that restores the given +// settings. +// +// Because we don't know how to get to that wrapper script from the current +// working directory, callers must create a symbolic link to the `env_git` file +// in their own `testdata` directories. +func EnableGitProtocolV2Support() func() { + oldGitBinPath := config.Config.Git.BinPath + oldGitProtocolV2Enabled := config.Config.Git.ProtocolV2Enabled + + config.Config.Git.BinPath = "testdata/env_git" + config.Config.Git.ProtocolV2Enabled = true + + return func() { + config.Config.Git.BinPath = oldGitBinPath + config.Config.Git.ProtocolV2Enabled = oldGitProtocolV2Enabled + } +} |