From 5a11d14082806c47b5c17d42d81257d39096bfa6 Mon Sep 17 00:00:00 2001
From: Michael Eischer <michael.eischer@fau.de>
Date: Wed, 4 Aug 2021 22:56:18 +0200
Subject: [PATCH] redacted keys/token in backend config debug log

---
 cmd/restic/global.go                   | 18 ++++-----
 internal/backend/azure/azure.go        |  2 +-
 internal/backend/azure/azure_test.go   |  5 ++-
 internal/backend/azure/config.go       |  2 +-
 internal/backend/b2/b2.go              |  2 +-
 internal/backend/b2/b2_test.go         |  3 +-
 internal/backend/b2/config.go          |  2 +-
 internal/backend/s3/config.go          | 15 +++----
 internal/backend/s3/s3.go              |  2 +-
 internal/backend/s3/s3_test.go         |  5 ++-
 internal/backend/swift/config.go       | 17 ++++++--
 internal/backend/swift/swift.go        |  4 +-
 internal/options/secret_string.go      | 24 +++++++++++
 internal/options/secret_string_test.go | 55 ++++++++++++++++++++++++++
 14 files changed, 124 insertions(+), 32 deletions(-)
 create mode 100644 internal/options/secret_string.go
 create mode 100644 internal/options/secret_string_test.go

diff --git a/cmd/restic/global.go b/cmd/restic/global.go
index 65dbbb6be..85ea704bc 100644
--- a/cmd/restic/global.go
+++ b/cmd/restic/global.go
@@ -555,13 +555,13 @@ func parseConfig(loc location.Location, opts options.Options) (interface{}, erro
 			cfg.KeyID = os.Getenv("AWS_ACCESS_KEY_ID")
 		}
 
-		if cfg.Secret == "" {
-			cfg.Secret = os.Getenv("AWS_SECRET_ACCESS_KEY")
+		if cfg.Secret.String() == "" {
+			cfg.Secret = options.NewSecretString(os.Getenv("AWS_SECRET_ACCESS_KEY"))
 		}
 
-		if cfg.KeyID == "" && cfg.Secret != "" {
+		if cfg.KeyID == "" && cfg.Secret.String() != "" {
 			return nil, errors.Fatalf("unable to open S3 backend: Key ID ($AWS_ACCESS_KEY_ID) is empty")
-		} else if cfg.KeyID != "" && cfg.Secret == "" {
+		} else if cfg.KeyID != "" && cfg.Secret.String() == "" {
 			return nil, errors.Fatalf("unable to open S3 backend: Secret ($AWS_SECRET_ACCESS_KEY) is empty")
 		}
 
@@ -595,8 +595,8 @@ func parseConfig(loc location.Location, opts options.Options) (interface{}, erro
 			cfg.AccountName = os.Getenv("AZURE_ACCOUNT_NAME")
 		}
 
-		if cfg.AccountKey == "" {
-			cfg.AccountKey = os.Getenv("AZURE_ACCOUNT_KEY")
+		if cfg.AccountKey.String() == "" {
+			cfg.AccountKey = options.NewSecretString(os.Getenv("AZURE_ACCOUNT_KEY"))
 		}
 
 		if err := opts.Apply(loc.Scheme, &cfg); err != nil {
@@ -631,11 +631,11 @@ func parseConfig(loc location.Location, opts options.Options) (interface{}, erro
 			return nil, errors.Fatalf("unable to open B2 backend: Account ID ($B2_ACCOUNT_ID) is empty")
 		}
 
-		if cfg.Key == "" {
-			cfg.Key = os.Getenv("B2_ACCOUNT_KEY")
+		if cfg.Key.String() == "" {
+			cfg.Key = options.NewSecretString(os.Getenv("B2_ACCOUNT_KEY"))
 		}
 
-		if cfg.Key == "" {
+		if cfg.Key.String() == "" {
 			return nil, errors.Fatalf("unable to open B2 backend: Key ($B2_ACCOUNT_KEY) is empty")
 		}
 
diff --git a/internal/backend/azure/azure.go b/internal/backend/azure/azure.go
index ff89a6b01..8e209f356 100644
--- a/internal/backend/azure/azure.go
+++ b/internal/backend/azure/azure.go
@@ -39,7 +39,7 @@ var _ restic.Backend = &Backend{}
 func open(cfg Config, rt http.RoundTripper) (*Backend, error) {
 	debug.Log("open, config %#v", cfg)
 
-	client, err := storage.NewBasicClient(cfg.AccountName, cfg.AccountKey)
+	client, err := storage.NewBasicClient(cfg.AccountName, cfg.AccountKey.Unwrap())
 	if err != nil {
 		return nil, errors.Wrap(err, "NewBasicClient")
 	}
diff --git a/internal/backend/azure/azure_test.go b/internal/backend/azure/azure_test.go
index a9ed94cd2..f1e58dea3 100644
--- a/internal/backend/azure/azure_test.go
+++ b/internal/backend/azure/azure_test.go
@@ -13,6 +13,7 @@ import (
 	"github.com/restic/restic/internal/backend/azure"
 	"github.com/restic/restic/internal/backend/test"
 	"github.com/restic/restic/internal/errors"
+	"github.com/restic/restic/internal/options"
 	"github.com/restic/restic/internal/restic"
 	rtest "github.com/restic/restic/internal/test"
 )
@@ -36,7 +37,7 @@ func newAzureTestSuite(t testing.TB) *test.Suite {
 
 			cfg := azcfg.(azure.Config)
 			cfg.AccountName = os.Getenv("RESTIC_TEST_AZURE_ACCOUNT_NAME")
-			cfg.AccountKey = os.Getenv("RESTIC_TEST_AZURE_ACCOUNT_KEY")
+			cfg.AccountKey = options.NewSecretString(os.Getenv("RESTIC_TEST_AZURE_ACCOUNT_KEY"))
 			cfg.Prefix = fmt.Sprintf("test-%d", time.Now().UnixNano())
 			return cfg, nil
 		},
@@ -146,7 +147,7 @@ func TestUploadLargeFile(t *testing.T) {
 
 	cfg := azcfg.(azure.Config)
 	cfg.AccountName = os.Getenv("RESTIC_TEST_AZURE_ACCOUNT_NAME")
-	cfg.AccountKey = os.Getenv("RESTIC_TEST_AZURE_ACCOUNT_KEY")
+	cfg.AccountKey = options.NewSecretString(os.Getenv("RESTIC_TEST_AZURE_ACCOUNT_KEY"))
 	cfg.Prefix = fmt.Sprintf("test-upload-large-%d", time.Now().UnixNano())
 
 	tr, err := backend.Transport(backend.TransportOptions{})
diff --git a/internal/backend/azure/config.go b/internal/backend/azure/config.go
index 682356b01..0a6079797 100644
--- a/internal/backend/azure/config.go
+++ b/internal/backend/azure/config.go
@@ -12,7 +12,7 @@ import (
 // server.
 type Config struct {
 	AccountName string
-	AccountKey  string
+	AccountKey  options.SecretString
 	Container   string
 	Prefix      string
 
diff --git a/internal/backend/b2/b2.go b/internal/backend/b2/b2.go
index 7f8019a74..ff384e8e8 100644
--- a/internal/backend/b2/b2.go
+++ b/internal/backend/b2/b2.go
@@ -34,7 +34,7 @@ var _ restic.Backend = &b2Backend{}
 func newClient(ctx context.Context, cfg Config, rt http.RoundTripper) (*b2.Client, error) {
 	opts := []b2.ClientOption{b2.Transport(rt)}
 
-	c, err := b2.NewClient(ctx, cfg.AccountID, cfg.Key, opts...)
+	c, err := b2.NewClient(ctx, cfg.AccountID, cfg.Key.Unwrap(), opts...)
 	if err != nil {
 		return nil, errors.Wrap(err, "b2.NewClient")
 	}
diff --git a/internal/backend/b2/b2_test.go b/internal/backend/b2/b2_test.go
index 9f97de4f9..123a61d7c 100644
--- a/internal/backend/b2/b2_test.go
+++ b/internal/backend/b2/b2_test.go
@@ -10,6 +10,7 @@ import (
 	"github.com/restic/restic/internal/backend"
 	"github.com/restic/restic/internal/backend/b2"
 	"github.com/restic/restic/internal/backend/test"
+	"github.com/restic/restic/internal/options"
 	"github.com/restic/restic/internal/restic"
 
 	rtest "github.com/restic/restic/internal/test"
@@ -37,7 +38,7 @@ func newB2TestSuite(t testing.TB) *test.Suite {
 
 			cfg := b2cfg.(b2.Config)
 			cfg.AccountID = os.Getenv("RESTIC_TEST_B2_ACCOUNT_ID")
-			cfg.Key = os.Getenv("RESTIC_TEST_B2_ACCOUNT_KEY")
+			cfg.Key = options.NewSecretString(os.Getenv("RESTIC_TEST_B2_ACCOUNT_KEY"))
 			cfg.Prefix = fmt.Sprintf("test-%d", time.Now().UnixNano())
 			return cfg, nil
 		},
diff --git a/internal/backend/b2/config.go b/internal/backend/b2/config.go
index f10e4d10d..98e8e1445 100644
--- a/internal/backend/b2/config.go
+++ b/internal/backend/b2/config.go
@@ -13,7 +13,7 @@ import (
 // server.
 type Config struct {
 	AccountID string
-	Key       string
+	Key       options.SecretString
 	Bucket    string
 	Prefix    string
 
diff --git a/internal/backend/s3/config.go b/internal/backend/s3/config.go
index 77b712fc7..9e83f4004 100644
--- a/internal/backend/s3/config.go
+++ b/internal/backend/s3/config.go
@@ -12,13 +12,14 @@ import (
 // Config contains all configuration necessary to connect to an s3 compatible
 // server.
 type Config struct {
-	Endpoint      string
-	UseHTTP       bool
-	KeyID, Secret string
-	Bucket        string
-	Prefix        string
-	Layout        string `option:"layout" help:"use this backend layout (default: auto-detect)"`
-	StorageClass  string `option:"storage-class" help:"set S3 storage class (STANDARD, STANDARD_IA, ONEZONE_IA, INTELLIGENT_TIERING or REDUCED_REDUNDANCY)"`
+	Endpoint     string
+	UseHTTP      bool
+	KeyID        string
+	Secret       options.SecretString
+	Bucket       string
+	Prefix       string
+	Layout       string `option:"layout" help:"use this backend layout (default: auto-detect)"`
+	StorageClass string `option:"storage-class" help:"set S3 storage class (STANDARD, STANDARD_IA, ONEZONE_IA, INTELLIGENT_TIERING or REDUCED_REDUNDANCY)"`
 
 	Connections   uint   `option:"connections" help:"set a limit for the number of concurrent connections (default: 5)"`
 	MaxRetries    uint   `option:"retries" help:"set the number of retries attempted"`
diff --git a/internal/backend/s3/s3.go b/internal/backend/s3/s3.go
index ac1a1d5ce..276f953ab 100644
--- a/internal/backend/s3/s3.go
+++ b/internal/backend/s3/s3.go
@@ -56,7 +56,7 @@ func open(ctx context.Context, cfg Config, rt http.RoundTripper) (*Backend, erro
 		&credentials.Static{
 			Value: credentials.Value{
 				AccessKeyID:     cfg.KeyID,
-				SecretAccessKey: cfg.Secret,
+				SecretAccessKey: cfg.Secret.Unwrap(),
 			},
 		},
 		&credentials.EnvMinio{},
diff --git a/internal/backend/s3/s3_test.go b/internal/backend/s3/s3_test.go
index ba7a408dd..46d96fcbd 100644
--- a/internal/backend/s3/s3_test.go
+++ b/internal/backend/s3/s3_test.go
@@ -18,6 +18,7 @@ import (
 	"github.com/restic/restic/internal/backend"
 	"github.com/restic/restic/internal/backend/s3"
 	"github.com/restic/restic/internal/backend/test"
+	"github.com/restic/restic/internal/options"
 	"github.com/restic/restic/internal/restic"
 	rtest "github.com/restic/restic/internal/test"
 )
@@ -141,7 +142,7 @@ func newMinioTestSuite(ctx context.Context, t testing.TB) *test.Suite {
 			cfg.Config.Prefix = fmt.Sprintf("test-%d", time.Now().UnixNano())
 			cfg.Config.UseHTTP = true
 			cfg.Config.KeyID = key
-			cfg.Config.Secret = secret
+			cfg.Config.Secret = options.NewSecretString(secret)
 			return cfg, nil
 		},
 
@@ -239,7 +240,7 @@ func newS3TestSuite(t testing.TB) *test.Suite {
 
 			cfg := s3cfg.(s3.Config)
 			cfg.KeyID = os.Getenv("RESTIC_TEST_S3_KEY")
-			cfg.Secret = os.Getenv("RESTIC_TEST_S3_SECRET")
+			cfg.Secret = options.NewSecretString(os.Getenv("RESTIC_TEST_S3_SECRET"))
 			cfg.Prefix = fmt.Sprintf("test-%d", time.Now().UnixNano())
 			return cfg, nil
 		},
diff --git a/internal/backend/swift/config.go b/internal/backend/swift/config.go
index 8ca26a918..d2751dd1a 100644
--- a/internal/backend/swift/config.go
+++ b/internal/backend/swift/config.go
@@ -24,12 +24,12 @@ type Config struct {
 	TrustID        string
 
 	StorageURL string
-	AuthToken  string
+	AuthToken  options.SecretString
 
 	// auth v3 only
 	ApplicationCredentialID     string
 	ApplicationCredentialName   string
-	ApplicationCredentialSecret string
+	ApplicationCredentialSecret options.SecretString
 
 	Container              string
 	Prefix                 string
@@ -111,11 +111,9 @@ func ApplyEnvironment(prefix string, cfg interface{}) error {
 		// Application Credential auth
 		{&c.ApplicationCredentialID, prefix + "OS_APPLICATION_CREDENTIAL_ID"},
 		{&c.ApplicationCredentialName, prefix + "OS_APPLICATION_CREDENTIAL_NAME"},
-		{&c.ApplicationCredentialSecret, prefix + "OS_APPLICATION_CREDENTIAL_SECRET"},
 
 		// Manual authentication
 		{&c.StorageURL, prefix + "OS_STORAGE_URL"},
-		{&c.AuthToken, prefix + "OS_AUTH_TOKEN"},
 
 		{&c.DefaultContainerPolicy, prefix + "SWIFT_DEFAULT_CONTAINER_POLICY"},
 	} {
@@ -123,5 +121,16 @@ func ApplyEnvironment(prefix string, cfg interface{}) error {
 			*val.s = os.Getenv(val.env)
 		}
 	}
+	for _, val := range []struct {
+		s   *options.SecretString
+		env string
+	}{
+		{&c.ApplicationCredentialSecret, prefix + "OS_APPLICATION_CREDENTIAL_SECRET"},
+		{&c.AuthToken, prefix + "OS_AUTH_TOKEN"},
+	} {
+		if val.s.String() == "" {
+			*val.s = options.NewSecretString(os.Getenv(val.env))
+		}
+	}
 	return nil
 }
diff --git a/internal/backend/swift/swift.go b/internal/backend/swift/swift.go
index b127cb832..90f7cec98 100644
--- a/internal/backend/swift/swift.go
+++ b/internal/backend/swift/swift.go
@@ -60,10 +60,10 @@ func Open(ctx context.Context, cfg Config, rt http.RoundTripper) (restic.Backend
 			TenantDomainId:              cfg.TenantDomainID,
 			TrustId:                     cfg.TrustID,
 			StorageUrl:                  cfg.StorageURL,
-			AuthToken:                   cfg.AuthToken,
+			AuthToken:                   cfg.AuthToken.Unwrap(),
 			ApplicationCredentialId:     cfg.ApplicationCredentialID,
 			ApplicationCredentialName:   cfg.ApplicationCredentialName,
-			ApplicationCredentialSecret: cfg.ApplicationCredentialSecret,
+			ApplicationCredentialSecret: cfg.ApplicationCredentialSecret.Unwrap(),
 			ConnectTimeout:              time.Minute,
 			Timeout:                     time.Minute,
 
diff --git a/internal/options/secret_string.go b/internal/options/secret_string.go
new file mode 100644
index 000000000..26e8da5af
--- /dev/null
+++ b/internal/options/secret_string.go
@@ -0,0 +1,24 @@
+package options
+
+type SecretString struct {
+	s *string
+}
+
+func NewSecretString(s string) SecretString {
+	return SecretString{s: &s}
+}
+
+func (s SecretString) GoString() string {
+	return `"` + s.String() + `"`
+}
+
+func (s SecretString) String() string {
+	if len(*s.s) == 0 {
+		return ``
+	}
+	return `**redacted**`
+}
+
+func (s *SecretString) Unwrap() string {
+	return *s.s
+}
diff --git a/internal/options/secret_string_test.go b/internal/options/secret_string_test.go
new file mode 100644
index 000000000..7e616ab50
--- /dev/null
+++ b/internal/options/secret_string_test.go
@@ -0,0 +1,55 @@
+package options_test
+
+import (
+	"fmt"
+	"strings"
+	"testing"
+
+	"github.com/restic/restic/internal/options"
+	"github.com/restic/restic/internal/test"
+)
+
+type secretTest struct {
+	str options.SecretString
+}
+
+func assertNotIn(t *testing.T, str string, substr string) {
+	if strings.Contains(str, substr) {
+		t.Fatalf("'%s' should not contain '%s'", str, substr)
+	}
+}
+
+func TestSecretString(t *testing.T) {
+	keyStr := "secret-key"
+	secret := options.NewSecretString(keyStr)
+
+	test.Equals(t, "**redacted**", secret.String())
+	test.Equals(t, `"**redacted**"`, secret.GoString())
+	test.Equals(t, "**redacted**", fmt.Sprint(secret))
+	test.Equals(t, "**redacted**", fmt.Sprintf("%v", secret))
+	test.Equals(t, `"**redacted**"`, fmt.Sprintf("%#v", secret))
+	test.Equals(t, keyStr, secret.Unwrap())
+}
+
+func TestSecretStringStruct(t *testing.T) {
+	keyStr := "secret-key"
+	secretStruct := &secretTest{
+		str: options.NewSecretString(keyStr),
+	}
+
+	assertNotIn(t, fmt.Sprint(secretStruct), keyStr)
+	assertNotIn(t, fmt.Sprintf("%v", secretStruct), keyStr)
+	assertNotIn(t, fmt.Sprintf("%#v", secretStruct), keyStr)
+}
+
+func TestSecretStringEmpty(t *testing.T) {
+	keyStr := ""
+	secret := options.NewSecretString(keyStr)
+
+	test.Equals(t, "", secret.String())
+	test.Equals(t, `""`, secret.GoString())
+	test.Equals(t, "", fmt.Sprint(secret))
+	test.Equals(t, "", fmt.Sprintf("%v", secret))
+	test.Equals(t, `""`, fmt.Sprintf("%#v", secret))
+	test.Equals(t, keyStr, secret.Unwrap())
+}