-
Notifications
You must be signed in to change notification settings - Fork 30
cache sigv4 compute #1652
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
base: main
Are you sure you want to change the base?
cache sigv4 compute #1652
Changes from 6 commits
731cf94
ce9764f
77b01c0
6212dec
e023a10
8d5592e
8e5c235
df69806
10670bb
e98bcf4
ff7b256
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,6 @@ | ||
| { | ||
| "id": "0be35766-2b6a-41ab-aa66-251af5ffd7bf", | ||
| "type": "feature", | ||
| "description": "Cache SigV4 signing key derivation to avoid redundant HMAC computations when credentials, region, service, and date are unchanged", | ||
| "module": "aws-signing-default" | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,26 +4,51 @@ | |
| */ | ||
| package aws.smithy.kotlin.runtime.auth.awssigning | ||
|
|
||
| import aws.smithy.kotlin.runtime.collections.PeriodicSweepCache | ||
| import aws.smithy.kotlin.runtime.hashing.HashSupplier | ||
| import aws.smithy.kotlin.runtime.hashing.Sha256 | ||
| import aws.smithy.kotlin.runtime.hashing.hmac | ||
| import aws.smithy.kotlin.runtime.text.encoding.encodeToHex | ||
| import aws.smithy.kotlin.runtime.time.Clock | ||
| import aws.smithy.kotlin.runtime.time.TimestampFormat | ||
| import aws.smithy.kotlin.runtime.util.ExpiringValue | ||
| import kotlinx.coroutines.runBlocking | ||
| import kotlin.time.Duration.Companion.hours | ||
|
|
||
| /** | ||
| * [SignatureCalculator] for the SigV4 ("AWS4-HMAC-SHA256") algorithm | ||
| * @param sha256Provider the [HashSupplier] to use for computing SHA-256 hashes | ||
| */ | ||
| internal class SigV4SignatureCalculator(override val sha256Provider: HashSupplier = ::Sha256) : BaseSigV4SignatureCalculator(AwsSigningAlgorithm.SIGV4, sha256Provider) { | ||
| private val signingKeyCache = PeriodicSweepCache<SigningKeyCacheKey, ByteArray>( | ||
| minimumSweepPeriod = 24.hours, | ||
| ) | ||
|
|
||
| override fun calculate(signingKey: ByteArray, stringToSign: String): String = hmac(signingKey, stringToSign.encodeToByteArray(), sha256Provider).encodeToHex() | ||
|
|
||
| override fun signingKey(config: AwsSigningConfig): ByteArray { | ||
| override fun signingKey(config: AwsSigningConfig): ByteArray = runBlocking { | ||
| val date = config.signingDate.format(TimestampFormat.ISO_8601_CONDENSED_DATE) | ||
| val cacheKey = SigningKeyCacheKey(config.credentials.secretAccessKey, config.region, config.service, date) | ||
|
|
||
| signingKeyCache.get(cacheKey) { | ||
| ExpiringValue(deriveKey(it.secretKey, it.date, it.region, it.service), Clock.System.now() + 24.hours) | ||
| } | ||
| } | ||
|
Collaborator
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: We should not block on a coroutine in a non- Can we withstand cache misses? If so, If we cannot withstand cache misses then I think we need a different lock mechanism. kotlinx.atomicfu provides some lock primitives that spin rather than suspend. If we decide to go this route we should create a new interface/implementation of the cache.
Collaborator
Author
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. Cache misses is fine so I think How SigV4a is different in behavior to SigV4? What is the reason of having the block in SigV4a? |
||
|
|
||
| private fun deriveKey(secretKey: String, date: String, region: String, service: String): ByteArray { | ||
| fun hmac(key: ByteArray, message: String) = hmac(key, message.encodeToByteArray(), sha256Provider) | ||
|
|
||
| val initialKey = ("AWS4" + config.credentials.secretAccessKey).encodeToByteArray() | ||
| val kDate = hmac(initialKey, config.signingDate.format(TimestampFormat.ISO_8601_CONDENSED_DATE)) | ||
| val kRegion = hmac(kDate, config.region) | ||
| val kService = hmac(kRegion, config.service) | ||
| val initialKey = ("AWS4$secretKey").encodeToByteArray() | ||
| val kDate = hmac(initialKey, date) | ||
| val kRegion = hmac(kDate, region) | ||
| val kService = hmac(kRegion, service) | ||
| return hmac(kService, "aws4_request") | ||
| } | ||
| } | ||
|
|
||
| private data class SigningKeyCacheKey( | ||
| val secretKey: String, | ||
| val region: String, | ||
| val service: String, | ||
| val date: String, | ||
| ) | ||
|
Collaborator
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. Suggestion: This combination of data don't just represent a cache key, they represent the signing key regardless of caching. I suggest renaming it to |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,6 +13,8 @@ import aws.smithy.kotlin.runtime.time.Instant | |
| import kotlinx.coroutines.test.runTest | ||
| import kotlin.test.Test | ||
| import kotlin.test.assertEquals | ||
| import kotlin.test.assertNotSame | ||
| import kotlin.test.assertSame | ||
|
|
||
| class SigV4SignatureCalculatorTest { | ||
| // Test adapted from https://docs.aws.amazon.com/general/latest/gr/sigv4-calculate-signature.html | ||
|
|
@@ -80,6 +82,90 @@ class SigV4SignatureCalculatorTest { | |
|
|
||
| private data class ChunkStringToSignTest(val signatureType: AwsSignatureType, val expectedNonSignatureHeaderHash: String) | ||
|
|
||
| @Test | ||
| fun testSigningKeyCacheHit() { | ||
| val calculator = SigV4SignatureCalculator() | ||
| val config = AwsSigningConfig { | ||
| signingDate = Instant.fromIso8601("20150830") | ||
| region = "us-east-1" | ||
| service = "iam" | ||
| credentials = Credentials("AKID", "wJalrXUtnFEMI/K7MDENG+bPxRfiCYEXAMPLEKEY") | ||
| } | ||
|
|
||
| val first = calculator.signingKey(config) | ||
| val second = calculator.signingKey(config) | ||
| assertSame(first, second) | ||
| } | ||
|
|
||
| @Test | ||
| fun testSigningKeyCacheMissOnDateChange() { | ||
| val calculator = SigV4SignatureCalculator() | ||
| val config1 = AwsSigningConfig { | ||
| signingDate = Instant.fromIso8601("20150830") | ||
| region = "us-east-1" | ||
| service = "iam" | ||
| credentials = Credentials("AKID", "wJalrXUtnFEMI/K7MDENG+bPxRfiCYEXAMPLEKEY") | ||
| } | ||
| val config2 = AwsSigningConfig { | ||
| signingDate = Instant.fromIso8601("20150831") | ||
| region = "us-east-1" | ||
| service = "iam" | ||
| credentials = Credentials("AKID", "wJalrXUtnFEMI/K7MDENG+bPxRfiCYEXAMPLEKEY") | ||
| } | ||
|
|
||
| val key1 = calculator.signingKey(config1) | ||
| val key2 = calculator.signingKey(config2) | ||
| assertNotSame(key1, key2) | ||
| assertEquals(false, key1.contentEquals(key2)) | ||
|
Collaborator
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. Nit: This
Collaborator
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. Nit: Don't use |
||
| } | ||
|
|
||
| @Test | ||
| fun testSigningKeyCacheMissOnCredentialChange() { | ||
| val calculator = SigV4SignatureCalculator() | ||
| val config1 = AwsSigningConfig { | ||
| signingDate = Instant.fromIso8601("20150830") | ||
| region = "us-east-1" | ||
| service = "iam" | ||
| credentials = Credentials("AKID1", "secretKey1") | ||
| } | ||
| val config2 = AwsSigningConfig { | ||
| signingDate = Instant.fromIso8601("20150830") | ||
| region = "us-east-1" | ||
| service = "iam" | ||
| credentials = Credentials("AKID2", "secretKey2") | ||
| } | ||
|
|
||
| val key1 = calculator.signingKey(config1) | ||
| val key2 = calculator.signingKey(config2) | ||
| assertEquals(false, key1.contentEquals(key2)) | ||
| } | ||
|
|
||
| @Test | ||
| fun testSigningKeyCacheMultipleAccountsRetainEntries() { | ||
| val calculator = SigV4SignatureCalculator() | ||
| val configA = AwsSigningConfig { | ||
| signingDate = Instant.fromIso8601("20150830") | ||
| region = "us-east-1" | ||
| service = "iam" | ||
| credentials = Credentials("AKID_A", "secretKeyA") | ||
| } | ||
| val configB = AwsSigningConfig { | ||
| signingDate = Instant.fromIso8601("20150830") | ||
| region = "us-east-1" | ||
| service = "iam" | ||
| credentials = Credentials("AKID_B", "secretKeyB") | ||
| } | ||
|
|
||
| val keyA1 = calculator.signingKey(configA) | ||
| val keyB1 = calculator.signingKey(configB) | ||
| val keyA2 = calculator.signingKey(configA) | ||
| val keyB2 = calculator.signingKey(configB) | ||
|
|
||
| assertSame(keyA1, keyA2) | ||
| assertSame(keyB1, keyB2) | ||
| assertEquals(false, keyA1.contentEquals(keyB1)) | ||
| } | ||
|
|
||
| @Test | ||
| fun testChunkStringToSign() { | ||
| // Test event stream signing | ||
|
|
||
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.
Question: A 24 hour sweep period combined with short-term credentials, multiservice usage pattern, and multiple accounts could stack up to quite a lot of entries. Should we be concerned about memory usage?
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.
You're right, I forgot about short-term credentials. Java SDK is using a FIFO cache with 300 entries. Do you have any suggestion on this?
The short-term credentials can last from few minutes to several hours. Computing SigV4 is cheap, the only problem is when it is in a high-concurrency environment then it becomes expensive, so probably using 1 hour like SigV4a?