From a2a2401a681c2ee89ffce113fc875454d600ec0a Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Sun, 7 Jul 2024 11:31:04 +0200 Subject: [PATCH 1/4] s3: prevent repeated credential queries with anonymous authentication --- internal/backend/s3/s3.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/internal/backend/s3/s3.go b/internal/backend/s3/s3.go index bddb57741..1b7f6e3d2 100644 --- a/internal/backend/s3/s3.go +++ b/internal/backend/s3/s3.go @@ -132,6 +132,9 @@ func getCredentials(cfg Config) (*credentials.Credentials, error) { if c.SignerType == credentials.SignatureAnonymous { debug.Log("using anonymous access for %#v", cfg.Endpoint) + // short circuit credentials resolution when using anonymous access + // otherwise the IAM provider would continuously try to (unsuccessfully) retrieve new credentials + creds = credentials.New(&credentials.Static{}) } roleArn := os.Getenv("RESTIC_AWS_ASSUME_ROLE_ARN") From 4b364940aa19bc3f6db9fa733b8bd8ebeb9cc034 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Sun, 7 Jul 2024 11:32:40 +0200 Subject: [PATCH 2/4] s3: use http client with configured timeouts for s3 IAM communication The default client has no timeouts configured opening network connections. Thus, if 169.254.169.254 is inaccessible, then the client would wait for until the operating system gives up, which will take several minutes. --- internal/backend/s3/s3.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/backend/s3/s3.go b/internal/backend/s3/s3.go index 1b7f6e3d2..6fe9e384b 100644 --- a/internal/backend/s3/s3.go +++ b/internal/backend/s3/s3.go @@ -52,7 +52,7 @@ func open(ctx context.Context, cfg Config, rt http.RoundTripper) (*Backend, erro minio.MaxRetry = int(cfg.MaxRetries) } - creds, err := getCredentials(cfg) + creds, err := getCredentials(cfg, rt) if err != nil { return nil, errors.Wrap(err, "s3.getCredentials") } @@ -97,7 +97,7 @@ func open(ctx context.Context, cfg Config, rt http.RoundTripper) (*Backend, erro // getCredentials -- runs through the various credential types and returns the first one that works. // additionally if the user has specified a role to assume, it will do that as well. -func getCredentials(cfg Config) (*credentials.Credentials, error) { +func getCredentials(cfg Config, tr http.RoundTripper) (*credentials.Credentials, error) { // Chains all credential types, in the following order: // - Static credentials provided by user // - AWS env vars (i.e. AWS_ACCESS_KEY_ID) @@ -120,7 +120,7 @@ func getCredentials(cfg Config) (*credentials.Credentials, error) { &credentials.FileMinioClient{}, &credentials.IAM{ Client: &http.Client{ - Transport: http.DefaultTransport, + Transport: tr, }, }, }) From f74e70cc36d52e05d1b7da69d9d694e4b5f6fde9 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Mon, 8 Jul 2024 19:42:00 +0200 Subject: [PATCH 3/4] s3: forbid anonymous authentication unless explicitly requested --- internal/backend/s3/config.go | 11 ++++++----- internal/backend/s3/s3.go | 13 +++++++++++-- internal/feature/registry.go | 2 ++ 3 files changed, 19 insertions(+), 7 deletions(-) diff --git a/internal/backend/s3/config.go b/internal/backend/s3/config.go index 4aea4c3d1..be2a78ce5 100644 --- a/internal/backend/s3/config.go +++ b/internal/backend/s3/config.go @@ -23,11 +23,12 @@ type Config struct { Layout string `option:"layout" help:"use this backend layout (default: auto-detect) (deprecated)"` 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"` - Region string `option:"region" help:"set region"` - BucketLookup string `option:"bucket-lookup" help:"bucket lookup style: 'auto', 'dns', or 'path'"` - ListObjectsV1 bool `option:"list-objects-v1" help:"use deprecated V1 api for ListObjects calls"` + 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"` + Region string `option:"region" help:"set region"` + BucketLookup string `option:"bucket-lookup" help:"bucket lookup style: 'auto', 'dns', or 'path'"` + ListObjectsV1 bool `option:"list-objects-v1" help:"use deprecated V1 api for ListObjects calls"` + UnsafeAnonymousAuth bool `option:"unsafe-anonymous-auth" help:"use anonymous authentication"` } // NewConfig returns a new Config with the default values filled in. diff --git a/internal/backend/s3/s3.go b/internal/backend/s3/s3.go index 6fe9e384b..019f8471b 100644 --- a/internal/backend/s3/s3.go +++ b/internal/backend/s3/s3.go @@ -98,6 +98,10 @@ func open(ctx context.Context, cfg Config, rt http.RoundTripper) (*Backend, erro // getCredentials -- runs through the various credential types and returns the first one that works. // additionally if the user has specified a role to assume, it will do that as well. func getCredentials(cfg Config, tr http.RoundTripper) (*credentials.Credentials, error) { + if cfg.UnsafeAnonymousAuth { + return credentials.New(&credentials.Static{}), nil + } + // Chains all credential types, in the following order: // - Static credentials provided by user // - AWS env vars (i.e. AWS_ACCESS_KEY_ID) @@ -131,9 +135,14 @@ func getCredentials(cfg Config, tr http.RoundTripper) (*credentials.Credentials, } if c.SignerType == credentials.SignatureAnonymous { + // Fail if no credentials were found to prevent repeated attempts to (unsuccessfully) retrieve new credentials. + // The first attempt still has to timeout which slows down restic usage considerably. Thus, migrate towards forcing + // users to explicitly decide between authenticated and anonymous access. + if feature.Flag.Enabled(feature.ExplicitS3AnonymousAuth) { + return nil, fmt.Errorf("no credentials found. Use `-o s3.unsafe-anonymous-auth=true` for anonymous authentication") + } + debug.Log("using anonymous access for %#v", cfg.Endpoint) - // short circuit credentials resolution when using anonymous access - // otherwise the IAM provider would continuously try to (unsuccessfully) retrieve new credentials creds = credentials.New(&credentials.Static{}) } diff --git a/internal/feature/registry.go b/internal/feature/registry.go index 74d8a2f61..6b8f6b397 100644 --- a/internal/feature/registry.go +++ b/internal/feature/registry.go @@ -9,6 +9,7 @@ const ( DeprecateLegacyIndex FlagName = "deprecate-legacy-index" DeprecateS3LegacyLayout FlagName = "deprecate-s3-legacy-layout" DeviceIDForHardlinks FlagName = "device-id-for-hardlinks" + ExplicitS3AnonymousAuth FlagName = "explicit-s3-anonymous-auth" SafeForgetKeepTags FlagName = "safe-forget-keep-tags" ) @@ -18,6 +19,7 @@ func init() { DeprecateLegacyIndex: {Type: Beta, Description: "disable support for index format used by restic 0.1.0. Use `restic repair index` to update the index if necessary."}, DeprecateS3LegacyLayout: {Type: Beta, Description: "disable support for S3 legacy layout used up to restic 0.7.0. Use `RESTIC_FEATURES=deprecate-s3-legacy-layout=false restic migrate s3_layout` to migrate your S3 repository if necessary."}, DeviceIDForHardlinks: {Type: Alpha, Description: "store deviceID only for hardlinks to reduce metadata changes for example when using btrfs subvolumes. Will be removed in a future restic version after repository format 3 is available"}, + ExplicitS3AnonymousAuth: {Type: Beta, Description: "forbid anonymous S3 authentication unless `-o s3.unsafe-anonymous-auth=true` is set"}, SafeForgetKeepTags: {Type: Beta, Description: "prevent deleting all snapshots if the tag passed to `forget --keep-tags tagname` does not exist"}, }) } From dc0db4eda47a366f40b043f2c778e74842b83824 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Mon, 8 Jul 2024 19:47:05 +0200 Subject: [PATCH 4/4] add s3 anonymous authentication changelog entry --- changelog/unreleased/issue-4707 | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100644 changelog/unreleased/issue-4707 diff --git a/changelog/unreleased/issue-4707 b/changelog/unreleased/issue-4707 new file mode 100644 index 000000000..3c5ffa2ad --- /dev/null +++ b/changelog/unreleased/issue-4707 @@ -0,0 +1,14 @@ +Change: Disallow S3 anonymous authentication by default + +When using the S3 backend with anonymous authentication, it continuously tried +to retrieve new authentication credentials, which caused bad performance. + +Now, to use anonymous authentication, it is necessary to pass the option `-o +s3.unsafe-anonymous-auth=true` to restic. + +It is temporarily possible to revert to the old behavior by setting the +environment variable `RESTIC_FEATURES=explicit-s3-anonymous-auth=false`. Note +that this feature flag will be removed in the next minor restic version. + +https://github.com/restic/restic/issues/4707 +https://github.com/restic/restic/pull/4908