diff options
author | GitLab Release Tools Bot <delivery-team+release-tools@gitlab.com> | 2020-10-30 18:10:54 +0300 |
---|---|---|
committer | GitLab Release Tools Bot <delivery-team+release-tools@gitlab.com> | 2020-10-30 18:10:54 +0300 |
commit | 01bfc98e1bee52e9625bc09c114b97dd613187ce (patch) | |
tree | 5aa45ded0e762320e7cc0cf3bc802d7e384c10b5 | |
parent | e2faf9c14a626740fc415d637648b649482945a1 (diff) | |
parent | 26389269d35d82265940f2937dcd2e9e86782fbe (diff) |
Merge branch 'security-ps-clean-creds-13.4' into '13-4-stable'
Removal of all http.*.extraHeader config values
See merge request gitlab-org/security/gitaly!20
-rw-r--r-- | changelogs/unreleased/security-ps-clean-creds.yml | 5 | ||||
-rw-r--r-- | internal/gitaly/service/repository/optimize.go | 100 | ||||
-rw-r--r-- | internal/gitaly/service/repository/optimize_test.go | 29 |
3 files changed, 134 insertions, 0 deletions
diff --git a/changelogs/unreleased/security-ps-clean-creds.yml b/changelogs/unreleased/security-ps-clean-creds.yml new file mode 100644 index 000000000..9d5866b3e --- /dev/null +++ b/changelogs/unreleased/security-ps-clean-creds.yml @@ -0,0 +1,5 @@ +--- +title: Removal of all http.*.extraHeader config values +merge_request: +author: +type: security diff --git a/internal/gitaly/service/repository/optimize.go b/internal/gitaly/service/repository/optimize.go index 9d0d58056..0d8520133 100644 --- a/internal/gitaly/service/repository/optimize.go +++ b/internal/gitaly/service/repository/optimize.go @@ -1,11 +1,16 @@ package repository import ( + "bufio" "context" + "errors" "fmt" + "io" "io/ioutil" "os" + "os/exec" "path/filepath" + "strings" "github.com/prometheus/client_golang/prometheus" "gitlab.com/gitlab-org/gitaly/internal/git" @@ -130,6 +135,12 @@ func (s *server) optimizeRepository(ctx context.Context, repository *gitalypb.Re return fmt.Errorf("OptimizeRepository: remove empty refs: %w", err) } + // TODO: https://gitlab.com/gitlab-org/gitaly/-/issues/3138 + // This is a temporary code and needs to be removed once it will be run on all repositories at least once. + if err := s.unsetAllConfigsByRegexp(ctx, repository, "^http\\..+\\.extraHeader$"); err != nil { + return fmt.Errorf("OptimizeRepository: unset all configs by regexp: %w", err) + } + return nil } @@ -157,3 +168,92 @@ func (s *server) validateOptimizeRepositoryRequest(in *gitalypb.OptimizeReposito return nil } + +func (s *server) unsetAllConfigsByRegexp(ctx context.Context, repository *gitalypb.Repository, regexp string) error { + keys, err := getConfigKeys(ctx, repository, regexp) + if err != nil { + return fmt.Errorf("get config keys: %w", err) + } + + if err := unsetConfigKeys(ctx, repository, keys); err != nil { + return fmt.Errorf("unset all keys: %w", err) + } + + return nil +} + +func getConfigKeys(ctx context.Context, repository *gitalypb.Repository, regexp string) ([]string, error) { + cmd, err := git.SafeCmd(ctx, repository, nil, git.SubCmd{ + Name: "config", + Flags: []git.Option{ + git.Flag{Name: "--name-only"}, + git.ValueFlag{Name: "--get-regexp", Value: regexp}, + }, + }) + if err != nil { + return nil, fmt.Errorf("creation of 'git config': %w", err) + } + + keys, err := parseConfigKeys(cmd) + if err != nil { + return nil, fmt.Errorf("parse config keys: %w", err) + } + + if err := cmd.Wait(); err != nil { + var termErr *exec.ExitError + if errors.As(err, &termErr) { + if termErr.ExitCode() == 1 { + // https://git-scm.com/docs/git-config#_description: The section or key is invalid (ret=1) + // This means no matching values were found. + return nil, nil + } + } + return nil, fmt.Errorf("wait for 'git config': %w", err) + } + + return keys, nil +} + +func parseConfigKeys(reader io.Reader) ([]string, error) { + var keys []string + + scanner := bufio.NewScanner(reader) + for scanner.Scan() { + keys = append(keys, scanner.Text()) + } + if err := scanner.Err(); err != nil { + return nil, err + } + + return keys, nil +} + +func unsetConfigKeys(ctx context.Context, repository *gitalypb.Repository, names []string) error { + for _, name := range names { + if err := unsetAll(ctx, repository, name); err != nil { + return fmt.Errorf("unset all: %w", err) + } + } + + return nil +} + +func unsetAll(ctx context.Context, repository *gitalypb.Repository, name string) error { + if strings.TrimSpace(name) == "" { + return nil + } + + cmd, err := git.SafeCmd(ctx, repository, nil, git.SubCmd{ + Name: "config", + Flags: []git.Option{git.ValueFlag{Name: "--unset-all", Value: name}}, + }) + if err != nil { + return fmt.Errorf("creation of 'git config': %w", err) + } + + if err := cmd.Wait(); err != nil { + return fmt.Errorf("wait for 'git config': %w", err) + } + + return nil +} diff --git a/internal/gitaly/service/repository/optimize_test.go b/internal/gitaly/service/repository/optimize_test.go index cba2caa29..ee5fafb6c 100644 --- a/internal/gitaly/service/repository/optimize_test.go +++ b/internal/gitaly/service/repository/optimize_test.go @@ -1,6 +1,8 @@ package repository import ( + "bytes" + "io/ioutil" "os" "path/filepath" "testing" @@ -52,6 +54,23 @@ func TestOptimizeRepository(t *testing.T) { testhelper.CreateCommit(t, testRepoPath, "master", nil) + testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "config", "http.http://localhost:51744/60631c8695bf041a808759a05de53e36a73316aacb502824fabbb0c6055637c1.git.extraHeader", "Authorization: Basic secret-password") + testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "config", "http.http://localhost:51744/60631c8695bf041a808759a05de53e36a73316aacb502824fabbb0c6055637c2.git.extraHeader", "Authorization: Basic secret-password") + testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "config", "randomStart-http.http://localhost:51744/60631c8695bf041a808759a05de53e36a73316aacb502824fabbb0c6055637c3.git.extraHeader", "Authorization: Basic secret-password") + testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "config", "http.http://localhost:51744/60631c8695bf041a808759a05de53e36a73316aacb502824fabbb0c6055637c4.git.extraHeader-randomEnd", "Authorization: Basic secret-password") + testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "config", "hTTp.http://localhost:51744/60631c8695bf041a808759a05de53e36a73316aacb502824fabbb0c6055637c5.git.ExtrAheaDeR", "Authorization: Basic secret-password") + testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "config", "http.http://extraHeader/extraheader/EXTRAHEADER.git.extraHeader", "Authorization: Basic secret-password") + testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "config", "https.https://localhost:51744/60631c8695bf041a808759a05de53e36a73316aacb502824fabbb0c6055637c5.git.extraHeader", "Authorization: Basic secret-password") + confFileData, err := ioutil.ReadFile(filepath.Join(testRepoPath, "config")) + require.NoError(t, err) + require.True(t, bytes.Contains(confFileData, []byte("http://localhost:51744/60631c8695bf041a808759a05de53e36a73316aacb502824fabbb0c6055637c1.git"))) + require.True(t, bytes.Contains(confFileData, []byte("http://localhost:51744/60631c8695bf041a808759a05de53e36a73316aacb502824fabbb0c6055637c2.git"))) + require.True(t, bytes.Contains(confFileData, []byte("http://localhost:51744/60631c8695bf041a808759a05de53e36a73316aacb502824fabbb0c6055637c3"))) + require.True(t, bytes.Contains(confFileData, []byte("http://localhost:51744/60631c8695bf041a808759a05de53e36a73316aacb502824fabbb0c6055637c4.git"))) + require.True(t, bytes.Contains(confFileData, []byte("http://localhost:51744/60631c8695bf041a808759a05de53e36a73316aacb502824fabbb0c6055637c5.git"))) + require.True(t, bytes.Contains(confFileData, []byte("http://extraHeader/extraheader/EXTRAHEADER.git"))) + require.True(t, bytes.Contains(confFileData, []byte("https://localhost:51744/60631c8695bf041a808759a05de53e36a73316aacb502824fabbb0c6055637c5.git"))) + serverSocketPath, stop := runRepoServer(t) defer stop() @@ -61,6 +80,16 @@ func TestOptimizeRepository(t *testing.T) { _, err = repoClient.OptimizeRepository(ctx, &gitalypb.OptimizeRepositoryRequest{Repository: testRepo}) require.NoError(t, err) + confFileData, err = ioutil.ReadFile(filepath.Join(testRepoPath, "config")) + require.NoError(t, err) + require.False(t, bytes.Contains(confFileData, []byte("http://localhost:51744/60631c8695bf041a808759a05de53e36a73316aacb502824fabbb0c6055637c1.git"))) + require.False(t, bytes.Contains(confFileData, []byte("http://localhost:51744/60631c8695bf041a808759a05de53e36a73316aacb502824fabbb0c6055637c2.git"))) + require.True(t, bytes.Contains(confFileData, []byte("http://localhost:51744/60631c8695bf041a808759a05de53e36a73316aacb502824fabbb0c6055637c3"))) + require.True(t, bytes.Contains(confFileData, []byte("http://localhost:51744/60631c8695bf041a808759a05de53e36a73316aacb502824fabbb0c6055637c4.git"))) + require.False(t, bytes.Contains(confFileData, []byte("http://localhost:51744/60631c8695bf041a808759a05de53e36a73316aacb502824fabbb0c6055637c5.git"))) + require.False(t, bytes.Contains(confFileData, []byte("http://extraHeader/extraheader/EXTRAHEADER.git.extraHeader"))) + require.True(t, bytes.Contains(confFileData, []byte("https://localhost:51744/60631c8695bf041a808759a05de53e36a73316aacb502824fabbb0c6055637c5.git"))) + require.Equal(t, getNewestPackfileModtime(t, testRepoPath), newestsPackfileTime, "there should not have been a new packfile created") testRepo, testRepoPath, cleanupBare := testhelper.InitBareRepo(t) |