Add validation of encryption key length and method

.github/workflows/jobs-linux-run-tests.yml

@@ -88,14 +88,43 @@ jobs:
  - run: mkdir build && cd build && cmake -DWITH_OPENSSL=0 -DWITH_SNAPPY=0 -DWITH_ZLIB=0 -DWITH_BZ2=0 -DWITH_LZ4=0 -DWITH_ZSTD=0 .. && make V=1 -j5 && ctest -j5 -V
  - run: "cd build/tools && ./sst_dump --help | grep -E -q 'Supported compression types: kNoCompression'"
  - uses: "./.github/actions/post-steps"
- build-linux-encrypted_env-openssl:
+ build-linux-encrypted_env-openssl-basic:
  runs-on: ubuntu-latest
  container:
  image: zjay437/rocksdb:0.6
+ strategy:
+ fail-fast: false
+ matrix:
+ test_enc_env:
+ - id=AES;hex_instance_key=0123456789ABCDEF0123456789ABCDEF;method=AES128CTR
+ - id=AES;hex_instance_key=0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF;method=AES192CTR
+ - id=AES;hex_instance_key=0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF;method=AES256CTR
+ - id=AES;hex_instance_key=0123456789ABCDEF0123456789ABCDEF;method=SM4CTR
  steps:
  - uses: actions/checkout@v3.5.0
  - uses: "./.github/actions/pre-steps"
  - run: |
- export ENCRYPTED_ENV=AES
+ export ENCRYPTED_ENV="${{ matrix.test_enc_env }}"
  mkdir build && cd build && cmake -DWITH_OPENSSL=1 .. && make V=1 -j5 && ctest -j5 -V
  - uses: "./.github/actions/post-steps"
+ build-linux-encrypted_env-openssl-special:
+ runs-on: ubuntu-latest
+ container:
+ image: zjay437/rocksdb:0.6
+ strategy:
+ fail-fast: false
+ matrix:
+ test_fs_uri:
+ - provider={id=AES;hex_instance_key=0123456789ABCDEF0123456789ABCDEF;method=AES128CTR};id=EncryptedFileSystem
+ - provider={id=AES;hex_instance_key=0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF;method=AES192CTR};id=EncryptedFileSystem
+ - provider={id=AES;hex_instance_key=0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF;method=AES256CTR};id=EncryptedFileSystem
+ - provider={id=AES;hex_instance_key=0123456789ABCDEF0123456789ABCDEF;method=SM4CTR};id=EncryptedFileSystem
+ steps:
+ - uses: actions/checkout@v3.5.0
+ - uses: "./.github/actions/pre-steps"
+ - run: |
+ mkdir build && cd build && cmake -DWITH_OPENSSL=1 .. && make V=1 -j5
+ export TEST_FS_URI="${{ matrix.test_fs_uri }}"
+ ./env_basic_test --gtest_filter=*CustomEnv*
+ ./env_test --gtest_filter=CreateEnvTest.CreateEncryptedFileSystem
+ - uses: "./.github/actions/post-steps"

encryption/encryption.cc

@@ -32,13 +32,14 @@
 #include "file/filename.h"
 #include "port/likely.h"
 #include "port/port.h"
+#include "rocksdb/configurable.h"
 #include "rocksdb/utilities/options_type.h"
 #include "test_util/sync_point.h"
 
 namespace ROCKSDB_NAMESPACE {
 namespace encryption {
 
-inline size_t KeySize(EncryptionMethod method) {
+size_t KeySize(EncryptionMethod method) {
  switch (method) {
  case EncryptionMethod::kAES128_CTR:
  return 16;
@@ -78,19 +79,6 @@ size_t BlockSize(EncryptionMethod method) {
  }
 }
 
-EncryptionMethod EncryptionMethodStringToEnum(const std::string& method) {
- if (!strcasecmp(method.c_str(), "AES128CTR")) {
- return ROCKSDB_NAMESPACE::encryption::EncryptionMethod::kAES128_CTR;
- } else if (!strcasecmp(method.c_str(), "AES192CTR")) {
- return ROCKSDB_NAMESPACE::encryption::EncryptionMethod::kAES192_CTR;
- } else if (!strcasecmp(method.c_str(), "AES256CTR")) {
- return ROCKSDB_NAMESPACE::encryption::EncryptionMethod::kAES256_CTR;
- } else if (!strcasecmp(method.c_str(), "SM4CTR")) {
- return ROCKSDB_NAMESPACE::encryption::EncryptionMethod::kSM4_CTR;
- }
- return ROCKSDB_NAMESPACE::encryption::EncryptionMethod::kUnknown;
-}
-
 const EVP_CIPHER* GetEVPCipher(EncryptionMethod method) {
  switch (method) {
  case EncryptionMethod::kAES128_CTR:
@@ -99,12 +87,6 @@ const EVP_CIPHER* GetEVPCipher(EncryptionMethod method) {
  return EVP_aes_192_ctr();
  case EncryptionMethod::kAES256_CTR:
  return EVP_aes_256_ctr();
- // case EncryptionMethod::AES128ECB:
- // return EVP_aes_128_ecb();
- // case EncryptionMethod::AES192ECB:
- // return EVP_aes_192_ecb();
- // case EncryptionMethod::AES256ECB:
- // return EVP_aes_256_ecb();
  case EncryptionMethod::kSM4_CTR:
 #if OPENSSL_VERSION_NUMBER < 0x1010100fL || defined(OPENSSL_NO_SM4)
  return nullptr;
@@ -176,11 +158,10 @@ void PutBigEndian64(uint64_t value, unsigned char* dst) {
  }
 }
 
-Status GenerateFileKey(EncryptionMethod method, char* file_key) {
- auto key_bytes = static_cast<int>(KeySize(method) / 8);
- OPENSSL_RET_NOT_OK(
- RAND_bytes(reinterpret_cast<unsigned char*>(file_key), key_bytes),
- "Failed to generate random key");
+Status GenerateFileKey(size_t key_size, char* file_key) {
+ OPENSSL_RET_NOT_OK(RAND_bytes(reinterpret_cast<unsigned char*>(file_key),
+ static_cast<int>(key_size)),
+ "Failed to generate random key");
  return Status::OK();
 }
 
@@ -207,6 +188,7 @@ Status Cipher(const EncryptionMethod method, const std::string& key,
  (void)encrypt_type;
  return Status::NotSupported("OpenSSL version < 1.0.2");
 #else
+ assert(key.size() == encryption::KeySize(method));
  evp_ctx_unique_ptr ctx(EVP_CIPHER_CTX_new(), EVP_CIPHER_CTX_free);
  if (UNLIKELY(!ctx)) {
  return Status::IOError("Failed to create cipher context.");
@@ -337,6 +319,80 @@ Status AESCTRCipherStream::Cipher(uint64_t file_offset, char* data,
  data_size, encrypt_type);
 }
 
+static std::unordered_map<std::string, OptionTypeInfo> aes_options_map = {
+ // TODO(yingchun): the relationship of "hex_instance_key" and "method"
+ // has not been validated, it seems there is no chance to validate
+ // this, now this is validated in CreateNewPrefix() and
+ // CreateCipherStream().
+ {"hex_instance_key",
+ OptionTypeInfo(offsetof(struct AESEncryptionOptions, instance_key),
+ OptionType::kString, OptionVerificationType::kNormal,
+ OptionTypeFlags::kNone)
+ .SetParseFunc([](const ConfigOptions& /*opts*/,
+ const std::string& /*name*/, const std::string& value,
+ void* addr) {
+ if (value.empty()) {
+ return Status::InvalidArgument("'hex_instance_key' is not set");
+ }
+ std::string bin_instance_key;
+ if (!Slice(value).DecodeHex(&bin_instance_key)) {
+ return Status::InvalidArgument(
+ "'hex_instance_key' is not a hexadecimal string in even "
+ "number");
+ }
+ size_t key_size = bin_instance_key.size();
+ if (key_size != KeySize(EncryptionMethod::kAES128_CTR) &&
+ key_size != KeySize(EncryptionMethod::kAES192_CTR) &&
+ key_size != KeySize(EncryptionMethod::kAES256_CTR)) {
+ return Status::InvalidArgument(
+ "'hex_instance_key' length is not valid");
+ }
+ auto target = static_cast<std::string*>(addr);
+ *target = bin_instance_key;
+ return Status::OK();
+ })
+ .SetSerializeFunc([](const ConfigOptions& /*opts*/,
+ const std::string& /*name*/, const void* addr,
+ std::string* value) {
+ std::string hex_instance_key =
+ Slice(*(static_cast<const std::string*>(addr))).ToString(true);
+ *value = hex_instance_key;
+ return Status::OK();
+ })
+ .SetPrepareFunc([](const ConfigOptions& /*opts*/,
+ const std::string& /*name*/, void* addr) {
+ if (static_cast<const std::string*>(addr)->empty()) {
+ return Status::InvalidArgument("'hex_instance_key' is not set");
+ }
+ return Status::OK();
+ })},
+ {"method",
+ OptionTypeInfo::Enum(offsetof(struct AESEncryptionOptions, method),
+ &encryption_method_enum_map)
+ .SetValidateFunc([](const DBOptions& /*db_opts*/,
+ const ColumnFamilyOptions& /*cf_opts*/,
+ const std::string& /*name*/, const void* addr) {
+ EncryptionMethod method =
+ *(static_cast<const EncryptionMethod*>(addr));
+ if (method == EncryptionMethod::kUnknown) {
+ return Status::InvalidArgument("'method' is not valid");
+ }
+ return Status::OK();
+ })
+ .SetPrepareFunc([](const ConfigOptions& /*opts*/,
+ const std::string& /*name*/, void* addr) {
+ EncryptionMethod method =
+ *(static_cast<const EncryptionMethod*>(addr));
+ if (method == EncryptionMethod::kUnknown) {
+ return Status::InvalidArgument("'method' is not set");
+ }
+ return Status::OK();
+ })}};
+
+AESEncryptionProvider::AESEncryptionProvider() {
+ RegisterOptions("aes_options", &aes_options_, &aes_options_map);
+}
+
 bool AESEncryptionProvider::IsInstanceOf(const std::string& name) const {
  return EncryptionProvider::IsInstanceOf(name);
 }
@@ -356,70 +412,98 @@ Status AESEncryptionProvider::ReadEncryptionHeader(
  std::to_string(static_cast<char>(method)));
  }
 
- // 3. Read the encrypted file key and decrypt it.
- char encrypted_file_key[key_size];
- memcpy(encrypted_file_key, prefix.data() + kEncryptionHeaderMagicLength + 1,
- key_size);
- Status s = Cipher(method_, instance_key_, 0, 0, 0, encrypted_file_key,
-  key_size, AESCTRCipherStream::EncryptType::kDecrypt);
+ // 3. Read the encrypted file key.
+ char file_key[key_size];
+ memcpy(file_key, prefix.data() + kEncryptionHeaderMagicLength + 1, key_size);
+
+ // 4. Decrypt the file key.
+ Status s = DecryptFileKey(file_key, key_size);
  if (UNLIKELY(!s.ok())) {
  return s;
  }
 
- // 4. Fill the FileEncryptionInfo.
+ // 5. Fill the FileEncryptionInfo.
  file_info->method = method;
- file_info->key.assign(encrypted_file_key, key_size);
+ file_info->key.assign(file_key, key_size);
  // TODO(yingchun): write a real IV to header_buf.
  static std::string fake_iv(AES_BLOCK_SIZE, '0');
  file_info->iv = fake_iv;
  return Status::OK();
 }
 
 Status AESEncryptionProvider::WriteEncryptionHeader(char* header_buf) const {
- auto method = EncryptionMethod(method_);
- size_t key_size = KeySize(method_);
+ size_t key_size = KeySize(aes_options_.method);
  assert(key_size != 0);
+ assert(key_size % 8 == 0);
 
  // 1. Write the encryption header magic.
  size_t offset = 0;
  memcpy(header_buf, kEncryptionHeaderMagic, kEncryptionHeaderMagicLength);
  offset += kEncryptionHeaderMagicLength;
 
  // 2. Write the encryption method.
- header_buf[offset] = static_cast<char>(method_);
+ header_buf[offset] = static_cast<char>(aes_options_.method);
  offset += 1;
 
- // 3. Generate a file key, encrypt and write it.
+ // 3. Generate a file key.
  char file_key[key_size];
- Status s = GenerateFileKey(method, file_key);
+ Status s = GenerateFileKey(key_size, file_key);
  if (UNLIKELY(!s.ok())) {
  return s;
  }
- s = Cipher(method_, instance_key_, 0, 0, 0, file_key, key_size,
- AESCTRCipherStream::EncryptType::kEncrypt);
+
+ // 4. Encrypt the file key.
+ s = EncryptFileKey(file_key, key_size);
  if (UNLIKELY(!s.ok())) {
  return s;
  }
+
+ // 5. Write the encrypted file key.
  memcpy(header_buf + offset, file_key, key_size);
  offset += key_size;
 
- // 4. Pad with 0.
+ // 6. Pad with 0.
  memset(header_buf + offset, 0, (64 - offset));
 
  // TODO(yingchun): write IV to header_buf.
  return Status::OK();
 }
 
+Status AESEncryptionProvider::EncryptFileKey(char* file_key,
+ size_t file_key_size) const {
+ return Cipher(aes_options_.method, aes_options_.instance_key, 0, 0, 0,
+ file_key, file_key_size,
+ AESCTRCipherStream::EncryptType::kEncrypt);
+}
+
+Status AESEncryptionProvider::DecryptFileKey(char* file_key,
+ size_t file_key_size) const {
+ return Cipher(aes_options_.method, aes_options_.instance_key, 0, 0, 0,
+ file_key, file_key_size,
+ AESCTRCipherStream::EncryptType::kDecrypt);
+}
+
+// TODO(yingchun): it would be better to do the validation when construct
+// AESEncryptionProvider object.
+#define VALIDATE_AES_OPTIONS(options) \
+ if (UNLIKELY(options.instance_key.size() != \
+ encryption::KeySize(options.method))) { \
+ return Status::InvalidArgument( \
+ "'hex_instance_key' length and 'method' are not matched"); \
+ }
+
 Status AESEncryptionProvider::CreateNewPrefix(const std::string& fname,
  char* prefix,
  size_t prefix_length) const {
- if (prefix_length != GetPrefixLength()) {
+ VALIDATE_AES_OPTIONS(aes_options_);
+ if (UNLIKELY(prefix_length != GetPrefixLength())) {
  return IOStatus::Corruption("CreateNewPrefix with invalid prefix length: " +
  std::to_string(prefix_length) + " for " +
  fname);
  }
+
  auto s = WriteEncryptionHeader(prefix);
- if (!s.ok()) {
+ if (UNLIKELY(!s.ok())) {
  s = Status::CopyAppendMessage(s, " in ", fname);
  return s;
  }
@@ -430,16 +514,18 @@ Status AESEncryptionProvider::CreateCipherStream(
  const std::string& fname, const EnvOptions& /*options*/, Slice& prefix,
  std::unique_ptr<BlockAccessCipherStream>* result) {
  assert(result != nullptr);
+ VALIDATE_AES_OPTIONS(aes_options_);
+
  FileEncryptionInfo file_info;
  Status s = ReadEncryptionHeader(prefix, &file_info);
- if (!s.ok()) {
+ if (UNLIKELY(!s.ok())) {
  s = Status::CopyAppendMessage(s, " in ", fname);
  return s;
  }
  std::unique_ptr<AESCTRCipherStream> cipher_stream;
  s = NewAESCTRCipherStream(file_info.method, file_info.key, file_info.iv,
  &cipher_stream);
- if (!s.ok()) {
+ if (UNLIKELY(!s.ok())) {
  s = Status::CopyAppendMessage(s, " in ", fname);
  return s;
  }
@@ -469,7 +555,8 @@ Status NewAESCTRCipherStream(EncryptionMethod method,
  reinterpret_cast<const unsigned char*>(file_key_iv.data()));
  uint64_t iv_low = GetBigEndian64(reinterpret_cast<const unsigned char*>(
  file_key_iv.data() + sizeof(uint64_t)));
- result->reset(new AESCTRCipherStream(method, file_key, iv_high, iv_low));
+ *result =
+ std::make_unique<AESCTRCipherStream>(method, file_key, iv_high, iv_low);
  return Status::OK();
 }