From 1afae7623b54b212b2ab30a907cdb73cfb7f08f4 Mon Sep 17 00:00:00 2001 From: sfc-gh-tclinkenbeard Date: Fri, 25 Jun 2021 22:33:26 -0700 Subject: [PATCH] Added /backup/containers/localdir/encrypted unit test --- fdbbackup/backup.actor.cpp | 2 +- fdbclient/BackupContainerFileSystem.actor.cpp | 43 +++++++++++++------ .../BackupContainerLocalDirectory.actor.cpp | 17 ++++---- fdbrpc/AsyncFileEncrypted.actor.cpp | 14 +++--- fdbrpc/AsyncFileEncrypted.h | 1 + fdbserver/workloads/UnitTests.actor.cpp | 16 +++++-- 6 files changed, 63 insertions(+), 30 deletions(-) diff --git a/fdbbackup/backup.actor.cpp b/fdbbackup/backup.actor.cpp index f69b731260..0213bece1a 100644 --- a/fdbbackup/backup.actor.cpp +++ b/fdbbackup/backup.actor.cpp @@ -697,9 +697,9 @@ CSimpleOpt::SOption g_rgRestoreOptions[] = { { OPT_DEVHELP, "--dev-help", SO_NONE }, { OPT_BLOB_CREDENTIALS, "--blob_credentials", SO_REQ_SEP }, { OPT_INCREMENTALONLY, "--incremental", SO_NONE }, - { OPT_ENCRYPTION_KEY_FILE, "--encryption_key_file", SO_REQ_SEP }, { OPT_RESTORE_BEGIN_VERSION, "--begin_version", SO_REQ_SEP }, { OPT_RESTORE_INCONSISTENT_SNAPSHOT_ONLY, "--inconsistent_snapshot_only", SO_NONE }, + { OPT_ENCRYPTION_KEY_FILE, "--encryption_key_file", SO_REQ_SEP }, #ifndef TLS_DISABLED TLS_OPTION_FLAGS #endif diff --git a/fdbclient/BackupContainerFileSystem.actor.cpp b/fdbclient/BackupContainerFileSystem.actor.cpp index 87e4ffcbf0..5d5a7fa722 100644 --- a/fdbclient/BackupContainerFileSystem.actor.cpp +++ b/fdbclient/BackupContainerFileSystem.actor.cpp @@ -1466,7 +1466,7 @@ ACTOR Future writeAndVerifyFile(Reference c, Reference inputFile = wait(c->readFile(f->getFileName())); int64_t fileSize = wait(inputFile->size()); - ASSERT(size == fileSize); + ASSERT_EQ(size, fileSize); if (size > 0) { state Standalone> buf; buf.resize(buf.arena(), fileSize); @@ -1506,12 +1506,28 @@ ACTOR static Future testWriteSnapshotFile(Reference file, Key return Void(); } -ACTOR static Future testBackupContainer(std::string url) { +ACTOR Future createTestEncryptionKeyFile(std::string filename) { + state Reference keyFile = wait(IAsyncFileSystem::filesystem()->open( + filename, + IAsyncFile::OPEN_ATOMIC_WRITE_AND_CREATE | IAsyncFile::OPEN_READWRITE | IAsyncFile::OPEN_CREATE, + 0600)); + std::array testKey; + generateRandomData(testKey.data(), testKey.size()); + keyFile->write(testKey.data(), testKey.size(), 0); + wait(keyFile->sync()); + return Void(); +} + +ACTOR Future testBackupContainer(std::string url, Optional encryptionKeyFileName) { state FlowLock lock(100e6); + if (encryptionKeyFileName.present()) { + wait(createTestEncryptionKeyFile(encryptionKeyFileName.get())); + } + printf("BackupContainerTest URL %s\n", url.c_str()); - state Reference c = IBackupContainer::openContainer(url); + state Reference c = IBackupContainer::openContainer(url, encryptionKeyFileName); // Make sure container doesn't exist, then create it. try { @@ -1655,22 +1671,25 @@ ACTOR static Future testBackupContainer(std::string url) { return Void(); } -TEST_CASE("/backup/containers/localdir") { - if (g_network->isSimulated()) - wait(testBackupContainer(format("file://simfdb/backups/%llx", timer_int()))); - else - wait(testBackupContainer(format("file:///private/tmp/fdb_backups/%llx", timer_int()))); +TEST_CASE("/backup/containers/localdir/unencrypted") { + wait(testBackupContainer(format("file://%s/fdb_backups/%llx", params.getDataDir().c_str(), timer_int()), {})); return Void(); -}; +} + +TEST_CASE("/backup/containers/localdir/encrypted") { + wait(testBackupContainer(format("file://%s/fdb_backups/%llx", params.getDataDir().c_str(), timer_int()), + format("%s/test_encryption_key", params.getDataDir().c_str()))); + return Void(); +} TEST_CASE("/backup/containers/url") { if (!g_network->isSimulated()) { const char* url = getenv("FDB_TEST_BACKUP_URL"); ASSERT(url != nullptr); - wait(testBackupContainer(url)); + wait(testBackupContainer(url, {})); } return Void(); -}; +} TEST_CASE("/backup/containers_list") { if (!g_network->isSimulated()) { @@ -1683,7 +1702,7 @@ TEST_CASE("/backup/containers_list") { } } return Void(); -}; +} TEST_CASE("/backup/time") { // test formatTime() diff --git a/fdbclient/BackupContainerLocalDirectory.actor.cpp b/fdbclient/BackupContainerLocalDirectory.actor.cpp index d94c92441a..e15423df74 100644 --- a/fdbclient/BackupContainerLocalDirectory.actor.cpp +++ b/fdbclient/BackupContainerLocalDirectory.actor.cpp @@ -134,13 +134,10 @@ std::string BackupContainerLocalDirectory::getURLFormat() { ACTOR static Future readEncryptionKey(std::string encryptionKeyFileName) { state Reference keyFile = wait(IAsyncFileSystem::filesystem()->open(encryptionKeyFileName, 0x0, 0400)); - int64_t fileSize = wait(keyFile->size()); - // TODO: Use new error code and avoid hard-coding expected size - if (fileSize != 16) { - throw internal_error(); - } state std::array key; - wait(success(keyFile->read(key.data(), key.size(), 0))); + int bytesRead = wait(keyFile->read(key.data(), key.size(), 0)); + // TODO: Throw new error (fail gracefully) + ASSERT_EQ(bytesRead, key.size()); StreamCipher::Key::initializeKey(std::move(key)); return Void(); } @@ -216,6 +213,10 @@ Future> BackupContainerLocalDirectory::listURLs(const s } Future BackupContainerLocalDirectory::create() { + if (usesEncryption()) { + return encryptionSetupFuture; + } + // TODO: Update this comment: // Nothing should be done here because create() can be called by any process working with the container URL, // such as fdbbackup. Since "local directory" containers are by definition local to the machine they are // accessed from, the container's creation (in this case the creation of a directory) must be ensured prior to @@ -284,8 +285,8 @@ Future> BackupContainerLocalDirectory::readFile(const std: } Future> BackupContainerLocalDirectory::writeFile(const std::string& path) { - int flags = IAsyncFile::OPEN_NO_AIO | IAsyncFile::OPEN_UNCACHED | IAsyncFile::OPEN_CREATE | IAsyncFile::OPEN_ATOMIC_WRITE_AND_CREATE | - IAsyncFile::OPEN_READWRITE; + int flags = IAsyncFile::OPEN_NO_AIO | IAsyncFile::OPEN_UNCACHED | IAsyncFile::OPEN_CREATE | + IAsyncFile::OPEN_ATOMIC_WRITE_AND_CREATE | IAsyncFile::OPEN_READWRITE; if (usesEncryption()) { flags |= IAsyncFile::OPEN_ENCRYPTED; } diff --git a/fdbrpc/AsyncFileEncrypted.actor.cpp b/fdbrpc/AsyncFileEncrypted.actor.cpp index ae37821f50..ffecfae3c1 100644 --- a/fdbrpc/AsyncFileEncrypted.actor.cpp +++ b/fdbrpc/AsyncFileEncrypted.actor.cpp @@ -30,7 +30,10 @@ public: // the filename. static auto getFirstBlockIV(const std::string& filename) { StreamCipher::IV iv; - auto hash = XXH3_128bits(filename.c_str(), filename.size()); + auto salt = basename(filename); + auto pos = salt.find('.'); + salt = salt.substr(0, pos); + auto hash = XXH3_128bits(salt.c_str(), salt.size()); auto high = reinterpret_cast(&hash.high64); auto low = reinterpret_cast(&hash.low64); std::copy(high, high + 8, &iv[0]); @@ -67,7 +70,6 @@ public: self->readBuffers.insert(block, _plaintext); plaintext = _plaintext; } - ASSERT(plaintext.size() == FLOW_KNOBS->ENCRYPTION_BLOCK_SIZE); auto start = (block == firstBlock) ? plaintext.begin() + (offset % FLOW_KNOBS->ENCRYPTION_BLOCK_SIZE) : plaintext.begin(); auto end = (block == lastBlock) @@ -86,7 +88,7 @@ public: ACTOR static Future write(AsyncFileEncrypted* self, void const* data, int length, int64_t offset) { ASSERT(self->canWrite); // All writes must append to the end of the file: - ASSERT(offset == self->currentBlock * FLOW_KNOBS->ENCRYPTION_BLOCK_SIZE + self->offsetInBlock); + ASSERT_EQ(offset, self->currentBlock * FLOW_KNOBS->ENCRYPTION_BLOCK_SIZE + self->offsetInBlock); state unsigned char const* input = reinterpret_cast(data); while (length > 0) { const auto chunkSize = std::min(length, FLOW_KNOBS->ENCRYPTION_BLOCK_SIZE - self->offsetInBlock); @@ -156,11 +158,13 @@ Future AsyncFileEncrypted::zeroRange(int64_t offset, int64_t length) { } Future AsyncFileEncrypted::truncate(int64_t size) { - ASSERT(false); // TODO: Not yet implemented + // FIXME: Not yet implemented + ASSERT(canWrite); return Void(); } Future AsyncFileEncrypted::sync() { + ASSERT(canWrite); return AsyncFileEncryptedImpl::sync(this); } @@ -169,7 +173,7 @@ Future AsyncFileEncrypted::flush() { } Future AsyncFileEncrypted::size() const { - return currentBlock * FLOW_KNOBS->ENCRYPTION_BLOCK_SIZE + offsetInBlock; + return file->size(); } std::string AsyncFileEncrypted::getFilename() const { diff --git a/fdbrpc/AsyncFileEncrypted.h b/fdbrpc/AsyncFileEncrypted.h index b474198e1c..af123f2650 100644 --- a/fdbrpc/AsyncFileEncrypted.h +++ b/fdbrpc/AsyncFileEncrypted.h @@ -57,6 +57,7 @@ class AsyncFileEncrypted : public IAsyncFile, public ReferenceCounted writeBuffer; + Future initialize(); public: AsyncFileEncrypted(Reference, bool canWrite); diff --git a/fdbserver/workloads/UnitTests.actor.cpp b/fdbserver/workloads/UnitTests.actor.cpp index 285eb6fca5..a3e0dd7508 100644 --- a/fdbserver/workloads/UnitTests.actor.cpp +++ b/fdbserver/workloads/UnitTests.actor.cpp @@ -39,6 +39,7 @@ struct UnitTestWorkload : TestWorkload { std::string testPattern; int testRunLimit; UnitTestParameters testParams; + bool cleanupAfterTests; PerfIntCounter testsAvailable, testsExecuted, testsFailed; PerfDoubleCounter totalWallTime, totalSimTime; @@ -48,9 +49,14 @@ struct UnitTestWorkload : TestWorkload { testsFailed("Test Cases Failed"), totalWallTime("Total wall clock time (s)"), totalSimTime("Total flow time (s)") { enabled = !clientId; // only do this on the "first" client - testPattern = getOption(options, LiteralStringRef("testsMatching"), Value()).toString(); - testRunLimit = getOption(options, LiteralStringRef("maxTestCases"), -1); - testParams.setDataDir(getOption(options, LiteralStringRef("dataDir"), "simfdb/unittests/"_sr).toString()); + testPattern = getOption(options, "testsMatching"_sr, Value()).toString(); + testRunLimit = getOption(options, "maxTestCases"_sr, -1); + if (g_network->isSimulated()) { + testParams.setDataDir(getOption(options, "dataDir"_sr, "simfdb/unittests/"_sr).toString()); + } else { + testParams.setDataDir(getOption(options, "dataDir"_sr, "/private/tmp/"_sr).toString()); + } + cleanupAfterTests = getOption(options, "cleanupAfterTests"_sr, true); // Consume all remaining options as testParams which the unit test can access for (auto& kv : options) { @@ -121,7 +127,9 @@ struct UnitTestWorkload : TestWorkload { ++self->testsFailed; result = e; } - platform::eraseDirectoryRecursive(self->testParams.getDataDir()); + if (self->cleanupAfterTests) { + platform::eraseDirectoryRecursive(self->testParams.getDataDir()); + } ++self->testsExecuted; double wallTime = timer() - start_timer; double simTime = now() - start_now;