[SERVER-48513] User collections can be renamed to a valid system collection, leading to server crash. Created: 01/Jun/20  Updated: 06/Dec/22  Resolved: 04/Jun/20

Status: Closed
Project: Core Server
Component/s: Catalog
Affects Version/s: None
Fix Version/s: None

Type: Bug Priority: Major - P3
Reporter: Suganthi Mani Assignee: Backlog - Storage Execution Team
Resolution: Duplicate Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Depends
Duplicate
duplicates SERVER-5955 Forbid renaming system collections Closed
is duplicated by SERVER-5955 Forbid renaming system collections Closed
Assigned Teams:
Storage Execution
Operating System: ALL
Steps To Reproduce:

load('jstests/libs/parallelTester.js');
load("jstests/replsets/rslib.js");
 
(function() {
 
"use strict";
 
const dbName = jsTest.name();
const collName = "coll";
 
var rst = new ReplSetTest({nodes: [{}, {rsConfig: {priority: 1}}]});
rst.startSet();
rst.initiate();
 
const primary = rst.getPrimary();
const primaryDB = primary.getDB(dbName);
const primaryAdmin = primary.getDB("admin");
const primaryColl = primaryDB[collName];
const collNss = primaryColl.getFullName();
const secondary = rst.getSecondary();
 
TestData.dbName = dbName;
TestData.collName = collName;
 
jsTestLog("Do a document write");
assert.commandWorked(
        primaryColl.insert({_id: 0}, {"writeConcern": {"w": "majority"}}));
 
jsTestLog("Insert duplicate records to admin.users");
assert.commandWorked(
        primaryAdmin.users.insert({user: 'backdoor', db: 'admin', pwd: 'hashed', roles: '--port'}, {"writeConcern": {"w": "majority"}}));
 
assert.commandWorked(
        primaryAdmin.users.insert({user: 'backdoor', db: 'admin', pwd: 'hashed', roles: '--port'}, {"writeConcern": {"w": "majority"}}));
 
print("Listing admin.users doc  " + tojson(primaryAdmin.users.find().toArray()));
 
// This step makes admin.system.users collection to have a records that would violate if the indexes 'user_1_db_1' gets built.
jsTestLog("Rename collection from admin.users to admin.systemusers");
assert.commandWorked(primaryAdmin.adminCommand({renameCollection: "admin.users" , to: "admin.system.users", dropTarget: true}));
 
print("Listing admin.system.users doc  " + tojson(primaryAdmin.system.users.find().toArray()));
rst.awaitReplication();
 
jsTestLog("Make primary step down");
assert.commandWorked(primary.adminCommand({"replSetStepDown": 1000, "force": true}));
 
jsTestLog("Make secondary step up");
assert.commandWorked(secondary.adminCommand({"replSetStepUp": 1}));
 
// Now, the new primary would try to build index 'user_1_db_1' for  admin.system.users collection as part of onTransitionToPrimary hook, leading to fassert failure due to duplicate key error.
// Wait for the secondary to become master.
rst.getPrimary();
 
rst.stopSet();
})();

Participants:
Linked BF Score: 0

 Description   

Noticed a build failure whose root cause is that we allow user collections to get renamed to a valid system collections if the target system collection is not present. As a result, that system collection can have docs that would violate the index key constraints when it's indexes gets built in future, leading to fassert failure. But, on the other hand, if the target system collection is present, then we would fail the rename either due to
1) 'NamespaceExists' error if 'droptarget' param of rename cmd is set as false.
2)'can't drop system collection <system collection name>' error if droptarget param of rename cmd is set true.



 Comments   
Comment by Eric Milkie [ 01/Jun/20 ]

SERVER-5955 is an existing ticket for this problem. I don't think option 1 will help because you can still rename collections out of system (which is effectively dropping them).

Comment by Suganthi Mani [ 01/Jun/20 ]

I am thinking of 2 solutions for this problem.
1) When the node steps up and before it transitions to master, possibly in onTransitionToPrimary hook, we should create empty system collections if the system collections are not present. As a result, if the rename cmd's target is system collection, it would fail either because of 'NamespaceExists' or 'can't drop system collection' error.
2) Rename cmd should be failed if the target collection namespace is a valid system collection namespace.

I also see, we can rename a system collection to a user collection. I think, even that's not safe. So, we should extend solution#2 "Fail rename cmd if either source or target namespace is a valid system collection namespace".

CC milkie

Generated at Thu Feb 08 05:17:20 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.