feat(libstore): AWS credential provider caching

Author: lovesegfault

src/libstore-tests/filetransfer-s3.cc

@@ -147,4 +147,4 @@ TEST_F(S3FileTransferTest, s3RegionQueryParameters)
 
 } // namespace nix
 
-#endif // NIX_WITH_S3_SUPPORT
+#endif // NIX_WITH_S3_SUPPORT

src/libstore/filetransfer.cc

@@ -6,6 +6,7 @@
 #include "nix/util/finally.hh"
 #include "nix/util/callback.hh"
 #include "nix/util/signals.hh"
+#include "nix/util/sync.hh"
 #include "nix/store/store-reference.hh"
 
 #include "store-config-private.hh"
@@ -48,6 +49,61 @@ struct curlFileTransfer : public FileTransfer
     std::random_device rd;
     std::mt19937 mt19937;
 
+#if NIX_WITH_S3_SUPPORT
+public:
+    // Public method to clear credential provider cache (called from AWS CRT shutdown)
+    void clearCredentialCache();
+
+private:
+    struct CredentialProviderCache
+    {
+        // Key: profile name (empty string for default profile)
+        std::map<std::string, std::shared_ptr<AwsCredentialProvider>> providers;
+    };
+
+    Sync<CredentialProviderCache> credentialProviderCache;
+
+    /**
+     * Get or create a credential provider for the given profile.
+     * Thread-safe: holds lock for entire duration to prevent duplicate creation.
+     */
+    std::shared_ptr<AwsCredentialProvider> getOrCreateCredentialProvider(const std::string & profile)
+    {
+        auto cache(credentialProviderCache.lock());
+
+        // Check if provider exists
+        auto it = cache->providers.find(profile);
+        if (it != cache->providers.end()) {
+            return it->second;
+        }
+
+        try {
+            debug(
+                "creating new AWS credential provider for profile '%s'",
+                profile.empty() ? "(default)" : profile.c_str());
+            auto provider = profile.empty() ? AwsCredentialProvider::createDefault()
+                                            : AwsCredentialProvider::createProfile(profile);
+
+            auto sharedProvider = std::shared_ptr<AwsCredentialProvider>(std::move(provider));
+            cache->providers[profile] = sharedProvider;
+            return sharedProvider;
+
+        } catch (const AwsAuthError & e) {
+            // Don't cache failed providers, just propagate the error
+            throw;
+        }
+    }
+
+    /**
+     * Invalidate a cached credential provider (e.g., on auth failure)
+     */
+    void invalidateCredentialProvider(const std::string & profile)
+    {
+        auto cache(credentialProviderCache.lock());
+        cache->providers.erase(profile);
+    }
+#endif
+
     struct TransferItem : public std::enable_shared_from_this<TransferItem>
     {
         curlFileTransfer & fileTransfer;
@@ -157,9 +213,9 @@ struct curlFileTransfer : public FileTransfer
                     // Get credentials
                     try {
                         std::string profile = parsed.profile.value_or("");
-                        auto credProvider = profile.empty() ? AwsCredentialProvider::createDefault()
-                                                            : AwsCredentialProvider::createProfile(profile);
 
+                        // Use cached credential provider
+                        auto credProvider = fileTransfer.getOrCreateCredentialProvider(profile);
                         auto creds = credProvider->getCredentials();
                         awsCredentials = creds.accessKeyId + ":" + creds.secretAccessKey;
 
@@ -174,6 +230,11 @@ struct curlFileTransfer : public FileTransfer
                         }
                     } catch (const AwsAuthError & e) {
                         warn("AWS authentication failed for S3 request %s: %s", request.uri, e.what());
+
+                        // Invalidate the cached provider so next request will retry
+                        std::string profile = parsed.profile.value_or("");
+                        fileTransfer.invalidateCredentialProvider(profile);
+
                         // Continue without authentication - might be a public bucket
                     }
                 } catch (std::exception & e) {
@@ -677,6 +738,12 @@ struct curlFileTransfer : public FileTransfer
 
     std::thread workerThread;
 
+public:
+    bool hasQuit()
+    {
+        return state_.lock()->quit;
+    }
+
     curlFileTransfer()
         : mt19937(rd())
     {
@@ -703,9 +770,18 @@ struct curlFileTransfer : public FileTransfer
     ~curlFileTransfer()
     {
         stopWorkerThread();
-
         workerThread.join();
 
+#if NIX_WITH_S3_SUPPORT
+        {
+            auto cache(credentialProviderCache.lock());
+            // IMPORTANT: We must clear the providers here to ensure they are destroyed
+            // BEFORE the AWS CRT ApiHandle is destroyed during static destruction.
+            // This prevents a deadlock where AWS CRT waits for resources we're holding.
+            cache->providers.clear();
+        }
+#endif
+
         if (curlm)
             curl_multi_cleanup(curlm);
     }
@@ -930,12 +1006,25 @@ ref<curlFileTransfer> makeCurlFileTransfer()
 
 ref<FileTransfer> getFileTransfer()
 {
-    static ref<curlFileTransfer> fileTransfer = makeCurlFileTransfer();
+    struct StaticFileTransfer
+    {
+        ref<curlFileTransfer> fileTransfer;
+
+        StaticFileTransfer()
+            : fileTransfer(makeCurlFileTransfer())
+        {
+        }
 
-    if (fileTransfer->state_.lock()->quit)
-        fileTransfer = makeCurlFileTransfer();
+        ~StaticFileTransfer() {}
+    };
+
+    static StaticFileTransfer staticFT;
+
+    if (staticFT.fileTransfer->hasQuit()) {
+        staticFT.fileTransfer = makeCurlFileTransfer();
+    }
 
-    return fileTransfer;
+    return staticFT.fileTransfer;
 }
 
 ref<FileTransfer> makeFileTransfer()
@@ -1091,4 +1180,28 @@ FileTransferError::FileTransferError(
         err.msg = hf;
 }
 
+#if NIX_WITH_S3_SUPPORT
+void curlFileTransfer::clearCredentialCache()
+{
+    auto cache(credentialProviderCache.lock());
+    cache->providers.clear();
+}
+
+// Function called by AWS CRT shutdown to clear credential provider cache
+void cleanupCredentialProviderCache()
+{
+    // Access the curlFileTransfer singleton and clear its cache
+    try {
+        auto ft = getFileTransfer();
+        if (auto curlFt = ft.dynamic_pointer_cast<curlFileTransfer>()) {
+            curlFt->clearCredentialCache();
+        }
+    } catch (const std::exception & e) {
+        warn("Error clearing credential cache during AWS CRT shutdown: %s", e.what());
+    } catch (...) {
+        warn("Unknown error clearing credential cache during AWS CRT shutdown");
+    }
+}
+#endif
+
 } // namespace nix

src/libstore/s3.cc

@@ -16,6 +16,7 @@
 #  include <condition_variable>
 #  include <mutex>
 #  include <string_view>
+#  include <pthread.h>
 
 using namespace std::string_view_literals;
 
@@ -25,13 +26,31 @@ namespace nix {
 static std::once_flag crtInitFlag;
 static bool crtInitialized = false;
 
+// Forward declaration for cleanup function
+void cleanupCredentialProviderCache();
+
 static void initAwsCrt()
 {
     std::call_once(crtInitFlag, []() {
         try {
             // Use a static local variable instead of global to control destruction order
-            static Aws::Crt::ApiHandle apiHandle;
-            apiHandle.InitializeLogging(Aws::Crt::LogLevel::Warn, (FILE *) nullptr);
+            struct CrtWrapper
+            {
+                Aws::Crt::ApiHandle apiHandle;
+                CrtWrapper()
+                {
+                    apiHandle.InitializeLogging(Aws::Crt::LogLevel::Warn, (FILE *) nullptr);
+                }
+                ~CrtWrapper()
+                {
+                    // 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
+                    cleanupCredentialProviderCache();
+                    // Now it's safe for ApiHandle destructor to run
+                }
+            };
+            static CrtWrapper crt;
             crtInitialized = true;
         } catch (const std::exception & e) {
             debug("Failed to initialize AWS CRT: %s", e.what());
@@ -218,6 +237,9 @@ try {
     throw;
 }
 
+// Function to clean up credential provider cache (defined in filetransfer.cc)
+void cleanupCredentialProviderCache();
+
 } // namespace nix
 
 #endif // NIX_WITH_S3_SUPPORT

tests/nixos/s3-binary-cache-store.nix

@@ -61,6 +61,7 @@ in
 
   testScript =
     { nodes }:
+    # python
     ''
       # fmt: off
       start_all()
@@ -73,7 +74,11 @@ in
       server.succeed("mc config host add minio http://localhost:9000 ${accessKey} ${secretKey} --api s3v4")
       server.succeed("mc mb minio/my-cache")
 
-      server.succeed("${env} nix copy --to '${storeUrl}' ${pkgA}")
+      # Test copying from a store and credential caching works
+      server_cp_out = server.succeed("${env} nix copy --debug --to '${storeUrl}' ${pkgA}")
+      server_providers_created = server_cp_out.count("creating new AWS credential provider")
+      if server_providers_created > 1:
+          raise Exception(f"Expected only 1 credential provider to be created, but got {server_providers_created}. Credential provider caching is probably not working.")
 
       client.wait_for_unit("network-addresses-eth1.service")
 
@@ -105,7 +110,11 @@ in
       assert store_info.get("url"), "Store should have a URL"
       print(f"Store URL: {store_info.get('url')}")
 
-      client.succeed("${env} nix copy --no-check-sigs --from '${storeUrl}' ${pkgA}")
+      # Test copying from a store and credential caching works
+      client_cp_out = client.succeed("${env} nix copy --debug --no-check-sigs --from '${storeUrl}' ${pkgA} 2>&1")
+      client_providers_created = client_cp_out.count("creating new AWS credential provider")
+      if client_providers_created > 1:
+          raise Exception(f"Expected only 1 credential provider to be created, but got {client_providers_created}. Credential provider caching is probably not working.")
 
       client.succeed("nix path-info ${pkgA}")
     '';