Skip to content

Commit

Permalink
[#6833] Add flag to skip client endpoint verification
Browse files Browse the repository at this point in the history
Summary:
This diff adds new flag for client client endpoint verification: verify_client_endpoint - false by default.

Also added ability to specify certiciate name for yb-admin and yb-ts-cli to be able to connect to cluster that uses client certificates.
The name count be specified with client_certificate_name flag.
Please note that key file will have name "node.XXX.key" and cert file will be "node.XXX.crt" where XXX - client_certificate_name flag value.

Test Plan:
ybd --gtest_filter ExternalMiniClusterSecureTest.YbAdmin
ybd --gtest_filter ExternalMiniClusterSecureTest.YbTsCli

Reviewers: bogdan, dmitry, sanketh

Reviewed By: sanketh

Subscribers: arnav, ybase

Differential Revision: https://phabricator.dev.yugabyte.com/D10301
  • Loading branch information
spolitov committed Jan 28, 2021
1 parent 7852d54 commit e092e3f
Show file tree
Hide file tree
Showing 24 changed files with 635 additions and 80 deletions.
9 changes: 7 additions & 2 deletions ent/src/yb/integration-tests/external_mini_cluster_ent.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,10 @@

#include "yb/util/test_util.h"

using namespace std::literals;

DECLARE_string(certs_dir);
DECLARE_bool(node_to_node_encryption_use_client_certificates);

namespace yb {

Expand All @@ -30,14 +33,16 @@ void StartSecure(
const std::vector<std::string>& master_flags) {
rpc::MessengerBuilder messenger_builder("test_client");
*secure_context = ASSERT_RESULT(server::SetupSecureContext(
"", "", server::SecureContextType::kClientToServer, &messenger_builder));
"", "127.0.0.100", server::SecureContextType::kInternal, &messenger_builder));
*messenger = ASSERT_RESULT(messenger_builder.Build());
(**messenger).TEST_SetOutboundIpBase(ASSERT_RESULT(HostToAddress("127.0.0.1")));

ExternalMiniClusterOptions opts;
opts.extra_tserver_flags = {
"--use_node_to_node_encryption=true", "--allow_insecure_connections=false",
"--certs_dir=" + FLAGS_certs_dir};
"--certs_dir=" + FLAGS_certs_dir,
"--node_to_node_encryption_use_client_certificates="s +
(FLAGS_node_to_node_encryption_use_client_certificates ? "true" : "false")};
opts.extra_master_flags = opts.extra_tserver_flags;
opts.extra_master_flags.insert(
opts.extra_master_flags.end(), master_flags.begin(), master_flags.end());
Expand Down
24 changes: 24 additions & 0 deletions ent/src/yb/integration-tests/external_mini_cluster_secure_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

#include "yb/util/size_literals.h"
#include "yb/util/env_util.h"
#include "yb/util/subprocess.h"

#include "yb/yql/cql/ql/util/errcodes.h"
#include "yb/yql/cql/ql/util/statement_result.h"
Expand All @@ -30,6 +31,7 @@
DECLARE_bool(use_client_to_server_encryption);
DECLARE_bool(use_node_to_node_encryption);
DECLARE_bool(allow_insecure_connections);
DECLARE_bool(node_to_node_encryption_use_client_certificates);
DECLARE_string(certs_dir);

namespace yb {
Expand Down Expand Up @@ -88,4 +90,26 @@ TEST_F(ExternalMiniClusterSecureTest, Simple) {
}
}

class ExternalMiniClusterSecureWithClientCertsTest : public ExternalMiniClusterSecureTest {
void SetUp() override {
FLAGS_node_to_node_encryption_use_client_certificates = true;
ExternalMiniClusterSecureTest::SetUp();
}
};

TEST_F_EX(ExternalMiniClusterSecureTest, YbAdmin, ExternalMiniClusterSecureWithClientCertsTest) {
ASSERT_OK(Subprocess::Call(ToStringVector(
GetToolPath("yb-admin"), "--master_addresses", cluster_->GetMasterAddresses(),
"--certs_dir_name", GetToolPath("../ent/test_certs"),
"--client_node_name=127.0.0.100", "list_tables")));
}

TEST_F_EX(ExternalMiniClusterSecureTest, YbTsCli, ExternalMiniClusterSecureWithClientCertsTest) {
ASSERT_OK(Subprocess::Call(ToStringVector(
GetToolPath("yb-ts-cli"),
"--server_address", cluster_->tablet_server(0)->bound_rpc_addr(),
"--certs_dir_name", GetToolPath("../ent/test_certs"),
"--client_node_name=127.0.0.100", "list_tablets")));
}

} // namespace yb
37 changes: 33 additions & 4 deletions ent/src/yb/integration-tests/secure_connection_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,14 @@ DECLARE_bool(node_to_node_encryption_use_client_certificates);
DECLARE_bool(TEST_private_broadcast_address);
DECLARE_bool(use_client_to_server_encryption);
DECLARE_bool(use_node_to_node_encryption);
DECLARE_bool(verify_client_endpoint);
DECLARE_bool(verify_server_endpoint);
DECLARE_int32(TEST_nodes_per_cloud);
DECLARE_int32(yb_client_admin_operation_timeout_sec);
DECLARE_string(certs_dir);
DECLARE_string(cert_file_pattern);
DECLARE_string(key_file_pattern);
DECLARE_string(node_to_node_encryption_required_uid);
DECLARE_string(ssl_protocols);
DECLARE_string(TEST_public_hostname_suffix);

Expand All @@ -54,15 +59,19 @@ class SecureConnectionTest : public client::KeyValueTableTest<MiniCluster> {
FLAGS_allow_insecure_connections = false;
FLAGS_TEST_public_hostname_suffix = ".ip.yugabyte";
FLAGS_TEST_private_broadcast_address = true;
const auto sub_dir = JoinPathSegments("ent", "test_certs");
auto root_dir = env_util::GetRootDir(sub_dir);
FLAGS_certs_dir = JoinPathSegments(root_dir, sub_dir);
FLAGS_certs_dir = CertsDir();

KeyValueTableTest::SetUp();

DontVerifyClusterBeforeNextTearDown(); // Verify requires insecure connection.
}

virtual std::string CertsDir() {
const auto sub_dir = JoinPathSegments("ent", "test_certs");
auto root_dir = env_util::GetRootDir(sub_dir);
return JoinPathSegments(root_dir, sub_dir);
}

CHECKED_STATUS CreateClient() override {
auto host = "127.0.0.52";
client_ = VERIFY_RESULT(DoCreateClient(host, host, &secure_context_));
Expand All @@ -82,7 +91,7 @@ class SecureConnectionTest : public client::KeyValueTableTest<MiniCluster> {
std::unique_ptr<rpc::SecureContext>* secure_context) {
rpc::MessengerBuilder messenger_builder("test_client");
*secure_context = VERIFY_RESULT(server::SetupSecureContext(
FLAGS_certs_dir, name, server::SecureContextType::kServerToServer, &messenger_builder));
FLAGS_certs_dir, name, server::SecureContextType::kInternal, &messenger_builder));
auto messenger = VERIFY_RESULT(messenger_builder.Build());
messenger->TEST_SetOutboundIpBase(VERIFY_RESULT(HostToAddress(host)));
return cluster_->CreateClient(std::move(messenger));
Expand Down Expand Up @@ -167,6 +176,7 @@ class SecureConnectionWithClientCertificatesTest : public SecureConnectionTest {
void SetUp() override {
FLAGS_TEST_nodes_per_cloud = 100;
FLAGS_node_to_node_encryption_use_client_certificates = true;
FLAGS_verify_client_endpoint = true;
SecureConnectionTest::SetUp();
}
};
Expand All @@ -177,4 +187,23 @@ TEST_F_EX(SecureConnectionTest, ClientCertificates, SecureConnectionWithClientCe
ASSERT_NOK(CreateBadClient());
}

class SecureConnectionVerifyNameOnlyTest : public SecureConnectionTest {
void SetUp() override {
FLAGS_TEST_nodes_per_cloud = 100;
FLAGS_node_to_node_encryption_use_client_certificates = true;
FLAGS_node_to_node_encryption_required_uid = "yugabyte-test";
SecureConnectionTest::SetUp();
}

std::string CertsDir() override {
const auto sub_dir = JoinPathSegments("ent", "test_certs", "named");
auto root_dir = env_util::GetRootDir(sub_dir);
return JoinPathSegments(root_dir, sub_dir);
}
};

TEST_F_EX(SecureConnectionTest, VerifyNameOnly, SecureConnectionVerifyNameOnlyTest) {
TestSimpleOps();
}

} // namespace yb
2 changes: 1 addition & 1 deletion ent/src/yb/master/cdc_rpc_tasks.cc
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ Result<std::shared_ptr<CDCRpcTasks>> CDCRpcTasks::CreateWithMasterAddrs(
dir = JoinPathSegments(FLAGS_certs_for_cdc_dir, universe_id);
}
cdc_rpc_tasks->secure_context_ = VERIFY_RESULT(server::SetupSecureContext(
dir, "", "", server::SecureContextType::kServerToServer, &messenger_builder));
dir, "", "", server::SecureContextType::kInternal, &messenger_builder));
cdc_rpc_tasks->messenger_ = VERIFY_RESULT(messenger_builder.Build());
}

Expand Down
4 changes: 2 additions & 2 deletions ent/src/yb/master/master_ent.cc
Original file line number Diff line number Diff line change
Expand Up @@ -61,14 +61,14 @@ Status Master::SetupMessengerBuilder(rpc::MessengerBuilder* builder) {
secure_context_ = VERIFY_RESULT(server::SetupSecureContext(
server::DefaultRootDir(*fs_manager_),
FLAGS_cert_node_filename,
server::SecureContextType::kServerToServer,
server::SecureContextType::kInternal,
builder));
} else {
const string &hosts = !options_.server_broadcast_addresses.empty()
? options_.server_broadcast_addresses
: options_.rpc_opts.rpc_bind_addresses;
secure_context_ = VERIFY_RESULT(server::SetupSecureContext(
hosts, *fs_manager_, server::SecureContextType::kServerToServer, builder));
hosts, *fs_manager_, server::SecureContextType::kInternal, builder));
}

return Status::OK();
Expand Down
47 changes: 33 additions & 14 deletions ent/src/yb/server/secure.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ DEFINE_bool(node_to_node_encryption_use_client_certificates, false,
"Should client certificates be sent and verified for encrypted node to node "
"communication.");

DEFINE_string(node_to_node_encryption_required_uid, "",
"Allow only certificates with specified uid. Empty to allow any.");

DEFINE_string(certs_dir, "",
"Directory that contains certificate authority, private key and certificates for "
"this server. By default 'certs' subdir in data folder is used.");
Expand All @@ -46,6 +49,10 @@ DEFINE_string(cert_node_filename, "",
"If this flag is not set, then --server_broadcast_addresses will be "
"used if it is set, and if not, --rpc_bind_addresses will be used.");

DEFINE_string(key_file_pattern, "node.$0.key", "Pattern used for key file");

DEFINE_string(cert_file_pattern, "node.$0.crt", "Pattern used for certificate file");

namespace yb {
namespace server {
namespace {
Expand Down Expand Up @@ -83,16 +90,16 @@ Result<std::unique_ptr<rpc::SecureContext>> SetupSecureContext(
Result<std::unique_ptr<rpc::SecureContext>> SetupSecureContext(
const std::string& cert_dir, const std::string& root_dir, const std::string& name,
SecureContextType type, rpc::MessengerBuilder* builder) {
auto use = type == SecureContextType::kServerToServer ? FLAGS_use_node_to_node_encryption
: FLAGS_use_client_to_server_encryption;
auto use = type == SecureContextType::kInternal ? FLAGS_use_node_to_node_encryption
: FLAGS_use_client_to_server_encryption;
if (!use) {
return std::unique_ptr<rpc::SecureContext>();
}

std::string dir;
if (!cert_dir.empty()) {
dir = cert_dir;
} else if (type == SecureContextType::kClientToServer) {
} else if (type == SecureContextType::kExternal) {
dir = FLAGS_certs_for_client_dir;
}
if (dir.empty()) {
Expand All @@ -102,38 +109,50 @@ Result<std::unique_ptr<rpc::SecureContext>> SetupSecureContext(
dir = DefaultCertsDir(root_dir);
}

auto context = VERIFY_RESULT(CreateSecureContext(dir, name));
if (type == SecureContextType::kServerToServer &&
FLAGS_node_to_node_encryption_use_client_certificates) {
context->set_require_client_certificate(true);
context->set_use_client_certificate(true);
UseClientCerts use_client_certs = UseClientCerts::kFalse;
std::string required_uid;
if (type == SecureContextType::kInternal) {
use_client_certs = UseClientCerts(FLAGS_node_to_node_encryption_use_client_certificates);
required_uid = FLAGS_node_to_node_encryption_required_uid;
}
auto context = VERIFY_RESULT(CreateSecureContext(dir, use_client_certs, name, required_uid));
ApplySecureContext(context.get(), builder);
return context;
}

Result<std::unique_ptr<rpc::SecureContext>> CreateSecureContext(
const std::string& certs_dir, const std::string& name) {
const std::string& certs_dir, UseClientCerts use_client_certs, const std::string& node_name,
const std::string& required_uid) {

LOG(INFO) << "Certs directory: " << certs_dir << ", name: " << name;
LOG(INFO) << "Certs directory: " << certs_dir << ", node name: " << node_name;

auto result = std::make_unique<rpc::SecureContext>();
faststring data;
RETURN_NOT_OK(result->AddCertificateAuthorityFile(JoinPathSegments(certs_dir, "ca.crt")));

if (!name.empty()) {
if (!node_name.empty()) {
RETURN_NOT_OK(ReadFileToString(
Env::Default(), JoinPathSegments(certs_dir, Format("node.$0.key", name)), &data));
Env::Default(), JoinPathSegments(certs_dir, Format(FLAGS_key_file_pattern, node_name)),
&data));
RETURN_NOT_OK(result->UsePrivateKey(data));

RETURN_NOT_OK(ReadFileToString(
Env::Default(), JoinPathSegments(certs_dir, Format("node.$0.crt", name)), &data));
Env::Default(), JoinPathSegments(certs_dir, Format(FLAGS_cert_file_pattern, node_name)),
&data));
RETURN_NOT_OK(result->UseCertificate(data));
}

if (use_client_certs) {
result->set_require_client_certificate(true);
result->set_use_client_certificate(true);
}

result->set_required_uid(required_uid);

return result;
}

void ApplySecureContext(rpc::SecureContext* context, rpc::MessengerBuilder* builder) {
void ApplySecureContext(const rpc::SecureContext* context, rpc::MessengerBuilder* builder) {
auto parent_mem_tracker = builder->last_used_parent_mem_tracker();
auto buffer_tracker = MemTracker::FindOrCreateTracker(
-1, "Encrypted Read Buffer", parent_mem_tracker);
Expand Down
10 changes: 7 additions & 3 deletions ent/src/yb/server/secure.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ class SecureContext;

namespace server {

YB_DEFINE_ENUM(SecureContextType, (kServerToServer)(kClientToServer));
YB_DEFINE_ENUM(SecureContextType, (kInternal)(kExternal));

string DefaultRootDir(const FsManager& fs_manager);

Expand All @@ -52,10 +52,14 @@ Result<std::unique_ptr<rpc::SecureContext>> SetupSecureContext(
const std::string& cert_dir, const std::string& root_dir, const std::string& name,
SecureContextType type, rpc::MessengerBuilder* builder);

YB_STRONGLY_TYPED_BOOL(UseClientCerts);

Result<std::unique_ptr<rpc::SecureContext>> CreateSecureContext(
const std::string& certs_dir, const std::string& name = std::string());
const std::string& certs_dir, UseClientCerts use_client_certs,
const std::string& node_name = std::string(),
const std::string& required_uid = std::string());

void ApplySecureContext(rpc::SecureContext* context, rpc::MessengerBuilder* builder);
void ApplySecureContext(const rpc::SecureContext* context, rpc::MessengerBuilder* builder);

} // namespace server
} // namespace yb
Expand Down
4 changes: 2 additions & 2 deletions ent/src/yb/tserver/cdc_consumer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ Result<std::unique_ptr<CDCConsumer>> CDCConsumer::Create(
rpc::MessengerBuilder messenger_builder("cdc-consumer");

local_client->secure_context = VERIFY_RESULT(server::SetupSecureContext(
"", "", server::SecureContextType::kServerToServer, &messenger_builder));
"", "", server::SecureContextType::kInternal, &messenger_builder));

local_client->messenger = VERIFY_RESULT(messenger_builder.Build());
}
Expand Down Expand Up @@ -274,7 +274,7 @@ void CDCConsumer::TriggerPollForNewTablets() {
}

auto secure_context_result = server::SetupSecureContext(
dir, "", "", server::SecureContextType::kServerToServer, &messenger_builder);
dir, "", "", server::SecureContextType::kInternal, &messenger_builder);
if (!secure_context_result.ok()) {
LOG(WARNING) << "Could not create secure context for " << uuid
<< ": " << secure_context_result.status().ToString();
Expand Down
4 changes: 2 additions & 2 deletions ent/src/yb/tserver/factory.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,14 +57,14 @@ class CQLServerEnt : public cqlserver::CQLServer {
secure_context_ = VERIFY_RESULT(server::SetupSecureContext(
server::DefaultRootDir(*fs_manager_),
FLAGS_cert_node_filename,
server::SecureContextType::kClientToServer,
server::SecureContextType::kExternal,
builder));
} else {
const string &hosts = !options_.server_broadcast_addresses.empty()
? options_.server_broadcast_addresses
: options_.rpc_opts.rpc_bind_addresses;
secure_context_ = VERIFY_RESULT(server::SetupSecureContext(
hosts, *fs_manager_, server::SecureContextType::kClientToServer, builder));
hosts, *fs_manager_, server::SecureContextType::kExternal, builder));
}
return Status::OK();
}
Expand Down
4 changes: 2 additions & 2 deletions ent/src/yb/tserver/tablet_server_ent.cc
Original file line number Diff line number Diff line change
Expand Up @@ -94,14 +94,14 @@ Status TabletServer::SetupMessengerBuilder(rpc::MessengerBuilder* builder) {
secure_context_ = VERIFY_RESULT(server::SetupSecureContext(
server::DefaultRootDir(*fs_manager_),
FLAGS_cert_node_filename,
server::SecureContextType::kServerToServer,
server::SecureContextType::kInternal,
builder));
} else {
const string &hosts = !options_.server_broadcast_addresses.empty()
? options_.server_broadcast_addresses
: options_.rpc_opts.rpc_bind_addresses;
secure_context_ = VERIFY_RESULT(server::SetupSecureContext(
hosts, *fs_manager_, server::SecureContextType::kServerToServer, builder));
hosts, *fs_manager_, server::SecureContextType::kInternal, builder));
}
return Status::OK();
}
Expand Down
17 changes: 17 additions & 0 deletions ent/test_certs/named/ca.crt
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
-----BEGIN CERTIFICATE-----
MIICqDCCAZACCQCZzuWUK8cZ7jANBgkqhkiG9w0BAQsFADAWMRQwEgYDVQQDDAtZ
dWdhQnl0ZSBDQTAeFw0xODA2MDUyMjA5MTZaFw0yODA2MTIyMjA5MTZaMBYxFDAS
BgNVBAMMC1l1Z2FCeXRlIENBMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKC
AQEArmefuP3J1JBojKc710ATZaDnjDnIRjcRR4Af4jj2k8zZhw/5DoLPSmGCmTSq
t1ZvFe9a2TAAAEZLceV+6ysH+4G2E8haVCYPDlvz00Wt1NFhCUg/zEFpMxDT84HC
HSXPaZKjXR76lp3Afc/xMgK7Q5ZlOaTJ/kJVgVb/SLgq1DjuvQkqQLbQjZLa6vGf
kpU037dJUaptjIPTnmX9OOrOdDkF0lNzMbnL5gwWWIpJofBKT+d4u/d7oX00AC9H
q9oautJOUwNymZkgla12pOlhGOnXJ6tqhoY7EfKM2fpzAGIwAe69a8aIv8zWBBfQ
f8ASQT5TZa4JPrZiBNITPGjHrwIDAQABMA0GCSqGSIb3DQEBCwUAA4IBAQCZN1mL
0jdSLkLh0xaX4Z6icrz+8Hd2dn4Qg1VQtBtW2uYh2ds3SBHQqnoos+tdTJ4WzC7J
hSuIp0d6JaA4ZS7BO8ucH0UzPh0SFfx9NWNiv6aA8SJta0HLD6DBSftdU+YlLWbH
t7D96rOpdf+mJCTpHcJ2HLQWFF1i6cJe1rFhemb0+VHAV1JIC5C5XiJYZmlOYDeG
+IgScyuPmD0fmVdccofm1XxdEdpT2Y/PVZvbfqOfPBpXr6+kU1zjKURfgMyap6nv
JuIho07OU6iGgb41XoQLjC1Ft7AdgPqqCQ141ROpC4Nyne5RDvvCqLiHKJEWpmGo
tyT7FPRPXZhcROGq
-----END CERTIFICATE-----
Loading

0 comments on commit e092e3f

Please sign in to comment.