diff --git a/changelog/unreleased/issue-2241 b/changelog/unreleased/issue-2241 new file mode 100644 index 000000000..23eb5cac5 --- /dev/null +++ b/changelog/unreleased/issue-2241 @@ -0,0 +1,10 @@ +Bugfix: Hide password in REST backend repository URLs + +When using a password in the REST backend repository URL, +the password could in some cases be included in the output +from restic, e.g. when initializing a repo or during an error. + +The password is now replaced with "***" where applicable. + +https://github.com/restic/restic/issues/2241 +https://github.com/restic/restic/pull/2658 diff --git a/cmd/restic/cmd_init.go b/cmd/restic/cmd_init.go index 6abc2695b..ae831c4e3 100644 --- a/cmd/restic/cmd_init.go +++ b/cmd/restic/cmd_init.go @@ -2,6 +2,7 @@ package main import ( "github.com/restic/chunker" + "github.com/restic/restic/internal/backend/location" "github.com/restic/restic/internal/errors" "github.com/restic/restic/internal/repository" @@ -53,7 +54,7 @@ func runInit(opts InitOptions, gopts GlobalOptions, args []string) error { be, err := create(gopts.Repo, gopts.extended) if err != nil { - return errors.Fatalf("create repository at %s failed: %v\n", gopts.Repo, err) + return errors.Fatalf("create repository at %s failed: %v\n", location.StripPassword(gopts.Repo), err) } gopts.password, err = ReadPasswordTwice(gopts, @@ -67,10 +68,10 @@ func runInit(opts InitOptions, gopts GlobalOptions, args []string) error { err = s.Init(gopts.ctx, gopts.password, chunkerPolynomial) if err != nil { - return errors.Fatalf("create key in repository at %s failed: %v\n", gopts.Repo, err) + return errors.Fatalf("create key in repository at %s failed: %v\n", location.StripPassword(gopts.Repo), err) } - Verbosef("created restic repository %v at %s\n", s.Config().ID[:10], gopts.Repo) + Verbosef("created restic repository %v at %s\n", s.Config().ID[:10], location.StripPassword(gopts.Repo)) Verbosef("\n") Verbosef("Please note that knowledge of your password is required to access\n") Verbosef("the repository. Losing your password means that your data is\n") diff --git a/cmd/restic/global.go b/cmd/restic/global.go index 7b917d5aa..c7b55a3de 100644 --- a/cmd/restic/global.go +++ b/cmd/restic/global.go @@ -631,7 +631,7 @@ func parseConfig(loc location.Location, opts options.Options) (interface{}, erro // Open the backend specified by a location config. func open(s string, gopts GlobalOptions, opts options.Options) (restic.Backend, error) { - debug.Log("parsing location %v", s) + debug.Log("parsing location %v", location.StripPassword(s)) loc, err := location.Parse(s) if err != nil { return nil, errors.Fatalf("parsing repository location failed: %v", err) @@ -686,13 +686,13 @@ func open(s string, gopts GlobalOptions, opts options.Options) (restic.Backend, } if err != nil { - return nil, errors.Fatalf("unable to open repo at %v: %v", s, err) + return nil, errors.Fatalf("unable to open repo at %v: %v", location.StripPassword(s), err) } // check if config is there fi, err := be.Stat(globalOptions.ctx, restic.Handle{Type: restic.ConfigFile}) if err != nil { - return nil, errors.Fatalf("unable to open config file: %v\nIs there a repository at the following location?\n%v", err, s) + return nil, errors.Fatalf("unable to open config file: %v\nIs there a repository at the following location?\n%v", err, location.StripPassword(s)) } if fi.Size == 0 { diff --git a/internal/backend/location/display_location_test.go b/internal/backend/location/display_location_test.go new file mode 100644 index 000000000..30d3cc286 --- /dev/null +++ b/internal/backend/location/display_location_test.go @@ -0,0 +1,96 @@ +package location + +import "testing" + +var passwordTests = []struct { + input string + expected string +}{ + { + "local:/srv/repo", + "local:/srv/repo", + }, + { + "/dir1/dir2", + "/dir1/dir2", + }, + { + `c:\dir1\foobar\dir2`, + `c:\dir1\foobar\dir2`, + }, + { + "sftp:user@host:/srv/repo", + "sftp:user@host:/srv/repo", + }, + { + "s3://eu-central-1/bucketname", + "s3://eu-central-1/bucketname", + }, + { + "swift:container17:/prefix97", + "swift:container17:/prefix97", + }, + { + "b2:bucketname:/prefix", + "b2:bucketname:/prefix", + }, + { + "rest:", + "rest:/", + }, + { + "rest:localhost/", + "rest:localhost/", + }, + { + "rest::123/", + "rest::123/", + }, + { + "rest:http://", + "rest:http://", + }, + { + "rest:http://hostname.foo:1234/", + "rest:http://hostname.foo:1234/", + }, + { + "rest:http://user@hostname.foo:1234/", + "rest:http://user@hostname.foo:1234/", + }, + { + "rest:http://user:@hostname.foo:1234/", + "rest:http://user:***@hostname.foo:1234/", + }, + { + "rest:http://user:p@hostname.foo:1234/", + "rest:http://user:***@hostname.foo:1234/", + }, + { + "rest:http://user:pppppaaafhhfuuwiiehhthhghhdkjaoowpprooghjjjdhhwuuhgjsjhhfdjhruuhsjsdhhfhshhsppwufhhsjjsjs@hostname.foo:1234/", + "rest:http://user:***@hostname.foo:1234/", + }, + { + "rest:http://user:password@hostname", + "rest:http://user:***@hostname/", + }, + { + "rest:http://user:password@:123", + "rest:http://user:***@:123/", + }, + { + "rest:http://user:password@", + "rest:http://user:***@/", + }, +} + +func TestStripPassword(t *testing.T) { + for i, test := range passwordTests { + t.Run(test.input, func(t *testing.T) { + result := StripPassword(test.input) + if result != test.expected { + t.Errorf("test %d: expected '%s' but got '%s'", i, test.expected, result) + } + }) + } +} diff --git a/internal/backend/location/location.go b/internal/backend/location/location.go index 68f4dfaff..43d6fa5a6 100644 --- a/internal/backend/location/location.go +++ b/internal/backend/location/location.go @@ -24,22 +24,28 @@ type Location struct { } type parser struct { - scheme string - parse func(string) (interface{}, error) + scheme string + parse func(string) (interface{}, error) + stripPassword func(string) string } // parsers is a list of valid config parsers for the backends. The first parser // is the fallback and should always be set to the local backend. var parsers = []parser{ - {"b2", b2.ParseConfig}, - {"local", local.ParseConfig}, - {"sftp", sftp.ParseConfig}, - {"s3", s3.ParseConfig}, - {"gs", gs.ParseConfig}, - {"azure", azure.ParseConfig}, - {"swift", swift.ParseConfig}, - {"rest", rest.ParseConfig}, - {"rclone", rclone.ParseConfig}, + {"b2", b2.ParseConfig, noPassword}, + {"local", local.ParseConfig, noPassword}, + {"sftp", sftp.ParseConfig, noPassword}, + {"s3", s3.ParseConfig, noPassword}, + {"gs", gs.ParseConfig, noPassword}, + {"azure", azure.ParseConfig, noPassword}, + {"swift", swift.ParseConfig, noPassword}, + {"rest", rest.ParseConfig, rest.StripPassword}, + {"rclone", rclone.ParseConfig, noPassword}, +} + +// noPassword returns the repository location unchanged (there's no sensitive information there) +func noPassword(s string) string { + return s } func isPath(s string) bool { @@ -107,6 +113,19 @@ func Parse(s string) (u Location, err error) { return u, nil } +// StripPassword returns a displayable version of a repository location (with any sensitive information removed) +func StripPassword(s string) string { + scheme := extractScheme(s) + + for _, parser := range parsers { + if parser.scheme != scheme { + continue + } + return parser.stripPassword(s) + } + return s +} + func extractScheme(s string) string { data := strings.SplitN(s, ":", 2) return data[0] diff --git a/internal/backend/rest/config.go b/internal/backend/rest/config.go index 60c6bf92b..51ff3b27c 100644 --- a/internal/backend/rest/config.go +++ b/internal/backend/rest/config.go @@ -31,10 +31,7 @@ func ParseConfig(s string) (interface{}, error) { return nil, errors.New("invalid REST backend specification") } - s = s[5:] - if !strings.HasSuffix(s, "/") { - s += "/" - } + s = prepareURL(s) u, err := url.Parse(s) @@ -46,3 +43,31 @@ func ParseConfig(s string) (interface{}, error) { cfg.URL = u return cfg, nil } + +// StripPassword removes the password from the URL +// If the repository location cannot be parsed as a valid URL, it will be returned as is +// (it's because this function is used for logging errors) +func StripPassword(s string) string { + scheme := s[:5] + s = prepareURL(s) + + u, err := url.Parse(s) + if err != nil { + return scheme + s + } + + if _, set := u.User.Password(); !set { + return scheme + s + } + + // a password was set: we replace it with *** + return scheme + strings.Replace(u.String(), u.User.String()+"@", u.User.Username()+":***@", 1) +} + +func prepareURL(s string) string { + s = s[5:] + if !strings.HasSuffix(s, "/") { + s += "/" + } + return s +}