Adding temporary file encryption #18013
Conversation
| set.AddFunction( | ||
| PragmaFunction::PragmaStatement("debug_disable_temp_file_encryption", PragmaDisableTempFilesEncryption)); | ||
|
|
||
| set.AddFunction(PragmaFunction::PragmaStatement("enable_full_encryption", PragmaEnableFullEncryption)); |
There was a problem hiding this comment.
What's the difference between full_encryption and temp_file_encryption?
There was a problem hiding this comment.
Full encryption is standard db encryption + wal + temp file encryption. I might wanted to add a setting that in one go disables WAL and temp file encryption all together. This is mainly for performance reasons, since users might not care about encrypting WAL and temp files.
There was a problem hiding this comment.
It seems error-prone to have the extra setting. I'm not sure if we are testing all combinations correctly here. What happens if full_encryption=true but encrypt_wal=false and encrypt_temp_file=false? What is supposed to/expected to happen in that case?
I think having only the two separate settings (encrypt_wal + encrypt_temp_file) is easier to reason about. We can always add a dummy setting that enables these (i.e. SET full_encryption=true sets encrypt_wal=true and encrypt_temp_file=true).
There was a problem hiding this comment.
That seems better indeed, thanks!
|
|
||
| set.AddFunction(PragmaFunction::PragmaStatement("debug_enable_wal_encryption", PragmaEnableWalEncryption)); | ||
| set.AddFunction(PragmaFunction::PragmaStatement("debug_disable_wal_encryption", PragmaDisableWalEncryption)); | ||
| set.AddFunction( |
There was a problem hiding this comment.
Can we turn this into settings?
| AddTempKeyToCache(db); | ||
| } | ||
|
|
||
| auto temp_key = GetKeyFromCache(db, "temp_key"); |
There was a problem hiding this comment.
It looks like there is a lot of duplicated code between the two EncryptTemporaryBuffer methods - can we unify them?
src/common/encryption_functions.cpp
Outdated
| // zero-out the metadata buffer | ||
| memset(metadata, 0, DEFAULT_ENCRYPTED_BUFFER_HEADER_SIZE); | ||
|
|
||
| uint8_t tag[MainHeader::AES_TAG_LEN]; |
There was a problem hiding this comment.
This pattern seems to be present all over the code - perhaps it would be nicer to have a dedicated class for this? e.g.:
struct EncryptionTag {
EncryptionTag() {
memset(tag, 0, MainHeader::AES_TAG_LEN);
}
data_ptr_t data();
idx_t size();
uint8_t data[MainHeader::AES_TAG_LEN];
};Same with the nonce.
| @@ -43,6 +44,7 @@ unique_ptr<FileBuffer> StandardBufferManager::ConstructManagedBuffer(idx_t size, | |||
| if (source) { | |||
| auto tmp = std::move(source); | |||
| D_ASSERT(tmp->AllocSize() == BufferManager::GetAllocSize(size + block_header_size)); | |||
| tmp->Restructure(size, block_header_size); | |||
There was a problem hiding this comment.
Can we instead add the block_header_size to the constructor of the FileBuffer (FileBuffer(FileBuffer &source, FileBufferType type, idx_t block_header_size))? Then the file buffer itself can decide to restructure if required and we don't need to remember to do it ourselves outside of the FileBuffer class.
| } | ||
|
|
||
| static void PragmaEnableTempFilesEncryption(ClientContext &context, const FunctionParameters ¶meters) { | ||
| DBConfig::GetConfig(context).options.enable_temp_file_encryption = true; |
There was a problem hiding this comment.
Can we maybe verify there are no temporary files currently active when setting this - and throw an error if there are any - given that this will lead to not being able to read/interact with the existing temporary files?
Follow-up from #18013 This PR adds support for encrypting temporary files (i.e. files spilled to disk in out-of-memory situations). This can be enabled using the `temp_file_encryption` setting: ```sql SET temp_file_encryption=true; ``` When enabled, any files spilled to disk are also encrypted. The key for those files is kept in-memory and not stored anywhere - if the database loses connection, the contents of the temporary files cannot be read again.
Follow up on #17955.
This PR enables to encrypt temporary files by default when an encrypted database is used.
Temporary files can be of three different 'types':
For the latter type, the size is prefixed in plaintext of an encrypted (.block) temp file.
Usage
When using
temporary files will be encrypted by default. The key for temporary file encryption is randomly generated; this means that temp files that are leftover after e.g. a crash will be unreadable - because the key is also lost in that case.
Disabling temp file encryption
This ideally should be avoided, but e.g. for performance improvement temporary file encryption can also be en/disabled by
However, again, you should be careful with the use of these PRAGMAs since incorrect usage could lead to corrupted temp files (e.g. a mix of encrypted and plaintext temp files).
Tests
Test are in
test/sql/storage/encryption/temp_files/. They partly consist of some (adapted) existing tests that are available for temporary files.