[SERVER-64918] Add a log entry if OpMsgRequest.body is bigger than BSONObjMaxInternalSize Created: 24/Mar/22  Updated: 29/Oct/23  Resolved: 09/Jun/23

Status: Closed
Project: Core Server
Component/s: None
Affects Version/s: None
Fix Version/s: 7.1.0-rc0

Type: Improvement Priority: Minor - P4
Reporter: Mohammad Dashti (Inactive) Assignee: Kristina Znam (Inactive)
Resolution: Fixed Votes: 0
Labels: neweng, query-product-scope-1, query-product-urgency-2, query-product-value-2
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Related
Assigned Teams:
Query Execution
Backwards Compatibility: Fully Compatible
Sprint: QE 2023-06-12, QE 2023-06-26
Participants:
Case:

 Description   

When creating the body of OpMsgRequest it's possible that the combination of body, extraFields and $db goes beyond BSONObjMaxInternalSize. In this case, currently, only an assertion failure is issued. However, to improve the situation and help our support team with more useful data, we'd better log the size break-down of these fields (and even include the truncated values for them):

Here's a suggested change for here:

static std::string compactStr(const std::string& input) {
    if (input.length() > 2024) {
        return input.substr(0, 1000) + " ... " + input.substr(input.length() - 1000);
    }
    return input;
}
 
request.body = ([&] {
    BSONObjBuilder bodyBuilder(std::move(body));
    bodyBuilder.appendElements(extraFields);
    bodyBuilder.append("$db", db);
    if (bodyBuilder.len() > BSONObjMaxInternalSize) {
        warning() << "Request body exceeded limit with body.objsize() = " << body.objsize()
                    << ", db.size() = " << db.size()
                    << ", extraFields.objsize() = " << extraFields.objsize()
                    << ", body.toString() = " << compactStr(body.toString())
                    << ", db = " << db
                    << ", extraFields.toString() = " << compactStr(extraFields.toString());
    }
    return bodyBuilder.obj();
}());



 Comments   
Comment by Githook User [ 09/Jun/23 ]

Author:

{'name': 'Kristina Znam', 'email': 'kristina.znam@mongodb.com', 'username': 'kmznam'}

Message: SERVER-64918 Add a log entry if OpMsgRequest.body is bigger than BSONObjMaxInternalSize
Branch: master
https://github.com/mongodb/mongo/commit/7f8f4e31c8906e07bac41104da16e2a71a2ab49f

Comment by Xiaochen Wu [ 31/Mar/23 ]

Add to the QE quick win candidate. 

Comment by Mohammad Dashti (Inactive) [ 12/Apr/22 ]

The user-roles in the request are added as part of SERVER-5261. This metadata should only appear in requests from mongos to mongod.

There are already some tests to make sure that manually inserted impersonations can't escalate privileges:

You can take a look at these code blocks:

    • Struct that temporarily stores client information when an audit hook executes on a separate thread with a new Client. In those cases, ImpersonatedClientAttrs can bundle all relevant client attributes necessary for auditing and be safely passed into the new thread, where the new Client will be loaded with the userNames and roleNames stored in ImpersonatedClientAttrs.
    • Since index builds occur in a separate thread, client attributes that are audited must be extracted from the client object and passed into the thread separately.

Now, with the possibility of a large number of roles, we need to rethink this and either account for the size of impersonation data while creating the BSON request object, or set a limit on the size of impersonation data that can be included (and if it goes beyond the limit, we need to find another way (e.g., spilling to disk) for communication). The former solution does not seem feasible, as we want to be able to continue supporting 16MB user documents (i.e., BSONObjMaxUserSize), which only leaves a rigid 16KB for the rest of meta-data (i.e., BSONObjMaxUserSize).

Comment by Ethan Zhang (Inactive) [ 29/Mar/22 ]

christopher.harris 2 Questions:

  • Should we schedule this ticket soon to help better understand the issue?
  • Do we want to consider changing the things we want to include in OpMsgRequest? Like not including all the user roles in the request? You can refer to Mohammad's comment in the linked HELP ticket for more information.
Generated at Thu Feb 08 06:01:27 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.