From 3d6515bd149b848fb46ba6b6a1feca5de391cbc7 Mon Sep 17 00:00:00 2001 From: sfc-gh-tclinkenbeard Date: Sat, 26 Jun 2021 00:07:27 -0700 Subject: [PATCH] Support encryption for blob store backups (not yet tested) --- fdbclient/AsyncFileS3BlobStore.actor.h | 2 +- fdbclient/BackupContainer.actor.cpp | 8 ++-- .../BackupContainerAzureBlobStore.actor.cpp | 39 +++++++++++++------ fdbclient/BackupContainerAzureBlobStore.h | 3 +- fdbclient/BackupContainerFileSystem.actor.cpp | 22 +++++++++++ fdbclient/BackupContainerFileSystem.h | 8 ++++ .../BackupContainerLocalDirectory.actor.cpp | 24 ++---------- fdbclient/BackupContainerLocalDirectory.h | 2 - .../BackupContainerS3BlobStore.actor.cpp | 33 +++++++++++----- fdbclient/BackupContainerS3BlobStore.h | 3 +- fdbrpc/AsyncFileEncrypted.h | 1 + 11 files changed, 94 insertions(+), 51 deletions(-) diff --git a/fdbclient/AsyncFileS3BlobStore.actor.h b/fdbclient/AsyncFileS3BlobStore.actor.h index bc520bda90..db436755b3 100644 --- a/fdbclient/AsyncFileS3BlobStore.actor.h +++ b/fdbclient/AsyncFileS3BlobStore.actor.h @@ -256,7 +256,7 @@ public: m_concurrentUploads(bstore->knobs.concurrent_writes_per_file) { // Add first part - m_parts.push_back(Reference(new Part(1, m_bstore->knobs.multipart_min_part_size))); + m_parts.push_back(makeReference(1, m_bstore->knobs.multipart_min_part_size)); } }; diff --git a/fdbclient/BackupContainer.actor.cpp b/fdbclient/BackupContainer.actor.cpp index 29fef0486a..fb7edbf753 100644 --- a/fdbclient/BackupContainer.actor.cpp +++ b/fdbclient/BackupContainer.actor.cpp @@ -58,6 +58,7 @@ ACTOR Future appendStringRefWithLen(Reference file, Standalon wait(file->append(s.begin(), s.size())); return Void(); } + } // namespace IBackupFile_impl Future IBackupFile::appendStringRefWithLen(Standalone s) { @@ -278,7 +279,7 @@ Reference IBackupContainer::openContainer(const std::string& u for (auto c : resource) if (!isalnum(c) && c != '_' && c != '-' && c != '.' && c != '/') throw backup_invalid_url(); - r = makeReference(bstore, resource, backupParams); + r = makeReference(bstore, resource, backupParams, encryptionKeyFileName); } #ifdef BUILD_AZURE_BACKUP else if (u.startsWith("azure://"_sr)) { @@ -286,7 +287,8 @@ Reference IBackupContainer::openContainer(const std::string& u auto address = NetworkAddress::parse(u.eat("/"_sr).toString()); auto containerName = u.eat("/"_sr).toString(); auto accountName = u.eat("/"_sr).toString(); - r = makeReference(address, containerName, accountName); + r = makeReference( + address, containerName, accountName, encryptionKeyFileName); } #endif else { @@ -334,7 +336,7 @@ ACTOR Future> listContainers_impl(std::string baseURL) } // Create a dummy container to parse the backup-specific parameters from the URL and get a final bucket name - BackupContainerS3BlobStore dummy(bstore, "dummy", backupParams); + BackupContainerS3BlobStore dummy(bstore, "dummy", backupParams, {}); std::vector results = wait(BackupContainerS3BlobStore::listURLs(bstore, dummy.getBucket())); return results; diff --git a/fdbclient/BackupContainerAzureBlobStore.actor.cpp b/fdbclient/BackupContainerAzureBlobStore.actor.cpp index dea07c382e..3af1a4a7cd 100644 --- a/fdbclient/BackupContainerAzureBlobStore.actor.cpp +++ b/fdbclient/BackupContainerAzureBlobStore.actor.cpp @@ -19,6 +19,7 @@ */ #include "fdbclient/BackupContainerAzureBlobStore.h" +#include "fdbrpc/AsyncFileEncrypted.h" #include "flow/actorcompiler.h" // This must be the last #include. @@ -167,8 +168,13 @@ public: if (!exists) { throw file_not_found(); } - return Reference( - new ReadFile(self->asyncTaskThread, self->containerName, fileName, self->client.get())); + Reference f = + makeReference(self->asyncTaskThread, self->containerName, fileName, self->client.get()); + if (self->usesEncryption()) { + f = makeReference(f, false); + } + f = makeReference(self->asyncTaskThread, self->containerName, fileName, self->client.get()); + return f; } ACTOR static Future> writeFile(BackupContainerAzureBlobStore* self, std::string fileName) { @@ -177,10 +183,11 @@ public: auto outcome = client->create_append_blob(containerName, fileName).get(); return Void(); })); - return Reference( - new BackupFile(fileName, - Reference(new WriteFile( - self->asyncTaskThread, self->containerName, fileName, self->client.get())))); + auto f = makeReference(self->asyncTaskThread, self->containerName, fileName, self->client.get()); + if (self->usesEncryption()) { + f = makeReference(f, true); + } + return makeReference(fileName, f); } static void listFiles(AzureClient* client, @@ -213,6 +220,16 @@ public: } return Void(); } + + ACTOR static Future create(BackupContainerAzureBlobStore* self) { + state Future f1 = + self->asyncTaskThread.execAsync([containerName = self->containerName, client = self->client.get()] { + client->create_container(containerName).wait(); + return Void(); + }); + state Future f2 = self->usesEncryption() ? self->encryptionSetupComplete() : Void(); + return f1 && f2; + } }; Future BackupContainerAzureBlobStore::blobExists(const std::string& fileName) { @@ -225,10 +242,11 @@ Future BackupContainerAzureBlobStore::blobExists(const std::string& fileNa BackupContainerAzureBlobStore::BackupContainerAzureBlobStore(const NetworkAddress& address, const std::string& accountName, - const std::string& containerName) + const std::string& containerName, + const Optional& encryptionKeyFileName) : containerName(containerName) { + setEncryptionKey(encryptionKeyFileName); std::string accountKey = std::getenv("AZURE_KEY"); - auto credential = std::make_shared(accountName, accountKey); auto storageAccount = std::make_shared( accountName, credential, false, format("http://%s/%s", address.toString().c_str(), accountName.c_str())); @@ -244,10 +262,7 @@ void BackupContainerAzureBlobStore::delref() { } Future BackupContainerAzureBlobStore::create() { - return asyncTaskThread.execAsync([containerName = this->containerName, client = this->client.get()] { - client->create_container(containerName).wait(); - return Void(); - }); + return BackupContainerAzureBlobStoreImpl::create(this); } Future BackupContainerAzureBlobStore::exists() { return asyncTaskThread.execAsync([containerName = this->containerName, client = this->client.get()] { diff --git a/fdbclient/BackupContainerAzureBlobStore.h b/fdbclient/BackupContainerAzureBlobStore.h index 193fe4a301..aae378fcf4 100644 --- a/fdbclient/BackupContainerAzureBlobStore.h +++ b/fdbclient/BackupContainerAzureBlobStore.h @@ -44,7 +44,8 @@ class BackupContainerAzureBlobStore final : public BackupContainerFileSystem, public: BackupContainerAzureBlobStore(const NetworkAddress& address, const std::string& accountName, - const std::string& containerName); + const std::string& containerName, + const Optional& encryptionKeyFileName); void addref() override; void delref() override; diff --git a/fdbclient/BackupContainerFileSystem.actor.cpp b/fdbclient/BackupContainerFileSystem.actor.cpp index 5d5a7fa722..94b2c10d99 100644 --- a/fdbclient/BackupContainerFileSystem.actor.cpp +++ b/fdbclient/BackupContainerFileSystem.actor.cpp @@ -23,6 +23,7 @@ #include "fdbclient/BackupContainerFileSystem.h" #include "fdbclient/BackupContainerLocalDirectory.h" #include "fdbclient/JsonBuilder.h" +#include "flow/StreamCipher.h" #include "flow/UnitTest.h" #include @@ -1126,6 +1127,16 @@ public: return false; } + ACTOR static Future readEncryptionKey(std::string encryptionKeyFileName) { + state Reference keyFile = + wait(IAsyncFileSystem::filesystem()->open(encryptionKeyFileName, 0x0, 0400)); + state std::array key; + 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(); + } }; // class BackupContainerFileSystemImpl Future> BackupContainerFileSystem::writeLogFile(Version beginVersion, @@ -1432,6 +1443,17 @@ BackupContainerFileSystem::VersionProperty BackupContainerFileSystem::unreliable BackupContainerFileSystem::VersionProperty BackupContainerFileSystem::logType() { return { Reference::addRef(this), "mutation_log_type" }; } +bool BackupContainerFileSystem::usesEncryption() const { + return encryptionSetupFuture.isValid(); +} +Future BackupContainerFileSystem::encryptionSetupComplete() const { + return encryptionSetupFuture; +} +void BackupContainerFileSystem::setEncryptionKey(Optional const& encryptionKeyFileName) { + if (encryptionKeyFileName.present()) { + encryptionSetupFuture = BackupContainerFileSystemImpl::readEncryptionKey(encryptionKeyFileName.get()); + } +} namespace backup_test { diff --git a/fdbclient/BackupContainerFileSystem.h b/fdbclient/BackupContainerFileSystem.h index cd0ddf4435..8c0967e701 100644 --- a/fdbclient/BackupContainerFileSystem.h +++ b/fdbclient/BackupContainerFileSystem.h @@ -186,6 +186,14 @@ private: Future> old_listRangeFiles(Version beginVersion, Version endVersion); friend class BackupContainerFileSystemImpl; + +protected: + bool usesEncryption() const; + void setEncryptionKey(Optional const& encryptionKeyFileName); + Future encryptionSetupComplete() const; + +private: + Future encryptionSetupFuture; }; #endif diff --git a/fdbclient/BackupContainerLocalDirectory.actor.cpp b/fdbclient/BackupContainerLocalDirectory.actor.cpp index e15423df74..b89d085a64 100644 --- a/fdbclient/BackupContainerLocalDirectory.actor.cpp +++ b/fdbclient/BackupContainerLocalDirectory.actor.cpp @@ -23,7 +23,6 @@ #include "fdbrpc/IAsyncFile.h" #include "flow/Platform.actor.h" #include "flow/Platform.h" -#include "flow/StreamCipher.h" #include "fdbrpc/simulator.h" #include "flow/actorcompiler.h" // This must be the last #include. @@ -132,25 +131,9 @@ std::string BackupContainerLocalDirectory::getURLFormat() { return "file://"; } -ACTOR static Future readEncryptionKey(std::string encryptionKeyFileName) { - state Reference keyFile = wait(IAsyncFileSystem::filesystem()->open(encryptionKeyFileName, 0x0, 0400)); - state std::array key; - 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(); -} - -bool BackupContainerLocalDirectory::usesEncryption() const { - return encryptionSetupFuture.isValid(); -} - BackupContainerLocalDirectory::BackupContainerLocalDirectory(const std::string& url, const Optional& encryptionKeyFileName) { - if (encryptionKeyFileName.present()) { - encryptionSetupFuture = readEncryptionKey(encryptionKeyFileName.get()); - } + setEncryptionKey(encryptionKeyFileName); std::string path; if (url.find("file://") != 0) { @@ -214,10 +197,9 @@ Future> BackupContainerLocalDirectory::listURLs(const s Future BackupContainerLocalDirectory::create() { if (usesEncryption()) { - return encryptionSetupFuture; + return encryptionSetupComplete(); } - // TODO: Update this comment: - // Nothing should be done here because create() can be called by any process working with the container URL, + // No directory should be created 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 // every file creation, which is done in openFile(). Creating the directory here will result in unnecessary diff --git a/fdbclient/BackupContainerLocalDirectory.h b/fdbclient/BackupContainerLocalDirectory.h index 52cd810907..f7c77e4636 100644 --- a/fdbclient/BackupContainerLocalDirectory.h +++ b/fdbclient/BackupContainerLocalDirectory.h @@ -54,8 +54,6 @@ public: private: std::string m_path; - Future encryptionSetupFuture; - bool usesEncryption() const; }; #endif diff --git a/fdbclient/BackupContainerS3BlobStore.actor.cpp b/fdbclient/BackupContainerS3BlobStore.actor.cpp index 4e89402ae0..2144347cab 100644 --- a/fdbclient/BackupContainerS3BlobStore.actor.cpp +++ b/fdbclient/BackupContainerS3BlobStore.actor.cpp @@ -20,6 +20,7 @@ #include "fdbclient/AsyncFileS3BlobStore.actor.h" #include "fdbclient/BackupContainerS3BlobStore.h" +#include "fdbrpc/AsyncFileEncrypted.h" #include "fdbrpc/AsyncFileReadAhead.actor.h" #include "flow/actorcompiler.h" // This must be the last #include. @@ -103,6 +104,10 @@ public: wait(bc->m_bstore->writeEntireFile(bc->m_bucket, bc->indexEntry(), "")); } + if (bc->usesEncryption()) { + wait(bc->encryptionSetupComplete()); + } + return Void(); } @@ -137,9 +142,10 @@ std::string BackupContainerS3BlobStore::indexEntry() { BackupContainerS3BlobStore::BackupContainerS3BlobStore(Reference bstore, const std::string& name, - const S3BlobStoreEndpoint::ParametersT& params) + const S3BlobStoreEndpoint::ParametersT& params, + const Optional& encryptionKeyFileName) : m_bstore(bstore), m_name(name), m_bucket("FDB_BACKUPS_V2") { - + setEncryptionKey(encryptionKeyFileName); // Currently only one parameter is supported, "bucket" for (const auto& [name, value] : params) { if (name == "bucket") { @@ -164,12 +170,16 @@ std::string BackupContainerS3BlobStore::getURLFormat() { } Future> BackupContainerS3BlobStore::readFile(const std::string& path) { - return Reference(new AsyncFileReadAheadCache( - Reference(new AsyncFileS3BlobStoreRead(m_bstore, m_bucket, dataPath(path))), - m_bstore->knobs.read_block_size, - m_bstore->knobs.read_ahead_blocks, - m_bstore->knobs.concurrent_reads_per_file, - m_bstore->knobs.read_cache_blocks_per_file)); + Reference f = makeReference(m_bstore, m_bucket, dataPath(path)); + if (usesEncryption()) { + f = makeReference(f, false); + } + f = makeReference(f, + m_bstore->knobs.read_block_size, + m_bstore->knobs.read_ahead_blocks, + m_bstore->knobs.concurrent_reads_per_file, + m_bstore->knobs.read_cache_blocks_per_file); + return f; } Future> BackupContainerS3BlobStore::listURLs(Reference bstore, @@ -178,8 +188,11 @@ Future> BackupContainerS3BlobStore::listURLs(Reference< } Future> BackupContainerS3BlobStore::writeFile(const std::string& path) { - return Reference(new BackupContainerS3BlobStoreImpl::BackupFile( - path, Reference(new AsyncFileS3BlobStoreWrite(m_bstore, m_bucket, dataPath(path))))); + Reference f = makeReference(m_bstore, m_bucket, dataPath(path)); + if (usesEncryption()) { + f = makeReference(f, true); + } + return Future>(makeReference(path, f)); } Future BackupContainerS3BlobStore::deleteFile(const std::string& path) { diff --git a/fdbclient/BackupContainerS3BlobStore.h b/fdbclient/BackupContainerS3BlobStore.h index 57199fcb85..9e47483adf 100644 --- a/fdbclient/BackupContainerS3BlobStore.h +++ b/fdbclient/BackupContainerS3BlobStore.h @@ -43,7 +43,8 @@ class BackupContainerS3BlobStore final : public BackupContainerFileSystem, public: BackupContainerS3BlobStore(Reference bstore, const std::string& name, - const S3BlobStoreEndpoint::ParametersT& params); + const S3BlobStoreEndpoint::ParametersT& params, + const Optional& encryptionKeyFileName); void addref() override; void delref() override; diff --git a/fdbrpc/AsyncFileEncrypted.h b/fdbrpc/AsyncFileEncrypted.h index af123f2650..6be26c8793 100644 --- a/fdbrpc/AsyncFileEncrypted.h +++ b/fdbrpc/AsyncFileEncrypted.h @@ -60,6 +60,7 @@ class AsyncFileEncrypted : public IAsyncFile, public ReferenceCounted initialize(); public: + // TODO: Remove boolean parameter here: AsyncFileEncrypted(Reference, bool canWrite); void addref() override; void delref() override;