From f90961dadf9f009ff0982e5cf21888611eb16642 Mon Sep 17 00:00:00 2001 From: Aaron J Todd Date: Tue, 7 Mar 2023 13:46:13 -0500 Subject: [PATCH 1/8] remove permanent IMDSv1 fallback in favor of per/request fallback --- feature/ec2/imds/request_middleware_test.go | 3 ++ feature/ec2/imds/token_provider.go | 54 --------------------- 2 files changed, 3 insertions(+), 54 deletions(-) diff --git a/feature/ec2/imds/request_middleware_test.go b/feature/ec2/imds/request_middleware_test.go index 0e751b0ce2f..cf5710b38c5 100644 --- a/feature/ec2/imds/request_middleware_test.go +++ b/feature/ec2/imds/request_middleware_test.go @@ -395,6 +395,7 @@ func TestRequestGetToken(t *testing.T) { ExpectTrace: []string{ getTokenPath, "/latest/foo", + getTokenPath, "/latest/foo", }, APICallCount: 2, @@ -416,6 +417,7 @@ func TestRequestGetToken(t *testing.T) { ExpectTrace: []string{ getTokenPath, "/latest/foo", + getTokenPath, "/latest/foo", }, APICallCount: 2, @@ -437,6 +439,7 @@ func TestRequestGetToken(t *testing.T) { ExpectTrace: []string{ getTokenPath, "/latest/foo", + getTokenPath, "/latest/foo", }, APICallCount: 2, diff --git a/feature/ec2/imds/token_provider.go b/feature/ec2/imds/token_provider.go index 275fade488a..d52ff476dc6 100644 --- a/feature/ec2/imds/token_provider.go +++ b/feature/ec2/imds/token_provider.go @@ -6,10 +6,8 @@ import ( "fmt" "net/http" "sync" - "sync/atomic" "time" - smithy "github.com/aws/smithy-go" "github.com/aws/smithy-go/middleware" smithyhttp "github.com/aws/smithy-go/transport/http" ) @@ -26,8 +24,6 @@ type tokenProvider struct { token *apiToken tokenMux sync.RWMutex - - disabled uint32 // Atomic updated } func newTokenProvider(client *Client, ttl time.Duration) *tokenProvider { @@ -60,18 +56,11 @@ func (t *tokenProvider) ID() string { return "APITokenProvider" } // token is not cached, it will be retrieved in a separate API call, getToken. // // For retry attempts, handler must be added after attempt retryer. -// -// If request for getToken fails the token provider may be disabled from future -// requests, depending on the response status code. func (t *tokenProvider) HandleFinalize( ctx context.Context, input middleware.FinalizeInput, next middleware.FinalizeHandler, ) ( out middleware.FinalizeOutput, metadata middleware.Metadata, err error, ) { - if !t.enabled() { - // short-circuits to insecure data flow if token provider is disabled. - return next.HandleFinalize(ctx, input) - } req, ok := input.Request.(*smithyhttp.Request) if !ok { @@ -116,7 +105,6 @@ func (t *tokenProvider) HandleDeserialize( if resp.StatusCode == http.StatusUnauthorized { // unauthorized err = &retryableError{Err: err} - t.enable() } return out, metadata, err @@ -131,12 +119,6 @@ func (*retryableError) RetryableError() bool { return true } func (e *retryableError) Error() string { return e.Err.Error() } func (t *tokenProvider) getToken(ctx context.Context) (tok *apiToken, err error) { - if !t.enabled() { - return nil, &bypassTokenRetrievalError{ - Err: fmt.Errorf("cannot get API token, provider disabled"), - } - } - t.tokenMux.RLock() tok = t.token t.tokenMux.RUnlock() @@ -167,31 +149,15 @@ func (t *tokenProvider) updateToken(ctx context.Context) (*apiToken, error) { TokenTTL: t.tokenTTL, }) if err != nil { - // change the disabled flag on token provider to true, when error is request timeout error. var statusErr interface{ HTTPStatusCode() int } if errors.As(err, &statusErr) { switch statusErr.HTTPStatusCode() { - - // Disable get token if failed because of 403, 404, or 405 - case http.StatusForbidden, - http.StatusNotFound, - http.StatusMethodNotAllowed: - - t.disable() - // 400 errors are terminal, and need to be upstreamed case http.StatusBadRequest: return nil, err } } - // Disable if request send failed or timed out getting response - var re *smithyhttp.RequestSendError - var ce *smithy.CanceledError - if errors.As(err, &re) || errors.As(err, &ce) { - atomic.StoreUint32(&t.disabled, 1) - } - // Token couldn't be retrieved, but bypass this, and allow the // request to continue. return nil, &bypassTokenRetrievalError{Err: err} @@ -215,23 +181,3 @@ func (e *bypassTokenRetrievalError) Error() string { } func (e *bypassTokenRetrievalError) Unwrap() error { return e.Err } - -// enabled returns if the token provider is current enabled or not. -func (t *tokenProvider) enabled() bool { - return atomic.LoadUint32(&t.disabled) == 0 -} - -// disable disables the token provider and it will no longer attempt to inject -// the token, nor request updates. -func (t *tokenProvider) disable() { - atomic.StoreUint32(&t.disabled, 1) -} - -// enable enables the token provide to start refreshing tokens, and adding them -// to the pending request. -func (t *tokenProvider) enable() { - t.tokenMux.Lock() - t.token = nil - t.tokenMux.Unlock() - atomic.StoreUint32(&t.disabled, 0) -} From f0d0a98fabec5b954ddcec5146310a3431d7c2ac Mon Sep 17 00:00:00 2001 From: Aaron J Todd Date: Wed, 8 Mar 2023 14:08:03 -0500 Subject: [PATCH 2/8] add flag to disable fallback to imdsv1 --- feature/ec2/imds/api_client.go | 10 ++++ feature/ec2/imds/request_middleware_test.go | 66 ++++++++++++++++++--- feature/ec2/imds/token_provider.go | 36 +++++++---- 3 files changed, 92 insertions(+), 20 deletions(-) diff --git a/feature/ec2/imds/api_client.go b/feature/ec2/imds/api_client.go index f97730bd931..d7156697a43 100644 --- a/feature/ec2/imds/api_client.go +++ b/feature/ec2/imds/api_client.go @@ -174,6 +174,16 @@ type Options struct { // The logger writer interface to write logging messages to. Logger logging.Logger + // Configure IMDSv1 fallback behavior. By default, the client will attempt + // to fall back to IMDSv1 as needed for backwards compatibility. When set to `true` + // the client will return any errors encountered from attempting to fetch a token + // instead of silently using the insecure data flow of IMDSv1. + // + // See [configuring IMDS] for more information. + // + // [configuring IMDS]: https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/configuring-instance-metadata-service.html + DisableFallback bool + // provides the caching of API tokens used for operation calls. If unset, // the API token will not be retrieved for the operation. tokenProvider *tokenProvider diff --git a/feature/ec2/imds/request_middleware_test.go b/feature/ec2/imds/request_middleware_test.go index cf5710b38c5..23cc8ce404e 100644 --- a/feature/ec2/imds/request_middleware_test.go +++ b/feature/ec2/imds/request_middleware_test.go @@ -5,10 +5,13 @@ import ( "context" "encoding/hex" "fmt" + "github.com/aws/aws-sdk-go-v2/aws" + "github.com/aws/smithy-go/logging" "io" "io/ioutil" "net/http" "net/http/httptest" + "os" "strings" "testing" "time" @@ -330,11 +333,12 @@ func (h *successAPIResponseHandler) ServeHTTP(w http.ResponseWriter, r *http.Req func TestRequestGetToken(t *testing.T) { cases := map[string]struct { - GetHandler func(*testing.T) http.Handler - APICallCount int - ExpectTrace []string - ExpectContent []byte - ExpectErr string + GetHandler func(*testing.T) http.Handler + APICallCount int + ExpectTrace []string + ExpectContent []byte + ExpectErr string + DisableFallback bool }{ "secure": { ExpectTrace: []string{ @@ -457,7 +461,7 @@ func TestRequestGetToken(t *testing.T) { ExpectContent: []byte("hello"), }, - // Token disabled and becomes re-enabled + // Token failure, fallback for one call, successful token then secure flow for following requests "unauthorized 401 re-enable": { ExpectTrace: []string{ getTokenPath, @@ -499,8 +503,51 @@ func TestRequestGetToken(t *testing.T) { }), )) }, + ExpectErr: "failed to get API token", + }, + + // retryable token error with fallback enabled (default) + "token failure fallback enabled": { + ExpectTrace: []string{ + getTokenPath, + getTokenPath, + getTokenPath, + "/latest/foo", + }, + APICallCount: 1, + GetHandler: func(t *testing.T) http.Handler { + return newTestServeMux(t, + newInsecureAPIHandler(t, + 500, + &successAPIResponseHandler{t: t, + path: "/latest/foo", + method: "GET", + body: []byte("hello"), + }, + )) + }, ExpectContent: []byte("hello"), - ExpectErr: "EC2 IMDS failed", + }, + // retryable token error with fallback disabled + "token failure fallback disabled": { + ExpectTrace: []string{ + getTokenPath, + getTokenPath, + getTokenPath, + }, + APICallCount: 1, + GetHandler: func(t *testing.T) http.Handler { + return newTestServeMux(t, + newInsecureAPIHandler(t, + 500, + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + t.Errorf("expected no call to API handler") + http.Error(w, "", 400) + }), + )) + }, + ExpectErr: "failed to get API token", + DisableFallback: true, }, } @@ -518,7 +565,10 @@ func TestRequestGetToken(t *testing.T) { defer server.Close() client := New(Options{ - Endpoint: server.URL, + Endpoint: server.URL, + DisableFallback: c.DisableFallback, + ClientLogMode: aws.LogRequest | aws.LogRetries, + Logger: logging.NewStandardLogger(os.Stdout), }) ctx := context.Background() diff --git a/feature/ec2/imds/token_provider.go b/feature/ec2/imds/token_provider.go index d52ff476dc6..4c1bfd7c8bb 100644 --- a/feature/ec2/imds/token_provider.go +++ b/feature/ec2/imds/token_provider.go @@ -104,20 +104,12 @@ func (t *tokenProvider) HandleDeserialize( } if resp.StatusCode == http.StatusUnauthorized { // unauthorized - err = &retryableError{Err: err} + err = &retryableError{Err: err, isRetryable: true} } return out, metadata, err } -type retryableError struct { - Err error -} - -func (*retryableError) RetryableError() bool { return true } - -func (e *retryableError) Error() string { return e.Err.Error() } - func (t *tokenProvider) getToken(ctx context.Context) (tok *apiToken, err error) { t.tokenMux.RLock() tok = t.token @@ -129,7 +121,7 @@ func (t *tokenProvider) getToken(ctx context.Context) (tok *apiToken, err error) tok, err = t.updateToken(ctx) if err != nil { - return nil, fmt.Errorf("cannot get API token, %w", err) + return nil, err } return tok, nil @@ -158,8 +150,19 @@ func (t *tokenProvider) updateToken(ctx context.Context) (*apiToken, error) { } } - // Token couldn't be retrieved, but bypass this, and allow the - // request to continue. + if t.client.options.DisableFallback { + // do not fallback to IMDSv1 insecure flow if token retrieval fails + // + // NOTE: getToken() is an implementation detail of some outer operation + // (e.g. GetMetadata). It has its own retries that have already been exhausted. + // Mark the underlying error as a terminal error. + err = &retryableError{Err: err, isRetryable: false} + return nil, err + } + + // Token couldn't be retrieved, fallback to IMDSv1 insecure flow for this request + // and allow the request to proceed. Future requests will re-attempt fetching a + // token. return nil, &bypassTokenRetrievalError{Err: err} } @@ -181,3 +184,12 @@ func (e *bypassTokenRetrievalError) Error() string { } func (e *bypassTokenRetrievalError) Unwrap() error { return e.Err } + +type retryableError struct { + Err error + isRetryable bool +} + +func (e *retryableError) RetryableError() bool { return e.isRetryable } + +func (e *retryableError) Error() string { return e.Err.Error() } From f65f1e48e03a2fa7f51613bab0750b92ee49cb9d Mon Sep 17 00:00:00 2001 From: Aaron J Todd Date: Wed, 8 Mar 2023 14:09:29 -0500 Subject: [PATCH 3/8] remove debug output --- feature/ec2/imds/request_middleware_test.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/feature/ec2/imds/request_middleware_test.go b/feature/ec2/imds/request_middleware_test.go index 23cc8ce404e..4a546d75dc2 100644 --- a/feature/ec2/imds/request_middleware_test.go +++ b/feature/ec2/imds/request_middleware_test.go @@ -5,13 +5,10 @@ import ( "context" "encoding/hex" "fmt" - "github.com/aws/aws-sdk-go-v2/aws" - "github.com/aws/smithy-go/logging" "io" "io/ioutil" "net/http" "net/http/httptest" - "os" "strings" "testing" "time" @@ -567,8 +564,6 @@ func TestRequestGetToken(t *testing.T) { client := New(Options{ Endpoint: server.URL, DisableFallback: c.DisableFallback, - ClientLogMode: aws.LogRequest | aws.LogRetries, - Logger: logging.NewStandardLogger(os.Stdout), }) ctx := context.Background() From ceb9642e62234934d4df212204881e4427308eea Mon Sep 17 00:00:00 2001 From: Aaron J Todd Date: Wed, 8 Mar 2023 14:15:46 -0500 Subject: [PATCH 4/8] add changelog --- .changelog/a1050420a5f7409fbc4d1d82c882d168.json | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 .changelog/a1050420a5f7409fbc4d1d82c882d168.json diff --git a/.changelog/a1050420a5f7409fbc4d1d82c882d168.json b/.changelog/a1050420a5f7409fbc4d1d82c882d168.json new file mode 100644 index 00000000000..4996783e782 --- /dev/null +++ b/.changelog/a1050420a5f7409fbc4d1d82c882d168.json @@ -0,0 +1,8 @@ +{ + "id": "a1050420-a5f7-409f-bc4d-1d82c882d168", + "type": "feature", + "description": "Add flag to disable IMDSv1 fallback", + "modules": [ + "feature/ec2/imds" + ] +} \ No newline at end of file From a82bdc98e65b439d3ea30374350a0f0f194b4302 Mon Sep 17 00:00:00 2001 From: Aaron J Todd Date: Wed, 8 Mar 2023 16:24:29 -0500 Subject: [PATCH 5/8] re-enable permanent fallback behavior --- feature/ec2/imds/request_middleware_test.go | 23 ++++++-- feature/ec2/imds/token_provider.go | 58 ++++++++++++++++++++- 2 files changed, 75 insertions(+), 6 deletions(-) diff --git a/feature/ec2/imds/request_middleware_test.go b/feature/ec2/imds/request_middleware_test.go index 4a546d75dc2..117e05741b2 100644 --- a/feature/ec2/imds/request_middleware_test.go +++ b/feature/ec2/imds/request_middleware_test.go @@ -396,7 +396,6 @@ func TestRequestGetToken(t *testing.T) { ExpectTrace: []string{ getTokenPath, "/latest/foo", - getTokenPath, "/latest/foo", }, APICallCount: 2, @@ -418,7 +417,6 @@ func TestRequestGetToken(t *testing.T) { ExpectTrace: []string{ getTokenPath, "/latest/foo", - getTokenPath, "/latest/foo", }, APICallCount: 2, @@ -440,7 +438,6 @@ func TestRequestGetToken(t *testing.T) { ExpectTrace: []string{ getTokenPath, "/latest/foo", - getTokenPath, "/latest/foo", }, APICallCount: 2, @@ -458,7 +455,7 @@ func TestRequestGetToken(t *testing.T) { ExpectContent: []byte("hello"), }, - // Token failure, fallback for one call, successful token then secure flow for following requests + // Token disabled and becomes re-enabled "unauthorized 401 re-enable": { ExpectTrace: []string{ getTokenPath, @@ -546,6 +543,24 @@ func TestRequestGetToken(t *testing.T) { ExpectErr: "failed to get API token", DisableFallback: true, }, + "insecure 403 fallback disabled": { + ExpectTrace: []string{ + getTokenPath, + }, + APICallCount: 1, + GetHandler: func(t *testing.T) http.Handler { + return newTestServeMux(t, + newInsecureAPIHandler(t, + 403, + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + t.Errorf("expected no call to API handler") + http.Error(w, "", 400) + }), + )) + }, + ExpectErr: "failed to get API token", + DisableFallback: true, + }, } type mockRequestOutput struct { diff --git a/feature/ec2/imds/token_provider.go b/feature/ec2/imds/token_provider.go index 4c1bfd7c8bb..49af7f9dec0 100644 --- a/feature/ec2/imds/token_provider.go +++ b/feature/ec2/imds/token_provider.go @@ -4,8 +4,10 @@ import ( "context" "errors" "fmt" + "github.com/aws/smithy-go" "net/http" "sync" + "sync/atomic" "time" "github.com/aws/smithy-go/middleware" @@ -24,6 +26,8 @@ type tokenProvider struct { token *apiToken tokenMux sync.RWMutex + + disabled uint32 // Atomic updated } func newTokenProvider(client *Client, ttl time.Duration) *tokenProvider { @@ -56,11 +60,18 @@ func (t *tokenProvider) ID() string { return "APITokenProvider" } // token is not cached, it will be retrieved in a separate API call, getToken. // // For retry attempts, handler must be added after attempt retryer. +// +// If request for getToken fails the token provider may be disabled from future +// requests, depending on the response status code. func (t *tokenProvider) HandleFinalize( ctx context.Context, input middleware.FinalizeInput, next middleware.FinalizeHandler, ) ( out middleware.FinalizeOutput, metadata middleware.Metadata, err error, ) { + if !t.enabled() { + // short-circuits to insecure data flow if token provider is disabled. + return next.HandleFinalize(ctx, input) + } req, ok := input.Request.(*smithyhttp.Request) if !ok { @@ -104,6 +115,7 @@ func (t *tokenProvider) HandleDeserialize( } if resp.StatusCode == http.StatusUnauthorized { // unauthorized + t.enable() err = &retryableError{Err: err, isRetryable: true} } @@ -111,6 +123,12 @@ func (t *tokenProvider) HandleDeserialize( } func (t *tokenProvider) getToken(ctx context.Context) (tok *apiToken, err error) { + if !t.enabled() { + return nil, &bypassTokenRetrievalError{ + Err: fmt.Errorf("cannot get API token, provider disabled"), + } + } + t.tokenMux.RLock() tok = t.token t.tokenMux.RUnlock() @@ -144,14 +162,30 @@ func (t *tokenProvider) updateToken(ctx context.Context) (*apiToken, error) { var statusErr interface{ HTTPStatusCode() int } if errors.As(err, &statusErr) { switch statusErr.HTTPStatusCode() { + // Disable future get token if failed because of 403, 404, or 405 + case http.StatusForbidden, + http.StatusNotFound, + http.StatusMethodNotAllowed: + + t.disable() + // 400 errors are terminal, and need to be upstreamed case http.StatusBadRequest: return nil, err } } + // Disable if request send failed or timed out getting response + var re *smithyhttp.RequestSendError + var ce *smithy.CanceledError + if errors.As(err, &re) || errors.As(err, &ce) { + atomic.StoreUint32(&t.disabled, 1) + } + if t.client.options.DisableFallback { // do not fallback to IMDSv1 insecure flow if token retrieval fails + atomic.StoreUint32(&t.disabled, 0) + // // NOTE: getToken() is an implementation detail of some outer operation // (e.g. GetMetadata). It has its own retries that have already been exhausted. @@ -161,8 +195,8 @@ func (t *tokenProvider) updateToken(ctx context.Context) (*apiToken, error) { } // Token couldn't be retrieved, fallback to IMDSv1 insecure flow for this request - // and allow the request to proceed. Future requests will re-attempt fetching a - // token. + // and allow the request to proceed. Future requests _may_ re-attempt fetching a + // token if not disabled. return nil, &bypassTokenRetrievalError{Err: err} } @@ -175,6 +209,26 @@ func (t *tokenProvider) updateToken(ctx context.Context) (*apiToken, error) { return tok, nil } +// enabled returns if the token provider is current enabled or not. +func (t *tokenProvider) enabled() bool { + return atomic.LoadUint32(&t.disabled) == 0 +} + +// disable disables the token provider and it will no longer attempt to inject +// the token, nor request updates. +func (t *tokenProvider) disable() { + atomic.StoreUint32(&t.disabled, 1) +} + +// enable enables the token provide to start refreshing tokens, and adding them +// to the pending request. +func (t *tokenProvider) enable() { + t.tokenMux.Lock() + t.token = nil + t.tokenMux.Unlock() + atomic.StoreUint32(&t.disabled, 0) +} + type bypassTokenRetrievalError struct { Err error } From 79982fe9bc3310d53ad0a67db5945867bfa570d2 Mon Sep 17 00:00:00 2001 From: Aaron J Todd Date: Wed, 8 Mar 2023 16:47:40 -0500 Subject: [PATCH 6/8] switch to positive config property using ternary --- feature/ec2/imds/api_client.go | 4 ++-- feature/ec2/imds/request_middleware_test.go | 25 +++++++++++---------- feature/ec2/imds/token_provider.go | 18 +++++++++++---- 3 files changed, 29 insertions(+), 18 deletions(-) diff --git a/feature/ec2/imds/api_client.go b/feature/ec2/imds/api_client.go index d7156697a43..e55edd992e2 100644 --- a/feature/ec2/imds/api_client.go +++ b/feature/ec2/imds/api_client.go @@ -175,14 +175,14 @@ type Options struct { Logger logging.Logger // Configure IMDSv1 fallback behavior. By default, the client will attempt - // to fall back to IMDSv1 as needed for backwards compatibility. When set to `true` + // to fall back to IMDSv1 as needed for backwards compatibility. When set to [aws.FalseTernary] // the client will return any errors encountered from attempting to fetch a token // instead of silently using the insecure data flow of IMDSv1. // // See [configuring IMDS] for more information. // // [configuring IMDS]: https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/configuring-instance-metadata-service.html - DisableFallback bool + EnableFallback aws.Ternary // provides the caching of API tokens used for operation calls. If unset, // the API token will not be retrieved for the operation. diff --git a/feature/ec2/imds/request_middleware_test.go b/feature/ec2/imds/request_middleware_test.go index 117e05741b2..53fd8b6ed73 100644 --- a/feature/ec2/imds/request_middleware_test.go +++ b/feature/ec2/imds/request_middleware_test.go @@ -5,6 +5,7 @@ import ( "context" "encoding/hex" "fmt" + "github.com/aws/aws-sdk-go-v2/aws" "io" "io/ioutil" "net/http" @@ -330,12 +331,12 @@ func (h *successAPIResponseHandler) ServeHTTP(w http.ResponseWriter, r *http.Req func TestRequestGetToken(t *testing.T) { cases := map[string]struct { - GetHandler func(*testing.T) http.Handler - APICallCount int - ExpectTrace []string - ExpectContent []byte - ExpectErr string - DisableFallback bool + GetHandler func(*testing.T) http.Handler + APICallCount int + ExpectTrace []string + ExpectContent []byte + ExpectErr string + EnableFallback aws.Ternary }{ "secure": { ExpectTrace: []string{ @@ -540,8 +541,8 @@ func TestRequestGetToken(t *testing.T) { }), )) }, - ExpectErr: "failed to get API token", - DisableFallback: true, + ExpectErr: "failed to get API token", + EnableFallback: aws.BoolTernary(false), }, "insecure 403 fallback disabled": { ExpectTrace: []string{ @@ -558,8 +559,8 @@ func TestRequestGetToken(t *testing.T) { }), )) }, - ExpectErr: "failed to get API token", - DisableFallback: true, + ExpectErr: "failed to get API token", + EnableFallback: aws.BoolTernary(false), }, } @@ -577,8 +578,8 @@ func TestRequestGetToken(t *testing.T) { defer server.Close() client := New(Options{ - Endpoint: server.URL, - DisableFallback: c.DisableFallback, + Endpoint: server.URL, + EnableFallback: c.EnableFallback, }) ctx := context.Background() diff --git a/feature/ec2/imds/token_provider.go b/feature/ec2/imds/token_provider.go index 49af7f9dec0..0400966f9bc 100644 --- a/feature/ec2/imds/token_provider.go +++ b/feature/ec2/imds/token_provider.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "github.com/aws/aws-sdk-go-v2/aws" "github.com/aws/smithy-go" "net/http" "sync" @@ -68,7 +69,7 @@ func (t *tokenProvider) HandleFinalize( ) ( out middleware.FinalizeOutput, metadata middleware.Metadata, err error, ) { - if !t.enabled() { + if t.fallbackEnabled() && !t.enabled() { // short-circuits to insecure data flow if token provider is disabled. return next.HandleFinalize(ctx, input) } @@ -123,7 +124,7 @@ func (t *tokenProvider) HandleDeserialize( } func (t *tokenProvider) getToken(ctx context.Context) (tok *apiToken, err error) { - if !t.enabled() { + if t.fallbackEnabled() && !t.enabled() { return nil, &bypassTokenRetrievalError{ Err: fmt.Errorf("cannot get API token, provider disabled"), } @@ -182,11 +183,10 @@ func (t *tokenProvider) updateToken(ctx context.Context) (*apiToken, error) { atomic.StoreUint32(&t.disabled, 1) } - if t.client.options.DisableFallback { + if !t.fallbackEnabled() { // do not fallback to IMDSv1 insecure flow if token retrieval fails atomic.StoreUint32(&t.disabled, 0) - // // NOTE: getToken() is an implementation detail of some outer operation // (e.g. GetMetadata). It has its own retries that have already been exhausted. // Mark the underlying error as a terminal error. @@ -214,6 +214,16 @@ func (t *tokenProvider) enabled() bool { return atomic.LoadUint32(&t.disabled) == 0 } +// fallbackEnabled returns false if EnableFallback is [aws.FalseTernary], true otherwise +func (t *tokenProvider) fallbackEnabled() bool { + switch t.client.options.EnableFallback { + case aws.FalseTernary: + return false + default: + return true + } +} + // disable disables the token provider and it will no longer attempt to inject // the token, nor request updates. func (t *tokenProvider) disable() { From 350ebfc4e928e3a7ade54cfdc6803c30a1982d8d Mon Sep 17 00:00:00 2001 From: Aaron J Todd Date: Thu, 9 Mar 2023 09:52:44 -0500 Subject: [PATCH 7/8] log when fallback happens --- feature/ec2/imds/token_provider.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/feature/ec2/imds/token_provider.go b/feature/ec2/imds/token_provider.go index 0400966f9bc..dc557748213 100644 --- a/feature/ec2/imds/token_provider.go +++ b/feature/ec2/imds/token_provider.go @@ -6,6 +6,7 @@ import ( "fmt" "github.com/aws/aws-sdk-go-v2/aws" "github.com/aws/smithy-go" + "github.com/aws/smithy-go/logging" "net/http" "sync" "sync/atomic" @@ -168,7 +169,10 @@ func (t *tokenProvider) updateToken(ctx context.Context) (*apiToken, error) { http.StatusNotFound, http.StatusMethodNotAllowed: - t.disable() + if t.fallbackEnabled() { + t.client.options.Logger.Logf(logging.Warn, "falling back to IMDSv1: %v", err) + t.disable() + } // 400 errors are terminal, and need to be upstreamed case http.StatusBadRequest: @@ -184,9 +188,6 @@ func (t *tokenProvider) updateToken(ctx context.Context) (*apiToken, error) { } if !t.fallbackEnabled() { - // do not fallback to IMDSv1 insecure flow if token retrieval fails - atomic.StoreUint32(&t.disabled, 0) - // NOTE: getToken() is an implementation detail of some outer operation // (e.g. GetMetadata). It has its own retries that have already been exhausted. // Mark the underlying error as a terminal error. From 4744abc4de2b031cd6af9998aeffd29e6bd1be23 Mon Sep 17 00:00:00 2001 From: Aaron J Todd Date: Thu, 9 Mar 2023 10:53:47 -0500 Subject: [PATCH 8/8] fix logger panic --- feature/ec2/imds/token_provider.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/feature/ec2/imds/token_provider.go b/feature/ec2/imds/token_provider.go index dc557748213..5703c6e16ad 100644 --- a/feature/ec2/imds/token_provider.go +++ b/feature/ec2/imds/token_provider.go @@ -170,7 +170,8 @@ func (t *tokenProvider) updateToken(ctx context.Context) (*apiToken, error) { http.StatusMethodNotAllowed: if t.fallbackEnabled() { - t.client.options.Logger.Logf(logging.Warn, "falling back to IMDSv1: %v", err) + logger := middleware.GetLogger(ctx) + logger.Logf(logging.Warn, "falling back to IMDSv1: %v", err) t.disable() }