From 0936d864a423bf16a265557b11e465cbdd56d82f Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Wed, 4 Aug 2021 22:19:44 +0200 Subject: [PATCH 1/3] redact http authorization header in debug log output --- internal/debug/round_tripper_debug.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/internal/debug/round_tripper_debug.go b/internal/debug/round_tripper_debug.go index 22219f9b7..339c49833 100644 --- a/internal/debug/round_tripper_debug.go +++ b/internal/debug/round_tripper_debug.go @@ -76,6 +76,12 @@ func RoundTripper(upstream http.RoundTripper) http.RoundTripper { } func (tr loggingRoundTripper) RoundTrip(req *http.Request) (res *http.Response, err error) { + // save original auth and redact it + origAuth, hasAuth := req.Header["Authorization"] + if hasAuth { + req.Header["Authorization"] = []string{"**redacted**"} + } + trace, err := httputil.DumpRequestOut(req, false) if err != nil { Log("DumpRequestOut() error: %v\n", err) @@ -83,6 +89,11 @@ func (tr loggingRoundTripper) RoundTrip(req *http.Request) (res *http.Response, Log("------------ HTTP REQUEST -----------\n%s", trace) } + // restore auth + if hasAuth { + req.Header["Authorization"] = origAuth + } + res, err = tr.RoundTripper.RoundTrip(req) if err != nil { Log("RoundTrip() returned error: %v", err) From 5a11d14082806c47b5c17d42d81257d39096bfa6 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Wed, 4 Aug 2021 22:56:18 +0200 Subject: [PATCH 2/3] 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()) +} From 6923353c43cd59ce55e7a88d49e010d5e2e3fc0e Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Tue, 17 Aug 2021 18:43:13 +0200 Subject: [PATCH 3/3] redact swift auth token in debug output --- internal/debug/round_tripper_debug.go | 34 ++++++++++++---- internal/debug/round_tripper_debug_test.go | 46 ++++++++++++++++++++++ 2 files changed, 72 insertions(+), 8 deletions(-) create mode 100644 internal/debug/round_tripper_debug_test.go diff --git a/internal/debug/round_tripper_debug.go b/internal/debug/round_tripper_debug.go index 339c49833..020e798f0 100644 --- a/internal/debug/round_tripper_debug.go +++ b/internal/debug/round_tripper_debug.go @@ -75,12 +75,31 @@ func RoundTripper(upstream http.RoundTripper) http.RoundTripper { return eofRoundTripper } +func redactHeader(header http.Header) map[string][]string { + removedHeaders := make(map[string][]string) + for _, hdr := range []string{ + "Authorization", + "X-Auth-Token", // Swift headers + "X-Auth-Key", + } { + origHeader, hasHeader := header[hdr] + if hasHeader { + removedHeaders[hdr] = origHeader + header[hdr] = []string{"**redacted**"} + } + } + return removedHeaders +} + +func restoreHeader(header http.Header, origHeaders map[string][]string) { + for hdr, val := range origHeaders { + header[hdr] = val + } +} + func (tr loggingRoundTripper) RoundTrip(req *http.Request) (res *http.Response, err error) { // save original auth and redact it - origAuth, hasAuth := req.Header["Authorization"] - if hasAuth { - req.Header["Authorization"] = []string{"**redacted**"} - } + origHeaders := redactHeader(req.Header) trace, err := httputil.DumpRequestOut(req, false) if err != nil { @@ -89,10 +108,7 @@ func (tr loggingRoundTripper) RoundTrip(req *http.Request) (res *http.Response, Log("------------ HTTP REQUEST -----------\n%s", trace) } - // restore auth - if hasAuth { - req.Header["Authorization"] = origAuth - } + restoreHeader(req.Header, origHeaders) res, err = tr.RoundTripper.RoundTrip(req) if err != nil { @@ -100,7 +116,9 @@ func (tr loggingRoundTripper) RoundTrip(req *http.Request) (res *http.Response, } if res != nil { + origHeaders := redactHeader(res.Header) trace, err := httputil.DumpResponse(res, false) + restoreHeader(res.Header, origHeaders) if err != nil { Log("DumpResponse() error: %v\n", err) } else { diff --git a/internal/debug/round_tripper_debug_test.go b/internal/debug/round_tripper_debug_test.go new file mode 100644 index 000000000..2095bbc6e --- /dev/null +++ b/internal/debug/round_tripper_debug_test.go @@ -0,0 +1,46 @@ +// +build debug + +package debug + +import ( + "net/http" + "testing" + + "github.com/restic/restic/internal/test" +) + +func TestRedactHeader(t *testing.T) { + secretHeaders := []string{ + "Authorization", + "X-Auth-Token", + "X-Auth-Key", + } + + header := make(http.Header) + header["Authorization"] = []string{"123"} + header["X-Auth-Token"] = []string{"1234"} + header["X-Auth-Key"] = []string{"12345"} + header["Host"] = []string{"my.host"} + + origHeaders := redactHeader(header) + + for _, hdr := range secretHeaders { + test.Equals(t, "**redacted**", header[hdr][0]) + } + test.Equals(t, "my.host", header["Host"][0]) + + restoreHeader(header, origHeaders) + test.Equals(t, "123", header["Authorization"][0]) + test.Equals(t, "1234", header["X-Auth-Token"][0]) + test.Equals(t, "12345", header["X-Auth-Key"][0]) + test.Equals(t, "my.host", header["Host"][0]) + + delete(header, "X-Auth-Key") + origHeaders = redactHeader(header) + _, hasHeader := header["X-Auth-Key"] + test.Assert(t, !hasHeader, "Unexpected header: %v", header["X-Auth-Key"]) + + restoreHeader(header, origHeaders) + _, hasHeader = header["X-Auth-Key"] + test.Assert(t, !hasHeader, "Unexpected header: %v", header["X-Auth-Key"]) +}