This is an automated email from the ASF dual-hosted git repository.
abukor pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git
The following commit(s) were added to refs/heads/master by this push:
new d4023e4af KUDU-3522 Fail early when encryption init fails
d4023e4af is described below
commit d4023e4af6c20d3185172df69540025c50c4e7ca
Author: Attila Bukor <[email protected]>
AuthorDate: Fri Nov 3 15:22:14 2023 +0100
KUDU-3522 Fail early when encryption init fails
If encryption is enabled using --encrypt_data_at_rest, but the file
system was formatted without encryption enabled, Kudu would be unable to
bootstrap tablets as the encryption headers are missing, however, it
would still start happily without being able to host any tablets.
In some cases, for example when attempting to rebalance a cluster which
contain such "zombie" tablet servers, it can cause some cryptic error
message such as this:
Failed to initialize encryption: error:0607B083:digital envelope
routines:EVP_CipherInit_ex:no cipher set
This commit adds a check to the FS manager to disallow starting up with
encryption enabled and existing instance files without server keys to
address this issue as it's better to fail early in this case. It also
adds a regression test to verify it works as expected.
Change-Id: I7721953869452197f537b161bd6c6b095cc7e75f
Reviewed-on: http://gerrit.cloudera.org:8080/20651
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin <[email protected]>
---
src/kudu/fs/fs_manager-test.cc | 17 +++++++++++++++++
src/kudu/fs/fs_manager.cc | 10 +++++++++-
2 files changed, 26 insertions(+), 1 deletion(-)
diff --git a/src/kudu/fs/fs_manager-test.cc b/src/kudu/fs/fs_manager-test.cc
index 63923bfa0..3c2ca71fa 100644
--- a/src/kudu/fs/fs_manager-test.cc
+++ b/src/kudu/fs/fs_manager-test.cc
@@ -1372,6 +1372,23 @@ TEST_P(FsManagerTestBase, TestAncillaryDirsReported) {
ASSERT_STR_CONTAINS(report_str, "metadata directory: " + opts.metadata_root);
}
+// Regression test for KUDU-3522.
+TEST_P(FsManagerTestBase, TestFailToStartWithoutEncryptionKeys) {
+ if (!FLAGS_encrypt_data_at_rest) {
+ // Skipping this test if encryption is not enabled.
+ GTEST_SKIP();
+ }
+ // Disable encryption while creating file system.
+ FLAGS_encrypt_data_at_rest = false;
+ const auto& path = GetTestPath("unencrypted");
+ ReinitFsManagerWithPaths(path, { path });
+ ASSERT_OK(fs_manager()->CreateInitialFileSystemLayout());
+
+ // Re-enable encryption and attempt to open the FS.
+ FLAGS_encrypt_data_at_rest = true;
+ ASSERT_TRUE(fs_manager()->Open().IsIllegalState());
+}
+
class OpenFsTypeTest : public KuduTest,
public ::testing::WithParamInterface<std::tuple<string,
bool, bool>> {
public:
diff --git a/src/kudu/fs/fs_manager.cc b/src/kudu/fs/fs_manager.cc
index 3017209fd..702ca2b1b 100644
--- a/src/kudu/fs/fs_manager.cc
+++ b/src/kudu/fs/fs_manager.cc
@@ -543,6 +543,11 @@ Status FsManager::Open(FsReport* report, Timer*
read_instance_metadata_files,
"The '--enable_multi_tenancy' should set for the existed tenants.");
}
+ if (FLAGS_encrypt_data_at_rest &&
tenant_key(fs::kDefaultTenantID).empty()) {
+ return Status::IllegalState(
+ "Data at rest encryption is enabled and tenants exist, but no tenant
key found");
+ }
+
// TODO(kedeng) :
// After implementing tenant management, different tenants need to be
handled here.
//
@@ -556,7 +561,7 @@ Status FsManager::Open(FsReport* report, Timer*
read_instance_metadata_files,
// Just check whether the upgrade operation is needed for
'--enable_multi_tenancy'.
if (FLAGS_enable_multi_tenancy) {
return Status::IllegalState(
- "We should do server key upgrade with Kudu CLI tool for
'--enable_multi_tenancy'.");
+ "--enable_multi_tenancy is set, but no tenants exist.");
}
string server_key;
@@ -564,6 +569,9 @@ Status FsManager::Open(FsReport* report, Timer*
read_instance_metadata_files,
this->server_key_iv(),
this->server_key_version(),
&decrypted_key));
+ } else if (server_key().empty() && FLAGS_encrypt_data_at_rest) {
+ return Status::IllegalState(
+ "--encrypt_data_at_rest is set, but no server key found.");
}
if (!decrypted_key.empty()) {
