-
Notifications
You must be signed in to change notification settings - Fork 91
feat: Automatic clock skew adjustment #2023
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 15 commits
dc93edd
71ea35f
3459208
db78779
dafec4c
b0c4b1e
336919b
3faee78
465103d
620b246
f265647
9cf8f71
7568ac7
0436058
8c9d68f
cd2afd7
2114730
f0b716f
6227b8b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -574,6 +574,7 @@ private var runtimeTargets: [Target] { | |
| .SmithyIdentity, | ||
| .SmithyRetriesAPI, | ||
| .SmithyRetries, | ||
| .SmithyTimestamps, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See |
||
| .AWSSDKCommon, | ||
| .AWSSDKHTTPAuth, | ||
| .AWSSDKChecksums, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,104 @@ | ||
| // | ||
| // Copyright Amazon.com Inc. or its affiliates. | ||
| // All Rights Reserved. | ||
| // | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
| // | ||
|
|
||
| import struct Foundation.TimeInterval | ||
| import typealias ClientRuntime.ClockSkewProvider | ||
| import protocol ClientRuntime.ServiceError | ||
| import class SmithyHTTPAPI.HTTPRequest | ||
| import class SmithyHTTPAPI.HTTPResponse | ||
| @_spi(SmithyTimestamps) import struct SmithyTimestamps.TimestampFormatter | ||
|
|
||
| public enum AWSClockSkewProvider { | ||
| private static var absoluteThreshold: TimeInterval { 300.0 } // clock skew of < 5 minutes is not compensated | ||
| private static var changeThreshold: TimeInterval { 60.0 } // changes to clock skew of < 1 minute are ignored | ||
|
|
||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the AWS-specific clock skew provider. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No further explanation, see inline code comments. |
||
| public static func provider() -> ClockSkewProvider<HTTPRequest, HTTPResponse> { | ||
| return clockSkew(request:response:error:previous:) | ||
| } | ||
|
|
||
| @Sendable | ||
| private static func clockSkew( | ||
| request: HTTPRequest, | ||
| response: HTTPResponse, | ||
| error: Error, | ||
| previous: TimeInterval? | ||
| ) -> TimeInterval? { | ||
| // Check if this error could be the result of clock skew. | ||
| // If not, leave the current clock skew value unchanged. | ||
| guard isAClockSkewError(request: request, error: error) else { return previous } | ||
|
|
||
| // Get the new clock skew value based on server & client times. | ||
| let new = newClockSkew(request: request, response: response, error: error) | ||
|
|
||
| if let new, let previous { | ||
| // Update clock skew if it's changed by at least the change threshold | ||
| // Updating clock skew for insignificant changes in value will result | ||
| // in retry when not likely to succeed | ||
| return abs(new - previous) > changeThreshold ? new : previous | ||
| } else { | ||
| // If previous was nil but new is non-nil, return new. | ||
| // If previous was non-nil but new is nil, return nil. | ||
| // If previous and new are both nil, return nil. | ||
| return new | ||
| } | ||
| } | ||
|
|
||
| private static func isAClockSkewError(request: HTTPRequest, error: Error) -> Bool { | ||
| // Get the error code, which is a cue that clock skew is the cause of the error | ||
| guard let code = (error as? ServiceError)?.errorCode else { return false } | ||
|
|
||
| // Check the error code to see if this error could be due to clock skew | ||
| // If not, fail fast to prevent having to parse server datetime (slow) | ||
| return isDefiniteClockSkewError(code: code) || isProbableClockSkewError(code: code, request: request) | ||
| } | ||
|
|
||
| private static func isDefiniteClockSkewError(code: String) -> Bool { | ||
| definiteClockSkewErrorCodes.contains(code) | ||
| } | ||
|
|
||
| private static func isProbableClockSkewError(code: String, request: HTTPRequest) -> Bool { | ||
| // Certain S3 HEAD methods will return generic HTTP 403 errors when the cause of the | ||
| // failure is clock skew. To accommodate, check clock skew when the method is HEAD | ||
| probableClockSkewErrorCodes.contains(code) || request.method == .head | ||
| } | ||
|
|
||
| private static func newClockSkew( | ||
| request: HTTPRequest, | ||
| response: HTTPResponse, | ||
| error: Error | ||
| ) -> TimeInterval? { | ||
| // Get the datetime that the request was signed at. | ||
| // If not available, clock skew can't be determined. | ||
| // This should always be set when signing with sigv4 & sigv4a. | ||
| guard let clientDate = request.signedAt else { return nil } | ||
|
|
||
| // Need a server Date (from the HTTP response headers) to calculate clock skew. | ||
| // If not available, return no clock skew. | ||
| // This header should always be included on AWS service responses. | ||
| guard let httpDateString = response.headers.value(for: "Date") else { return nil } | ||
| guard let serverDate = TimestampFormatter(format: .httpDate).date(from: httpDateString) else { return nil } | ||
|
|
||
| // Calculate & return clock skew if more than the threshold, else return nil. | ||
| let clockSkew = serverDate.timeIntervalSince(clientDate) | ||
| return abs(clockSkew) > absoluteThreshold ? clockSkew : nil | ||
| } | ||
| } | ||
|
|
||
| // These error codes indicate that the cause of the failure was clock skew. | ||
| private let definiteClockSkewErrorCodes: Set = [ | ||
| "RequestTimeTooSkewed", | ||
| "RequestExpired", | ||
| "RequestInTheFuture", | ||
| ] | ||
|
|
||
| // These error codes indicate that a possible cause of the failure was clock skew. | ||
| // So, when these are received, check/set clock skew & retry to see if that helps. | ||
| private let probableClockSkewErrorCodes: Set = [ | ||
| "InvalidSignatureException", | ||
| "AuthFailure", | ||
| "SignatureDoesNotMatch", | ||
| ] | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -65,6 +65,8 @@ public class AuthTokenGenerator { | |
| requestBuilder.withQueryItem(URIQueryItem(name: "Action", value: "connect")) | ||
| requestBuilder.withQueryItem(URIQueryItem(name: "DBUser", value: username)) | ||
|
|
||
| let signedAt = Date() | ||
|
|
||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| let signingConfig = AWSSigningConfig( | ||
| credentials: self.awsCredentialIdentity, | ||
| expiration: expiration, | ||
|
|
@@ -74,7 +76,7 @@ public class AuthTokenGenerator { | |
| shouldNormalizeURIPath: true, | ||
| omitSessionToken: false | ||
| ), | ||
| date: Date(), | ||
| date: signedAt, | ||
| service: "rds-db", | ||
| region: region, | ||
| signatureType: .requestQueryParams, | ||
|
|
@@ -83,7 +85,8 @@ public class AuthTokenGenerator { | |
|
|
||
| let signedRequest = await AWSSigV4Signer().sigV4SignedRequest( | ||
| requestBuilder: requestBuilder, | ||
| signingConfig: signingConfig | ||
| signingConfig: signingConfig, | ||
| signedAt: signedAt | ||
| ) | ||
|
|
||
| guard let presignedURL = signedRequest?.destination.url else { | ||
|
|
@@ -119,6 +122,8 @@ public class AuthTokenGenerator { | |
| let actionQueryItemValue = isForAdmin ? "DbConnectAdmin" : "DbConnect" | ||
| requestBuilder.withQueryItem(URIQueryItem(name: "Action", value: actionQueryItemValue)) | ||
|
|
||
| let signedAt = Date() | ||
|
|
||
| let signingConfig = AWSSigningConfig( | ||
| credentials: self.awsCredentialIdentity, | ||
| expiration: expiration, | ||
|
|
@@ -128,7 +133,7 @@ public class AuthTokenGenerator { | |
| shouldNormalizeURIPath: true, | ||
| omitSessionToken: false | ||
| ), | ||
| date: Date(), | ||
| date: signedAt, | ||
| service: "dsql", | ||
| region: region, | ||
| signatureType: .requestQueryParams, | ||
|
|
@@ -137,7 +142,8 @@ public class AuthTokenGenerator { | |
|
|
||
| let signedRequest = await AWSSigV4Signer().sigV4SignedRequest( | ||
| requestBuilder: requestBuilder, | ||
| signingConfig: signingConfig | ||
| signingConfig: signingConfig, | ||
| signedAt: signedAt | ||
| ) | ||
|
|
||
| guard let presignedURL = signedRequest?.destination.url else { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,171 @@ | ||
| // | ||
| // Copyright Amazon.com Inc. or its affiliates. | ||
| // All Rights Reserved. | ||
| // | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
| // | ||
|
|
||
| import AWSClientRuntime | ||
| import XCTest | ||
| import Smithy | ||
| import ClientRuntime | ||
| import SmithyHTTPAPI | ||
| @_spi(SmithyTimestamps) import SmithyTimestamps | ||
|
|
||
| class AWSClockSkewProviderTests: XCTestCase { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tests for the behavior of the clock skew provider above. |
||
|
|
||
| // MARK: - nil previous clock skew | ||
|
|
||
| func test_clockSkewError_returnsNilWhenClientAndServerTimeAreTheSame() { | ||
| let previousClockSkew: TimeInterval? = nil | ||
| let client = "Sun, 02 Jan 2000 20:34:56.000 GMT" | ||
| let server = "Sun, 02 Jan 2000 20:34:56.000 GMT" | ||
| let clientDate = TimestampFormatter(format: .httpDate).date(from: client)! | ||
| let request = HTTPRequestBuilder().withMethod(.get).withSignedAt(clientDate).build() | ||
| let response = HTTPResponse(headers: Headers(["Date": server]), body: ByteStream.noStream, statusCode: .badRequest) | ||
| let error = ClockSkewTestError.definite | ||
| XCTAssertNil(AWSClockSkewProvider.provider()(request, response, error, previousClockSkew)) | ||
| } | ||
|
|
||
| func test_clockSkewError_returnsNilWhenClientAndServerTimeAreDifferentByLessThanThreshold() { | ||
| let previousClockSkew: TimeInterval? = nil | ||
| let client = "Sun, 02 Jan 2000 20:35:26.000 GMT" // +30 seconds | ||
| let server = "Sun, 02 Jan 2000 20:34:56.000 GMT" | ||
| let clientDate = TimestampFormatter(format: .httpDate).date(from: client)! | ||
| let request = HTTPRequestBuilder().withMethod(.get).withSignedAt(clientDate).build() | ||
| let response = HTTPResponse(headers: Headers(["Date": server]), body: ByteStream.noStream, statusCode: .badRequest) | ||
| let error = ClockSkewTestError.definite | ||
| XCTAssertNil(AWSClockSkewProvider.provider()(request, response, error, previousClockSkew)) | ||
| } | ||
|
|
||
| func test_clockSkewError_returnsIntervalWhenClientAndServerTimeAreDifferentByMoreThanThreshold() { | ||
| let previousClockSkew: TimeInterval? = nil | ||
| let client = "Sun, 02 Jan 2000 20:44:56.000 GMT" // server + 600 seconds | ||
| let server = "Sun, 02 Jan 2000 20:34:56.000 GMT" | ||
| let clientDate = TimestampFormatter(format: .httpDate).date(from: client)! | ||
| let request = HTTPRequestBuilder().withMethod(.get).withSignedAt(clientDate).build() | ||
| let response = HTTPResponse(headers: Headers(["Date": server]), body: ByteStream.noStream, statusCode: .badRequest) | ||
| let error = ClockSkewTestError.definite | ||
| XCTAssertEqual(AWSClockSkewProvider.provider()(request, response, error, previousClockSkew), -600.0) | ||
| } | ||
|
|
||
| func test_nonClockSkewError_returnsNilWhenClientAndServerTimeAreTheSame() { | ||
| let previousClockSkew: TimeInterval? = nil | ||
| let client = "Sun, 02 Jan 2000 20:34:56.000 GMT" | ||
| let server = "Sun, 02 Jan 2000 20:34:56.000 GMT" | ||
| let clientDate = TimestampFormatter(format: .httpDate).date(from: client)! | ||
| let request = HTTPRequestBuilder().withMethod(.get).withSignedAt(clientDate).build() | ||
| let response = HTTPResponse(headers: Headers(["Date": server]), body: ByteStream.noStream, statusCode: .badRequest) | ||
| let error = ClockSkewTestError.notDueToClockSkew | ||
| XCTAssertNil(AWSClockSkewProvider.provider()(request, response, error, previousClockSkew)) | ||
| } | ||
|
|
||
| func test_nonClockSkewError_returnsNilWhenClientAndServerTimeAreDifferentByLessThanThreshold() { | ||
| let previousClockSkew: TimeInterval? = nil | ||
| let client = "Sun, 02 Jan 2000 20:35:26.000 GMT" // +30 seconds | ||
| let server = "Sun, 02 Jan 2000 20:34:56.000 GMT" | ||
| let clientDate = TimestampFormatter(format: .httpDate).date(from: client)! | ||
| let request = HTTPRequestBuilder().withMethod(.get).withSignedAt(clientDate).build() | ||
| let response = HTTPResponse(headers: Headers(["Date": server]), body: ByteStream.noStream, statusCode: .badRequest) | ||
| let error = ClockSkewTestError.notDueToClockSkew | ||
| XCTAssertNil(AWSClockSkewProvider.provider()(request, response, error, previousClockSkew)) | ||
| } | ||
|
|
||
| func test_nonClockSkewError_returnsNilWhenClientAndServerTimeAreDifferentByMoreThanThreshold() { | ||
| let previousClockSkew: TimeInterval? = nil | ||
| let client = "Sun, 02 Jan 2000 20:36:26.000 GMT" // +90 seconds | ||
| let server = "Sun, 02 Jan 2000 20:34:56.000 GMT" | ||
| let clientDate = TimestampFormatter(format: .httpDate).date(from: client)! | ||
| let request = HTTPRequestBuilder().withMethod(.get).withSignedAt(clientDate).build() | ||
| let response = HTTPResponse(headers: Headers(["Date": server]), body: ByteStream.noStream, statusCode: .badRequest) | ||
| let error = ClockSkewTestError.notDueToClockSkew | ||
| XCTAssertNil(AWSClockSkewProvider.provider()(request, response, error, previousClockSkew)) | ||
| } | ||
|
|
||
| func test_headRequest_returnsNilWhenClientAndServerTimeAreTheSame() { | ||
| let previousClockSkew: TimeInterval? = nil | ||
| let client = "Sun, 02 Jan 2000 20:34:56.000 GMT" | ||
| let server = "Sun, 02 Jan 2000 20:34:56.000 GMT" | ||
| let clientDate = TimestampFormatter(format: .httpDate).date(from: client)! | ||
| let request = HTTPRequestBuilder().withMethod(.head).withSignedAt(clientDate).build() | ||
| let response = HTTPResponse(headers: Headers(["Date": server]), body: ByteStream.noStream, statusCode: .badRequest) | ||
| let error = ClockSkewTestError.notDueToClockSkew | ||
| XCTAssertNil(AWSClockSkewProvider.provider()(request, response, error, previousClockSkew)) | ||
| } | ||
|
|
||
| func test_headRequest_returnsNilWhenClientAndServerTimeAreDifferentByLessThanThreshold() { | ||
| let previousClockSkew: TimeInterval? = nil | ||
| let client = "Sun, 02 Jan 2000 20:35:26.000 GMT" // +30 seconds | ||
| let server = "Sun, 02 Jan 2000 20:34:56.000 GMT" | ||
| let clientDate = TimestampFormatter(format: .httpDate).date(from: client)! | ||
| let request = HTTPRequestBuilder().withMethod(.head).withSignedAt(clientDate).build() | ||
| let response = HTTPResponse(headers: Headers(["Date": server]), body: ByteStream.noStream, statusCode: .badRequest) | ||
| let error = ClockSkewTestError.notDueToClockSkew | ||
| XCTAssertNil(AWSClockSkewProvider.provider()(request, response, error, previousClockSkew)) | ||
| } | ||
|
|
||
| func test_headRequest_returnsIntervalWhenClientAndServerTimeAreDifferentByMoreThanThreshold() { | ||
| let previousClockSkew: TimeInterval? = nil | ||
| let client = "Sun, 02 Jan 2000 20:44:56.000 GMT" // server + 600 seconds | ||
| let server = "Sun, 02 Jan 2000 20:34:56.000 GMT" | ||
| let clientDate = TimestampFormatter(format: .httpDate).date(from: client)! | ||
| let request = HTTPRequestBuilder().withMethod(.head).withSignedAt(clientDate).build() | ||
| let response = HTTPResponse(headers: Headers(["Date": server]), body: ByteStream.noStream, statusCode: .badRequest) | ||
| let error = ClockSkewTestError.notDueToClockSkew | ||
| XCTAssertEqual(AWSClockSkewProvider.provider()(request, response, error, previousClockSkew), -600.0) | ||
| } | ||
|
|
||
| // MARK: - Non-nil previous clock skew | ||
|
|
||
| func test_nonNilPrevious_returnsNilWhenNewClockSkewIsNil() { | ||
| let previousClockSkew: TimeInterval = -400.0 | ||
| let client = "Sun, 02 Jan 2000 20:34:56.000 GMT" // server + 0 seconds | ||
| let server = "Sun, 02 Jan 2000 20:34:56.000 GMT" | ||
| let clientDate = TimestampFormatter(format: .httpDate).date(from: client)! | ||
| let request = HTTPRequestBuilder().withMethod(.get).withSignedAt(clientDate).build() | ||
| let response = HTTPResponse(headers: Headers(["Date": server]), body: ByteStream.noStream, statusCode: .badRequest) | ||
| let error = ClockSkewTestError.definite | ||
| XCTAssertEqual(AWSClockSkewProvider.provider()(request, response, error, previousClockSkew), nil) | ||
| } | ||
|
|
||
| func test_nonNilPrevious_returnsPreviousWhenClientAndServerTimeAreDifferentByLessThanThreshold() { | ||
| let previousClockSkew: TimeInterval = -400.0 | ||
| let client = "Sun, 02 Jan 2000 20:40:56.000 GMT" // server + 360 seconds | ||
| let server = "Sun, 02 Jan 2000 20:34:56.000 GMT" | ||
| let clientDate = TimestampFormatter(format: .httpDate).date(from: client)! | ||
| let request = HTTPRequestBuilder().withMethod(.get).withSignedAt(clientDate).build() | ||
| let response = HTTPResponse(headers: Headers(["Date": server]), body: ByteStream.noStream, statusCode: .badRequest) | ||
| let error = ClockSkewTestError.definite | ||
| XCTAssertEqual(AWSClockSkewProvider.provider()(request, response, error, previousClockSkew), previousClockSkew) | ||
| } | ||
|
|
||
| func test_nonNilPrevious_returnsNewWhenClientAndServerTimeAreDifferentByMoreThanThreshold() { | ||
| let previousClockSkew: TimeInterval = -400.0 | ||
| let client = "Sun, 02 Jan 2000 20:44:56.000 GMT" // server + 600 seconds | ||
| let server = "Sun, 02 Jan 2000 20:34:56.000 GMT" | ||
| let clientDate = TimestampFormatter(format: .httpDate).date(from: client)! | ||
| let request = HTTPRequestBuilder().withMethod(.get).withSignedAt(clientDate).build() | ||
| let response = HTTPResponse(headers: Headers(["Date": server]), body: ByteStream.noStream, statusCode: .badRequest) | ||
| let error = ClockSkewTestError.definite | ||
| XCTAssertEqual(AWSClockSkewProvider.provider()(request, response, error, previousClockSkew), -600.0) | ||
| } | ||
|
|
||
| func test_nonNilPrevious_returnsPreviousWhenErrorIsNotAClockSkewError() { | ||
| let previousClockSkew: TimeInterval = -400.0 | ||
| let client = "Sun, 02 Jan 2000 20:44:56.000 GMT" // server + 600 seconds | ||
| let server = "Sun, 02 Jan 2000 20:34:56.000 GMT" | ||
| let clientDate = TimestampFormatter(format: .httpDate).date(from: client)! | ||
| let request = HTTPRequestBuilder().withMethod(.get).withSignedAt(clientDate).build() | ||
| let response = HTTPResponse(headers: Headers(["Date": server]), body: ByteStream.noStream, statusCode: .badRequest) | ||
| let error = ClockSkewTestError.notDueToClockSkew | ||
| XCTAssertEqual(AWSClockSkewProvider.provider()(request, response, error, previousClockSkew), previousClockSkew) | ||
| } | ||
| } | ||
|
|
||
| private struct ClockSkewTestError: Error, ServiceError { | ||
| static var definite: Self { .init(typeName: "RequestTimeTooSkewed") } | ||
| static var notDueToClockSkew: Self { .init(typeName: "NotAClockSkewError") } | ||
|
|
||
| var typeName: String? | ||
| var message: String? { "" } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -57,7 +57,13 @@ final class RetryIntegrationTests: XCTestCase { | |
| .attributes(context) | ||
| .retryErrorInfoProvider(DefaultRetryErrorInfoProvider.errorInfo(for:)) | ||
| .retryStrategy(subject) | ||
| .deserialize({ _, _ in TestOutputResponse() }) | ||
| .deserialize({ response, _ in | ||
| if response.statusCode == .ok { | ||
| return TestOutputResponse() | ||
| } else { | ||
| throw TestHTTPError(statusCode: response.statusCode) | ||
| } | ||
| }) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This test file changed comparable to the retry integration tests in smithy-swift. |
||
| .executeRequest(next) | ||
| builder.interceptors.add(AmzSdkInvocationIdMiddleware()) | ||
| builder.interceptors.add(AmzSdkRequestMiddleware(maxRetries: subject.options.maxRetriesBase)) | ||
|
|
@@ -229,9 +235,10 @@ private class TestOutputHandler: ExecuteRequest { | |
| // Return either a successful response or a HTTP error, depending on the directions in the test step. | ||
| switch testStep.response { | ||
| case .success: | ||
| return HTTPResponse() | ||
| return HTTPResponse(statusCode: .ok) | ||
| case .httpError(let statusCode): | ||
| throw TestHTTPError(statusCode: statusCode) | ||
| let httpStatusCode = HTTPStatusCode(rawValue: statusCode)! | ||
| return HTTPResponse(statusCode: httpStatusCode) | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -310,9 +317,8 @@ private class TestOutputHandler: ExecuteRequest { | |
| private struct TestHTTPError: HTTPError, Error { | ||
| var httpResponse: HTTPResponse | ||
|
|
||
| init(statusCode: Int) { | ||
| guard let statusCodeValue = HTTPStatusCode(rawValue: statusCode) else { fatalError("Unrecognized HTTP code") } | ||
| self.httpResponse = HTTPResponse(statusCode: statusCodeValue) | ||
| init(statusCode: HTTPStatusCode) { | ||
| self.httpResponse = HTTPResponse(statusCode: statusCode) | ||
| } | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new AWS-specific clock skew provider uses
SmithyTimestampsto read the date header in HTTP responses, hence the dependency is added.