SERVER-68122 Remove encryption from storageOptions on secondary replication

(cherry picked from commit ef120ac5552574fb9b84d36d842ead8519588c31)

15 files changed, 212 insertions, 5 deletions

diff --git a/src/mongo/db/repl/collection_cloner.cpp b/src/mongo/db/repl/collection_cloner.cpp
index 96e4f1c4540..8a395d09f72 100644
--- a/src/mongo/db/repl/collection_cloner.cpp
+++ b/src/mongo/db/repl/collection_cloner.cpp
@@ -29,6 +29,8 @@
 
 #define MONGO_LOGV2_DEFAULT_COMPONENT ::mongo::logv2::LogComponent::kReplicationInitialSync
 
+#include "mongo/db/index/index_descriptor_fwd.h"
+#include "mongo/db/service_context.h"
 #include "mongo/platform/basic.h"
 
 #include "mongo/base/string_data.h"
@@ -218,8 +220,21 @@ BaseCloner::AfterStageBehavior CollectionCloner::listIndexesStage() {
                       "source"_attr = getSource());
     }
 
+    const auto storageEngine = getGlobalServiceContext()->getStorageEngine();
     // Parse the index specs into their respective state, ready or unfinished.
     for (auto&& spec : indexSpecs) {
+        // Sanitize storage engine options to remove options which might not apply to this node. See
+        // SERVER-68122.
+        if (auto storageEngineElem = spec.getField(IndexDescriptor::kStorageEngineFieldName)) {
+            auto sanitizedStorageEngineOpts =
+                storageEngine->getSanitizedStorageOptionsForSecondaryReplication(
+                    storageEngineElem.embeddedObject());
+            fassert(6812200, sanitizedStorageEngineOpts);
+            spec = spec.addField(BSON(IndexDescriptor::kStorageEngineFieldName
+                                      << sanitizedStorageEngineOpts.getValue())
+                                     .firstElement());
+        }
+
         if (spec.hasField("buildUUID")) {
             invariant(includeBuildUUIDs);
             _unfinishedIndexSpecs.push_back(spec.getOwned());
diff --git a/src/mongo/db/repl/database_cloner.cpp b/src/mongo/db/repl/database_cloner.cpp
index ce5bb9053cc..88b3aead08e 100644
--- a/src/mongo/db/repl/database_cloner.cpp
+++ b/src/mongo/db/repl/database_cloner.cpp
@@ -29,6 +29,7 @@
 
 #define MONGO_LOGV2_DEFAULT_COMPONENT ::mongo::logv2::LogComponent::kReplicationInitialSync
 
+#include "mongo/db/service_context.h"
 #include "mongo/platform/basic.h"
 
 #include "mongo/base/string_data.h"
@@ -72,6 +73,7 @@ BaseCloner::AfterStageBehavior DatabaseCloner::listCollectionsStage() {
     BSONObj res;
     auto collectionInfos =
         getClient()->getCollectionInfos(_dbName, ListCollectionsFilter::makeTypeCollectionFilter());
+    const auto storageEngine = getGlobalServiceContext()->getStorageEngine();
 
     stdx::unordered_set<std::string> seen;
     for (auto&& info : collectionInfos) {
@@ -107,6 +109,13 @@ BaseCloner::AfterStageBehavior DatabaseCloner::listCollectionsStage() {
                               << "'" << result.getName() << "': " << info,
                 isDuplicate);
 
+        // Sanitize storage engine options to remove options which might not apply to this node. See
+        // SERVER-68122.
+        auto sanitizedStorageOptions =
+            uassertStatusOK(storageEngine->getSanitizedStorageOptionsForSecondaryReplication(
+                result.getOptions().storageEngine));
+        result.getOptions().storageEngine = sanitizedStorageOptions;
+
         // While UUID is a member of CollectionOptions, listCollections does not return the
         // collectionUUID there as part of the options, but instead places it in the 'info' field.
         // We need to move it back to CollectionOptions to create the collection properly.
diff --git a/src/mongo/db/repl/oplog.cpp b/src/mongo/db/repl/oplog.cpp
index 6cd1a8cc6b3..06001d4f7fc 100644
--- a/src/mongo/db/repl/oplog.cpp
+++ b/src/mongo/db/repl/oplog.cpp
@@ -59,6 +59,7 @@
 #include "mongo/db/catalog/rename_collection.h"
 #include "mongo/db/client.h"
 #include "mongo/db/commands.h"
+#include "mongo/db/commands/create_gen.h"
 #include "mongo/db/commands/feature_compatibility_version_parser.h"
 #include "mongo/db/concurrency/write_conflict_exception.h"
 #include "mongo/db/db_raii.h"
@@ -725,6 +726,23 @@ NamespaceString extractNsFromUUIDorNs(OperationContext* opCtx,
     return ui ? extractNsFromUUID(opCtx, ui.get()) : extractNs(ns, cmd);
 }
 
+StatusWith<BSONObj> getObjWithSanitizedStorageEngineOptions(OperationContext* opCtx,
+                                                            const BSONObj& cmd) {
+    if (auto storageEngineElem = cmd[IndexDescriptor::kStorageEngineFieldName]) {
+        auto storageEngine = opCtx->getServiceContext()->getStorageEngine();
+        auto engineObj = storageEngineElem.embeddedObject();
+        auto sanitizedObj =
+            storageEngine->getSanitizedStorageOptionsForSecondaryReplication(engineObj);
+        if (!sanitizedObj.isOK()) {
+            return sanitizedObj.getStatus();
+        }
+        return cmd.addField(
+            BSON(IndexDescriptor::kStorageEngineFieldName << sanitizedObj.getValue())
+                .firstElement());
+    }
+    return cmd;
+}
+
 using OpApplyFn = std::function<Status(
     OperationContext* opCtx, const OplogEntry& entry, OplogApplication::Mode mode)>;
 
@@ -746,7 +764,14 @@ const StringMap<ApplyOpMetadata> kOpsMap = {
     {"create",
      {[](OperationContext* opCtx, const OplogEntry& entry, OplogApplication::Mode mode) -> Status {
           const auto& ui = entry.getUuid();
-          const auto& cmd = entry.getObject();
+          // Sanitize storage engine options to remove options which might not apply to this node.
+          // See SERVER-68122.
+          const auto sanitizedCmdOrStatus =
+              getObjWithSanitizedStorageEngineOptions(opCtx, entry.getObject());
+          if (!sanitizedCmdOrStatus.isOK()) {
+              return sanitizedCmdOrStatus.getStatus();
+          }
+          const auto& cmd = sanitizedCmdOrStatus.getValue();
           const NamespaceString nss(extractNs(entry.getNss(), cmd));
 
           // Mode SECONDARY steady state replication should not allow create collection to rename an
@@ -784,7 +809,15 @@ const StringMap<ApplyOpMetadata> kOpsMap = {
       {ErrorCodes::NamespaceExists}}},
     {"createIndexes",
      {[](OperationContext* opCtx, const OplogEntry& entry, OplogApplication::Mode mode) -> Status {
-          const auto& cmd = entry.getObject();
+          // Sanitize storage engine options to remove options which might not apply to this node.
+          // See SERVER-68122.
+          const auto sanitizedCmdOrStatus =
+              getObjWithSanitizedStorageEngineOptions(opCtx, entry.getObject());
+          if (!sanitizedCmdOrStatus.isOK()) {
+              return sanitizedCmdOrStatus.getStatus();
+          }
+          const auto& cmd = sanitizedCmdOrStatus.getValue();
+
           if (OplogApplication::Mode::kApplyOpsCmd == mode &&
               IndexBuildsCoordinator::supportsTwoPhaseIndexBuild()) {
               return {ErrorCodes::CommandNotSupported,
@@ -824,6 +857,16 @@ const StringMap<ApplyOpMetadata> kOpsMap = {
                   "Error parsing 'startIndexBuild' oplog entry");
           }
 
+          // Sanitize storage engine options to remove options which might not apply to this node.
+          // See SERVER-68122.
+          for (auto& spec : swOplogEntry.getValue().indexSpecs) {
+              auto sanitizedObj = getObjWithSanitizedStorageEngineOptions(opCtx, spec);
+              if (!sanitizedObj.isOK()) {
+                  return swOplogEntry.getStatus();
+              }
+              spec = sanitizedObj.getValue();
+          }
+
           IndexBuildsCoordinator::ApplicationMode applicationMode =
               IndexBuildsCoordinator::ApplicationMode::kNormal;
           if (mode == OplogApplication::Mode::kInitialSync) {
diff --git a/src/mongo/db/storage/kv/kv_engine.h b/src/mongo/db/storage/kv/kv_engine.h
index 88af5f02fe7..fb91ca8b95e 100644
--- a/src/mongo/db/storage/kv/kv_engine.h
+++ b/src/mongo/db/storage/kv/kv_engine.h
@@ -473,6 +473,16 @@ public:
     virtual void setPinnedOplogTimestamp(const Timestamp& pinnedTimestamp) = 0;
 
     /**
+     * Returns the input storage engine options, sanitized to remove options that may not apply to
+     * this node, such as encryption. Might be called for both collection and index options. See
+     * SERVER-68122.
+     */
+    virtual StatusWith<BSONObj> getSanitizedStorageOptionsForSecondaryReplication(
+        const BSONObj& options) const {
+        return options;
+    }
+
+    /**
      * The destructor will never be called from mongod, but may be called from tests.
      * Engines may assume that this will only be called in the case of clean shutdown, even if
      * cleanShutdown() hasn't been called.
diff --git a/src/mongo/db/storage/storage_engine.h b/src/mongo/db/storage/storage_engine.h
index dc9db28e145..1a5514c100b 100644
--- a/src/mongo/db/storage/storage_engine.h
+++ b/src/mongo/db/storage/storage_engine.h
@@ -641,6 +641,14 @@ public:
      * it.
      */
     virtual void setPinnedOplogTimestamp(const Timestamp& pinnedTimestamp) = 0;
+
+    /**
+     * Returns the input storage engine options, sanitized to remove options that may not apply to
+     * this node, such as encryption. Might be called for both collection and index options. See
+     * SERVER-68122.
+     */
+    virtual StatusWith<BSONObj> getSanitizedStorageOptionsForSecondaryReplication(
+        const BSONObj& options) const = 0;
 };
 
 }  // namespace mongo
diff --git a/src/mongo/db/storage/storage_engine_impl.cpp b/src/mongo/db/storage/storage_engine_impl.cpp
index 59e4bf3cfc9..056c52967cb 100644
--- a/src/mongo/db/storage/storage_engine_impl.cpp
+++ b/src/mongo/db/storage/storage_engine_impl.cpp
@@ -1132,4 +1132,8 @@ void StorageEngineImpl::setPinnedOplogTimestamp(const Timestamp& pinnedTimestamp
     _engine->setPinnedOplogTimestamp(pinnedTimestamp);
 }
 
+StatusWith<BSONObj> StorageEngineImpl::getSanitizedStorageOptionsForSecondaryReplication(
+    const BSONObj& options) const {
+    return _engine->getSanitizedStorageOptionsForSecondaryReplication(options);
+}
 }  // namespace mongo
diff --git a/src/mongo/db/storage/storage_engine_impl.h b/src/mongo/db/storage/storage_engine_impl.h
index b0fc3df31d2..7608b708e8a 100644
--- a/src/mongo/db/storage/storage_engine_impl.h
+++ b/src/mongo/db/storage/storage_engine_impl.h
@@ -361,6 +361,9 @@ public:
 
     void setPinnedOplogTimestamp(const Timestamp& pinnedTimestamp) override;
 
+    StatusWith<BSONObj> getSanitizedStorageOptionsForSecondaryReplication(
+        const BSONObj& options) const override;
+
 private:
     using CollIter = std::list<std::string>::iterator;
 
diff --git a/src/mongo/db/storage/storage_engine_mock.h b/src/mongo/db/storage/storage_engine_mock.h
index 86a450371ce..b52f8e7c967 100644
--- a/src/mongo/db/storage/storage_engine_mock.h
+++ b/src/mongo/db/storage/storage_engine_mock.h
@@ -193,6 +193,11 @@ public:
     }
 
     void setPinnedOplogTimestamp(const Timestamp& pinnedTimestamp) final {}
+
+    StatusWith<BSONObj> getSanitizedStorageOptionsForSecondaryReplication(
+        const BSONObj& options) const final {
+        return options;
+    }
 };
 
 }  // namespace mongo
diff --git a/src/mongo/db/storage/wiredtiger/wiredtiger_index.cpp b/src/mongo/db/storage/wiredtiger/wiredtiger_index.cpp
index f81f2823df8..04200ac805b 100644
--- a/src/mongo/db/storage/wiredtiger/wiredtiger_index.cpp
+++ b/src/mongo/db/storage/wiredtiger/wiredtiger_index.cpp
@@ -122,7 +122,7 @@ void WiredTigerIndex::getKey(WT_CURSOR* cursor, WT_ITEM* key) {
 StatusWith<std::string> WiredTigerIndex::parseIndexOptions(const BSONObj& options) {
     StringBuilder ss;
     BSONForEach(elem, options) {
-        if (elem.fieldNameStringData() == "configString") {
+        if (elem.fieldNameStringData() == WiredTigerUtil::kConfigStringField) {
             Status status = WiredTigerUtil::checkTableCreationOptions(elem);
             if (!status.isOK()) {
                 return status;
diff --git a/src/mongo/db/storage/wiredtiger/wiredtiger_kv_engine.cpp b/src/mongo/db/storage/wiredtiger/wiredtiger_kv_engine.cpp
index 9c62481e373..6c4cf625f66 100644
--- a/src/mongo/db/storage/wiredtiger/wiredtiger_kv_engine.cpp
+++ b/src/mongo/db/storage/wiredtiger/wiredtiger_kv_engine.cpp
@@ -2348,4 +2348,31 @@ std::uint64_t WiredTigerKVEngine::_getCheckpointTimestamp() const {
     return tmp;
 }
 
+StatusWith<BSONObj> WiredTigerKVEngine::getSanitizedStorageOptionsForSecondaryReplication(
+    const BSONObj& options) const {
+
+    if (options.isEmpty()) {
+        return options;
+    }
+
+    auto firstElem = options.firstElement();
+    if (firstElem.fieldName() != kWiredTigerEngineName) {
+        return Status(ErrorCodes::InvalidOptions,
+                      str::stream() << "Expected \"" << kWiredTigerEngineName
+                                    << "\" field, but got: " << firstElem.fieldName());
+    }
+
+    BSONObj wtObj = firstElem.Obj();
+    if (auto configStringElem = wtObj.getField(WiredTigerUtil::kConfigStringField)) {
+        auto configString = configStringElem.String();
+        WiredTigerUtil::removeEncryptionFromConfigString(&configString);
+        // Return a new BSONObj with the configString field sanitized.
+        return options.addField(
+            BSON(kWiredTigerEngineName << wtObj.addField(
+                     BSON(WiredTigerUtil::kConfigStringField << configString).firstElement()))
+                .firstElement());
+    }
+
+    return options;
+}
 }  // namespace mongo
diff --git a/src/mongo/db/storage/wiredtiger/wiredtiger_kv_engine.h b/src/mongo/db/storage/wiredtiger/wiredtiger_kv_engine.h
index 7c5b95f68dd..e48db10d5ac 100644
--- a/src/mongo/db/storage/wiredtiger/wiredtiger_kv_engine.h
+++ b/src/mongo/db/storage/wiredtiger/wiredtiger_kv_engine.h
@@ -348,6 +348,9 @@ public:
 
     void setPinnedOplogTimestamp(const Timestamp& pinnedTimestamp) override;
 
+    StatusWith<BSONObj> getSanitizedStorageOptionsForSecondaryReplication(
+        const BSONObj& options) const override;
+
 private:
     class WiredTigerSessionSweeper;
     class WiredTigerCheckpointThread;
diff --git a/src/mongo/db/storage/wiredtiger/wiredtiger_record_store.cpp b/src/mongo/db/storage/wiredtiger/wiredtiger_record_store.cpp
index 30b451c81ec..48c1dea7283 100644
--- a/src/mongo/db/storage/wiredtiger/wiredtiger_record_store.cpp
+++ b/src/mongo/db/storage/wiredtiger/wiredtiger_record_store.cpp
@@ -645,7 +645,7 @@ void WiredTigerRecordStore::OplogStones::adjust(int64_t maxSize) {
 StatusWith<std::string> WiredTigerRecordStore::parseOptionsField(const BSONObj options) {
     StringBuilder ss;
     BSONForEach(elem, options) {
-        if (elem.fieldNameStringData() == "configString") {
+        if (elem.fieldNameStringData() == WiredTigerUtil::kConfigStringField) {
             Status status = WiredTigerUtil::checkTableCreationOptions(elem);
             if (!status.isOK()) {
                 return status;
diff --git a/src/mongo/db/storage/wiredtiger/wiredtiger_util.cpp b/src/mongo/db/storage/wiredtiger/wiredtiger_util.cpp
index 9b954ebf4ea..ee87aca4723 100644
--- a/src/mongo/db/storage/wiredtiger/wiredtiger_util.cpp
+++ b/src/mongo/db/storage/wiredtiger/wiredtiger_util.cpp
@@ -37,6 +37,7 @@
 
 #include <boost/filesystem.hpp>
 #include <boost/filesystem/path.hpp>
+#include <pcrecpp.h>
 
 #include "mongo/base/simple_string_data_comparator.h"
 #include "mongo/bson/bsonobjbuilder.h"
@@ -51,6 +52,7 @@
 #include "mongo/util/assert_util.h"
 #include "mongo/util/processinfo.h"
 #include "mongo/util/scopeguard.h"
+#include "mongo/util/static_immortal.h"
 #include "mongo/util/str.h"
 
 namespace mongo {
@@ -336,7 +338,7 @@ StatusWith<int64_t> WiredTigerUtil::checkApplicationMetadataFormatVersion(Operat
 
 // static
 Status WiredTigerUtil::checkTableCreationOptions(const BSONElement& configElem) {
-    invariant(configElem.fieldNameStringData() == "configString");
+    invariant(configElem.fieldNameStringData() == WiredTigerUtil::kConfigStringField);
 
     if (configElem.type() != String) {
         return {ErrorCodes::TypeMismatch, "'configString' must be a string."};
@@ -795,4 +797,9 @@ void WiredTigerUtil::appendSnapshotWindowSettings(WiredTigerKVEngine* engine,
                     oldestTimestamp.toStringPretty());
 }
 
+void WiredTigerUtil::removeEncryptionFromConfigString(std::string* configString) {
+    static const StaticImmortal<pcrecpp::RE> encryptionOptsRegex(R"re(encryption=\([^\)]*\),?)re");
+    encryptionOptsRegex->GlobalReplace("", configString);
+}
+
 }  // namespace mongo
diff --git a/src/mongo/db/storage/wiredtiger/wiredtiger_util.h b/src/mongo/db/storage/wiredtiger/wiredtiger_util.h
index c6b5caf1068..cb20397eb5a 100644
--- a/src/mongo/db/storage/wiredtiger/wiredtiger_util.h
+++ b/src/mongo/db/storage/wiredtiger/wiredtiger_util.h
@@ -129,6 +129,8 @@ private:
     WiredTigerUtil();
 
 public:
+    static constexpr StringData kConfigStringField = "configString"_sd;
+
     /**
      * Fetch the type and source fields out of the colgroup metadata.  'tableUri' must be a
      * valid table: uri.
@@ -271,6 +273,14 @@ public:
     template <typename T>
     static T castStatisticsValue(uint64_t statisticsValue);
 
+    /**
+     * Removes encryption configuration from a config string. Should only be applied on custom
+     * config strings on secondaries. Fixes an issue where encryption configuration might be
+     * replicated to non-encrypted nodes, or nodes with different encryption options, causing
+     * initial sync or replication to fail. See SERVER-68122.
+     */
+    static void removeEncryptionFromConfigString(std::string* configString);
+
 private:
     /**
      * Casts unsigned 64-bit statistics value to T.
diff --git a/src/mongo/db/storage/wiredtiger/wiredtiger_util_test.cpp b/src/mongo/db/storage/wiredtiger/wiredtiger_util_test.cpp
index 81f1726267c..1d5aa1cf162 100644
--- a/src/mongo/db/storage/wiredtiger/wiredtiger_util_test.cpp
+++ b/src/mongo/db/storage/wiredtiger/wiredtiger_util_test.cpp
@@ -349,4 +349,67 @@ TEST(WiredTigerUtilTest, GetStatisticsValueValidKey) {
     ASSERT_EQUALS(0U, result.getValue());
 }
 
+TEST(WiredTigerUtilTest, RemoveEncryptionFromConfigString) {
+    {  // Found at the middle.
+        std::string input{
+            "debug_mode=(table_logging=true,checkpoint_retention=4),encryption=(name=AES256-CBC,"
+            "keyid="
+            "\".system\"),extensions=[local={entry=mongo_addWiredTigerEncryptors,early_load=true},,"
+            "],"};
+        const std::string expectedOutput{
+            "debug_mode=(table_logging=true,checkpoint_retention=4),extensions=[local={entry=mongo_"
+            "addWiredTigerEncryptors,early_load=true},,],"};
+        WiredTigerUtil::removeEncryptionFromConfigString(&input);
+        ASSERT_EQUALS(input, expectedOutput);
+    }
+    {  // Found at start.
+        std::string input{
+            "encryption=(name=AES256-CBC,keyid=\".system\"),extensions=[local={entry=mongo_"
+            "addWiredTigerEncryptors,early_load=true},,],"};
+        const std::string expectedOutput{
+            "extensions=[local={entry=mongo_addWiredTigerEncryptors,early_load=true},,],"};
+        WiredTigerUtil::removeEncryptionFromConfigString(&input);
+        ASSERT_EQUALS(input, expectedOutput);
+    }
+    {  // Found at the end.
+        std::string input{
+            "debug_mode=(table_logging=true,checkpoint_retention=4),encryption=(name=AES256-CBC,"
+            "keyid=\".system\")"};
+        const std::string expectedOutput{"debug_mode=(table_logging=true,checkpoint_retention=4),"};
+        WiredTigerUtil::removeEncryptionFromConfigString(&input);
+        ASSERT_EQUALS(input, expectedOutput);
+    }
+    {  // Matches full configString.
+        std::string input{"encryption=(name=AES256-CBC,keyid=\".system\")"};
+        const std::string expectedOutput{""};
+        WiredTigerUtil::removeEncryptionFromConfigString(&input);
+        ASSERT_EQUALS(input, expectedOutput);
+    }
+    {  // Matches full configString, trailing comma.
+        std::string input{"encryption=(name=AES256-CBC,keyid=\".system\"),"};
+        const std::string expectedOutput{""};
+        WiredTigerUtil::removeEncryptionFromConfigString(&input);
+        ASSERT_EQUALS(input, expectedOutput);
+    }
+    {  // No match.
+        std::string input{"debug_mode=(table_logging=true,checkpoint_retention=4)"};
+        const std::string expectedOutput{"debug_mode=(table_logging=true,checkpoint_retention=4)"};
+        WiredTigerUtil::removeEncryptionFromConfigString(&input);
+        ASSERT_EQUALS(input, expectedOutput);
+    }
+    {  // No match, empty.
+        std::string input{""};
+        const std::string expectedOutput{""};
+        WiredTigerUtil::removeEncryptionFromConfigString(&input);
+        ASSERT_EQUALS(input, expectedOutput);
+    }
+    {  // Removes multiple instances.
+        std::string input{
+            "encryption=(name=AES256-CBC,keyid=\".system\"),debug_mode=(table_logging=true,"
+            "checkpoint_retention=4),encryption=(name=AES256-CBC,keyid=\".system\")"};
+        const std::string expectedOutput{"debug_mode=(table_logging=true,checkpoint_retention=4),"};
+        WiredTigerUtil::removeEncryptionFromConfigString(&input);
+        ASSERT_EQUALS(input, expectedOutput);
+    }
+}
 }  // namespace mongo