SERVER-116327 & SERVER-118546: Correctly validate internal index types (#48204)

Co-authored-by: Benjamin Pearce <benjamin.pearce@mongodb.com>
GitOrigin-RevId: 2ae32bc103866f64de1cd10f3fa3c95e0f8c7063

etc/backports_required_for_multiversion_tests.yml

@@ -498,6 +498,8 @@ last-continuous:
       ticket: SERVER-113887
     - test_file: jstests/sharding/disable_resumable_range_deleter.js
       ticket: SERVER-112357
+    - test_file: jstests/core/index/index_on_incorrect_collection.js
+      ticket: SERVER-113888
     - test_file: jstests/core_sharding/ddl/cannot_track_temporary_collection.js
       ticket: SERVER-114666
     - test_file: jstests/core/query/regex/regex_max_pattern_length.js
@@ -1075,6 +1077,8 @@ last-lts:
       ticket: SERVER-113887
     - test_file: jstests/sharding/disable_resumable_range_deleter.js
       ticket: SERVER-112357
+    - test_file: jstests/core/index/index_on_incorrect_collection.js
+      ticket: SERVER-113888
     - test_file: jstests/core_sharding/ddl/cannot_track_temporary_collection.js
       ticket: SERVER-114666
     - test_file: jstests/core/query/regex/regex_max_pattern_length.js

jstests/core/index/index_on_incorrect_collection.js

@@ -0,0 +1,53 @@
+/**
+ * If an incompatible index exists on a collection, the server should prevent updates to that index
+ * with non-fatal errors.
+ */
+
+import {after, afterEach, before, beforeEach, describe, it} from "jstests/libs/mochalite.js";
+
+describe(
+    "Nonfatal error when attempting to update an improper timeseries-only index on a non-timeseries collection.",
+    function() {
+        const collName = jsTestName();
+        before(function() {
+            this.coll = db.getCollection(collName);
+            this.coll.drop();
+        });
+
+        beforeEach(function() {
+            assert.commandWorked(db.createCollection(collName));
+            this.coll = db.getCollection(collName);
+        });
+
+        it("Prevents updating 2dsphere_bucket indices for top-level measurements", function() {
+            const indexSpec = {x: "2dsphere_bucket"};
+            // TODO SERVER-118911 index creation should not be possible
+            assert.commandWorked(this.coll.createIndex(indexSpec));
+            const sampleDoc = {control: {version: 2}, x: HexData(0, "00")};
+            assert.commandFailed(this.coll.insert(sampleDoc));
+            // Ensure the index can be dropped, allowing the sampleDoc to be inserted
+            const res = assert.commandWorked(this.coll.dropIndex(indexSpec));
+            assert.commandWorked(this.coll.insert(sampleDoc));
+        });
+
+        it("Prevents updating 2dsphere_bucket indices for nested measurements", function() {
+            const indexSpec = {"data.a.b.c": "2dsphere_bucket"};
+            // TODO SERVER-118911 index creation should not be possible
+            assert.commandWorked(this.coll.createIndex(indexSpec));
+            const sampleDoc = {control: {version: 2}, data: {a: {b: {c: [0, 0]}}}};
+            assert.commandFailed(this.coll.insert(sampleDoc));
+            // Ensure the index can be dropped, allowing the sampleDoc to be inserted
+            const res = assert.commandWorked(this.coll.dropIndex(indexSpec));
+            assert.commandWorked(this.coll.insert(sampleDoc));
+        });
+
+        it("Cannot build a 2dsphere_bucket index on a populated collection", function() {
+            assert.commandWorked(this.coll.insert({abc: "xyz"}));
+            assert.commandFailed(this.coll.createIndex({x: "2dsphere_bucket"}));
+            assert.commandFailed(this.coll.createIndex({"data.a.b.c": "2dsphere_bucket"}));
+        });
+
+        afterEach(function() {
+            this.coll.drop();
+        });
+    });

jstests/noPassthrough/index_builds/index_create_illegal_options.js

@@ -0,0 +1,85 @@
+/**
+ * Ensures that a createIndexes command request fails when creating an index with illegal options.
+ */
+
+import {after, afterEach, before, describe, it} from "jstests/libs/mochalite.js";
+
+const withOrWithoutAuth = [true, false];
+
+withOrWithoutAuth.forEach((withAuth) => {
+    describe(`Specifying index type in the createIndex command with auth=${withAuth}`, function() {
+        const collName = jsTestName();
+        const testUser = "mongo";
+        const testPass = "mongo";
+
+        before(function() {
+            let options = {};
+            if (withAuth) {
+                options = {auth: "", bind_ip: "127.0.0.1"};
+            }
+            this.conn = MongoRunner.runMongod(options);
+            this.admin = this.conn.getDB("admin");
+            if (withAuth) {
+                this.admin.createUser(
+                    {user: testUser, pwd: testPass, roles: jsTest.adminUserRoles});
+                this.admin.logout();
+                this.admin.auth({user: testUser, pwd: testPass});
+            }
+            this.db = this.admin.getSiblingDB("test");
+        });
+
+        const legalIndexTypes = [1, "2d", "2dsphere", "text", "hashed"];
+        const legalIndexTypesForTimeseries = [1, "2dsphere"];
+        const illegalIndexTypes = [
+            {type: "queryable_encrypted_range", codes: [ErrorCodes.CannotCreateIndex]},
+            {type: "wildcard", codes: [7246202]},
+            {type: "columnstore", codes: [ErrorCodes.NotImplemented]},
+            {type: "geoHaystack", codes: [ErrorCodes.CannotCreateIndex]},
+        ];
+
+        legalIndexTypes.forEach(function(indexType) {
+            it(`Can create a '${indexType == 1 ? "btree" : indexType}' index auth=${withAuth}`,
+               function() {
+                   this.testCollName = collName + "." + indexType;
+                   assert.commandWorked(this.db[this.testCollName].createIndex({"foo": indexType}));
+               });
+        });
+
+        legalIndexTypesForTimeseries.forEach(function(indexType) {
+            it(`Can create a '${
+                   indexType == 1 ? "btree"
+                                  : indexType}' index on a timeseries collection auth=${withAuth}`,
+               function() {
+                   this.testCollName = collName + "." + indexType;
+                   assert.commandWorked(this.db.runCommand(
+                       {create: this.testCollName, timeseries: {timeField: "t"}}));
+                   assert.commandWorked(this.db[this.testCollName].createIndex({"foo": indexType}));
+               });
+        });
+
+        illegalIndexTypes.forEach((args) => {
+            const indexType = args.type;
+            const expectedErrorCodes = args.codes;
+            it(`Cannot create a '${indexType}' index auth=${withAuth}`, function() {
+                this.testCollName = collName + "." + indexType;
+                assert.commandFailedWithCode(
+                    this.db[this.testCollName].createIndex({"foo": indexType}),
+                    expectedErrorCodes,
+                );
+                assert.doesNotContain(
+                    this.db.getCollectionNames(),
+                    [this.testCollName],
+                    `The ${this.testCollName} collection should not be implicitly created upon failing to create the index.`,
+                );
+            });
+        });
+
+        afterEach(function() {
+            this.db[this.testCollName].drop();
+        });
+
+        after(function() {
+            MongoRunner.stopMongod(this.conn);
+        });
+    });
+});

src/mongo/db/catalog/index_key_validate.cpp

@@ -214,6 +214,9 @@ Status validateKeyPattern(const BSONObj& key, IndexDescriptor::IndexVersion inde
             return {
                 ErrorCodes::CannotCreateIndex,
                 str::stream() << "GeoHaystack indexes cannot be created in version 5.0 and above"};
+        if (pluginName == IndexNames::ENCRYPTED_RANGE)
+            return {ErrorCodes::CannotCreateIndex,
+                    str::stream() << "EncryptedRange indexes cannot be created"};
         if (!IndexNames::isKnownName(pluginName))
             return Status(code, str::stream() << "Unknown index plugin '" << pluginName << '\'');
     }

src/mongo/db/commands/create_indexes_cmd.cpp

@@ -203,6 +203,20 @@ void validateTTLOptions(OperationContext* opCtx,
     }
 }
 
+/**
+ * Ensures that the user is authorized to create an index of a given type.
+ */
+void validateIndexType(OperationContext* opCtx, const CreateIndexesCommand& cmd) {
+    for (const auto& elem : cmd.getIndexes()) {
+        for (const auto& key : elem.getField("key").Obj()) {
+            const auto type = key.str();  // will return "" for btree
+            uassert(ErrorCodes::CannotCreateIndex,
+                    fmt::format("Index Type {} is for internal use only", type),
+                    !IndexNames::isVirtualIndexType(type));
+        }
+    }
+}
+
 void checkEncryptedFieldIndexRestrictions(OperationContext* opCtx,
                                           const Collection* coll,
                                           const CreateIndexesCommand& cmd) {
@@ -561,6 +575,7 @@ CreateIndexesReply runCreateIndexesWithCoordinator(OperationContext* opCtx,
             }
 
             validateTTLOptions(opCtx, collection.getCollectionPtr().get(), cmd);
+            validateIndexType(opCtx, cmd);
 
             if (collection.exists() &&
                 !UncommittedCatalogUpdates::get(opCtx).isCreatedCollection(opCtx, ns)) {

src/mongo/db/index/index_access_method.cpp

@@ -130,8 +130,12 @@ std::unique_ptr<IndexAccessMethod> IndexAccessMethod::make(
         return std::make_unique<TwoDAccessMethod>(entry, makeSDI());
     else if (IndexNames::WILDCARD == type)
         return std::make_unique<WildcardAccessMethod>(entry, makeSDI());
-    LOGV2(20688, "Can't find index for keyPattern", "keyPattern"_attr = desc->keyPattern());
-    fassertFailed(31021);
+    LOGV2_ERROR_OPTIONS(20688,
+                        {logv2::UserAssertAfterLog(ErrorCodes::IndexOptionsConflict)},
+                        "Can't find index for keyPattern",
+                        "keyPattern"_attr = desc->keyPattern(),
+                        "type"_attr = type);
+    MONGO_UNREACHABLE;
 }
 
 namespace {

src/mongo/db/index_names.cpp

@@ -51,6 +51,7 @@ const string IndexNames::BTREE = "";
 const string IndexNames::WILDCARD = "wildcard";
 // We no longer support column store indexes. We use this value to reject creating them.
 const string IndexNames::COLUMN = "columnstore";
+// Encrypted range indexes are "pseudo-indexes", and as such, they cannot be created.
 const string IndexNames::ENCRYPTED_RANGE = "queryable_encrypted_range";
 // We no longer support geo haystack indexes. We use this value to reject creating them.
 const string IndexNames::GEO_HAYSTACK = "geoHaystack";
@@ -106,4 +107,17 @@ IndexType IndexNames::nameToType(StringData accessMethod) {
     return typeIt->second;
 }
 
+// static
+bool IndexNames::isVirtualIndexType(const std::string& name) {
+    if (isKnownName(name)) {
+        switch (nameToType(name)) {
+            case INDEX_ENCRYPTED_RANGE:
+                return true;
+            default:
+                return false;
+        }
+    }
+    return false;
+}
+
 }  // namespace mongo

src/mongo/db/index_names.h

@@ -86,6 +86,11 @@ public:
      * Convert an index name to an IndexType.
      */
     static IndexType nameToType(StringData accessMethod);
+
+    /**
+     * Index is not intended to be user facing.
+     */
+    static bool isVirtualIndexType(const std::string& name);
 };
 
 /**

src/mongo/db/timeseries/timeseries_dotted_path_support.cpp

@@ -174,7 +174,7 @@ boost::optional<BSONColumn> _extractAllElementsAlongBucketPath(
         case 0:
         case 1: {
             if (auto res = _splitPath(path)) {
-                auto& [left, next] = *res;
+                const auto& [left, next] = *res;
                 BSONElement e = obj.getField(left);
                 if (depth > 0 || left == timeseries::kBucketDataFieldName) {
                     if (e.type() == BSONType::object) {
@@ -229,7 +229,9 @@ boost::optional<BSONColumn> _extractAllElementsAlongBucketPath(
                     // measurement field (i.e. data.a) and we need to iterate over each of the
                     // numerically-indexed entries (i.e. data.a.1, data.a.5, etc.) to extract
                     // the actual field we want.
-                    invariant(depth == 1);
+                    massert(11388801,
+                            "Malformed measurement field in compressed timeseries bucket",
+                            depth == 1);
                     BSONColumn storage{e};
                     for (const BSONElement& e2 : storage) {
                         if (!e2.eoo()) {
@@ -251,7 +253,7 @@ boost::optional<BSONColumn> _extractAllElementsAlongBucketPath(
             // numerically-indexed entries (i.e. data.a.1, data.a.5, etc.) to extract the actual
             // field we want. If we are after a top-level field, then we already have the element we
             // want in 'e'. If we are after a nested field, then we need to recurse.
-            invariant(!isCompressed);
+            massert(11388802, "Expected uncompressed bucket", !isCompressed);
             for (const BSONElement& e : obj) {
                 if (path.empty()) {
                     // The top-level measurement field (i.e. data.a) is the indexed field we are