SERVER-58338 Return an error if client attempts an OP_QUERY command other than isMaster/hello

12 files changed, 105 insertions, 95 deletions

diff --git a/src/mongo/base/error_codes.yml b/src/mongo/base/error_codes.yml
index 6d127633f2d..1efb26030d4 100644
--- a/src/mongo/base/error_codes.yml
+++ b/src/mongo/base/error_codes.yml
@@ -443,6 +443,8 @@ error_codes:
 
     - {code: 351, name: ChangeStreamStartAfterInvalidate, extra: ChangeStreamStartAfterInvalidateInfo}
     
+    - {code: 352, name: UnsupportedOpQueryCommand}
+
     # Error codes 4000-8999 are reserved.
 
     # Non-sequential error codes for compatibility only)
diff --git a/src/mongo/client/dbclient_base.h b/src/mongo/client/dbclient_base.h
index 83b1f688819..173127103e2 100644
--- a/src/mongo/client/dbclient_base.h
+++ b/src/mongo/client/dbclient_base.h
@@ -834,15 +834,13 @@ private:
 
     /**
      * The rpc protocols this client supports.
-     *
      */
-    rpc::ProtocolSet _clientRPCProtocols{rpc::supports::kAll};
+    rpc::ProtocolSet _clientRPCProtocols{rpc::supports::kOpMsgOnly};
 
     /**
-     * The rpc protocol the remote server(s) support. We support 'opQueryOnly' by default unless
-     * we detect support for OP_MSG at connection time.
+     * The rpc protocol the remote server(s) support.
      */
-    rpc::ProtocolSet _serverRPCProtocols{rpc::supports::kOpQueryOnly};
+    rpc::ProtocolSet _serverRPCProtocols{rpc::supports::kOpMsgOnly};
 
     rpc::RequestMetadataWriter _metadataWriter;
     rpc::ReplyMetadataReader _metadataReader;
diff --git a/src/mongo/client/dbclient_connection.cpp b/src/mongo/client/dbclient_connection.cpp
index f40b32e76a6..6e643b75ad0 100644
--- a/src/mongo/client/dbclient_connection.cpp
+++ b/src/mongo/client/dbclient_connection.cpp
@@ -93,25 +93,6 @@ MONGO_FAIL_POINT_DEFINE(dbClientConnectionDisableChecksum);
 
 namespace {
 
-/**
- * RAII class to force usage of OP_QUERY on a connection.
- */
-class ScopedForceOpQuery {
-public:
-    ScopedForceOpQuery(DBClientBase* conn)
-        : _conn(conn), _oldProtos(conn->getClientRPCProtocols()) {
-        _conn->setClientRPCProtocols(rpc::supports::kOpQueryOnly);
-    }
-
-    ~ScopedForceOpQuery() {
-        _conn->setClientRPCProtocols(_oldProtos);
-    }
-
-private:
-    DBClientBase* const _conn;
-    const rpc::ProtocolSet _oldProtos;
-};
-
 StatusWith<bool> completeSpeculativeAuth(DBClientConnection* conn,
                                          auth::SpeculativeAuthType speculativeAuthType,
                                          std::shared_ptr<SaslClientSession> session,
@@ -180,10 +161,6 @@ executor::RemoteCommandResponse initWireVersion(
     std::vector<std::string>* saslMechsForAuth,
     auth::SpeculativeAuthType* speculativeAuthType,
     std::shared_ptr<SaslClientSession>* saslClientSession) try {
-    // We need to force the usage of OP_QUERY on this command, even if we have previously
-    // detected support for OP_MSG on a connection. This is necessary to handle the case
-    // where we reconnect to an older version of MongoDB running at the same host/port.
-    ScopedForceOpQuery forceOpQuery{conn};
 
     BSONObjBuilder bob;
     bob.append(conn->getApiParameters().getVersion() ? "hello" : "isMaster", 1);
@@ -294,7 +271,7 @@ Status DBClientConnection::connect(const HostAndPort& serverAddress,
     }
 
     // Clear the auto-detected protocols from any previous connection.
-    _setServerRPCProtocols(rpc::supports::kOpQueryOnly);
+    _setServerRPCProtocols(rpc::supports::kOpMsgOnly);
 
     // NOTE: If the 'applicationName' parameter is a view of the '_applicationName' member, as
     // happens, for instance, in the call to DBClientConnection::connect from
@@ -310,6 +287,8 @@ Status DBClientConnection::connect(const HostAndPort& serverAddress,
         this, _applicationName, _uri, &_saslMechsForAuth, &speculativeAuthType, &saslClientSession);
     if (!swIsMasterReply.isOK()) {
         _markFailed(kSetFlag);
+        swIsMasterReply.status.addContext(
+            "Connection handshake failed. Is your mongod/mongos 3.4 or older?"_sd);
         return swIsMasterReply.status;
     }
 
diff --git a/src/mongo/client/dbclient_cursor_test.cpp b/src/mongo/client/dbclient_cursor_test.cpp
index 96a1fe529d7..d267a11257a 100644
--- a/src/mongo/client/dbclient_cursor_test.cpp
+++ b/src/mongo/client/dbclient_cursor_test.cpp
@@ -409,54 +409,6 @@ TEST_F(DBClientCursorTest, DBClientCursorMoreThrowsExceptionWhenMoreToComeFlagSe
     ASSERT_THROWS_CODE(cursor.more(), DBException, 50935);
 }
 
-TEST_F(DBClientCursorTest, DBClientCursorIgnoresExhaustForOpQueryMessages) {
-    // Set up the DBClientCursor and a mock client connection. If we set the server RPC protocol to
-    // OpQuery, then when we assemble a command request in DBClientCursor, we will make a command
-    // style OpQuery request, as opposed to an OpMsg request. We want to make sure that for command
-    // style OpQuery requests, we ignore the exhaust option, and that cursor queries work normally.
-    DBClientConnectionForTest conn;
-    conn.setSupportedProtocols(rpc::supports::kOpQueryOnly);
-
-    const NamespaceString nss("test", "coll");
-    DBClientCursor cursor(
-        &conn, NamespaceStringOrUUID(nss), Query().obj, 0, 0, nullptr, QueryOption_Exhaust, 0);
-    cursor.setBatchSize(0);
-
-    // Set up mock 'find' response.
-    const long long cursorId = 42;
-    Message findResponseMsg = mockFindResponse(nss, cursorId, {});
-
-    conn.setCallResponse(findResponseMsg);
-    ASSERT(cursor.init());
-
-    // Verify that the initial 'find' request was sent.
-    auto m = conn.getLastSentMessage();
-    ASSERT(!m.empty());
-    QueryMessage queryMsg(m);
-    ASSERT_EQ(queryMsg.query.getStringField("find"), nss.coll());
-    ASSERT_EQ(queryMsg.query["batchSize"].number(), 0);
-    ASSERT_EQ(0, queryMsg.queryOptions);
-
-    // Create and set a non-exhaust getMore response.
-    cursor.setBatchSize(2);
-    auto getMoreResponseMsg = mockGetMoreResponse(nss, cursorId, {docObj(1), docObj(2)});
-    conn.setCallResponse(getMoreResponseMsg);
-
-    // Trigger another 'getMore' request.
-    conn.clearLastSentMessage();
-    ASSERT(cursor.more());
-
-    // Make sure the sent request has no exhaust query options set.
-    m = conn.getLastSentMessage();
-    ASSERT(!m.empty());
-    queryMsg = QueryMessage(m);
-    ASSERT_EQ(queryMsg.query["getMore"].number(), cursorId);
-    ASSERT_EQ(queryMsg.query["collection"].str(), nss.coll());
-    ASSERT_EQ(0, queryMsg.queryOptions);
-    ASSERT_BSONOBJ_EQ(docObj(1), cursor.next());
-    ASSERT_BSONOBJ_EQ(docObj(2), cursor.next());
-}
-
 TEST_F(DBClientCursorTest, DBClientCursorPassesReadOnceFlag) {
 
     // Set up the DBClientCursor and a mock client connection.
diff --git a/src/mongo/db/commands/user_management_commands.cpp b/src/mongo/db/commands/user_management_commands.cpp
index 9c42dec4997..2157df890d4 100644
--- a/src/mongo/db/commands/user_management_commands.cpp
+++ b/src/mongo/db/commands/user_management_commands.cpp
@@ -891,8 +891,7 @@ private:
         auto svcCtx = _client->getServiceContext();
         auto sep = svcCtx->getServiceEntryPoint();
         auto opMsgRequest = OpMsgRequest::fromDBAndBody(kAdminDB, cmdBuilder->obj());
-        auto requestMessage = rpc::messageFromOpMsgRequest(
-            rpc::supports::kAll, rpc::supports::kOpQueryOnly, opMsgRequest);
+        auto requestMessage = rpc::messageFromOpMsgRequest(rpc::Protocol::kOpMsg, opMsgRequest);
 
         // Switch to our local client and create a short-lived opCtx for this transaction op.
         AlternativeClientRegion clientRegion(_client);
diff --git a/src/mongo/db/service_entry_point_common.cpp b/src/mongo/db/service_entry_point_common.cpp
index 7d3d299d735..7d7320f9f25 100644
--- a/src/mongo/db/service_entry_point_common.cpp
+++ b/src/mongo/db/service_entry_point_common.cpp
@@ -1432,11 +1432,6 @@ void ExecCommandDatabase::_initiateCommand() {
         globalOpCounters.gotCommand();
     }
 
-    if (!isHello() && _execContext->getMessage().operation() == dbQuery) {
-        warnDeprecation(*client, networkOpToString(dbQuery));
-        globalOpCounters.gotQueryDeprecated();
-    }
-
     // Parse the 'maxTimeMS' command option, and use it to set a deadline for the operation on the
     // OperationContext. The 'maxTimeMS' option unfortunately has a different meaning for a getMore
     // command, where it is used to communicate the maximum time to wait for new inserts on tailable
@@ -1731,7 +1726,13 @@ void curOpCommandSetup(OperationContext* opCtx, const OpMsgRequest& request) {
 }
 
 Future<void> parseCommand(std::shared_ptr<HandleRequest::ExecutionContext> execContext) try {
-    execContext->setRequest(rpc::opMsgRequestFromAnyProtocol(execContext->getMessage()));
+    const auto& msg = execContext->getMessage();
+    auto opMsgReq = rpc::opMsgRequestFromAnyProtocol(msg);
+    if (msg.operation() == dbQuery) {
+        checkAllowedOpQueryCommand(*(execContext->getOpCtx()->getClient()),
+                                   opMsgReq.getCommandName());
+    }
+    execContext->setRequest(opMsgReq);
     return Status::OK();
 } catch (const DBException& ex) {
     // Need to set request as `makeCommandResponse` expects an empty request on failure.
diff --git a/src/mongo/rpc/SConscript b/src/mongo/rpc/SConscript
index 6a0c3cb3a3e..a831e2fdcd4 100644
--- a/src/mongo/rpc/SConscript
+++ b/src/mongo/rpc/SConscript
@@ -67,6 +67,7 @@ env.Library(
         '$BUILD_DIR/mongo/base',
         '$BUILD_DIR/mongo/db/dbmessage',
         '$BUILD_DIR/mongo/db/server_options_core',
+        '$BUILD_DIR/mongo/db/stats/counters',
         '$BUILD_DIR/mongo/idl/basic_types',
         '$BUILD_DIR/mongo/s/common_s',
         '$BUILD_DIR/mongo/util/net/network',
diff --git a/src/mongo/rpc/legacy_request.cpp b/src/mongo/rpc/legacy_request.cpp
index 2c05714d4f4..87c8e1bc071 100644
--- a/src/mongo/rpc/legacy_request.cpp
+++ b/src/mongo/rpc/legacy_request.cpp
@@ -60,7 +60,7 @@ OpMsgRequest opMsgRequestFromLegacyRequest(const Message& message) {
                           << ") for $cmd type ns - can only be 1 or -1",
             qm.ntoreturn == 1 || qm.ntoreturn == -1);
 
-    return rpc::upconvertRequest(
+    return upconvertRequest(
         ns.db(), qm.query.shareOwnershipWith(message.sharedBuffer()), qm.queryOptions);
 }
 
diff --git a/src/mongo/rpc/op_legacy_integration_test.cpp b/src/mongo/rpc/op_legacy_integration_test.cpp
index c2d8614364a..353c1446b17 100644
--- a/src/mongo/rpc/op_legacy_integration_test.cpp
+++ b/src/mongo/rpc/op_legacy_integration_test.cpp
@@ -343,7 +343,8 @@ TEST(OpLegacy, GenericCommandViaOpQuery) {
     QueryResult::ConstView qr = replyQuery.singleData().view2ptr();
     BufReader data(qr.data(), qr.dataLen());
     BSONObj obj = data.read<BSONObj>();
-    ASSERT_OK(getStatusFromCommandResult(obj));  // will fail after we remove the support for $cmd
+    auto status = getStatusFromCommandResult(obj);
+    ASSERT_EQ(status.code(), ErrorCodes::UnsupportedOpQueryCommand);
 
     // The logic around log severity for the deprecation logging is tested elsewhere. Here we check
     // that it gets logged at all.
@@ -356,7 +357,7 @@ TEST(OpLegacy, GenericCommandViaOpQuery) {
 }
 
 // 'hello' and 'isMaster' commands, issued via OP_QUERY protocol, are still fully supported.
-void testAllowedCommand(const char* command) {
+void testAllowedCommand(const char* command, ErrorCodes::Error code = ErrorCodes::OK) {
     auto conn = getIntegrationTestConnection();
 
     auto serverStatusCmd = fromjson("{serverStatus: 1}");
@@ -379,7 +380,8 @@ void testAllowedCommand(const char* command) {
     QueryResult::ConstView qr = replyQuery.singleData().view2ptr();
     BufReader data(qr.data(), qr.dataLen());
     BSONObj obj = data.read<BSONObj>();
-    ASSERT_OK(getStatusFromCommandResult(obj));
+    auto status = getStatusFromCommandResult(obj);
+    ASSERT_EQ(status.code(), code);
 
     ASSERT_FALSE(wasLogged(conn.get(), "query", ""));
 
@@ -397,5 +399,48 @@ TEST(OpLegacy, IsMasterCommandViaOpQuery) {
     testAllowedCommand("{isMaster: 1}");
 }
 
+TEST(OpLegacy, IsmasterCommandViaOpQuery) {
+    testAllowedCommand("{ismaster: 1}");
+}
+
+TEST(OpLegacy, IsSelfCommandViaOpQuery) {
+    testAllowedCommand("{_isSelf: 1}");
+}
+
+TEST(OpLegacy, SaslStartCommandViaOpQuery) {
+    // Here we verify that "saslStart" command passes parsing since the request is actually
+    // an invalid authentication request which is capture from a log. The AuthenticationFailed error
+    // code means that it passes request parsing.
+    testAllowedCommand(R"({
+                           saslStart: 1,
+                           "mechanism":"SCRAM-SHA-256",
+                           "options":{"skipEmptyExchange":true},
+                           "payload":{
+                               "$binary":{
+                                   "base64":"biwsbj1fX3N5c3RlbSxyPUlyNDVmQm1WNWNuUXJSS3FhdU9JUERCTUhkV2NrK01i",
+                                   "subType":"0"
+                               }
+                           }
+                       })",
+                       ErrorCodes::AuthenticationFailed);
+}
+
+TEST(OpLegacy, SaslContinueCommandViaOpQuery) {
+    // Here we verify that "saslContinue" command passes parsing since the request is actually
+    // an invalid authentication request which is captured from a log. The ProtocolError error code
+    // means that it passes request parsing.
+    testAllowedCommand(R"({
+                           saslContinue: 1,
+                           "payload":{
+                               "$binary":{
+                                   "base64":"Yz1iaXdzLHI9SXI0NWZCbVY1Y25RclJLcWF1T0lQREJNSGRXY2srTWJSNE81SnJrcnV4anorRDl2WXkrKzlnNlhBVHFCV0pMbSxwPUJTV3puZnNjcG8rYVhnc1YyT2xEa2NFSjF5NW9rM2xWSWQybjc4NlJ5MTQ9",
+                                   "subType":"0"
+                               }
+                           },
+                           "conversationId":1
+                       })",
+                       ErrorCodes::ProtocolError);
+}
+
 }  // namespace
 }  // namespace mongo
diff --git a/src/mongo/rpc/warn_deprecated_wire_ops.cpp b/src/mongo/rpc/warn_deprecated_wire_ops.cpp
index 24f1029194b..c73727d2cda 100644
--- a/src/mongo/rpc/warn_deprecated_wire_ops.cpp
+++ b/src/mongo/rpc/warn_deprecated_wire_ops.cpp
@@ -37,6 +37,7 @@
 #include <string>
 
 #include "mongo/db/client.h"
+#include "mongo/db/stats/counters.h"
 #include "mongo/logv2/log.h"
 #include "mongo/logv2/log_severity_suppressor.h"
 #include "mongo/rpc/deprecated_wire_ops_gen.h"
@@ -108,4 +109,24 @@ void warnDeprecation(Client& client, StringData op) {
                 "op"_attr = op,
                 "clientInfo"_attr = clientInfo);
 }
+
+void checkAllowedOpQueryCommand(Client& client, StringData cmd) {
+    static constexpr std::array allowedOpQueryCommands{
+        "_isSelf"_sd,
+        "hello"_sd,
+        "isMaster"_sd,
+        "ismaster"_sd,
+        "saslContinue"_sd,
+        "saslStart"_sd,
+    };
+
+    if (std::find(allowedOpQueryCommands.begin(), allowedOpQueryCommands.end(), cmd) ==
+        allowedOpQueryCommands.end()) {
+        warnDeprecation(client, networkOpToString(dbQuery));
+        globalOpCounters.gotQueryDeprecated();
+        uasserted(ErrorCodes::UnsupportedOpQueryCommand,
+                  "Unsupported OP_QUERY command: {}"_format(cmd));
+    }
+}
+
 }  // namespace mongo
diff --git a/src/mongo/rpc/warn_deprecated_wire_ops.h b/src/mongo/rpc/warn_deprecated_wire_ops.h
index ae626ee6dd1..fbfb49cbdb6 100644
--- a/src/mongo/rpc/warn_deprecated_wire_ops.h
+++ b/src/mongo/rpc/warn_deprecated_wire_ops.h
@@ -56,4 +56,17 @@ void warnDeprecation(Client& client, StringData op);
  */
 Status onUpdateOfWireOpsWarningPeriod(const int&);
 
+/**
+ * Logs a warning message and throws if 'cmd' is not an allowed 'OP_QUERY' command.
+ *
+ * OP_QUERY commands are no longer serviced by mongos or mongod processes, with the following
+ * exceptions.
+ *
+ * - The isMaster/ismaster/hello OP_QUERY commands are allowed for connection handshake.
+ *
+ * - The _isSelf/saslContinue/saslStart OP_QUERY commands are allowed for intra-cluster
+ * communication in a mixed version cluster. V5.0 nodes may send those commands as OP_QUERY ones.
+ */
+void checkAllowedOpQueryCommand(Client& client, StringData cmd);
+
 }  // namespace mongo
diff --git a/src/mongo/s/commands/strategy.cpp b/src/mongo/s/commands/strategy.cpp
index ee23be15884..90e9ca5a053 100644
--- a/src/mongo/s/commands/strategy.cpp
+++ b/src/mongo/s/commands/strategy.cpp
@@ -864,11 +864,6 @@ Status ParseAndRunCommand::RunInvocation::_setup() {
         _shouldAffectCommandCounter = true;
     }
 
-    if (_parc->_opType == dbQuery && !_parc->_isHello.get()) {
-        warnDeprecation(*opCtx->getClient(), networkOpToString(dbQuery));
-        globalOpCounters.gotQueryDeprecated();
-    }
-
     return Status::OK();
 }
 
@@ -1158,7 +1153,11 @@ private:
 void ClientCommand::_parseMessage() try {
     const auto& msg = _rec->getMessage();
     _rec->setReplyBuilder(rpc::makeReplyBuilder(rpc::protocolForMessage(msg)));
-    _rec->setRequest(rpc::opMsgRequestFromAnyProtocol(msg));
+    auto opMsgReq = rpc::opMsgRequestFromAnyProtocol(msg);
+    if (msg.operation() == dbQuery) {
+        checkAllowedOpQueryCommand(*(_rec->getOpCtx()->getClient()), opMsgReq.getCommandName());
+    }
+    _rec->setRequest(opMsgReq);
 } catch (const DBException& ex) {
     // If this error needs to fail the connection, propagate it out.
     if (ErrorCodes::isConnectionFatalMessageParseError(ex.code()))