From 21ecdc6cb98f65f61cbe9cb8b8bb5fb439ad82ff Mon Sep 17 00:00:00 2001 From: Brent Graveland Date: Tue, 27 Aug 2024 10:02:53 -0600 Subject: [PATCH] Re-read the web-id token for each S3 request The token file pointed to by the AWS_WEB_IDENTITY_TOKEN_FILE env var was read once at startup, but for long operations the token might expire before we're complete. This switches to reading it on each S3 request so we immediately use the renewed token. This is especially likely when running in kubernetes using a projected service account token. Fixes #2428 --- src/storage/s3/helper.c | 8 ++++---- src/storage/s3/storage.c | 19 +++++++++++++------ src/storage/s3/storage.h | 2 +- test/src/module/storage/s3Test.c | 5 +++-- 4 files changed, 21 insertions(+), 13 deletions(-) diff --git a/src/storage/s3/helper.c b/src/storage/s3/helper.c index 42fe093b76..5f0d82e039 100644 --- a/src/storage/s3/helper.c +++ b/src/storage/s3/helper.c @@ -53,9 +53,9 @@ storageS3Helper(const unsigned int repoIdx, const bool write, StoragePathExpress // Get role and token const StorageS3KeyType keyType = (StorageS3KeyType)cfgOptionIdxStrId(cfgOptRepoS3KeyType, repoIdx); const String *role = cfgOptionIdxStrNull(cfgOptRepoS3Role, repoIdx); - const String *webIdToken = NULL; + const String *webIdTokenFile = NULL; - // If web identity authentication then load the role and token from environment variables documented here: + // If web identity authentication then load the role and token filename from environment variables documented here: // https://docs.aws.amazon.com/eks/latest/userguide/iam-roles-for-service-accounts-technical-overview.html if (keyType == storageS3KeyTypeWebId) { @@ -75,7 +75,7 @@ storageS3Helper(const unsigned int repoIdx, const bool write, StoragePathExpress } role = strNewZ(roleZ); - webIdToken = strNewBuf(storageGetP(storageNewReadP(storagePosixNewP(FSLASH_STR), STR(webIdTokenFileZ)))); + webIdTokenFile = strNewZ(webIdTokenFileZ); } MEM_CONTEXT_PRIOR_BEGIN() @@ -86,7 +86,7 @@ storageS3Helper(const unsigned int repoIdx, const bool write, StoragePathExpress (StorageS3UriStyle)cfgOptionIdxStrId(cfgOptRepoS3UriStyle, repoIdx), cfgOptionIdxStr(cfgOptRepoS3Region, repoIdx), keyType, cfgOptionIdxStrNull(cfgOptRepoS3Key, repoIdx), cfgOptionIdxStrNull(cfgOptRepoS3KeySecret, repoIdx), cfgOptionIdxStrNull(cfgOptRepoS3Token, repoIdx), cfgOptionIdxStrNull(cfgOptRepoS3KmsKeyId, repoIdx), - cfgOptionIdxStrNull(cfgOptRepoS3SseCustomerKey, repoIdx), role, webIdToken, + cfgOptionIdxStrNull(cfgOptRepoS3SseCustomerKey, repoIdx), role, webIdTokenFile, (size_t)cfgOptionIdxUInt64(cfgOptRepoStorageUploadChunkSize, repoIdx), cfgOptionIdxKvNull(cfgOptRepoStorageTag, repoIdx), host, port, ioTimeoutMs(), cfgOptionIdxBool(cfgOptRepoStorageVerifyTls, repoIdx), cfgOptionIdxStrNull(cfgOptRepoStorageCaFile, repoIdx), diff --git a/src/storage/s3/storage.c b/src/storage/s3/storage.c index 8592fe4d51..7aed2079f7 100644 --- a/src/storage/s3/storage.c +++ b/src/storage/s3/storage.c @@ -3,6 +3,7 @@ S3 Storage ***********************************************************************************************************************************/ #include "build.auto.h" +#include #include #include "common/crypto/hash.h" @@ -16,6 +17,7 @@ S3 Storage #include "common/type/json.h" #include "common/type/object.h" #include "common/type/xml.h" +#include "storage/posix/storage.h" #include "storage/s3/read.h" #include "storage/s3/write.h" @@ -110,7 +112,8 @@ struct StorageS3 HttpClient *credHttpClient; // HTTP client to service credential requests const String *credHost; // Credentials host const String *credRole; // Role to use for credential requests - const String *webIdToken; // Token to use for credential requests + const String *webIdTokenFile; // The file containing the token to use for web-id credential + // requests time_t credExpirationTime; // Time the temporary credentials expire // Current signing key and date it is valid for @@ -386,13 +389,17 @@ storageS3AuthWebId(StorageS3 *const this, const HttpHeader *const header) MEM_CONTEXT_TEMP_BEGIN() { + // We load the token from the given file for each request since the token can be updated during the middle + // of a long-running execution. + const String *webIdToken = strNewBuf(storageGetP(storageNewReadP(storagePosixNewP(FSLASH_STR), this->webIdTokenFile))); + // Get credentials HttpQuery *const query = httpQueryNewP(); httpQueryAdd(query, STRDEF("Action"), STRDEF("AssumeRoleWithWebIdentity")); httpQueryAdd(query, STRDEF("RoleArn"), this->credRole); httpQueryAdd(query, STRDEF("RoleSessionName"), STRDEF(PROJECT_NAME)); httpQueryAdd(query, STRDEF("Version"), STRDEF("2011-06-15")); - httpQueryAdd(query, STRDEF("WebIdentityToken"), this->webIdToken); + httpQueryAdd(query, STRDEF("WebIdentityToken"), webIdToken); HttpRequest *const request = httpRequestNewP( this->credHttpClient, HTTP_VERB_GET_STR, FSLASH_STR, .header = header, .query = query); @@ -1097,7 +1104,7 @@ storageS3New( const String *const path, const bool write, StoragePathExpressionCallback pathExpressionFunction, const String *const bucket, const String *const endPoint, const StorageS3UriStyle uriStyle, const String *const region, const StorageS3KeyType keyType, const String *const accessKey, const String *const secretAccessKey, const String *const securityToken, - const String *const kmsKeyId, const String *sseCustomerKey, const String *const credRole, const String *const webIdToken, + const String *const kmsKeyId, const String *sseCustomerKey, const String *const credRole, const String *const webIdTokenFile, const size_t partSize, const KeyValue *const tag, const String *host, const unsigned int port, const TimeMSec timeout, const bool verifyPeer, const String *const caFile, const String *const caPath) { @@ -1116,7 +1123,7 @@ storageS3New( FUNCTION_TEST_PARAM(STRING, kmsKeyId); FUNCTION_TEST_PARAM(STRING, sseCustomerKey); FUNCTION_TEST_PARAM(STRING, credRole); - FUNCTION_TEST_PARAM(STRING, webIdToken); + FUNCTION_TEST_PARAM(STRING, webIdTokenFile); FUNCTION_LOG_PARAM(SIZE, partSize); FUNCTION_LOG_PARAM(KEY_VALUE, tag); FUNCTION_LOG_PARAM(STRING, host); @@ -1192,10 +1199,10 @@ storageS3New( { ASSERT(accessKey == NULL && secretAccessKey == NULL && securityToken == NULL); ASSERT(credRole != NULL); - ASSERT(webIdToken != NULL); + ASSERT(webIdTokenFile != NULL); this->credRole = strDup(credRole); - this->webIdToken = strDup(webIdToken); + this->webIdTokenFile = strDup(webIdTokenFile); this->credHost = S3_STS_HOST_STR; this->credExpirationTime = time(NULL); this->credHttpClient = httpClientNew( diff --git a/src/storage/s3/storage.h b/src/storage/s3/storage.h index 3154c00a6c..d5be86e915 100644 --- a/src/storage/s3/storage.h +++ b/src/storage/s3/storage.h @@ -37,7 +37,7 @@ FN_EXTERN Storage *storageS3New( const String *path, bool write, StoragePathExpressionCallback pathExpressionFunction, const String *bucket, const String *endPoint, StorageS3UriStyle uriStyle, const String *region, StorageS3KeyType keyType, const String *accessKey, const String *secretAccessKey, const String *securityToken, const String *kmsKeyId, const String *sseCustomerKey, - const String *credRole, const String *webIdToken, size_t partSize, const KeyValue *tag, const String *host, unsigned int port, + const String *credRole, const String *webIdTokenFile, size_t partSize, const KeyValue *tag, const String *host, unsigned int port, TimeMSec timeout, bool verifyPeer, const String *caFile, const String *caPath); #endif diff --git a/test/src/module/storage/s3Test.c b/test/src/module/storage/s3Test.c index 5c3f466610..cf9207f469 100644 --- a/test/src/module/storage/s3Test.c +++ b/test/src/module/storage/s3Test.c @@ -933,6 +933,7 @@ testRun(void) #define TEST_SERVICE_ROLE "arn:aws:iam::123456789012:role/TestRole" #define TEST_SERVICE_TOKEN "TOKEN" + #define TEST_SERVICE_TOKEN_FILE TEST_PATH "web-id-token" #define TEST_SERVICE_URI \ "/?Action=AssumeRoleWithWebIdentity&RoleArn=arn%3Aaws%3Aiam%3A%3A123456789012%3Arole%2FTestRole" \ "&RoleSessionName=pgBackRest&Version=2011-06-15&WebIdentityToken=" TEST_SERVICE_TOKEN @@ -967,13 +968,13 @@ testRun(void) storageRepoGet(0, true), OptionInvalidError, "option 'repo1-s3-key-type' is 'web-id' but 'AWS_ROLE_ARN' and 'AWS_WEB_IDENTITY_TOKEN_FILE' are not set"); - setenv("AWS_WEB_IDENTITY_TOKEN_FILE", TEST_PATH "/web-id-token", true); + setenv("AWS_WEB_IDENTITY_TOKEN_FILE", TEST_SERVICE_TOKEN_FILE, true); s3 = storageRepoGet(0, true); driver = (StorageS3 *)storageDriver(s3); TEST_RESULT_STR_Z(driver->credRole, TEST_SERVICE_ROLE, "check role"); - TEST_RESULT_STR_Z(driver->webIdToken, TEST_SERVICE_TOKEN, "check token"); + TEST_RESULT_STR_Z(driver->webIdTokenFile, TEST_SERVICE_TOKEN_FILE, "check token file"); // Set partSize to a small value for testing driver->partSize = 16;