Merge pull request #2658 from creativeprojects/issue-2241

Don't echo authentication passwords (rest backend)

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

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")

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 {

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)
+			}
+		})
+	}
+}

internal/backend/location/location.go

@@ -24,22 +24,28 @@ type Location struct {
 }
 
 type parser struct {
-	scheme string
+	scheme        string
-	parse  func(string) (interface{}, error)
+	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},
+	{"b2", b2.ParseConfig, noPassword},
-	{"local", local.ParseConfig},
+	{"local", local.ParseConfig, noPassword},
-	{"sftp", sftp.ParseConfig},
+	{"sftp", sftp.ParseConfig, noPassword},
-	{"s3", s3.ParseConfig},
+	{"s3", s3.ParseConfig, noPassword},
-	{"gs", gs.ParseConfig},
+	{"gs", gs.ParseConfig, noPassword},
-	{"azure", azure.ParseConfig},
+	{"azure", azure.ParseConfig, noPassword},
-	{"swift", swift.ParseConfig},
+	{"swift", swift.ParseConfig, noPassword},
-	{"rest", rest.ParseConfig},
+	{"rest", rest.ParseConfig, rest.StripPassword},
-	{"rclone", rclone.ParseConfig},
+	{"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]

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:]
+	s = prepareURL(s)
-	if !strings.HasSuffix(s, "/") {
-		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
+}