empiredan commented on code in PR #1706:
URL:
https://github.com/apache/incubator-pegasus/pull/1706#discussion_r1472827571
##########
src/replica/replica_stub.cpp:
##########
@@ -389,9 +428,52 @@ void replica_stub::initialize(const replication_options
&opts, bool clear /* = f
}
}
+ std::string kms_path =
Review Comment:
```suggestion
const auto &kms_path =
```
##########
src/replica/kms_key_provider.cpp:
##########
@@ -0,0 +1,38 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include <string>
+
+#include "replica/kms_key_provider.h"
+#include "utils/errors.h"
+
+namespace dsn {
+namespace security {
+
+dsn::error_s KMSKeyProvider::DecryptEncryptionKey(const
dsn::replication::kms_info &kms_info,
Review Comment:
Though the name that are the same with class could be used, we'd better make
it differ:
```suggestion
dsn::error_s KMSKeyProvider::DecryptEncryptionKey(const
dsn::replication::kms_info &info,
```
##########
src/replica/kms_key_provider.cpp:
##########
@@ -0,0 +1,38 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include <string>
+
+#include "replica/kms_key_provider.h"
+#include "utils/errors.h"
+
+namespace dsn {
+namespace security {
+
+dsn::error_s KMSKeyProvider::DecryptEncryptionKey(const
dsn::replication::kms_info &kms_info,
+ std::string *decrypted_key)
+{
+ return client_.DecryptEncryptionKey(kms_info, decrypted_key);
Review Comment:
```suggestion
return client_.DecryptEncryptionKey(info, decrypted_key);
```
##########
src/replica/kms_key_provider.cpp:
##########
@@ -0,0 +1,38 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include <string>
+
+#include "replica/kms_key_provider.h"
+#include "utils/errors.h"
+
+namespace dsn {
+namespace security {
+
+dsn::error_s KMSKeyProvider::DecryptEncryptionKey(const
dsn::replication::kms_info &kms_info,
+ std::string *decrypted_key)
+{
+ return client_.DecryptEncryptionKey(kms_info, decrypted_key);
+}
+
+dsn::error_s KMSKeyProvider::GenerateEncryptionKey(dsn::replication::kms_info
*kms_info)
Review Comment:
```suggestion
dsn::error_s
KMSKeyProvider::GenerateEncryptionKey(dsn::replication::kms_info *info)
```
##########
src/replica/kms_key_provider.h:
##########
@@ -0,0 +1,56 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#pragma once
+
+#include <string>
+#include <utility>
+#include <vector>
+
+#include "security/kms_client.h"
+#include "utils/errors.h"
+
+namespace dsn {
+namespace replication {
+struct kms_info;
+} // namespace replication
+
+namespace security {
+// This class generates EEK IV KV from KMS (a.k.a Key Management Service) and
retrieves DEK from
+// KMS.
+class KMSKeyProvider
+{
+public:
+ ~KMSKeyProvider() {}
+
+ KMSKeyProvider(const std::vector<std::string> &kms_url, std::string
cluster_key_name)
+ : client_(kms_url, std::move(cluster_key_name))
+ {
+ }
+
+ // Decrypt the encryption key in 'kms_info' via KMS. The 'decrypted_key'
will be a hex string.
+ dsn::error_s DecryptEncryptionKey(const dsn::replication::kms_info
&kms_info,
+ std::string *decrypted_key);
+
+ // Generate an encryption key from KMS.
+ dsn::error_s GenerateEncryptionKey(dsn::replication::kms_info *kms_info);
Review Comment:
```suggestion
dsn::error_s GenerateEncryptionKey(dsn::replication::kms_info *info);
```
##########
src/replica/replica_stub.cpp:
##########
@@ -389,9 +428,52 @@ void replica_stub::initialize(const replication_options
&opts, bool clear /* = f
}
}
+ std::string kms_path =
+ utils::filesystem::path_combine(_options.data_dirs[0],
kms_info::kKmsInfo);
+ // FLAGS_data_dirs may be empty when load configuration, use LOG_FATAL
instead of group
+ // validator.
+ if (!FLAGS_encrypt_data_at_rest &&
utils::filesystem::path_exists(kms_path)) {
+ LOG_FATAL("The kms_info file exists at ({}), but [pegasus.server] "
+ "encrypt_data_at_rest is enbale."
+ "Encryption in Pegasus is irreversible after its initial
activation.",
+ kms_path);
+ }
+
+ std::string server_key;
+ dsn::replication::kms_info kms_info;
+ if (FLAGS_encrypt_data_at_rest && !utils::is_empty(FLAGS_hadoop_kms_url)) {
+ key_provider.reset(new dsn::security::KMSKeyProvider(
+ ::absl::StrSplit(FLAGS_hadoop_kms_url, ",", ::absl::SkipEmpty()),
FLAGS_cluster_name));
+ auto ec = dsn::utils::load_rjobj_from_file(
+ kms_path, dsn::utils::FileDataType::kNonSensitive, &kms_info);
+ if (ec != dsn::ERR_PATH_NOT_FOUND && ec != dsn::ERR_OK) {
+ CHECK_EQ_MSG(dsn::ERR_OK, ec, "Can't load kms key from kms-info
file");
+ }
+ // Upon the first launch, the encryption key should be empty. The
process will then retrieve
+ // EEK, IV, and KV from KMS.
+ // After the first launch, the encryption key, obtained from the
kms-info file, should not
+ // be empty. The process will then acquire the DEK from KMS.
+ if (ec == dsn::ERR_PATH_NOT_FOUND) {
+ LOG_WARNING("It's normal to encounter a temporary inability to
open the kms-info file "
+ "during the first process launch.");
+ CHECK_OK(key_provider->GenerateEncryptionKey(&kms_info),
+ "Generate encryption key from kms failed");
+ }
+ CHECK_OK(key_provider->DecryptEncryptionKey(kms_info, &server_key),
+ "Get decryption key failed from {}",
+ kms_path);
+ FLAGS_server_key = server_key.c_str();
Review Comment:
`server_key` is a local variable, and the type of `FLAGS_server_key` is
`const char *`.
##########
src/replica/kms_key_provider.h:
##########
@@ -0,0 +1,56 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#pragma once
+
+#include <string>
+#include <utility>
+#include <vector>
+
+#include "security/kms_client.h"
+#include "utils/errors.h"
+
+namespace dsn {
+namespace replication {
+struct kms_info;
+} // namespace replication
+
+namespace security {
+// This class generates EEK IV KV from KMS (a.k.a Key Management Service) and
retrieves DEK from
+// KMS.
+class KMSKeyProvider
+{
+public:
+ ~KMSKeyProvider() {}
+
+ KMSKeyProvider(const std::vector<std::string> &kms_url, std::string
cluster_key_name)
+ : client_(kms_url, std::move(cluster_key_name))
+ {
+ }
+
+ // Decrypt the encryption key in 'kms_info' via KMS. The 'decrypted_key'
will be a hex string.
+ dsn::error_s DecryptEncryptionKey(const dsn::replication::kms_info
&kms_info,
Review Comment:
```suggestion
dsn::error_s DecryptEncryptionKey(const dsn::replication::kms_info &info,
```
##########
src/replica/replica_stub.cpp:
##########
@@ -389,9 +428,52 @@ void replica_stub::initialize(const replication_options
&opts, bool clear /* = f
}
}
+ std::string kms_path =
+ utils::filesystem::path_combine(_options.data_dirs[0],
kms_info::kKmsInfo);
+ // FLAGS_data_dirs may be empty when load configuration, use LOG_FATAL
instead of group
+ // validator.
+ if (!FLAGS_encrypt_data_at_rest &&
utils::filesystem::path_exists(kms_path)) {
+ LOG_FATAL("The kms_info file exists at ({}), but [pegasus.server] "
+ "encrypt_data_at_rest is enbale."
+ "Encryption in Pegasus is irreversible after its initial
activation.",
+ kms_path);
+ }
+
+ std::string server_key;
+ dsn::replication::kms_info kms_info;
+ if (FLAGS_encrypt_data_at_rest && !utils::is_empty(FLAGS_hadoop_kms_url)) {
+ key_provider.reset(new dsn::security::KMSKeyProvider(
+ ::absl::StrSplit(FLAGS_hadoop_kms_url, ",", ::absl::SkipEmpty()),
FLAGS_cluster_name));
+ auto ec = dsn::utils::load_rjobj_from_file(
+ kms_path, dsn::utils::FileDataType::kNonSensitive, &kms_info);
+ if (ec != dsn::ERR_PATH_NOT_FOUND && ec != dsn::ERR_OK) {
+ CHECK_EQ_MSG(dsn::ERR_OK, ec, "Can't load kms key from kms-info
file");
+ }
+ // Upon the first launch, the encryption key should be empty. The
process will then retrieve
+ // EEK, IV, and KV from KMS.
+ // After the first launch, the encryption key, obtained from the
kms-info file, should not
+ // be empty. The process will then acquire the DEK from KMS.
+ if (ec == dsn::ERR_PATH_NOT_FOUND) {
+ LOG_WARNING("It's normal to encounter a temporary inability to
open the kms-info file "
+ "during the first process launch.");
+ CHECK_OK(key_provider->GenerateEncryptionKey(&kms_info),
+ "Generate encryption key from kms failed");
+ }
+ CHECK_OK(key_provider->DecryptEncryptionKey(kms_info, &server_key),
+ "Get decryption key failed from {}",
+ kms_path);
+ FLAGS_server_key = server_key.c_str();
+ }
+
// Initialize the file system manager.
_fs_manager.initialize(_options.data_dirs, _options.data_dir_tags);
+ if (key_provider && !utils::filesystem::path_exists(kms_path)) {
+ auto err = dsn::utils::dump_rjobj_to_file(
Review Comment:
```suggestion
const auto &err = dsn::utils::dump_rjobj_to_file(
```
##########
src/replica/kms_key_provider.cpp:
##########
@@ -0,0 +1,38 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include <string>
+
+#include "replica/kms_key_provider.h"
+#include "utils/errors.h"
+
+namespace dsn {
+namespace security {
+
+dsn::error_s KMSKeyProvider::DecryptEncryptionKey(const
dsn::replication::kms_info &kms_info,
+ std::string *decrypted_key)
+{
+ return client_.DecryptEncryptionKey(kms_info, decrypted_key);
+}
+
+dsn::error_s KMSKeyProvider::GenerateEncryptionKey(dsn::replication::kms_info
*kms_info)
+{
+ return client_.GenerateEncryptionKey(kms_info);
Review Comment:
```suggestion
return client_.GenerateEncryptionKey(info);
```
##########
src/replica/replica_stub.cpp:
##########
@@ -389,9 +428,52 @@ void replica_stub::initialize(const replication_options
&opts, bool clear /* = f
}
}
+ std::string kms_path =
+ utils::filesystem::path_combine(_options.data_dirs[0],
kms_info::kKmsInfo);
+ // FLAGS_data_dirs may be empty when load configuration, use LOG_FATAL
instead of group
+ // validator.
+ if (!FLAGS_encrypt_data_at_rest &&
utils::filesystem::path_exists(kms_path)) {
+ LOG_FATAL("The kms_info file exists at ({}), but [pegasus.server] "
+ "encrypt_data_at_rest is enbale."
+ "Encryption in Pegasus is irreversible after its initial
activation.",
+ kms_path);
+ }
+
+ std::string server_key;
+ dsn::replication::kms_info kms_info;
+ if (FLAGS_encrypt_data_at_rest && !utils::is_empty(FLAGS_hadoop_kms_url)) {
+ key_provider.reset(new dsn::security::KMSKeyProvider(
+ ::absl::StrSplit(FLAGS_hadoop_kms_url, ",", ::absl::SkipEmpty()),
FLAGS_cluster_name));
+ auto ec = dsn::utils::load_rjobj_from_file(
Review Comment:
```suggestion
const auto &ec = dsn::utils::load_rjobj_from_file(
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
