Fix MongoClient leak in auto-encryption (#1142)

When AutoEncryptionSettings#keyVaultMongoClientSettings is non-null and AutoEncryptionSettings#isBypassAutoEncryption is false, the second internal MongoClient is now closed when the containing MongoClient is closed. The bug is fixed in both the synchronous and reactive streams drivers. JAVA-4993
mongodb · Jun 12, 2023 · 6332c14 · 6332c14
1 parent f256e42
commit 6332c14
Show file tree

Hide file tree

Showing 4 changed files with 39 additions and 25 deletions.
diff --git a/...er-reactive-streams/src/main/com/mongodb/reactivestreams/client/internal/crypt/Crypt.java b/...er-reactive-streams/src/main/com/mongodb/reactivestreams/client/internal/crypt/Crypt.java
@@ -64,7 +64,9 @@ public class Crypt implements Closeable {
     private final KeyManagementService keyManagementService;
     private final boolean bypassAutoEncryption;
     @Nullable
-    private final MongoClient internalClient;
+    private final MongoClient collectionInfoRetrieverClient;
+    @Nullable
+    private final MongoClient keyVaultClient;
 
     /**
      * Create an instance to use for explicit encryption and decryption, and data key creation.
@@ -81,7 +83,7 @@ public class Crypt implements Closeable {
             final Map<String, Map<String, Object>> kmsProviders,
             final Map<String, Supplier<Map<String, Object>>> kmsProviderPropertySuppliers) {
         this(mongoCrypt, keyRetriever, keyManagementService, kmsProviders, kmsProviderPropertySuppliers,
-                false, null, null, null);
+                false, null, null, null, null);
     }
 
     /**
@@ -95,7 +97,8 @@ public class Crypt implements Closeable {
      * @param bypassAutoEncryption          the bypass auto encryption flag
      * @param collectionInfoRetriever       the collection info retriever
      * @param commandMarker                 the command marker
-     * @param internalClient                the internal mongo client
+     * @param collectionInfoRetrieverClient the collection info retriever mongo client
+     * @param keyVaultClient                the key vault mongo client
      */
     Crypt(final MongoCrypt mongoCrypt,
             final KeyRetriever keyRetriever,
@@ -105,7 +108,8 @@ public class Crypt implements Closeable {
             final boolean bypassAutoEncryption,
             @Nullable final CollectionInfoRetriever collectionInfoRetriever,
             @Nullable final CommandMarker commandMarker,
-            @Nullable final MongoClient internalClient) {
+            @Nullable final MongoClient collectionInfoRetrieverClient,
+            @Nullable final MongoClient keyVaultClient) {
         this.mongoCrypt = mongoCrypt;
         this.keyRetriever = keyRetriever;
         this.keyManagementService = keyManagementService;
@@ -114,7 +118,8 @@ public class Crypt implements Closeable {
         this.bypassAutoEncryption = bypassAutoEncryption;
         this.collectionInfoRetriever = collectionInfoRetriever;
         this.commandMarker = commandMarker;
-        this.internalClient = internalClient;
+        this.collectionInfoRetrieverClient = collectionInfoRetrieverClient;
+        this.keyVaultClient = keyVaultClient;
     }
 
     /**
@@ -227,8 +232,9 @@ public void close() {
         //noinspection EmptyTryBlock
         try (MongoCrypt ignored = this.mongoCrypt;
              CommandMarker ignored1 = this.commandMarker;
-             MongoClient ignored2 = this.internalClient;
-             KeyManagementService ignored3 = this.keyManagementService
+             MongoClient ignored2 = this.collectionInfoRetrieverClient;
+             MongoClient ignored3 = this.keyVaultClient;
+             KeyManagementService ignored4 = this.keyManagementService
         ) {
             // just using try-with-resources to ensure they all get closed, even in the case of exceptions
         }

diff --git a/...r-reactive-streams/src/main/com/mongodb/reactivestreams/client/internal/crypt/Crypts.java b/...r-reactive-streams/src/main/com/mongodb/reactivestreams/client/internal/crypt/Crypts.java
@@ -42,17 +42,17 @@ private Crypts() {
     }
 
     public static Crypt createCrypt(final MongoClientImpl client, final AutoEncryptionSettings settings) {
-        MongoClient internalClient = null;
+        MongoClient sharedInternalClient = null;
         MongoClientSettings keyVaultMongoClientSettings = settings.getKeyVaultMongoClientSettings();
         if (keyVaultMongoClientSettings == null || !settings.isBypassAutoEncryption()) {
-            MongoClientSettings mongoClientSettings = MongoClientSettings.builder(client.getSettings())
+            MongoClientSettings defaultInternalMongoClientSettings = MongoClientSettings.builder(client.getSettings())
                     .applyToConnectionPoolSettings(builder -> builder.minSize(0))
                     .autoEncryptionSettings(null)
                     .build();
-            internalClient = MongoClients.create(mongoClientSettings);
+            sharedInternalClient = MongoClients.create(defaultInternalMongoClientSettings);
         }
         MongoClient keyVaultClient = keyVaultMongoClientSettings == null
-                ? internalClient : MongoClients.create(keyVaultMongoClientSettings);
+                ? sharedInternalClient : MongoClients.create(keyVaultMongoClientSettings);
         MongoCrypt mongoCrypt = MongoCrypts.create(createMongoCryptOptions(settings));
         return new Crypt(
                 mongoCrypt,
@@ -61,9 +61,10 @@ public static Crypt createCrypt(final MongoClientImpl client, final AutoEncrypti
                 settings.getKmsProviders(),
                 settings.getKmsProviderPropertySuppliers(),
                 settings.isBypassAutoEncryption(),
-                settings.isBypassAutoEncryption() ? null : new CollectionInfoRetriever(internalClient),
+                settings.isBypassAutoEncryption() ? null : new CollectionInfoRetriever(sharedInternalClient),
                 new CommandMarker(mongoCrypt, settings),
-                internalClient);
+                sharedInternalClient,
+                keyVaultClient);
     }
 
     public static Crypt create(final MongoClient keyVaultClient, final ClientEncryptionSettings settings) {

diff --git a/driver-sync/src/main/com/mongodb/client/internal/Crypt.java b/driver-sync/src/main/com/mongodb/client/internal/Crypt.java
@@ -63,7 +63,10 @@ public class Crypt implements Closeable {
     private final KeyRetriever keyRetriever;
     private final KeyManagementService keyManagementService;
     private final boolean bypassAutoEncryption;
-    private final MongoClient internalClient;
+    @Nullable
+    private final MongoClient collectionInfoRetrieverClient;
+    @Nullable
+    private final MongoClient keyVaultClient;
 
 
     /**
@@ -81,7 +84,7 @@ public class Crypt implements Closeable {
             final Map<String, Map<String, Object>> kmsProviders,
             final Map<String, Supplier<Map<String, Object>>> kmsProviderPropertySuppliers) {
         this(mongoCrypt, keyRetriever, keyManagementService, kmsProviders, kmsProviderPropertySuppliers,
-                false, null, null, null);
+                false, null, null, null, null);
     }
 
     /**
@@ -95,7 +98,8 @@ public class Crypt implements Closeable {
      * @param bypassAutoEncryption          the bypass auto encryption flag
      * @param collectionInfoRetriever       the collection info retriever
      * @param commandMarker                 the command marker
-     * @param internalClient                the internal mongo client
+     * @param collectionInfoRetrieverClient the collection info retriever mongo client
+     * @param keyVaultClient                the key vault mongo client
      */
     Crypt(final MongoCrypt mongoCrypt,
             final KeyRetriever keyRetriever,
@@ -105,7 +109,8 @@ public class Crypt implements Closeable {
             final boolean bypassAutoEncryption,
             @Nullable final CollectionInfoRetriever collectionInfoRetriever,
             @Nullable final CommandMarker commandMarker,
-            @Nullable final MongoClient internalClient) {
+            @Nullable final MongoClient collectionInfoRetrieverClient,
+            @Nullable final MongoClient keyVaultClient) {
         this.mongoCrypt = mongoCrypt;
         this.keyRetriever = keyRetriever;
         this.keyManagementService = keyManagementService;
@@ -114,7 +119,8 @@ public class Crypt implements Closeable {
         this.bypassAutoEncryption = bypassAutoEncryption;
         this.collectionInfoRetriever = collectionInfoRetriever;
         this.commandMarker = commandMarker;
-        this.internalClient = internalClient;
+        this.collectionInfoRetrieverClient = collectionInfoRetrieverClient;
+        this.keyVaultClient = keyVaultClient;
     }
 
     /**
@@ -260,7 +266,8 @@ public void close() {
         //noinspection EmptyTryBlock
         try (MongoCrypt ignored = this.mongoCrypt;
              CommandMarker ignored1 = this.commandMarker;
-             MongoClient ignored2 = this.internalClient
+             MongoClient ignored2 = this.collectionInfoRetrieverClient;
+             MongoClient ignored3 = this.keyVaultClient
         ) {
             // just using try-with-resources to ensure they all get closed, even in the case of exceptions
         }

diff --git a/driver-sync/src/main/com/mongodb/client/internal/Crypts.java b/driver-sync/src/main/com/mongodb/client/internal/Crypts.java
@@ -36,17 +36,17 @@
 public final class Crypts {
 
     public static Crypt createCrypt(final MongoClientImpl client, final AutoEncryptionSettings settings) {
-        MongoClient internalClient = null;
+        MongoClient sharedInternalClient = null;
         MongoClientSettings keyVaultMongoClientSettings = settings.getKeyVaultMongoClientSettings();
         if (keyVaultMongoClientSettings == null || !settings.isBypassAutoEncryption()) {
-            MongoClientSettings mongoClientSettings = MongoClientSettings.builder(client.getSettings())
+            MongoClientSettings defaultInternalMongoClientSettings = MongoClientSettings.builder(client.getSettings())
                     .applyToConnectionPoolSettings(builder -> builder.minSize(0))
                     .autoEncryptionSettings(null)
                     .build();
-            internalClient = MongoClients.create(mongoClientSettings);
+            sharedInternalClient = MongoClients.create(defaultInternalMongoClientSettings);
         }
         MongoClient keyVaultClient = keyVaultMongoClientSettings == null
-                ? internalClient : MongoClients.create(keyVaultMongoClientSettings);
+                ? sharedInternalClient : MongoClients.create(keyVaultMongoClientSettings);
         MongoCrypt mongoCrypt = MongoCrypts.create(createMongoCryptOptions(settings));
         return new Crypt(
                 mongoCrypt,
@@ -55,9 +55,9 @@ public static Crypt createCrypt(final MongoClientImpl client, final AutoEncrypti
                 settings.getKmsProviders(),
                 settings.getKmsProviderPropertySuppliers(),
                 settings.isBypassAutoEncryption(),
-                settings.isBypassAutoEncryption() ? null : new CollectionInfoRetriever(internalClient),
+                settings.isBypassAutoEncryption() ? null : new CollectionInfoRetriever(sharedInternalClient),
                 new CommandMarker(mongoCrypt, settings),
-                internalClient);
+                sharedInternalClient, keyVaultClient);
     }
 
     static Crypt create(final MongoClient keyVaultClient, final ClientEncryptionSettings settings) {