[SERVER-75651] Exhaustively identify internal namespaces that should not be modified by user commands Created: 04/Apr/23  Updated: 13/Dec/23

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

Type: Task Priority: Major - P3
Reporter: Dianna Hohensee (Inactive) Assignee: Backlog - Catalog and Routing
Resolution: Unresolved Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Related
related to SERVER-77003 Allow renames of time-series collecti... Closed
is related to SERVER-72090 system.users can be renamed to regula... Closed
is related to SERVER-82934 Disallow system.preimages collection ... Closed
Assigned Teams:
Catalog and Routing
Sprint: Execution NAMR Team 2023-06-26, CAR team targeted by 8.0
Participants:

 Description   

Periodically a new SERVER ticket pops up where fuzz testing has messed up internal namespace state via drop/rename/etc.

We should explore creating a new auth role/permission for internal namespaces – so that downstream products can still modify internal collections as necessary – and labeling internal namespaces such that they can be identified as such.

Perhaps we can add a tag/field to NamespaceString for internal collection identification. And then create a list of commands (maybe via a test) that are not allowed to modify internal collections.



 Comments   
Comment by Connie Chen [ 31/Jul/23 ]

We should ensure we include the Security team to consult on how this interacts with the authorization system

Comment by Dianna Hohensee (Inactive) [ 07/Jul/23 ]

Re-assigning to the backlog. The next steps here, agreed on in triage meeting, are basically to try implementing what I proposed.

Comment by Dianna Hohensee (Inactive) [ 26/Jun/23 ]

We need some mechanism to ensure engineers enumerate command prohibitions / access control whenever a new internal namespace is created. But at the same time, there's no need to pass that around with NamespaceString instances, so the system has to be alongside somehow. Maybe a lint rule could be created to pick up whenever engineers create a new namespace without specifying access control.

First code idea that came to mind:

enum class AccessControl {
    kDrop,
    kCreate,
    kRename,
    kWrite,
    .... // define enum per command requiring namespace access control
}

// isRestoreRepairAuthorized  is provided by the caller and represents AuthorizationSession::get(opCtx->getClient())->isAuthorizedForActionsOnResource(.....)
 
NamespaceString::accessAllowed(OperationContext* opCtx, NamespaceString::AccessControl operation, bool isRestoreRepairAuthorized) {
    // Check coll() is not disallowed.
 
    if (operation == NamespaceString::AccessControl::kDrop) {
        if (isRestoreRepairAuthorized) {
            // check namespaces always disallowed.
        } else {
            // check namespaces always disallowed and access controlled.
        }
    } else if (operation == NamespaceString::AccessControl::kCreate) {
        // ....
    } else if (operation == NamespaceString::AccessControl::kRename) {
        // ....
    } else ....
 
}

Comment by Dianna Hohensee (Inactive) [ 23/Jun/23 ]

After thinking about this some more, I think there are two categories of namespaces:

1) Namespaces to which we would like to limit access, essentially allow only repair/restore type operations to alter.

2) Namespaces to which we allow no access.

We're much stricter with drop/rename than regular writes to internal collections, internationally or not. Today we do a lot of checks via NamespaceString helpers, grouping similarly treated namespaces into helper functions and then checking for and taking action on those namespaces in various command paths.

Comment by Geert Bosch [ 25/Apr/23 ]

Thanks for your insights Max, I agree with your summary. I'd like the fuzzer to run with access control instead of a list of exceptions, as that seems less error-prone and actually allows us to catch cases where there are errors in access control.

Comment by Max Hirschhorn [ 25/Apr/23 ]

We should explore creating a new auth role/permission for internal namespaces

I had discussed this ticket a little with Andy after also seeing SERVER-76282 and SERVER-74679 go by jira-server-new. We're both in favor of pursuing solutions (if any change is to be made) which use the access control system.

It wasn't clear to me from reading this ticket if the mutational (jstestfuzz) fuzzer is the primary motivation or not. My personal view is that it must be possible for a human to manually remediate a collection with direct writes in both a replicated context (--replSet) and a non-replicated context (standalone mongod). Absolute restrictions unconditionally enforced in C++ code would otherwise mean an operator in a dire situation is limited in how they can recover or, sometimes more importantly, how quickly they can recover.

Periodically a new SERVER ticket pops up where fuzz testing has messed up internal namespace state via drop/rename/etc.

The mutational (jstestfuzz) fuzzer has a framework for prohibiting certain operations. Many internal namespaces are already part of the list. It is straightforward to enumerate and add others. We could also look into having the jstestfuzz* Evergreen tasks run with the access control system enabled if that would lower the maintenance burden on Server engineers as new internal namespaces are introduced.

Generated at Thu Feb 08 06:30:43 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.