From 1ce0d13e46f720b35ae01e37d16bcd7b3789a045 Mon Sep 17 00:00:00 2001 From: "A. Jesse Jiryu Davis" Date: Thu, 9 Jul 2020 18:19:07 -0400 Subject: SERVER-48967 Ban non-primary writes with empty namespace --- src/mongo/db/op_observer_impl_test.cpp | 15 ++++++ src/mongo/db/repl/SConscript | 1 + .../db/repl/always_allow_non_local_writes.cpp | 55 ++++++++++++++++++++ src/mongo/db/repl/always_allow_non_local_writes.h | 59 ++++++++++++++++++++++ src/mongo/db/repl/oplog.cpp | 2 +- ...replication_coordinator_external_state_impl.cpp | 4 +- src/mongo/db/repl/replication_coordinator_impl.cpp | 33 ++---------- 7 files changed, 137 insertions(+), 32 deletions(-) create mode 100644 src/mongo/db/repl/always_allow_non_local_writes.cpp create mode 100644 src/mongo/db/repl/always_allow_non_local_writes.h diff --git a/src/mongo/db/op_observer_impl_test.cpp b/src/mongo/db/op_observer_impl_test.cpp index 8e4b093fafa..16c59b16ef3 100644 --- a/src/mongo/db/op_observer_impl_test.cpp +++ b/src/mongo/db/op_observer_impl_test.cpp @@ -41,6 +41,7 @@ #include "mongo/db/read_write_concern_defaults.h" #include "mongo/db/read_write_concern_defaults_cache_lookup_mock.h" #include "mongo/db/repl/oplog.h" +#include "mongo/db/repl/oplog_entry.h" #include "mongo/db/repl/oplog_interface_local.h" #include "mongo/db/repl/repl_client_info.h" #include "mongo/db/repl/replication_coordinator_mock.h" @@ -445,6 +446,20 @@ TEST_F(OpObserverTest, OnRenameCollectionOmitsDropTargetFieldIfDropTargetUuidIsN ASSERT_BSONOBJ_EQ(oExpected, o); } +TEST_F(OpObserverTest, MustBePrimaryToWriteOplogEntries) { + OpObserverImpl opObserver; + auto opCtx = cc().makeOperationContext(); + + ASSERT_OK(repl::ReplicationCoordinator::get(opCtx.get()) + ->setFollowerMode(repl::MemberState::RS_SECONDARY)); + + Lock::GlobalWrite globalWrite(opCtx.get()); + WriteUnitOfWork wunit(opCtx.get()); + + // No-op writes should be prohibited. + ASSERT_THROWS_CODE(opObserver.onOpMessage(opCtx.get(), {}), DBException, ErrorCodes::NotMaster); +} + /** * Test fixture for testing OpObserver behavior specific to the SessionCatalog. */ diff --git a/src/mongo/db/repl/SConscript b/src/mongo/db/repl/SConscript index 21d7b496a20..b0d1660b815 100644 --- a/src/mongo/db/repl/SConscript +++ b/src/mongo/db/repl/SConscript @@ -740,6 +740,7 @@ env.Library( env.Library( target='repl_coordinator_interface', source=[ + 'always_allow_non_local_writes.cpp', 'repl_client_info.cpp', 'replication_coordinator.cpp', 'replication_coordinator_noop.cpp', diff --git a/src/mongo/db/repl/always_allow_non_local_writes.cpp b/src/mongo/db/repl/always_allow_non_local_writes.cpp new file mode 100644 index 00000000000..0417debe798 --- /dev/null +++ b/src/mongo/db/repl/always_allow_non_local_writes.cpp @@ -0,0 +1,55 @@ +/** + * Copyright (C) 2020-present MongoDB, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the Server Side Public License, version 1, + * as published by MongoDB, Inc. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * Server Side Public License for more details. + * + * You should have received a copy of the Server Side Public License + * along with this program. If not, see + * . + * + * As a special exception, the copyright holders give permission to link the + * code of portions of this program with the OpenSSL library under certain + * conditions as described in each individual source file and distribute + * linked combinations including the program with the OpenSSL library. You + * must comply with the Server Side Public License in all respects for + * all of the code used other than as permitted herein. If you modify file(s) + * with this exception, you may extend this exception to your version of the + * file(s), but you are not obligated to do so. If you do not wish to do so, + * delete this exception statement from your version. If you delete this + * exception statement from all source files in the program, then also delete + * it in the license file. + */ + +#include "mongo/platform/basic.h" + +#include "mongo/db/repl/always_allow_non_local_writes.h" + +namespace mongo { +namespace repl { + +// Overrides _canAcceptLocalWrites for the decorated OperationContext. +const OperationContext::Decoration alwaysAllowNonLocalWritesOnOpCtx = + OperationContext::declareDecoration(); + +AllowNonLocalWritesBlock::AllowNonLocalWritesBlock(OperationContext* opCtx) + : _opCtx(opCtx), _initialState(alwaysAllowNonLocalWritesOnOpCtx(_opCtx)) { + alwaysAllowNonLocalWritesOnOpCtx(_opCtx) = true; +} + +AllowNonLocalWritesBlock::~AllowNonLocalWritesBlock() { + alwaysAllowNonLocalWritesOnOpCtx(_opCtx) = _initialState; +} + +bool alwaysAllowNonLocalWrites(const OperationContext* opCtx) { + return alwaysAllowNonLocalWritesOnOpCtx(opCtx); +} + +} // namespace repl +} // namespace mongo diff --git a/src/mongo/db/repl/always_allow_non_local_writes.h b/src/mongo/db/repl/always_allow_non_local_writes.h new file mode 100644 index 00000000000..e5759ba418f --- /dev/null +++ b/src/mongo/db/repl/always_allow_non_local_writes.h @@ -0,0 +1,59 @@ +/** + * Copyright (C) 2020-present MongoDB, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the Server Side Public License, version 1, + * as published by MongoDB, Inc. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * Server Side Public License for more details. + * + * You should have received a copy of the Server Side Public License + * along with this program. If not, see + * . + * + * As a special exception, the copyright holders give permission to link the + * code of portions of this program with the OpenSSL library under certain + * conditions as described in each individual source file and distribute + * linked combinations including the program with the OpenSSL library. You + * must comply with the Server Side Public License in all respects for + * all of the code used other than as permitted herein. If you modify file(s) + * with this exception, you may extend this exception to your version of the + * file(s), but you are not obligated to do so. If you do not wish to do so, + * delete this exception statement from your version. If you delete this + * exception statement from all source files in the program, then also delete + * it in the license file. + */ + +#pragma once + +#include "mongo/db/operation_context.h" + +namespace mongo { +namespace repl { + +/** + * Allows non-local writes despite _canAcceptNonLocalWrites being false on a single OperationContext + * while in scope. + * + * Resets to original value when leaving scope so it is safe to nest. + */ +class AllowNonLocalWritesBlock { + AllowNonLocalWritesBlock(const AllowNonLocalWritesBlock&) = delete; + AllowNonLocalWritesBlock& operator=(const AllowNonLocalWritesBlock&) = delete; + +public: + AllowNonLocalWritesBlock(OperationContext* opCtx); + ~AllowNonLocalWritesBlock(); + +private: + OperationContext* const _opCtx; + const bool _initialState; +}; + +bool alwaysAllowNonLocalWrites(const OperationContext* opCtx); + +} // namespace repl +} // namespace mongo diff --git a/src/mongo/db/repl/oplog.cpp b/src/mongo/db/repl/oplog.cpp index 8588bc9255f..0181230f177 100644 --- a/src/mongo/db/repl/oplog.cpp +++ b/src/mongo/db/repl/oplog.cpp @@ -214,7 +214,7 @@ void _logOpsInner(OperationContext* opCtx, OpTime finalOpTime, Date_t wallTime) { auto replCoord = ReplicationCoordinator::get(opCtx); - if (nss.size() && replCoord->getReplicationMode() == ReplicationCoordinator::modeReplSet && + if (replCoord->getReplicationMode() == ReplicationCoordinator::modeReplSet && !replCoord->canAcceptWritesFor(opCtx, nss)) { str::stream ss; ss << "logOp() but can't accept write to collection " << nss; diff --git a/src/mongo/db/repl/replication_coordinator_external_state_impl.cpp b/src/mongo/db/repl/replication_coordinator_external_state_impl.cpp index 717cba25db6..ccbcc74e0cd 100644 --- a/src/mongo/db/repl/replication_coordinator_external_state_impl.cpp +++ b/src/mongo/db/repl/replication_coordinator_external_state_impl.cpp @@ -60,6 +60,7 @@ #include "mongo/db/logical_time_metadata_hook.h" #include "mongo/db/logical_time_validator.h" #include "mongo/db/op_observer.h" +#include "mongo/db/repl/always_allow_non_local_writes.h" #include "mongo/db/repl/bgsync.h" #include "mongo/db/repl/drop_pending_collection_reaper.h" #include "mongo/db/repl/isself.h" @@ -437,8 +438,9 @@ Status ReplicationCoordinatorExternalStateImpl::initializeReplSetStorage(Operati "initiate oplog entry", NamespaceString::kRsOplogNamespace.toString(), [this, &opCtx, &config] { + // Permit writing to the oplog before we step up to primary. + AllowNonLocalWritesBlock allowNonLocalWrites(opCtx); Lock::GlobalWrite globalWrite(opCtx); - WriteUnitOfWork wuow(opCtx); Helpers::putSingleton(opCtx, configCollectionName, config); const auto msgObj = BSON("msg" << kInitiatingSetMsg); diff --git a/src/mongo/db/repl/replication_coordinator_impl.cpp b/src/mongo/db/repl/replication_coordinator_impl.cpp index 027a75ba474..310bef218a3 100644 --- a/src/mongo/db/repl/replication_coordinator_impl.cpp +++ b/src/mongo/db/repl/replication_coordinator_impl.cpp @@ -63,6 +63,7 @@ #include "mongo/db/kill_sessions_local.h" #include "mongo/db/mongod_options_storage_gen.h" #include "mongo/db/prepare_conflict_tracker.h" +#include "mongo/db/repl/always_allow_non_local_writes.h" #include "mongo/db/repl/check_quorum_for_config_change.h" #include "mongo/db/repl/data_replicator_external_state_initial_sync.h" #include "mongo/db/repl/is_master_response.h" @@ -174,34 +175,6 @@ using NextAction = Fetcher::NextAction; namespace { const char kLocalDB[] = "local"; -// Overrides _canAcceptLocalWrites for the decorated OperationContext. -const OperationContext::Decoration alwaysAllowNonLocalWrites = - OperationContext::declareDecoration(); - -/** - * Allows non-local writes despite _canAcceptNonLocalWrites being false on a single OperationContext - * while in scope. - * - * Resets to original value when leaving scope so it is safe to nest. - */ -class AllowNonLocalWritesBlock { - AllowNonLocalWritesBlock(const AllowNonLocalWritesBlock&) = delete; - AllowNonLocalWritesBlock& operator=(const AllowNonLocalWritesBlock&) = delete; - -public: - AllowNonLocalWritesBlock(OperationContext* opCtx) - : _opCtx(opCtx), _initialState(alwaysAllowNonLocalWrites(_opCtx)) { - alwaysAllowNonLocalWrites(_opCtx) = true; - } - - ~AllowNonLocalWritesBlock() { - alwaysAllowNonLocalWrites(_opCtx) = _initialState; - } - -private: - OperationContext* const _opCtx; - const bool _initialState; -}; void lockAndCall(stdx::unique_lock* lk, const std::function& fn) { if (!lk->owns_lock()) { @@ -2843,7 +2816,7 @@ bool ReplicationCoordinatorImpl::canAcceptWritesForDatabase_UNSAFE(OperationCont // // Stand-alone nodes and drained replica set primaries can always accept writes. Writes are // always permitted to the "local" database. - if (_readWriteAbility->canAcceptNonLocalWrites_UNSAFE() || alwaysAllowNonLocalWrites(*opCtx)) { + if (_readWriteAbility->canAcceptNonLocalWrites_UNSAFE() || alwaysAllowNonLocalWrites(opCtx)) { return true; } if (dbName == kLocalDB) { @@ -2888,7 +2861,7 @@ bool ReplicationCoordinatorImpl::canAcceptWritesFor_UNSAFE(OperationContext* opC // If we can accept non local writes (ie we're PRIMARY) then we must not be in ROLLBACK. // This check is redundant of the check of _memberState below, but since this can be checked // without locking, we do it as an optimization. - if (_readWriteAbility->canAcceptNonLocalWrites_UNSAFE() || alwaysAllowNonLocalWrites(*opCtx)) { + if (_readWriteAbility->canAcceptNonLocalWrites_UNSAFE() || alwaysAllowNonLocalWrites(opCtx)) { return true; } -- cgit v1.2.1