acelyc111 commented on code in PR #1706: URL: https://github.com/apache/incubator-pegasus/pull/1706#discussion_r1433381454
########## src/runtime/security/replica_kms_info.cpp: ########## @@ -0,0 +1,70 @@ +/* + * 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 GCC diagnostic ignored "-Wclass-memaccess" +#include <stdint.h> +#include <typeinfo> + +#include "replica/replication_app_base.h" +#include "runtime/api_layer1.h" +#include "runtime/security/replica_kms_info.h" +#include "utils/blob.h" +#include "utils/env.h" +#include "utils/filesystem.h" +#include "utils/fmt_logging.h" +#include "utils/macros.h" + +namespace dsn { + +namespace replication { + +const std::string replica_kms_info::kKmsInfo = ".kms-info"; + +error_code replica_kms_info::load(const std::string &dir) +{ + std::string info_path = utils::filesystem::path_combine(dir, kKmsInfo); + LOG_AND_RETURN_NOT_TRUE(ERROR, + utils::filesystem::path_exists(info_path), + ERR_PATH_NOT_FOUND, + "file({}) not exist", + info_path); + auto err = load_json_generic<replica_kms_info, dsn::utils::FileDataType::kNonSensitive>( + info_path, *this); + LOG_AND_RETURN_NOT_OK(ERROR, err, "load replica_kms_info from {} failed", info_path); + LOG_INFO("load replica_kms_info from {} ", info_path); + return ERR_OK; +} + +error_code replica_kms_info::store(const std::string &dir) +{ + uint64_t start = dsn_now_ns(); + std::string info_path = utils::filesystem::path_combine(dir, kKmsInfo); + LOG_AND_RETURN_NOT_OK(ERROR, + write_blob_to_file(info_path, + json::json_forwarder<replica_kms_info>::encode(*this), + dsn::utils::FileDataType::kNonSensitive), + "store replica_kms_info to {} failed, time_used_ns = {}", + info_path, + dsn_now_ns() - start); + LOG_INFO( + "store replica_kms_info to {} succeed, time_used_ns = {}", info_path, dsn_now_ns() - start); + return ERR_OK; +} + +} // namespace replication +} // namespace dsn Review Comment: Add a lank line. ########## src/replica/replica_stub.cpp: ########## @@ -291,13 +302,35 @@ DSN_DEFINE_int32( 10, "if tcmalloc reserved but not-used memory exceed this percentage of application allocated " "memory, replica server will release the exceeding memory back to operating system"); +DSN_DEFINE_string(pegasus.server, + encryption_cluster_key_name, + "pegasus", + "The cluster name of encrypted server which use to get server key from kms."); + +DSN_DEFINE_string( + pegasus.server, + hadoop_kms_url, + "", + "Where the server key of file system can get from. " + "Url should be comma-separated list.Such as 'hostname1:1234/kms,hostname2:1234/kms'"); DSN_DECLARE_bool(duplication_enabled); DSN_DECLARE_int32(fd_beacon_interval_seconds); DSN_DECLARE_int32(fd_check_interval_seconds); DSN_DECLARE_int32(fd_grace_seconds); DSN_DECLARE_int32(fd_lease_seconds); DSN_DECLARE_int32(gc_interval_ms); +DSN_TAG_VARIABLE(encrypt_data_at_rest, FT_MUTABLE); +DSN_DEFINE_group_validator(encrypt_data_at_rest, [](std::string &message) -> bool { + if (!dsn::security::FLAGS_enable_acl && FLAGS_encrypt_data_at_rest) { + message = fmt::format("[security] enable_acl = ({}) [pegasus.server] encrypt_data_at_rest" + "= ({}),should be true at the same time", Review Comment: ```suggestion message = fmt::format("[pegasus.server] encrypt_data_at_rest should be enabled only if [security] enable_acl is enabled.", ``` ########## src/replica/replica_stub.cpp: ########## @@ -291,13 +302,35 @@ DSN_DEFINE_int32( 10, "if tcmalloc reserved but not-used memory exceed this percentage of application allocated " "memory, replica server will release the exceeding memory back to operating system"); +DSN_DEFINE_string(pegasus.server, + encryption_cluster_key_name, + "pegasus", + "The cluster name of encrypted server which use to get server key from kms."); + +DSN_DEFINE_string( + pegasus.server, + hadoop_kms_url, + "", + "Where the server key of file system can get from. " + "Url should be comma-separated list.Such as 'hostname1:1234/kms,hostname2:1234/kms'"); DSN_DECLARE_bool(duplication_enabled); DSN_DECLARE_int32(fd_beacon_interval_seconds); DSN_DECLARE_int32(fd_check_interval_seconds); DSN_DECLARE_int32(fd_grace_seconds); DSN_DECLARE_int32(fd_lease_seconds); DSN_DECLARE_int32(gc_interval_ms); +DSN_TAG_VARIABLE(encrypt_data_at_rest, FT_MUTABLE); Review Comment: encrypt_data_at_rest should not be mutable. ########## src/replica/replica_stub.cpp: ########## @@ -389,9 +422,44 @@ void replica_stub::initialize(const replication_options &opts, bool clear /* = f } } + if (FLAGS_encrypt_data_at_rest) { + key_provider.reset(new dsn::security::KMSKeyProvider( + ::absl::StrSplit(FLAGS_hadoop_kms_url, ",", ::absl::SkipEmpty()), + FLAGS_encryption_cluster_key_name)); + } + + std::string server_key; + dsn::replication::replica_kms_info kms_info; + if (key_provider && !utils::is_empty(FLAGS_hadoop_kms_url)) { + auto err = kms_info.load(_options.data_dirs[0]); + if (err != dsn::ERR_OK) { + LOG_WARNING("Can't open kms-info file to read, this is normal when first launch " + "process. err = {}", + err); + } + // The encryption key should empty when process upon the first launch. And the process will + // get eek,iv,kv from kms. + // After first launch, the encryption key should not empty and get from kms-info file. The + // process get dek(a.k.a Decrypted Encryption Key) from kms. + if (kms_info.eek.empty()) { + auto err = + key_provider->GenerateEncryptionKey(&kms_info.eek, &kms_info.iv, &kms_info.kv); + CHECK(err, "get encryption key failed, err = {}", err); + } + CHECK( + key_provider->DecryptEncryptionKey(kms_info.eek, kms_info.iv, kms_info.kv, &server_key), + "get decryption key failed"); + 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::is_empty(FLAGS_hadoop_kms_url)) { Review Comment: Why not merge to the if-statement above? ########## src/replica/kms_key_provider.h: ########## @@ -0,0 +1,55 @@ +// 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 "replica/key_provider.h" +#include "runtime/security/kms_client.h" +#include "utils/errors.h" + +namespace dsn { +namespace security { +class KMSKeyProvider : public KeyProvider Review Comment: Add comments for this class. ########## src/replica/replica_stub.cpp: ########## @@ -291,13 +302,35 @@ DSN_DEFINE_int32( 10, "if tcmalloc reserved but not-used memory exceed this percentage of application allocated " "memory, replica server will release the exceeding memory back to operating system"); +DSN_DEFINE_string(pegasus.server, + encryption_cluster_key_name, + "pegasus", + "The cluster name of encrypted server which use to get server key from kms."); + +DSN_DEFINE_string( + pegasus.server, + hadoop_kms_url, + "", + "Where the server key of file system can get from. " + "Url should be comma-separated list.Such as 'hostname1:1234/kms,hostname2:1234/kms'"); Review Comment: ```suggestion "Url should be comma-separated list, such as 'hostname1:1234/kms,hostname2:1234/kms'"); ``` ########## src/runtime/security/kms_client.h: ########## @@ -0,0 +1,72 @@ +// 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 "absl/strings/str_split.h" +#include "utils/errors.h" + +namespace dsn { +namespace security { +// A class to generate encryption_key from kms for writing file which implemented based on http +// client +// This class is not thread-safe. Thus maintain one instance for each thread. +// +// Example of using Kms client: +// -------------------------------------------------------- +// Generate and get the Encrypted Encryption Key(eek),Initialization Vector(iv),Key Version from +// kms: +// std::string encryption_key; +// std::string iv; +// std::string key_version; +// err = GenerateEncryptionKey(key_name, &encryption_key, &iv, &key_version); +// +// Get the Decrypted Encryption Key(dek) back from kms. The eek,iv,kv need generated from kms: +// std::string decryption_key; +// err = DecryptEncryptionKey(encryption_key, iv, key_version, &decrypted_key); Review Comment: Didn't update? ########## src/replica/replica_stub.cpp: ########## @@ -389,9 +422,44 @@ void replica_stub::initialize(const replication_options &opts, bool clear /* = f } } + if (FLAGS_encrypt_data_at_rest) { + key_provider.reset(new dsn::security::KMSKeyProvider( + ::absl::StrSplit(FLAGS_hadoop_kms_url, ",", ::absl::SkipEmpty()), + FLAGS_encryption_cluster_key_name)); + } + + std::string server_key; + dsn::replication::replica_kms_info kms_info; + if (key_provider && !utils::is_empty(FLAGS_hadoop_kms_url)) { + auto err = kms_info.load(_options.data_dirs[0]); Review Comment: Another validation should be added to check the kms_info file is not exist when FLAGS_encrypt_data_at_rest is false, because we don't support disable encryption if the cluster enable it ever. ########## src/replica/replica_stub.cpp: ########## @@ -389,9 +422,44 @@ void replica_stub::initialize(const replication_options &opts, bool clear /* = f } } + if (FLAGS_encrypt_data_at_rest) { + key_provider.reset(new dsn::security::KMSKeyProvider( + ::absl::StrSplit(FLAGS_hadoop_kms_url, ",", ::absl::SkipEmpty()), + FLAGS_encryption_cluster_key_name)); + } + + std::string server_key; + dsn::replication::replica_kms_info kms_info; + if (key_provider && !utils::is_empty(FLAGS_hadoop_kms_url)) { + auto err = kms_info.load(_options.data_dirs[0]); Review Comment: Judge the whether the key manager can be initialized before creating key_provider, so this if statement can be combined with line 425. -- 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]
