Skip to content

Commit b886c0c

Browse files
committed
Add clock skew correction on the client
Fixes #2321 This change adjusts signing request to account for the difference in time between client and server Caka clock skew). This PR makes values that we know are caused by clock skew retryable by default, and adds logic to classify probable clock skew errors as retryable. This value is set on the client so subsequent requests can read the adjusted time.
1 parent 4b0b7ef commit b886c0c

File tree

10 files changed

+538
-4
lines changed

10 files changed

+538
-4
lines changed

aws/middleware/middleware.go

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package middleware
33
import (
44
"context"
55
"fmt"
6+
"sync/atomic"
67
"time"
78

89
"github.com/aws/aws-sdk-go-v2/internal/rand"
@@ -124,6 +125,19 @@ func setAttemptSkew(metadata *middleware.Metadata, v time.Duration) {
124125
metadata.Set(attemptSkewKey{}, v)
125126
}
126127

128+
type clockSkew struct{}
129+
130+
// SetAttemptSkewContext sets the clock skew value on the context
131+
func SetAttemptSkewContext(ctx context.Context, v time.Duration) context.Context {
132+
return middleware.WithStackValue(ctx, clockSkew{}, v)
133+
}
134+
135+
// GetAttemptSkewContext gets the clock skew value from the context
136+
func GetAttemptSkewContext(ctx context.Context) time.Duration {
137+
x, _ := middleware.GetStackValue(ctx, clockSkew{}).(time.Duration)
138+
return x
139+
}
140+
127141
// AddClientRequestIDMiddleware adds ClientRequestID to the middleware stack
128142
func AddClientRequestIDMiddleware(stack *middleware.Stack) error {
129143
return stack.Build.Add(&ClientRequestID{}, middleware.After)
@@ -166,3 +180,45 @@ func AddRawResponseToMetadata(stack *middleware.Stack) error {
166180
func GetRawResponse(metadata middleware.Metadata) interface{} {
167181
return metadata.Get(rawResponseKey{})
168182
}
183+
184+
// AddTimeOffsetBuildMiddleware sets a value representing clock skew on the request context.
185+
// This can be read by other operations (such as signing) to correct the date value they send
186+
// on the request
187+
type AddTimeOffsetBuildMiddleware struct {
188+
Offset *atomic.Int64
189+
}
190+
191+
// ID the identifier for AddTimeOffsetBuildMiddleware
192+
func (m *AddTimeOffsetBuildMiddleware) ID() string { return "AddTimeOffsetMiddleware" }
193+
194+
// HandleBuild sets a value for attemptSkew on the request context if one is set on the client.
195+
func (m AddTimeOffsetBuildMiddleware) HandleBuild(ctx context.Context, in middleware.BuildInput, next middleware.BuildHandler) (
196+
out middleware.BuildOutput, metadata middleware.Metadata, err error,
197+
) {
198+
if m.Offset != nil {
199+
offset := time.Duration(m.Offset.Load())
200+
ctx = SetAttemptSkewContext(ctx, offset)
201+
}
202+
return next.HandleBuild(ctx, in)
203+
}
204+
205+
// AddTimeOffsetDeserializeMiddleware sets the clock skew on the client if it's present on the context
206+
// at the end of the request
207+
type AddTimeOffsetDeserializeMiddleware struct {
208+
Offset *atomic.Int64
209+
}
210+
211+
// ID the identifier for AddTimeOffsetDeserializeMiddleware
212+
func (m *AddTimeOffsetDeserializeMiddleware) ID() string { return "AddTimeOffsetDeserializeMiddleware" }
213+
214+
// HandleDeserialize gets the clock skew context from the context, and if set, sets it on the pointer
215+
// held by AddTimeOffsetDeserializeMiddleware
216+
func (m *AddTimeOffsetDeserializeMiddleware) HandleDeserialize(ctx context.Context, in middleware.DeserializeInput, next middleware.DeserializeHandler) (
217+
out middleware.DeserializeOutput, metadata middleware.Metadata, err error,
218+
) {
219+
v := GetAttemptSkewContext(ctx)
220+
if v != 0 {
221+
m.Offset.Store(v.Nanoseconds())
222+
}
223+
return next.HandleDeserialize(ctx, in)
224+
}

aws/middleware/middleware_test.go

Lines changed: 240 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,13 @@ package middleware_test
33
import (
44
"bytes"
55
"context"
6+
"fmt"
7+
"github.com/aws/aws-sdk-go-v2/aws"
8+
"github.com/aws/aws-sdk-go-v2/aws/retry"
69
"net/http"
710
"reflect"
811
"strings"
12+
"sync/atomic"
913
"testing"
1014
"time"
1115

@@ -187,3 +191,239 @@ func TestAttemptClockSkewHandler(t *testing.T) {
187191
})
188192
}
189193
}
194+
195+
type HTTPClient interface {
196+
Do(*http.Request) (*http.Response, error)
197+
}
198+
199+
type Options struct {
200+
HTTPClient HTTPClient
201+
RetryMode aws.RetryMode
202+
Retryer aws.Retryer
203+
Offset *atomic.Int64
204+
}
205+
206+
type MockClient struct {
207+
options Options
208+
}
209+
210+
func addRetry(stack *smithymiddleware.Stack, o Options) error {
211+
attempt := retry.NewAttemptMiddleware(o.Retryer, smithyhttp.RequestCloner, func(m *retry.Attempt) {
212+
m.LogAttempts = false
213+
})
214+
return stack.Finalize.Add(attempt, smithymiddleware.After)
215+
}
216+
217+
func addOffset(stack *smithymiddleware.Stack, o Options) error {
218+
buildOffset := middleware.AddTimeOffsetBuildMiddleware{Offset: o.Offset}
219+
deserializeOffset := middleware.AddTimeOffsetDeserializeMiddleware{Offset: o.Offset}
220+
err := stack.Build.Add(&buildOffset, smithymiddleware.After)
221+
if err != nil {
222+
return err
223+
}
224+
err = stack.Deserialize.Add(&deserializeOffset, smithymiddleware.Before)
225+
if err != nil {
226+
return err
227+
}
228+
return nil
229+
}
230+
231+
// Middleware to set a `Date` object that includes sdk time and offset
232+
type MockAddDateHeader struct {
233+
}
234+
235+
func (l *MockAddDateHeader) ID() string {
236+
return "MockAddDateHeader"
237+
}
238+
239+
func (l *MockAddDateHeader) HandleFinalize(
240+
ctx context.Context, in smithymiddleware.FinalizeInput, next smithymiddleware.FinalizeHandler,
241+
) (
242+
out smithymiddleware.FinalizeOutput, metadata smithymiddleware.Metadata, attemptError error,
243+
) {
244+
req := in.Request.(*smithyhttp.Request)
245+
date := sdk.NowTime()
246+
skew := middleware.GetAttemptSkewContext(ctx)
247+
date = date.Add(skew)
248+
req.Header.Set("Date", date.Format(time.RFC850))
249+
return next.HandleFinalize(ctx, in)
250+
}
251+
252+
// Middleware to deserialize the response which just says "OK" if the response is 200
253+
type DeserializeFailIfNotHTTP200 struct {
254+
}
255+
256+
func (*DeserializeFailIfNotHTTP200) ID() string {
257+
return "DeserializeFailIfNotHTTP200"
258+
}
259+
260+
func (m *DeserializeFailIfNotHTTP200) HandleDeserialize(ctx context.Context, in smithymiddleware.DeserializeInput, next smithymiddleware.DeserializeHandler) (
261+
out smithymiddleware.DeserializeOutput, metadata smithymiddleware.Metadata, err error,
262+
) {
263+
out, metadata, err = next.HandleDeserialize(ctx, in)
264+
if err != nil {
265+
return out, metadata, err
266+
}
267+
response, ok := out.RawResponse.(*smithyhttp.Response)
268+
if !ok {
269+
return out, metadata, fmt.Errorf("expected raw response to be set on testing")
270+
}
271+
if response.StatusCode != 200 {
272+
return out, metadata, mockRetryableError{true}
273+
}
274+
return out, metadata, err
275+
}
276+
277+
func (c *MockClient) setupMiddleware(stack *smithymiddleware.Stack) error {
278+
err := error(nil)
279+
if c.options.Retryer != nil {
280+
err = addRetry(stack, c.options)
281+
if err != nil {
282+
return err
283+
}
284+
}
285+
if c.options.Offset != nil {
286+
err = addOffset(stack, c.options)
287+
if err != nil {
288+
return err
289+
}
290+
}
291+
err = stack.Finalize.Add(&MockAddDateHeader{}, smithymiddleware.After)
292+
if err != nil {
293+
return err
294+
}
295+
err = middleware.AddRecordResponseTiming(stack)
296+
if err != nil {
297+
return err
298+
}
299+
err = stack.Deserialize.Add(&DeserializeFailIfNotHTTP200{}, smithymiddleware.After)
300+
if err != nil {
301+
return err
302+
}
303+
return nil
304+
}
305+
306+
func (c *MockClient) Do(ctx context.Context) (interface{}, error) {
307+
// setup middlewares
308+
ctx = smithymiddleware.ClearStackValues(ctx)
309+
stack := smithymiddleware.NewStack("stack", smithyhttp.NewStackRequest)
310+
err := c.setupMiddleware(stack)
311+
if err != nil {
312+
return nil, err
313+
}
314+
handler := smithymiddleware.DecorateHandler(smithyhttp.NewClientHandler(c.options.HTTPClient), stack)
315+
result, _, err := handler.Handle(ctx, 1)
316+
if err != nil {
317+
return nil, err
318+
}
319+
return result, err
320+
}
321+
322+
type mockRetryableError struct{ b bool }
323+
324+
func (m mockRetryableError) RetryableError() bool { return m.b }
325+
func (m mockRetryableError) Error() string {
326+
return fmt.Sprintf("mock retryable %t", m.b)
327+
}
328+
329+
func failRequestIfSkewed() smithyhttp.ClientDoFunc {
330+
return func(req *http.Request) (*http.Response, error) {
331+
dateHeader := req.Header.Get("Date")
332+
if dateHeader == "" {
333+
return nil, fmt.Errorf("expected `Date` header to be set")
334+
}
335+
reqDate, err := time.Parse(time.RFC850, dateHeader)
336+
if err != nil {
337+
return nil, err
338+
}
339+
parsedReqTime := time.Now().Sub(reqDate)
340+
parsedReqTime = time.Duration.Abs(parsedReqTime)
341+
thresholdForSkewError := 4 * time.Minute
342+
if thresholdForSkewError-parsedReqTime <= 0 {
343+
return &http.Response{
344+
StatusCode: 403,
345+
Header: http.Header{
346+
"Date": {time.Now().Format(time.RFC850)},
347+
},
348+
}, nil
349+
}
350+
// else, return OK
351+
return &http.Response{
352+
StatusCode: 200,
353+
Header: http.Header{},
354+
}, nil
355+
}
356+
}
357+
358+
func TestSdkOffsetIsSet(t *testing.T) {
359+
nowTime := sdk.NowTime
360+
defer func() {
361+
sdk.NowTime = nowTime
362+
}()
363+
fiveMinuteSkew := func() time.Time {
364+
return time.Now().Add(5 * time.Minute)
365+
}
366+
sdk.NowTime = fiveMinuteSkew
367+
c := MockClient{
368+
Options{
369+
HTTPClient: failRequestIfSkewed(),
370+
},
371+
}
372+
resp, err := c.Do(context.Background())
373+
if err == nil {
374+
t.Errorf("Expected first request to fail since clock skew logic has not run. Got %v and err %v", resp, err)
375+
}
376+
}
377+
378+
func TestRetrySetsSkewInContext(t *testing.T) {
379+
defer resetDefaults(sdk.TestingUseNopSleep())
380+
fiveMinuteSkew := func() time.Time {
381+
return time.Now().Add(5 * time.Minute)
382+
}
383+
sdk.NowTime = fiveMinuteSkew
384+
c := MockClient{
385+
Options{
386+
HTTPClient: failRequestIfSkewed(),
387+
Retryer: retry.NewStandard(func(s *retry.StandardOptions) {
388+
}),
389+
},
390+
}
391+
resp, err := c.Do(context.Background())
392+
if err != nil {
393+
t.Errorf("Expected request to succeed on retry. Got %v and err %v", resp, err)
394+
}
395+
}
396+
397+
func TestSkewIsSetOnTheWholeClient(t *testing.T) {
398+
defer resetDefaults(sdk.TestingUseNopSleep())
399+
fiveMinuteSkew := func() time.Time {
400+
return time.Now().Add(5 * time.Minute)
401+
}
402+
sdk.NowTime = fiveMinuteSkew
403+
var offset atomic.Int64
404+
offset.Store(0)
405+
c := MockClient{
406+
Options{
407+
HTTPClient: failRequestIfSkewed(),
408+
Retryer: retry.NewStandard(func(s *retry.StandardOptions) {
409+
}),
410+
Offset: &offset,
411+
},
412+
}
413+
resp, err := c.Do(context.Background())
414+
if err != nil {
415+
t.Errorf("Expected request to succeed on retry. Got %v and err %v", resp, err)
416+
}
417+
// Remove retryer so it has to succeed on first call
418+
c.options.Retryer = nil
419+
// same client, new request
420+
resp, err = c.Do(context.Background())
421+
if err != nil {
422+
t.Errorf("Expected second request to succeed since the skew should be set on the client. Got %v and err %v", resp, err)
423+
}
424+
}
425+
426+
func resetDefaults(restoreSleepFunc func()) {
427+
sdk.NowTime = time.Now
428+
restoreSleepFunc()
429+
}

aws/middleware/private/metrics/testutils/test_util.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,10 @@ type StreamingBodyBuildHandler struct {
5252
Result interface{}
5353
}
5454

55+
func NoopRequestCloner(i interface{}) interface{} {
56+
return i
57+
}
58+
5559
func (NoopInitializeHandler) HandleInitialize(ctx context.Context, in middleware.InitializeInput) (
5660
out middleware.InitializeOutput, metadata middleware.Metadata, err error,
5761
) {

aws/retry/middleware.go

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package retry
22

33
import (
44
"context"
5+
"errors"
56
"fmt"
67
"github.com/aws/aws-sdk-go-v2/aws/middleware/private/metrics"
78
"strconv"
@@ -86,6 +87,9 @@ func (r *Attempt) HandleFinalize(ctx context.Context, in smithymiddle.FinalizeIn
8687
AttemptClockSkew: attemptClockSkew,
8788
})
8889

90+
// Setting clock skew to be used on other context (like signing)
91+
ctx = awsmiddle.SetAttemptSkewContext(ctx, attemptClockSkew)
92+
8993
var attemptResult AttemptResult
9094
out, attemptResult, releaseRetryToken, err = r.handleAttempt(attemptCtx, attemptInput, releaseRetryToken, next)
9195
attemptClockSkew, _ = awsmiddle.GetAttemptSkew(attemptResult.ResponseMetadata)
@@ -185,6 +189,8 @@ func (r *Attempt) handleAttempt(
185189
return out, attemptResult, nopRelease, err
186190
}
187191

192+
err = wrapAsClockSkew(ctx, err)
193+
188194
//------------------------------
189195
// Is Retryable and Should Retry
190196
//------------------------------
@@ -247,6 +253,32 @@ func (r *Attempt) handleAttempt(
247253
return out, attemptResult, releaseRetryToken, err
248254
}
249255

256+
// Note that there are errors that are known to be definitely caused by clock
257+
// skew, which are defined on the list of retryable errors
258+
var possibleSkewCodes = map[string]struct{}{
259+
"InvalidSignatureException": {},
260+
"SignatureDoesNotMatch": {},
261+
"AuthFailure": {},
262+
}
263+
264+
// wrapAsClockSkew checks if this error could be related to a clock skew
265+
// error and if so, wrap the error.
266+
func wrapAsClockSkew(ctx context.Context, err error) error {
267+
var v interface{ ErrorCode() string }
268+
if !errors.As(err, &v) {
269+
return err
270+
}
271+
_, ok := possibleSkewCodes[v.ErrorCode()]
272+
if !ok {
273+
return err
274+
}
275+
skew := awsmiddle.GetAttemptSkewContext(ctx)
276+
if !(skew > 4*time.Minute) {
277+
return err
278+
}
279+
return &ProbClockSkewError{Err: err}
280+
}
281+
250282
// MetricsHeader attaches SDK request metric header for retries to the transport
251283
type MetricsHeader struct{}
252284

0 commit comments

Comments
 (0)