From 7e56c46bb08d030f7a16904edc7e99b8262008c6 Mon Sep 17 00:00:00 2001 From: Sebastian Messmer Date: Tue, 5 Sep 2017 00:43:43 +0100 Subject: [PATCH] CryFS tells the operating system to not swap the encryption key to the disk (note: this is best-effort and cannot be guaranteed. Hibernation, for example, will still write the encryption key to the disk) --- ChangeLog.txt | 5 +- src/cpp-utils/CMakeLists.txt | 2 + src/cpp-utils/crypto/symmetric/CFB_Cipher.h | 9 +- src/cpp-utils/crypto/symmetric/Cipher.h | 2 +- .../crypto/symmetric/EncryptionKey.cpp | 1 + .../crypto/symmetric/EncryptionKey.h | 107 ++++++++++++++++++ src/cpp-utils/crypto/symmetric/GCM_Cipher.h | 9 +- src/cpp-utils/random/RandomGenerator.h | 6 + src/cpp-utils/system/memory.cpp | 26 +++++ src/cpp-utils/system/memory.h | 23 ++++ src/cryfs/config/CryCipher.cpp | 2 +- src/stats/main.cpp | 2 +- test/cpp-utils/CMakeLists.txt | 1 + .../testutils/FakeAuthenticatedCipher.h | 10 +- test/cpp-utils/system/MemoryTest.cpp | 25 ++++ test/cryfs/config/CryCipherTest.cpp | 2 +- test/cryfs/filesystem/testutils/CryTestBase.h | 2 +- 17 files changed, 209 insertions(+), 25 deletions(-) create mode 100644 src/cpp-utils/crypto/symmetric/EncryptionKey.cpp create mode 100644 src/cpp-utils/crypto/symmetric/EncryptionKey.h create mode 100644 src/cpp-utils/system/memory.cpp create mode 100644 src/cpp-utils/system/memory.h create mode 100644 test/cpp-utils/system/MemoryTest.cpp diff --git a/ChangeLog.txt b/ChangeLog.txt index 0b0200e1..06858342 100644 --- a/ChangeLog.txt +++ b/ChangeLog.txt @@ -1,14 +1,13 @@ Version 0.10.0 (unreleased) --------------- -New Features: +New Features & Improvements: * Integrity checks ensure you notice when someone modifies your file system. * File system nodes (files, directories, symlinks) store a parent pointer to the directory that contains them. This information can be used in later versions to resolve some synchronization conflicts. * Allow mounting using system mount tool and /etc/fstab (e.g. mount -t fuse.cryfs basedir mountdir) - -Improvements: * Performance improvements * Use relatime instead of strictatime (further performance improvement) * Pass fuse options directly to cryfs (i.e. 'cryfs basedir mountdir -o allow_other' instead of 'cryfs basedir mountdir -- -o allow_other') +* CryFS tells the operating system to not swap the encryption key to the disk (note: this is best-effort and cannot be guaranteed. Hibernation, for example, will still write the encryption key to the disk) Fixed bugs: * `du` shows correct file system size on Mac OS X. diff --git a/src/cpp-utils/CMakeLists.txt b/src/cpp-utils/CMakeLists.txt index ec361d3e..a9c000be 100644 --- a/src/cpp-utils/CMakeLists.txt +++ b/src/cpp-utils/CMakeLists.txt @@ -6,6 +6,7 @@ set(SOURCES crypto/kdf/SCryptParameters.cpp crypto/kdf/PasswordBasedKDF.cpp crypto/RandomPadding.cpp + crypto/symmetric/EncryptionKey.cpp process/daemonize.cpp process/subprocess.cpp tempfile/TempFile.cpp @@ -37,6 +38,7 @@ set(SOURCES assert/AssertFailed.cpp system/get_total_memory.cpp system/homedir.cpp + system/memory.cpp ) add_library(${PROJECT_NAME} STATIC ${SOURCES}) diff --git a/src/cpp-utils/crypto/symmetric/CFB_Cipher.h b/src/cpp-utils/crypto/symmetric/CFB_Cipher.h index 3d9d5e1d..9de10c3d 100644 --- a/src/cpp-utils/crypto/symmetric/CFB_Cipher.h +++ b/src/cpp-utils/crypto/symmetric/CFB_Cipher.h @@ -9,17 +9,14 @@ #include #include #include "Cipher.h" +#include "EncryptionKey.h" namespace cpputils { template class CFB_Cipher { public: - using EncryptionKey = FixedSizeData; - - static EncryptionKey CreateKey(RandomGenerator &randomGenerator) { - return randomGenerator.getFixedSize(); - } + using EncryptionKey = cpputils::EncryptionKey; static constexpr unsigned int ciphertextSize(unsigned int plaintextBlockSize) { return plaintextBlockSize + IV_SIZE; @@ -39,7 +36,7 @@ private: template Data CFB_Cipher::encrypt(const CryptoPP::byte *plaintext, unsigned int plaintextSize, const EncryptionKey &encKey) { FixedSizeData iv = Random::PseudoRandom().getFixedSize(); - auto encryption = typename CryptoPP::CFB_Mode::Encryption(encKey.data(), encKey.BINARY_LENGTH, iv.data()); + auto encryption = typename CryptoPP::CFB_Mode::Encryption((CryptoPP::byte*)encKey.data(), encKey.BINARY_LENGTH, iv.data()); Data ciphertext(ciphertextSize(plaintextSize)); std::memcpy(ciphertext.data(), iv.data(), IV_SIZE); encryption.ProcessData((CryptoPP::byte*)ciphertext.data() + IV_SIZE, plaintext, plaintextSize); diff --git a/src/cpp-utils/crypto/symmetric/Cipher.h b/src/cpp-utils/crypto/symmetric/Cipher.h index 196dbd92..e6327f48 100644 --- a/src/cpp-utils/crypto/symmetric/Cipher.h +++ b/src/cpp-utils/crypto/symmetric/Cipher.h @@ -17,7 +17,7 @@ public: BOOST_CONCEPT_USAGE(CipherConcept) { same_type(UINT32_C(0), X::ciphertextSize(UINT32_C(5))); same_type(UINT32_C(0), X::plaintextSize(UINT32_C(5))); - typename X::EncryptionKey key = X::CreateKey(Random::OSRandom()); + typename X::EncryptionKey key = X::EncryptionKey::CreateKey(Random::OSRandom()); same_type(Data(0), X::encrypt((uint8_t*)nullptr, UINT32_C(0), key)); same_type(boost::optional(Data(0)), X::decrypt((uint8_t*)nullptr, UINT32_C(0), key)); string name = X::NAME; diff --git a/src/cpp-utils/crypto/symmetric/EncryptionKey.cpp b/src/cpp-utils/crypto/symmetric/EncryptionKey.cpp new file mode 100644 index 00000000..18a97b45 --- /dev/null +++ b/src/cpp-utils/crypto/symmetric/EncryptionKey.cpp @@ -0,0 +1 @@ +#include "EncryptionKey.h" diff --git a/src/cpp-utils/crypto/symmetric/EncryptionKey.h b/src/cpp-utils/crypto/symmetric/EncryptionKey.h new file mode 100644 index 00000000..48ed8269 --- /dev/null +++ b/src/cpp-utils/crypto/symmetric/EncryptionKey.h @@ -0,0 +1,107 @@ +#pragma once +#ifndef MESSMER_CPPUTILS_CRYPTO_SYMMETRIC_ENCRYPTIONKEY_H_ +#define MESSMER_CPPUTILS_CRYPTO_SYMMETRIC_ENCRYPTIONKEY_H_ + +#include +#include +#include +#include "../cryptopp_byte.h" +#include + +namespace cpputils { + +/** + * Use this to store an encryption key and keep it safe in memory. + * It will only keep the key in one memory location, even if the EncryptionKey object is copied or moved. + * This one memory location will be prevented from swapping to disk. + * Note: This is a best-effort, but not a guarantee. System hibernation might still write the encryption key to the disk. + * Also, when (de)serializing the config file or calling a crypto library with the encryption key, it isn't guaranteed + * that there aren't any copies made to different memory regions. However, these other memory regions should be short-lived + * and therefore much less likely to swap. + */ +template +class EncryptionKey final { +private: + struct EncryptionKeyData final { + public: + constexpr static size_t BINARY_LENGTH = FixedSizeData::BINARY_LENGTH; + constexpr static size_t STRING_LENGTH = FixedSizeData::STRING_LENGTH; + + EncryptionKeyData(const FixedSizeData& keyData); + ~EncryptionKeyData(); + + // Disallow copying and moving + EncryptionKeyData(const EncryptionKeyData &rhs) = delete; + EncryptionKeyData(EncryptionKeyData &&rhs) = delete; + EncryptionKeyData &operator=(const EncryptionKeyData &rhs) = delete; + EncryptionKeyData &operator=(EncryptionKeyData &&rhs) = delete; + + FixedSizeData key; + DontSwapMemoryRAII memoryProtector; // this makes sure that the key data isn't swapped to disk + }; + +public: + constexpr static size_t BINARY_LENGTH = EncryptionKeyData::BINARY_LENGTH; + constexpr static size_t STRING_LENGTH = EncryptionKeyData::STRING_LENGTH; + + EncryptionKey(const FixedSizeData& keyData); + + static EncryptionKey FromBinary(const void *source); + static EncryptionKey FromString(const std::string& keyData); + std::string ToString() const; + + static EncryptionKey CreateKey(RandomGenerator &randomGenerator) { + EncryptionKey result(FixedSizeData::Null()); + randomGenerator.write(result._key->key.data(), BINARY_LENGTH); + return result; + } + + const void *data() const { + return _key->key.data(); + } + + void *data() { + return const_cast(const_cast(this)->data()); + } + +private: + + std::shared_ptr _key; +}; + +template +inline EncryptionKey::EncryptionKeyData::EncryptionKeyData(const FixedSizeData& keyData) +: key(keyData) +, memoryProtector(&key, sizeof(key)) { +} + +template +inline EncryptionKey::EncryptionKeyData::~EncryptionKeyData() { + // After destruction, the swap-protection is lifted, but we also don't need the key data anymore. + // Overwrite it with zeroes. + std::memset(key.data(), 0, KeySize); +} + +template +inline EncryptionKey::EncryptionKey(const FixedSizeData& keyData) +: _key(std::make_shared(keyData)) { +} + +template +EncryptionKey EncryptionKey::FromBinary(const void *source) { + return EncryptionKey(FixedSizeData::FromBinary(source)); +} + +template +EncryptionKey EncryptionKey::FromString(const std::string& keyData) { + return EncryptionKey(FixedSizeData::FromString(keyData)); +} + +template +std::string EncryptionKey::ToString() const { + return _key->key.ToString(); +} + +} + +#endif diff --git a/src/cpp-utils/crypto/symmetric/GCM_Cipher.h b/src/cpp-utils/crypto/symmetric/GCM_Cipher.h index c7bbd7a5..8f9952b2 100644 --- a/src/cpp-utils/crypto/symmetric/GCM_Cipher.h +++ b/src/cpp-utils/crypto/symmetric/GCM_Cipher.h @@ -8,17 +8,14 @@ #include "../../random/Random.h" #include #include "Cipher.h" +#include "EncryptionKey.h" namespace cpputils { template class GCM_Cipher { public: - using EncryptionKey = FixedSizeData; - - static EncryptionKey CreateKey(RandomGenerator &randomGenerator) { - return randomGenerator.getFixedSize(); - } + using EncryptionKey = cpputils::EncryptionKey; static constexpr unsigned int ciphertextSize(unsigned int plaintextBlockSize) { return plaintextBlockSize + IV_SIZE + TAG_SIZE; @@ -40,7 +37,7 @@ template Data GCM_Cipher::encrypt(const CryptoPP::byte *plaintext, unsigned int plaintextSize, const EncryptionKey &encKey) { FixedSizeData iv = Random::PseudoRandom().getFixedSize(); typename CryptoPP::GCM::Encryption encryption; - encryption.SetKeyWithIV(encKey.data(), encKey.BINARY_LENGTH, iv.data(), IV_SIZE); + encryption.SetKeyWithIV((CryptoPP::byte*)encKey.data(), encKey.BINARY_LENGTH, iv.data(), IV_SIZE); Data ciphertext(ciphertextSize(plaintextSize)); std::memcpy(ciphertext.data(), iv.data(), IV_SIZE); diff --git a/src/cpp-utils/random/RandomGenerator.h b/src/cpp-utils/random/RandomGenerator.h index b0d0ba97..8cb2dd0e 100644 --- a/src/cpp-utils/random/RandomGenerator.h +++ b/src/cpp-utils/random/RandomGenerator.h @@ -12,6 +12,8 @@ namespace cpputils { template FixedSizeData getFixedSize(); Data get(size_t size); + void write(void *target, size_t size); + protected: virtual void _get(void *target, size_t bytes) = 0; private: @@ -23,6 +25,10 @@ namespace cpputils { inline RandomGenerator::RandomGenerator() { } + inline void RandomGenerator::write(void *target, size_t size) { + _get(target, size); + } + template inline FixedSizeData RandomGenerator::getFixedSize() { FixedSizeData result = FixedSizeData::Null(); _get(result.data(), SIZE); diff --git a/src/cpp-utils/system/memory.cpp b/src/cpp-utils/system/memory.cpp new file mode 100644 index 00000000..04ddd776 --- /dev/null +++ b/src/cpp-utils/system/memory.cpp @@ -0,0 +1,26 @@ +#include "memory.h" +#include +#include +#include +#include + +using namespace cpputils::logging; + +namespace cpputils { + +DontSwapMemoryRAII::DontSwapMemoryRAII(const void *addr, size_t len) +: addr_(addr), len_(len) { + const int result = ::mlock(addr_, len_); + if (0 != result) { + throw std::runtime_error("Error calling mlock. Errno: " + std::to_string(errno)); + } +} + +DontSwapMemoryRAII::~DontSwapMemoryRAII() { + const int result = ::munlock(addr_, len_); + if (0 != result) { + LOG(WARN, "Error calling munlock. Errno: {}", errno); + } +} + +} diff --git a/src/cpp-utils/system/memory.h b/src/cpp-utils/system/memory.h new file mode 100644 index 00000000..95f0c664 --- /dev/null +++ b/src/cpp-utils/system/memory.h @@ -0,0 +1,23 @@ +#pragma once +#ifndef MESSMER_CPPUTILS_SYSTEM_MEMORY_H +#define MESSMER_CPPUTILS_SYSTEM_MEMORY_H + +#include + +namespace cpputils { + +// While this RAII object exists, it locks a given memory address into RAM, +// i.e. tells the operating system not to swap it. +class DontSwapMemoryRAII final { +public: + DontSwapMemoryRAII(const void* addr, size_t len); + ~DontSwapMemoryRAII(); + +private: + const void* addr_; + const size_t len_; +}; + +} + +#endif diff --git a/src/cryfs/config/CryCipher.cpp b/src/cryfs/config/CryCipher.cpp index e881bc4d..bb84c040 100644 --- a/src/cryfs/config/CryCipher.cpp +++ b/src/cryfs/config/CryCipher.cpp @@ -44,7 +44,7 @@ public: } string createKey(cpputils::RandomGenerator &randomGenerator) const override { - return Cipher::CreateKey(randomGenerator).ToString(); + return Cipher::EncryptionKey::CreateKey(randomGenerator).ToString(); } unique_ref createInnerConfigEncryptor(const FixedSizeData &key) const override { diff --git a/src/stats/main.cpp b/src/stats/main.cpp index 15b885b3..575960f7 100644 --- a/src/stats/main.cpp +++ b/src/stats/main.cpp @@ -58,7 +58,7 @@ set _getBlockstoreUnaccountedBlocks(const CryConfig &config) { i = 0; cout << "\nRemove blocks that have a parent" << endl; //Remove root block from unaccountedBlocks - unaccountedBlocks.erase(Key::FromString(config.RootBlob())); + unaccountedBlocks.erase(Key::FrComString(config.RootBlob())); //Remove all blocks that have a parent node from unaccountedBlocks for (auto file = directory_iterator("/home/heinzi/basedir"); file != directory_iterator(); ++file) { cout << "\r" << (++i) << "/" << numBlocks << flush; diff --git a/test/cpp-utils/CMakeLists.txt b/test/cpp-utils/CMakeLists.txt index 5a2bc9f8..1a751855 100644 --- a/test/cpp-utils/CMakeLists.txt +++ b/test/cpp-utils/CMakeLists.txt @@ -46,6 +46,7 @@ set(SOURCES assert/backtrace_include_test.cpp assert/assert_include_test.cpp assert/assert_debug_test.cpp + system/MemoryTest.cpp ) add_executable(${PROJECT_NAME} ${SOURCES}) diff --git a/test/cpp-utils/crypto/symmetric/testutils/FakeAuthenticatedCipher.h b/test/cpp-utils/crypto/symmetric/testutils/FakeAuthenticatedCipher.h index 7ec77e27..7b10c008 100644 --- a/test/cpp-utils/crypto/symmetric/testutils/FakeAuthenticatedCipher.h +++ b/test/cpp-utils/crypto/symmetric/testutils/FakeAuthenticatedCipher.h @@ -17,6 +17,11 @@ namespace cpputils { static constexpr unsigned int BINARY_LENGTH = 1; + static FakeKey CreateKey(RandomGenerator &randomGenerator) { + auto data = randomGenerator.getFixedSize<1>(); + return FakeKey{*((uint8_t *) data.data())}; + } + uint8_t value; }; @@ -27,11 +32,6 @@ namespace cpputils { using EncryptionKey = FakeKey; - static EncryptionKey CreateKey(RandomGenerator &randomGenerator) { - auto data = randomGenerator.getFixedSize<1>(); - return FakeKey{*((uint8_t *) data.data())}; - } - static EncryptionKey Key1() { return FakeKey{5}; } diff --git a/test/cpp-utils/system/MemoryTest.cpp b/test/cpp-utils/system/MemoryTest.cpp new file mode 100644 index 00000000..eb875962 --- /dev/null +++ b/test/cpp-utils/system/MemoryTest.cpp @@ -0,0 +1,25 @@ +#include +#include +#include + +using cpputils::DontSwapMemoryRAII; + +TEST(MemoryTest, LockingSmallStackMemoryDoesntCrash) { + bool memory; + DontSwapMemoryRAII obj(&memory, sizeof(memory)); +} + +TEST(MemoryTest, LockingLargeStackMemoryDoesntCrash) { + bool memory[10*1024]; + DontSwapMemoryRAII obj(memory, sizeof(memory)); +} + +TEST(MemoryTest, LockingSmallHeapMemoryDoesntCrash) { + auto memory = std::make_unique(false); + DontSwapMemoryRAII obj(memory.get(), sizeof(*memory)); +} + +TEST(MemoryTest, LockingLargeHeapMemoryDoesntCrash) { + auto memory = std::make_unique(10*1024); + DontSwapMemoryRAII obj(memory.get(), 10*1024); +} diff --git a/test/cryfs/config/CryCipherTest.cpp b/test/cryfs/config/CryCipherTest.cpp index bde6408c..74c981c2 100644 --- a/test/cryfs/config/CryCipherTest.cpp +++ b/test/cryfs/config/CryCipherTest.cpp @@ -37,7 +37,7 @@ public: void EXPECT_CREATES_CORRECT_ENCRYPTED_BLOCKSTORE(const string &cipherName) { const auto &actualCipher = CryCiphers::find(cipherName); Data dataFixture = DataFixture::generate(1024); - string encKey = ExpectedCipher::CreateKey(Random::PseudoRandom()).ToString(); + string encKey = ExpectedCipher::EncryptionKey::CreateKey(Random::PseudoRandom()).ToString(); _EXPECT_ENCRYPTS_WITH_ACTUAL_BLOCKSTORE_DECRYPTS_CORRECTLY_WITH_EXPECTED_BLOCKSTORE(actualCipher, encKey, std::move(dataFixture)); } diff --git a/test/cryfs/filesystem/testutils/CryTestBase.h b/test/cryfs/filesystem/testutils/CryTestBase.h index c63e4cf9..50f0daa8 100644 --- a/test/cryfs/filesystem/testutils/CryTestBase.h +++ b/test/cryfs/filesystem/testutils/CryTestBase.h @@ -17,7 +17,7 @@ public: cryfs::CryConfigFile configFile() { cryfs::CryConfig config; config.SetCipher("aes-256-gcm"); - config.SetEncryptionKey(cpputils::AES256_GCM::CreateKey(cpputils::Random::PseudoRandom()).ToString()); + config.SetEncryptionKey(cpputils::AES256_GCM::EncryptionKey::CreateKey(cpputils::Random::PseudoRandom()).ToString()); config.SetBlocksizeBytes(10240); return cryfs::CryConfigFile::create(_configFile.path(), std::move(config), "mypassword", cpputils::SCrypt::TestSettings); }