From ec6718ef403e0b89f8b649aef225b94b2369725e Mon Sep 17 00:00:00 2001 From: Ethan Koenig Date: Fri, 8 Dec 2017 07:12:47 -0800 Subject: [PATCH] Sanitize logs for mirror sync (#3057, #3082) (#3078) * Sanitize logs for mirror sync * Fix error message sanitiziation (#3082) --- models/repo.go | 7 +++- models/repo_mirror.go | 70 ++++++++++++++++++++++--------------- modules/util/sanitize.go | 48 +++++++++++++++++++++++++ routers/api/v1/repo/repo.go | 6 ++-- routers/repo/repo.go | 8 +++-- 5 files changed, 105 insertions(+), 34 deletions(-) create mode 100644 modules/util/sanitize.go diff --git a/models/repo.go b/models/repo.go index 8d57ae51a..c570acd6f 100644 --- a/models/repo.go +++ b/models/repo.go @@ -605,9 +605,14 @@ func (repo *Repository) RepoPath() string { return repo.repoPath(x) } +// GitConfigPath returns the path to a repository's git config/ directory +func GitConfigPath(repoPath string) string { + return filepath.Join(repoPath, "config") +} + // GitConfigPath returns the repository git config path func (repo *Repository) GitConfigPath() string { - return filepath.Join(repo.RepoPath(), "config") + return GitConfigPath(repo.RepoPath()) } // RelLink returns the repository relative link diff --git a/models/repo_mirror.go b/models/repo_mirror.go index 77cd98faa..f52b3eb45 100644 --- a/models/repo_mirror.go +++ b/models/repo_mirror.go @@ -6,18 +6,18 @@ package models import ( "fmt" - "strings" "time" - "github.com/Unknwon/com" - "github.com/go-xorm/xorm" - "gopkg.in/ini.v1" - "code.gitea.io/git" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/process" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/sync" + "code.gitea.io/gitea/modules/util" + + "github.com/Unknwon/com" + "github.com/go-xorm/xorm" + "gopkg.in/ini.v1" ) // MirrorQueue holds an UniqueQueue object of the mirror @@ -76,41 +76,41 @@ func (m *Mirror) ScheduleNextUpdate() { m.NextUpdate = time.Now().Add(m.Interval) } +func remoteAddress(repoPath string) (string, error) { + cfg, err := ini.Load(GitConfigPath(repoPath)) + if err != nil { + return "", err + } + return cfg.Section("remote \"origin\"").Key("url").Value(), nil +} + func (m *Mirror) readAddress() { if len(m.address) > 0 { return } - - cfg, err := ini.Load(m.Repo.GitConfigPath()) + var err error + m.address, err = remoteAddress(m.Repo.RepoPath()) if err != nil { - log.Error(4, "Load: %v", err) - return + log.Error(4, "remoteAddress: %v", err) } - m.address = cfg.Section("remote \"origin\"").Key("url").Value() } -// HandleCloneUserCredentials replaces user credentials from HTTP/HTTPS URL -// with placeholder . -// It will fail for any other forms of clone addresses. -func HandleCloneUserCredentials(url string, mosaics bool) string { - i := strings.Index(url, "@") - if i == -1 { - return url +// sanitizeOutput sanitizes output of a command, replacing occurrences of the +// repository's remote address with a sanitized version. +func sanitizeOutput(output, repoPath string) (string, error) { + remoteAddr, err := remoteAddress(repoPath) + if err != nil { + // if we're unable to load the remote address, then we're unable to + // sanitize. + return "", err } - start := strings.Index(url, "://") - if start == -1 { - return url - } - if mosaics { - return url[:start+3] + "" + url[i:] - } - return url[:start+3] + url[i+1:] + return util.SanitizeMessage(output, remoteAddr), nil } // Address returns mirror address from Git repository config without credentials. func (m *Mirror) Address() string { m.readAddress() - return HandleCloneUserCredentials(m.address, false) + return util.SanitizeURLCredentials(m.address, false) } // FullAddress returns mirror address from Git repository config. @@ -145,7 +145,14 @@ func (m *Mirror) runSync() bool { if _, stderr, err := process.GetManager().ExecDir( timeout, repoPath, fmt.Sprintf("Mirror.runSync: %s", repoPath), "git", gitArgs...); err != nil { - desc := fmt.Sprintf("Failed to update mirror repository '%s': %s", repoPath, stderr) + // sanitize the output, since it may contain the remote address, which may + // contain a password + message, err := sanitizeOutput(stderr, repoPath) + if err != nil { + log.Error(4, "sanitizeOutput: %v", err) + return false + } + desc := fmt.Sprintf("Failed to update mirror repository '%s': %s", repoPath, message) log.Error(4, desc) if err = CreateRepositoryNotice(desc); err != nil { log.Error(4, "CreateRepositoryNotice: %v", err) @@ -170,7 +177,14 @@ func (m *Mirror) runSync() bool { if _, stderr, err := process.GetManager().ExecDir( timeout, wikiPath, fmt.Sprintf("Mirror.runSync: %s", wikiPath), "git", "remote", "update", "--prune"); err != nil { - desc := fmt.Sprintf("Failed to update mirror wiki repository '%s': %s", wikiPath, stderr) + // sanitize the output, since it may contain the remote address, which may + // contain a password + message, err := sanitizeOutput(stderr, wikiPath) + if err != nil { + log.Error(4, "sanitizeOutput: %v", err) + return false + } + desc := fmt.Sprintf("Failed to update mirror wiki repository '%s': %s", wikiPath, message) log.Error(4, desc) if err = CreateRepositoryNotice(desc); err != nil { log.Error(4, "CreateRepositoryNotice: %v", err) diff --git a/modules/util/sanitize.go b/modules/util/sanitize.go new file mode 100644 index 000000000..b1c17b29c --- /dev/null +++ b/modules/util/sanitize.go @@ -0,0 +1,48 @@ +// Copyright 2017 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package util + +import ( + "net/url" + "strings" +) + +// urlSafeError wraps an error whose message may contain a sensitive URL +type urlSafeError struct { + err error + unsanitizedURL string +} + +func (err urlSafeError) Error() string { + return SanitizeMessage(err.err.Error(), err.unsanitizedURL) +} + +// URLSanitizedError returns the sanitized version an error whose message may +// contain a sensitive URL +func URLSanitizedError(err error, unsanitizedURL string) error { + return urlSafeError{err: err, unsanitizedURL: unsanitizedURL} +} + +// SanitizeMessage sanitizes a message which may contains a sensitive URL +func SanitizeMessage(message, unsanitizedURL string) string { + sanitizedURL := SanitizeURLCredentials(unsanitizedURL, true) + return strings.Replace(message, unsanitizedURL, sanitizedURL, -1) +} + +// SanitizeURLCredentials sanitizes a url, either removing user credentials +// or replacing them with a placeholder. +func SanitizeURLCredentials(unsanitizedURL string, usePlaceholder bool) string { + u, err := url.Parse(unsanitizedURL) + if err != nil { + // don't log the error, since it might contain unsanitized URL. + return "(unparsable url)" + } + if u.User != nil && usePlaceholder { + u.User = url.User("") + } else { + u.User = nil + } + return u.String() +} diff --git a/routers/api/v1/repo/repo.go b/routers/api/v1/repo/repo.go index 36e644373..b86921be6 100644 --- a/routers/api/v1/repo/repo.go +++ b/routers/api/v1/repo/repo.go @@ -9,8 +9,6 @@ import ( "net/http" "strings" - api "code.gitea.io/sdk/gitea" - "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/auth" "code.gitea.io/gitea/modules/context" @@ -18,6 +16,7 @@ import ( "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/routers/api/v1/convert" + api "code.gitea.io/sdk/gitea" ) // Search repositories via options @@ -322,12 +321,13 @@ func Migrate(ctx *context.APIContext, form auth.MigrateRepoForm) { RemoteAddr: remoteAddr, }) if err != nil { + err = util.URLSanitizedError(err, remoteAddr) if repo != nil { if errDelete := models.DeleteRepository(ctx.User, ctxUser.ID, repo.ID); errDelete != nil { log.Error(4, "DeleteRepository: %v", errDelete) } } - ctx.Error(500, "MigrateRepository", models.HandleCloneUserCredentials(err.Error(), true)) + ctx.Error(500, "MigrateRepository", err) return } diff --git a/routers/repo/repo.go b/routers/repo/repo.go index dbe78f6d1..36105bfe1 100644 --- a/routers/repo/repo.go +++ b/routers/repo/repo.go @@ -20,6 +20,7 @@ import ( "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/util" ) const ( @@ -232,6 +233,9 @@ func MigratePost(ctx *context.Context, form auth.MigrateRepoForm) { return } + // remoteAddr may contain credentials, so we sanitize it + err = util.URLSanitizedError(err, remoteAddr) + if repo != nil { if errDelete := models.DeleteRepository(ctx.User, ctxUser.ID, repo.ID); errDelete != nil { log.Error(4, "DeleteRepository: %v", errDelete) @@ -241,11 +245,11 @@ func MigratePost(ctx *context.Context, form auth.MigrateRepoForm) { if strings.Contains(err.Error(), "Authentication failed") || strings.Contains(err.Error(), "could not read Username") { ctx.Data["Err_Auth"] = true - ctx.RenderWithErr(ctx.Tr("form.auth_failed", models.HandleCloneUserCredentials(err.Error(), true)), tplMigrate, &form) + ctx.RenderWithErr(ctx.Tr("form.auth_failed", err.Error()), tplMigrate, &form) return } else if strings.Contains(err.Error(), "fatal:") { ctx.Data["Err_CloneAddr"] = true - ctx.RenderWithErr(ctx.Tr("repo.migrate.failed", models.HandleCloneUserCredentials(err.Error(), true)), tplMigrate, &form) + ctx.RenderWithErr(ctx.Tr("repo.migrate.failed", err.Error()), tplMigrate, &form) return }