Skip to content

Commit 19d94ab

Browse files
committed
MB-51871: Enable AuditDroppedTest for windows
And ensure that the code works with and without execute permissions for the directory Change-Id: Ide8a52418ba67d90a3dda8e52f67e35b739461a7 Reviewed-on: https://review.couchbase.org/c/kv_engine/+/236669 Reviewed-by: Mohammad Zaeem <[email protected]> Tested-by: Build Bot <[email protected]>
1 parent bc1bd55 commit 19d94ab

File tree

2 files changed

+60
-30
lines changed

2 files changed

+60
-30
lines changed

auditd/src/auditfile.cc

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,7 @@ bool AuditFile::open() {
189189
// In order for this to happen in production you would generate
190190
// so much audit data that you would rotate within the same second
191191
int count = 0;
192+
std::error_code ec;
192193
do {
193194
if (count == 0) {
194195
open_file_name =
@@ -202,7 +203,7 @@ bool AuditFile::open() {
202203
extension);
203204
}
204205
++count;
205-
} while (exists(open_file_name));
206+
} while (exists(open_file_name, ec));
206207

207208
try {
208209
file = cb::crypto::FileWriter::create(
@@ -286,15 +287,15 @@ void AuditFile::set_log_directory(const std::string& new_directory) {
286287
}
287288

288289
log_directory = cb::io::makeExtendedLengthPath(new_directory);
289-
try {
290-
create_directories(log_directory);
291-
} catch (const std::runtime_error& error) {
290+
std::error_code ec;
291+
create_directories(log_directory, ec);
292+
if (ec) {
292293
// The directory does not exist and we failed to create
293294
// it. This is not a fatal error, but it does mean that the
294295
// node won't be able to do any auditing
295296
LOG_WARNING_CTX("Audit: failed to create audit directory",
296297
{"path", new_directory},
297-
{"error", error.what()});
298+
{"error", ec.message()});
298299
}
299300
}
300301

@@ -327,13 +328,13 @@ bool AuditFile::flush() {
327328
}
328329

329330
void AuditFile::remove_file(const std::filesystem::path& path) {
330-
if (exists(path)) {
331-
try {
332-
remove(path);
333-
} catch (const std::exception& exception) {
331+
std::error_code ec;
332+
if (exists(path, ec)) {
333+
remove(path, ec);
334+
if (ec) {
334335
LOG_WARNING_CTX("Audit: Failed to remove file",
335336
{"path", path.generic_string()},
336-
{"error", exception.what()});
337+
{"error", ec.message()});
337338
}
338339
}
339340
}

tests/testapp/testapp_audit.cc

Lines changed: 49 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
#include "testapp_client_test.h"
1818
#include <auditd/couchbase_audit_events.h>
19+
#include <folly/ScopeGuard.h>
1920
#include <mcbp/codec/frameinfo.h>
2021
#include <nlohmann/json.hpp>
2122
#include <platform/dirutils.h>
@@ -654,38 +655,66 @@ TEST_P(AuditTest, MB51863) {
654655
ASSERT_EQ("0x0", document["collection_id"].get<std::string>());
655656
}
656657

657-
#ifdef WIN32
658-
#define AuditDroppedTest DISABLED_AuditDroppedTest
659-
#endif
660658
TEST_P(AuditTest, AuditDroppedTest) {
659+
using namespace std::filesystem;
661660
auto orgLogDir = mcd_env->getAuditLogDir();
662661
setEnabled(true);
663662

664663
auto stats = getAdminConnection().stats("audit");
665664
// Get the current count for dropped events:
666-
const auto org_dropped = stats["dropped_events"].get<size_t>();
665+
auto org_dropped = stats["dropped_events"].get<size_t>();
667666

668667
auto& json = mcd_env->getAuditConfig();
669-
// Set the audit log to a path which cannot be created
670-
// due to access permissions (not just the file but
671-
// missing path elements in the path which needs to be
672-
// created which we won't have access to create).
673-
json["log_path"] = "/AuditTest/auditlog/myaudit";
674-
try {
675-
mcd_env->rewriteAuditConfig();
676-
} catch (std::exception& e) {
677-
FAIL() << "Failed to toggle audit state: " << e.what();
678-
}
679668

680-
getAdminConnection().reloadAuditConfiguration();
669+
auto writeProtectedDir = orgLogDir / "writeprotected";
670+
auto filesystem_permission_guard = folly::makeGuard([&writeProtectedDir]() {
671+
std::error_code ec;
672+
permissions(
673+
writeProtectedDir, perms::owner_all, perm_options::replace, ec);
674+
if (ec) {
675+
std::cerr << "Failed to restore permissions on '"
676+
<< writeProtectedDir.string() << "': " << ec.message()
677+
<< std::endl;
678+
}
679+
});
681680

682-
stats = getAdminConnection().stats("audit");
683-
while (!stats["enabled"].get<bool>()) {
684-
std::this_thread::sleep_for(std::chrono::milliseconds{10});
685-
stats = getAdminConnection().stats("audit");
681+
create_directories(writeProtectedDir);
682+
#ifdef WIN32
683+
// Seems like we can't remove the ability for a user to create a directory
684+
// in a directory by removing the write permission on the parent directory.
685+
// Let's just create a file with the name of the audit directory. That
686+
// should cause creation of the file to fail ;)
687+
{
688+
auto* fp = fopen((writeProtectedDir / "myaudit").string().c_str(), "w");
689+
ASSERT_NE(nullptr, fp);
690+
fclose(fp);
691+
std::error_code ec;
692+
create_directories(writeProtectedDir / "myaudit", ec);
693+
ASSERT_TRUE(ec) << ec.message();
686694
}
695+
#endif
696+
// Verify that it work with both read-exec and read-only permissions
697+
// set for the directory
698+
for (const auto mode :
699+
{perms::owner_read | perms::owner_exec, perms::owner_read}) {
700+
permissions(writeProtectedDir, mode, perm_options::replace);
701+
json["log_path"] = writeProtectedDir / "myaudit";
702+
try {
703+
mcd_env->rewriteAuditConfig();
704+
} catch (std::exception& e) {
705+
FAIL() << "Failed to toggle audit state: " << e.what();
706+
}
687707

688-
EXPECT_LT(org_dropped, stats["dropped_events"].get<size_t>());
708+
getAdminConnection().reloadAuditConfiguration();
709+
stats = getAdminConnection().stats("audit");
710+
while (!stats["enabled"].get<bool>()) {
711+
std::this_thread::sleep_for(std::chrono::milliseconds{10});
712+
stats = getAdminConnection().stats("audit");
713+
}
714+
715+
EXPECT_LT(org_dropped, stats["dropped_events"].get<size_t>());
716+
org_dropped = stats["dropped_events"].get<size_t>();
717+
}
689718

690719
// Rewrite the config back to the original one
691720
json["log_path"] = orgLogDir;

0 commit comments

Comments
 (0)