From b1d067c9bb33bbe35507872636d5e1c499b4ea7c Mon Sep 17 00:00:00 2001 From: Sergei Zimmerman Date: Thu, 16 Oct 2025 20:34:15 +0300 Subject: [PATCH 1/4] tests/nixos: Rename back S3 store nixos test --- ci/gha/tests/default.nix | 2 +- tests/nixos/default.nix | 2 +- ...curl-s3-binary-cache-store.nix => s3-binary-cache-store.nix} | 0 3 files changed, 2 insertions(+), 2 deletions(-) rename tests/nixos/{curl-s3-binary-cache-store.nix => s3-binary-cache-store.nix} (100%) diff --git a/ci/gha/tests/default.nix b/ci/gha/tests/default.nix index fac4f9002..2bfdae17b 100644 --- a/ci/gha/tests/default.nix +++ b/ci/gha/tests/default.nix @@ -222,7 +222,7 @@ rec { }; vmTests = { - inherit (nixosTests) curl-s3-binary-cache-store; + inherit (nixosTests) s3-binary-cache-store; } // lib.optionalAttrs (!withSanitizers && !withCoverage) { # evalNixpkgs uses non-instrumented components from hydraJobs, so only run it diff --git a/tests/nixos/default.nix b/tests/nixos/default.nix index 0112d2e2f..edfa4124f 100644 --- a/tests/nixos/default.nix +++ b/tests/nixos/default.nix @@ -199,7 +199,7 @@ in user-sandboxing = runNixOSTest ./user-sandboxing; - curl-s3-binary-cache-store = runNixOSTest ./curl-s3-binary-cache-store.nix; + s3-binary-cache-store = runNixOSTest ./s3-binary-cache-store.nix; fsync = runNixOSTest ./fsync.nix; diff --git a/tests/nixos/curl-s3-binary-cache-store.nix b/tests/nixos/s3-binary-cache-store.nix similarity index 100% rename from tests/nixos/curl-s3-binary-cache-store.nix rename to tests/nixos/s3-binary-cache-store.nix From dc03c6a8121a42f597268dcfbee2087a5a80018d Mon Sep 17 00:00:00 2001 From: Sergei Zimmerman Date: Thu, 16 Oct 2025 21:13:04 +0300 Subject: [PATCH 2/4] libstore: Put all the AWS credentials logic behind interface class AwsCredentialProvider This makes it so we don't need to rely on global variables and hacky destructors to clean up another global variable. Just putting it in the correct order in the class is more than enough. --- src/libstore/aws-creds.cc | 88 +++++++------------ src/libstore/filetransfer.cc | 22 ++--- src/libstore/include/nix/store/aws-creds.hh | 46 ++++++---- src/libstore/unix/build/derivation-builder.cc | 2 +- 4 files changed, 69 insertions(+), 89 deletions(-) diff --git a/src/libstore/aws-creds.cc b/src/libstore/aws-creds.cc index 93fc3da33..4ba5b7dee 100644 --- a/src/libstore/aws-creds.cc +++ b/src/libstore/aws-creds.cc @@ -24,50 +24,6 @@ namespace nix { namespace { -// Global credential provider cache using boost's concurrent map -// Key: profile name (empty string for default profile) -using CredentialProviderCache = - boost::concurrent_flat_map>; - -static CredentialProviderCache credentialProviderCache; - -/** - * Clear all cached credential providers. - * Called automatically by CrtWrapper destructor during static destruction. - */ -static void clearAwsCredentialsCache() -{ - credentialProviderCache.clear(); -} - -static void initAwsCrt() -{ - struct CrtWrapper - { - Aws::Crt::ApiHandle apiHandle; - - CrtWrapper() - { - apiHandle.InitializeLogging(Aws::Crt::LogLevel::Warn, static_cast(nullptr)); - } - - ~CrtWrapper() - { - try { - // CRITICAL: Clear credential provider cache BEFORE AWS CRT shuts down - // This ensures all providers (which hold references to ClientBootstrap) - // are destroyed while AWS CRT is still valid - clearAwsCredentialsCache(); - // Now it's safe for ApiHandle destructor to run - } catch (...) { - ignoreExceptionInDestructor(); - } - } - }; - - static CrtWrapper crt; -} - static AwsCredentials getCredentialsFromProvider(std::shared_ptr provider) { if (!provider || !provider->IsValid()) { @@ -113,7 +69,35 @@ static AwsCredentials getCredentialsFromProvider(std::shared_ptr(nullptr)); + } + + AwsCredentials getCredentialsRaw(const std::string & profile); + + AwsCredentials getCredentials(const ParsedS3URL & url) override + { + auto profile = url.profile.value_or(""); + try { + return getCredentialsRaw(profile); + } catch (AwsAuthError & e) { + warn("AWS authentication failed for S3 request %s: %s", url.toHttpsUrl(), e.what()); + credentialProviderCache.erase(profile); + throw; + } + } + +private: + Aws::Crt::ApiHandle apiHandle; + boost::concurrent_flat_map> + credentialProviderCache; +}; + +AwsCredentials AwsCredentialProviderImpl::getCredentialsRaw(const std::string & profile) { // Get or create credential provider with caching std::shared_ptr provider; @@ -132,8 +116,6 @@ AwsCredentials getAwsCredentials(const std::string & profile) profile.empty() ? "(default)" : profile.c_str()); try { - initAwsCrt(); - if (profile.empty()) { Aws::Crt::Auth::CredentialsProviderChainDefaultConfig config; config.Bootstrap = Aws::Crt::ApiHandle::GetOrCreateStaticDefaultClientBootstrap(); @@ -173,17 +155,15 @@ AwsCredentials getAwsCredentials(const std::string & profile) return getCredentialsFromProvider(provider); } -void invalidateAwsCredentials(const std::string & profile) +ref makeAwsCredentialsProvider() { - credentialProviderCache.erase(profile); + return make_ref(); } -AwsCredentials preResolveAwsCredentials(const ParsedS3URL & s3Url) +ref getAwsCredentialsProvider() { - std::string profile = s3Url.profile.value_or(""); - - // Get credentials (automatically cached) - return getAwsCredentials(profile); + static auto instance = makeAwsCredentialsProvider(); + return instance; } } // namespace nix diff --git a/src/libstore/filetransfer.cc b/src/libstore/filetransfer.cc index 981d49d77..201f2984e 100644 --- a/src/libstore/filetransfer.cc +++ b/src/libstore/filetransfer.cc @@ -883,22 +883,12 @@ void FileTransferRequest::setupForS3() if (usernameAuth) { debug("Using pre-resolved AWS credentials from parent process"); sessionToken = preResolvedAwsSessionToken; - } else { - std::string profile = parsedS3.profile.value_or(""); - try { - auto creds = getAwsCredentials(profile); - usernameAuth = UsernameAuth{ - .username = creds.accessKeyId, - .password = creds.secretAccessKey, - }; - sessionToken = creds.sessionToken; - } catch (const AwsAuthError & e) { - warn("AWS authentication failed for S3 request %s: %s", uri, e.what()); - // Invalidate the cached credentials so next request will retry - invalidateAwsCredentials(profile); - // Continue without authentication - might be a public bucket - return; - } + } else if (auto creds = getAwsCredentialsProvider()->maybeGetCredentials(parsedS3)) { + usernameAuth = UsernameAuth{ + .username = creds->accessKeyId, + .password = creds->secretAccessKey, + }; + sessionToken = creds->sessionToken; } if (sessionToken) headers.emplace_back("x-amz-security-token", *sessionToken); diff --git a/src/libstore/include/nix/store/aws-creds.hh b/src/libstore/include/nix/store/aws-creds.hh index 6e653936c..d72290ced 100644 --- a/src/libstore/include/nix/store/aws-creds.hh +++ b/src/libstore/include/nix/store/aws-creds.hh @@ -5,6 +5,7 @@ #if NIX_WITH_AWS_AUTH # include "nix/store/s3-url.hh" +# include "nix/util/ref.hh" # include "nix/util/error.hh" # include @@ -38,30 +39,39 @@ struct AwsCredentials */ MakeError(AwsAuthError, Error); -/** - * Get AWS credentials for the given profile. - * This function automatically caches credential providers to avoid - * creating multiple providers for the same profile. - * - * @param profile The AWS profile name (empty string for default profile) - * @return AWS credentials - * @throws AwsAuthError if credentials cannot be resolved - */ -AwsCredentials getAwsCredentials(const std::string & profile = ""); +class AwsCredentialProvider +{ +public: + /** + * Get AWS credentials for the given URL. + * + * @param url The S3 url to get the credentials for + * @return AWS credentials + * @throws AwsAuthError if credentials cannot be resolved + */ + virtual AwsCredentials getCredentials(const ParsedS3URL & url) = 0; + + std::optional maybeGetCredentials(const ParsedS3URL & url) + { + try { + return getCredentials(url); + } catch (AwsAuthError & e) { + return std::nullopt; + } + } + + virtual ~AwsCredentialProvider() {} +}; /** - * Invalidate cached credentials for a profile (e.g., on authentication failure). - * The next request for this profile will create a new provider. - * - * @param profile The AWS profile name to invalidate + * Create a new instancee of AwsCredentialProvider. */ -void invalidateAwsCredentials(const std::string & profile); +ref makeAwsCredentialsProvider(); /** - * Pre-resolve AWS credentials for S3 URLs. - * Used to cache credentials in parent process before forking. + * Get a reference to the global AwsCredentialProvider. */ -AwsCredentials preResolveAwsCredentials(const ParsedS3URL & s3Url); +ref getAwsCredentialsProvider(); } // namespace nix #endif diff --git a/src/libstore/unix/build/derivation-builder.cc b/src/libstore/unix/build/derivation-builder.cc index 8a0fa5ef7..1246fbf26 100644 --- a/src/libstore/unix/build/derivation-builder.cc +++ b/src/libstore/unix/build/derivation-builder.cc @@ -958,7 +958,7 @@ std::optional DerivationBuilderImpl::preResolveAwsCredentials() auto s3Url = ParsedS3URL::parse(parsedUrl); // Use the preResolveAwsCredentials from aws-creds - auto credentials = nix::preResolveAwsCredentials(s3Url); + auto credentials = getAwsCredentialsProvider()->getCredentials(s3Url); debug("Successfully pre-resolved AWS credentials in parent process"); return credentials; } From 33e94fe19fdedca5dd89fdc0b292938ac58dc81a Mon Sep 17 00:00:00 2001 From: Sergei Zimmerman Date: Thu, 16 Oct 2025 21:48:13 +0300 Subject: [PATCH 3/4] libstore: Make AwsAuthError more legible Instead of the cryptic: > error: Failed to resolve AWS credentials: error code 6153` We now get more legible: > error: AWS authentication error: 'Valid credentials could not be sourced by the IMDS provider' (6153) --- src/libstore/aws-creds.cc | 9 +++++++-- src/libstore/include/nix/store/aws-creds.hh | 17 +++++++++++++---- src/libstore/meson.build | 2 ++ 3 files changed, 22 insertions(+), 6 deletions(-) diff --git a/src/libstore/aws-creds.cc b/src/libstore/aws-creds.cc index 4ba5b7dee..6c9bc99b2 100644 --- a/src/libstore/aws-creds.cc +++ b/src/libstore/aws-creds.cc @@ -22,6 +22,12 @@ namespace nix { +AwsAuthError::AwsAuthError(int errorCode) + : Error("AWS authentication error: '%s' (%d)", aws_error_str(errorCode), errorCode) + , errorCode(errorCode) +{ +} + namespace { static AwsCredentials getCredentialsFromProvider(std::shared_ptr provider) @@ -35,8 +41,7 @@ static AwsCredentials getCredentialsFromProvider(std::shared_ptrGetCredentials([prom](std::shared_ptr credentials, int errorCode) { if (errorCode != 0 || !credentials) { - prom->set_exception( - std::make_exception_ptr(AwsAuthError("Failed to resolve AWS credentials: error code %d", errorCode))); + prom->set_exception(std::make_exception_ptr(AwsAuthError(errorCode))); } else { auto accessKeyId = Aws::Crt::ByteCursorToStringView(credentials->GetAccessKeyId()); auto secretAccessKey = Aws::Crt::ByteCursorToStringView(credentials->GetSecretAccessKey()); diff --git a/src/libstore/include/nix/store/aws-creds.hh b/src/libstore/include/nix/store/aws-creds.hh index d72290ced..30f6592a0 100644 --- a/src/libstore/include/nix/store/aws-creds.hh +++ b/src/libstore/include/nix/store/aws-creds.hh @@ -34,10 +34,19 @@ struct AwsCredentials } }; -/** - * Exception thrown when AWS authentication fails - */ -MakeError(AwsAuthError, Error); +class AwsAuthError : public Error +{ + std::optional errorCode; + +public: + using Error::Error; + AwsAuthError(int errorCode); + + std::optional getErrorCode() const + { + return errorCode; + } +}; class AwsCredentialProvider { diff --git a/src/libstore/meson.build b/src/libstore/meson.build index 78a3dd9b3..40da06e6b 100644 --- a/src/libstore/meson.build +++ b/src/libstore/meson.build @@ -158,6 +158,8 @@ curl_s3_store_opt = get_option('curl-s3-store').require( if curl_s3_store_opt.enabled() deps_other += aws_crt_cpp + aws_c_common = cxx.find_library('aws-c-common', required : true) + deps_other += aws_c_common endif configdata_pub.set('NIX_WITH_AWS_AUTH', curl_s3_store_opt.enabled().to_int()) From e7047fde2549aa207ebd28cfb67a7eb21471c708 Mon Sep 17 00:00:00 2001 From: Sergei Zimmerman Date: Thu, 16 Oct 2025 21:33:42 +0300 Subject: [PATCH 4/4] libstore: Remove the unnecessary 'error: ' prefix in warning message --- src/libstore/aws-creds.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstore/aws-creds.cc b/src/libstore/aws-creds.cc index 6c9bc99b2..d58293560 100644 --- a/src/libstore/aws-creds.cc +++ b/src/libstore/aws-creds.cc @@ -90,7 +90,7 @@ public: try { return getCredentialsRaw(profile); } catch (AwsAuthError & e) { - warn("AWS authentication failed for S3 request %s: %s", url.toHttpsUrl(), e.what()); + warn("AWS authentication failed for S3 request %s: %s", url.toHttpsUrl(), e.message()); credentialProviderCache.erase(profile); throw; }