-
Notifications
You must be signed in to change notification settings - Fork 28
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
feat: implement clock skew interceptor #972
Changes from 50 commits
5b7e337
023c845
371e993
e2bbde5
5a42d82
10c8013
b21ae43
dd94acc
e83af28
3c8ca1a
6234df5
ee15721
be94c7c
f2cfa77
a22de25
4c50705
ad55bd8
cfc0ce5
5cfcb17
c37e2e8
d3520c8
bb57855
fd11afd
1230226
2849f66
bb2bce8
c4aa35b
0007895
52c5d3b
3fd3f9f
f44477d
b822870
7c9c012
6c01e69
672740e
b139e49
cf58ca7
839ef98
ae609bf
3b5d2e1
3924710
928d58f
34c1b3b
2a85575
343ae26
e590d52
5ffebe9
19a9ea9
6c4e3bf
badf986
7becf58
8d52429
7582dd0
66fa578
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 |
---|---|---|
@@ -0,0 +1,5 @@ | ||
{ | ||
"id": "6de10487-c3a0-4c63-929a-ba11a415ea8f", | ||
"type": "feature", | ||
"description": "Detect and automatically correct clock skew to prevent signing errors" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,119 @@ | ||
/* | ||
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
package aws.smithy.kotlin.runtime.awsprotocol | ||
|
||
import aws.smithy.kotlin.runtime.ErrorMetadata | ||
import aws.smithy.kotlin.runtime.SdkBaseException | ||
import aws.smithy.kotlin.runtime.ServiceErrorMetadata | ||
import aws.smithy.kotlin.runtime.client.ProtocolRequestInterceptorContext | ||
import aws.smithy.kotlin.runtime.client.ResponseInterceptorContext | ||
import aws.smithy.kotlin.runtime.http.interceptors.HttpInterceptor | ||
import aws.smithy.kotlin.runtime.http.operation.HttpOperationContext | ||
import aws.smithy.kotlin.runtime.http.request.HttpRequest | ||
import aws.smithy.kotlin.runtime.http.response.HttpResponse | ||
import aws.smithy.kotlin.runtime.http.response.header | ||
import aws.smithy.kotlin.runtime.telemetry.logging.logger | ||
import aws.smithy.kotlin.runtime.time.Instant | ||
import aws.smithy.kotlin.runtime.time.until | ||
import aws.smithy.kotlin.runtime.util.get | ||
import kotlinx.atomicfu.* | ||
import kotlin.coroutines.coroutineContext | ||
import kotlin.time.Duration | ||
import kotlin.time.Duration.Companion.minutes | ||
|
||
/** | ||
* An interceptor used to detect clock skew (difference between client and server clocks) and apply a correction. | ||
*/ | ||
public class ClockSkewInterceptor : HttpInterceptor { | ||
public companion object { | ||
/** | ||
* How much must the clock be skewed before attempting correction | ||
*/ | ||
public val CLOCK_SKEW_THRESHOLD: Duration = 4.minutes | ||
|
||
/** | ||
* Determine whether the client's clock is skewed relative to the server. | ||
* @return true if the service's response represents a definite clock skew error | ||
* OR a *possible* clock skew error AND the skew exists. false otherwise. | ||
* @param responseCodeDescription the server's response code description | ||
* @param serverTime the server's time | ||
*/ | ||
internal fun Instant.isSkewed(serverTime: Instant, responseCodeDescription: String?): Boolean = | ||
responseCodeDescription?.let { | ||
CLOCK_SKEW_ERROR_CODES.contains(it) || (POSSIBLE_CLOCK_SKEW_ERROR_CODES.contains(it) && until(serverTime).absoluteValue >= CLOCK_SKEW_THRESHOLD) | ||
} ?: false | ||
|
||
// Errors definitely caused by clock skew | ||
private val CLOCK_SKEW_ERROR_CODES = listOf( | ||
"RequestTimeTooSkewed", | ||
"RequestExpired", | ||
"RequestInTheFuture", | ||
) | ||
|
||
// Errors possibly caused by clock skew | ||
private val POSSIBLE_CLOCK_SKEW_ERROR_CODES = listOf( | ||
"PriorRequestNotComplete", | ||
"RequestTimeout", | ||
"RequestTimeoutException", | ||
"InternalError", | ||
) | ||
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. Correctness: These don't appear to be related to clock skew. Looks like Java considers 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. 🤦 Good catch. I took the wrong list of exceptions from Java and then convinced myself they could be caused by clock skew. |
||
} | ||
|
||
// Clock skew to be applied to all requests | ||
private val _currentSkew: AtomicRef<Duration?> = atomic(null) | ||
|
||
/** | ||
* Apply the previously-computed skew, if it's set, to the execution context before signing | ||
*/ | ||
public override suspend fun modifyBeforeSigning(context: ProtocolRequestInterceptorContext<Any, HttpRequest>): HttpRequest { | ||
val logger = coroutineContext.logger<ClockSkewInterceptor>() | ||
|
||
val skew = _currentSkew.value | ||
skew?.let { | ||
logger.info { "applying clock skew $skew to client" } | ||
context.executionContext[HttpOperationContext.ClockSkew] = skew | ||
} | ||
|
||
context.executionContext[HttpOperationContext.ClockSkewApproximateSigningTime] = Instant.now() | ||
|
||
return context.protocolRequest | ||
} | ||
|
||
/** | ||
* After receiving a response, check if the client clock is skewed and apply a correction if necessary. | ||
*/ | ||
public override suspend fun modifyBeforeAttemptCompletion(context: ResponseInterceptorContext<Any, Any, HttpRequest, HttpResponse?>): Result<Any> { | ||
val logger = coroutineContext.logger<ClockSkewInterceptor>() | ||
|
||
val serverTime = context.protocolResponse?.header("Date")?.let { | ||
Instant.fromRfc5322(it) | ||
} ?: run { | ||
logger.debug { "service did not return \"Date\" header, skipping skew calculation" } | ||
return context.response | ||
} | ||
|
||
val clientTime = context.protocolRequest.headers["Date"]?.let { | ||
Instant.fromRfc5322(it) | ||
} ?: context.protocolRequest.headers["x-amz-date"]?.let { | ||
Instant.fromIso8601(it) | ||
} ?: context.executionContext[HttpOperationContext.ClockSkewApproximateSigningTime] | ||
|
||
val ex = (context.response.exceptionOrNull() as? SdkBaseException) ?: return context.response | ||
val errorCode = ex.sdkErrorMetadata.attributes.getOrNull(ServiceErrorMetadata.ErrorCode) | ||
|
||
if (clientTime.isSkewed(serverTime, errorCode)) { | ||
val skew = clientTime.until(serverTime) | ||
logger.warn { "client clock ($clientTime) is skewed $skew from the server ($serverTime), applying correction" } | ||
_currentSkew.getAndSet(skew) | ||
context.executionContext[HttpOperationContext.ClockSkew] = skew | ||
|
||
// Mark the exception as retryable | ||
ex.sdkErrorMetadata.attributes[ErrorMetadata.Retryable] = true | ||
return Result.failure(ex) | ||
} | ||
|
||
return context.response | ||
} | ||
} |
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.
nit:
responseCodeDescription
->errorCode
Also
responseCodeDescription
should probably be non-nullable since this always returns false otherwise which means it's really a required parameter for determining if a clock is skewed.