Skip to content

Commit

Permalink
Re-read the web-id token for each S3 request
Browse files Browse the repository at this point in the history 
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 pgbackrest#2428
  • Loading branch information
@graveland
graveland committed Aug 27, 2024
1 parent 2ce01e5 commit 21ecdc6
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 13 deletions.
8 changes: 4 additions & 4 deletions src/storage/s3/helper.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand All @@ -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()
Expand All @@ -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),
Expand Down
19 changes: 13 additions & 6 deletions src/storage/s3/storage.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ S3 Storage
***********************************************************************************************************************************/
#include "build.auto.h"

#include <stdlib.h>
#include <string.h>

#include "common/crypto/hash.h"
Expand All @@ -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"

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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)
{
Expand All @@ -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);
Expand Down Expand Up @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion src/storage/s3/storage.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
5 changes: 3 additions & 2 deletions test/src/module/storage/s3Test.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit 21ecdc6

Please sign in to comment.