acelyc111 commented on code in PR #1706: URL: https://github.com/apache/incubator-pegasus/pull/1706#discussion_r1432185502
########## src/replica/default_key_provider.h: ########## @@ -0,0 +1,87 @@ +// 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 <openssl/rand.h> + +#include "absl/strings/escaping.h" +#include "replica/key_provider.h" +#include "utils/error_code.h" +#include "utils/fmt_logging.h" + +namespace dsn { +namespace security { + +class DefaultKeyProvider : public KeyProvider +{ +public: + ~DefaultKeyProvider() override {} + dsn::error_s DecryptEncryptionKey(const std::string &encryption_key, + const std::string & /*iv*/, + const std::string & /*key_version*/, + std::string *decrypted_key) override + { + *decrypted_key = ::absl::HexStringToBytes(encryption_key); + +#ifdef __linux__ + memfrob(decrypted_key->data(), decrypted_key->length()); +#else + // On Linux, memfrob() bitwise XORs the data with the magic number that is + // the answer to the ultimate question of life, the universe, and + // everything. On Mac, we do this manually. + const uint8_t kMagic = 42; + for (auto i = 0; i < decrypted_key->length(); ++i) { + decrypted_key->data()[i] ^= kMagic; + } +#endif + *decrypted_key = ::absl::BytesToHexString(*decrypted_key); + return dsn::error_s::ok(); + } + + dsn::error_s GenerateEncryptionKey(std::string *encryption_key, + std::string *iv, + std::string *key_version) override + { + unsigned char key_bytes[32]; + unsigned char iv_bytes[32]; Review Comment: You can use uint8_t to simplify code. ########## src/replica/default_key_provider.h: ########## @@ -0,0 +1,87 @@ +// 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 <openssl/rand.h> + +#include "absl/strings/escaping.h" +#include "replica/key_provider.h" +#include "utils/error_code.h" +#include "utils/fmt_logging.h" + +namespace dsn { +namespace security { + +class DefaultKeyProvider : public KeyProvider +{ +public: + ~DefaultKeyProvider() override {} + dsn::error_s DecryptEncryptionKey(const std::string &encryption_key, + const std::string & /*iv*/, + const std::string & /*key_version*/, + std::string *decrypted_key) override + { + *decrypted_key = ::absl::HexStringToBytes(encryption_key); + +#ifdef __linux__ + memfrob(decrypted_key->data(), decrypted_key->length()); +#else + // On Linux, memfrob() bitwise XORs the data with the magic number that is + // the answer to the ultimate question of life, the universe, and + // everything. On Mac, we do this manually. + const uint8_t kMagic = 42; + for (auto i = 0; i < decrypted_key->length(); ++i) { + decrypted_key->data()[i] ^= kMagic; + } +#endif + *decrypted_key = ::absl::BytesToHexString(*decrypted_key); + return dsn::error_s::ok(); + } + + dsn::error_s GenerateEncryptionKey(std::string *encryption_key, + std::string *iv, + std::string *key_version) override + { + unsigned char key_bytes[32]; + unsigned char iv_bytes[32]; + int num_bytes = 16; + RAND_bytes(key_bytes, num_bytes); Review Comment: Judge the return value of OpenSSL APIs. ########## src/replica/replica_stub.cpp: ########## @@ -362,6 +387,35 @@ void replica_stub::initialize(bool clear /* = false*/) _access_controller = std::make_unique<dsn::security::access_controller>(); } +dsn::error_s store_kms_key(const std::string &data_dir, + const std::string &encryption_key, + const std::string &iv, + const std::string &key_version) +{ + replica_kms_info kms_info(encryption_key, iv, key_version); + auto err = kms_info.store(data_dir); + if (err != dsn::ERR_OK) { + return dsn::error_s::make(err, "Can't open kms-info file to write"); + } + return dsn::error_s::ok(); +} + +void get_kms_key(const std::string &data_dir, Review Comment: It would be better to return error_s as well, distinguish errors like "not found" and other type of errors. Then judge the return value when call this function. ########## src/replica/replica_stub.cpp: ########## @@ -350,6 +370,11 @@ replica_stub::replica_stub(replica_state_subscriber subscriber /*= nullptr*/, _failure_detector = nullptr; _state = NS_Disconnected; _primary_address_str[0] = '\0'; + if (FLAGS_encrypt_data_at_rest) { Review Comment: Move these code to line 451, closer to the key provider initialization. ########## src/replica/replica_stub.cpp: ########## @@ -389,9 +443,36 @@ void replica_stub::initialize(const replication_options &opts, bool clear /* = f } } + std::string encryption_key; + std::string iv; + std::string key_version; + std::string server_key; + // get and store Encrypted Encryption Key(eek),Initialization Vector(iv),Key Version from kms Review Comment: ```suggestion // Get and store eek (a.k.a encrypted encryption key), iv (a.k.a. initialization vector), key version from KMS. ``` Or add the comments to the replica_kms_info struct defination if using its object here. ########## src/replica/replica_stub.cpp: ########## @@ -389,9 +443,36 @@ void replica_stub::initialize(const replication_options &opts, bool clear /* = f } } + std::string encryption_key; + std::string iv; + std::string key_version; Review Comment: How about using a replica_kms_info object instead? ########## src/replica/replica_stub.cpp: ########## @@ -291,6 +302,15 @@ 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."); Review Comment: Add description that it's a comma-separated list of KMS urls. -- 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]
