Skip to content

Commit

Permalink
Merge "Generalize TLS creds builder configuration" from Pavel E
Browse files Browse the repository at this point in the history 
"
There are 4 places out there that do the same steps parsing
"client_|server_encryption_options" and configuring the
seastar::tls::creds_builder with the values (messaging, redis,
alternator and transport).

Also to make redis and transport look slimmer main() cleans
the client_encryption_options by ... parsing it too.

This set introduces a (coroutinized) helper to configure the
creds_builder with map<string, string> and removes the options
beautification from main.

tests: unit(dev), dtest.internode_ssl_test(dev)
"

* 'br-generalize-tls-creds-builder-configuration' of https://github.com/xemul/scylla:
  code: Generalize tls::credentials_builder configuration
  transport, redis: Do not assume fixed encryption options
  messaging: Move encryption options parsing to ms
  main: Open-code internode encryption misconfig warning
  main, config: Move options parsing helpers
  • Loading branch information
@avikivity
avikivity committed Sep 1, 2021
2 parents 72bc37d + e02b39c commit 705f957
Show file tree
Hide file tree
Showing 8 changed files with 81 additions and 126 deletions.
21 changes: 3 additions & 18 deletions alternator/controller.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,6 @@ using namespace seastar;

namespace alternator {

template<typename K, typename V, typename... Args, typename K2, typename V2 = V>
V get_or_default(const std::unordered_map<K, V, Args...>& ss, const K2& key, const V2& def = V()) {
const auto iter = ss.find(key);
if (iter != ss.end()) {
return iter->second;
}
return def;
}

static logging::logger logger("alternator_controller");

controller::controller(sharded<service::storage_proxy>& proxy,
Expand Down Expand Up @@ -108,15 +99,9 @@ future<> controller::start() {
"switch to setting alternator_encryption_options instead.");
}
}
creds->set_dh_level(tls::dh_params::level::MEDIUM);
auto cert = get_or_default(opts, "certificate", db::config::get_conf_sub("scylla.crt").string());
auto key = get_or_default(opts, "keyfile", db::config::get_conf_sub("scylla.key").string());
creds->set_x509_key_file(cert, key, tls::x509_crt_format::PEM).get();
auto prio = get_or_default(opts, "priority_string", sstring());
creds->set_priority_string(db::config::default_tls_priority);
if (!prio.empty()) {
creds->set_priority_string(prio);
}
opts.erase("require_client_auth");
opts.erase("truststore");
utils::configure_tls_creds_builder(creds.value(), std::move(opts)).get();
}
bool alternator_enforce_authorization = _config.alternator_enforce_authorization();
_server.invoke_on_all(
Expand Down
22 changes: 22 additions & 0 deletions db/config.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,10 @@
#include <boost/program_options.hpp>
#include <yaml-cpp/yaml.h>

#include <seastar/core/coroutine.hh>
#include <seastar/core/print.hh>
#include <seastar/util/log.hh>
#include <seastar/net/tls.hh>

#include "cdc/cdc_extension.hh"
#include "config.hh"
Expand Down Expand Up @@ -1039,4 +1041,24 @@ sstring config_value_as_json(const std::unordered_map<sstring, log_level>& v) {
throw std::runtime_error("config_value_as_json(const std::unordered_map<sstring, log_level>& v) is not implemented");
}

future<> configure_tls_creds_builder(seastar::tls::credentials_builder& creds, db::config::string_map options) {
creds.set_dh_level(seastar::tls::dh_params::level::MEDIUM);
creds.set_priority_string(db::config::default_tls_priority);

if (options.contains("priority_string")) {
creds.set_priority_string(options.at("priority_string"));
}
if (is_true(get_or_default(options, "require_client_auth", "false"))) {
creds.set_client_auth(seastar::tls::client_auth::REQUIRE);
}

auto cert = get_or_default(options, "certificate", db::config::get_conf_sub("scylla.crt").string());
auto key = get_or_default(options, "keyfile", db::config::get_conf_sub("scylla.key").string());
co_await creds.set_x509_key_file(cert, key, seastar::tls::x509_crt_format::PEM);

if (options.contains("truststore")) {
co_await creds.set_x509_trust_file(options.at("truststore"), seastar::tls::x509_crt_format::PEM);
}
}

}
28 changes: 27 additions & 1 deletion db/config.hh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,13 @@
#include "utils/enum_option.hh"
#include "db/hints/host_filter.hh"

namespace seastar { class file; struct logging_settings; }
namespace seastar {
class file;
struct logging_settings;
namespace tls {
class credentials_builder;
}
}

namespace db {

Expand Down Expand Up @@ -396,3 +402,23 @@ private:
};

}

namespace utils {

template<typename K, typename V, typename... Args, typename K2, typename V2 = V>
V get_or_default(const std::unordered_map<K, V, Args...>& ss, const K2& key, const V2& def = V()) {
const auto iter = ss.find(key);
if (iter != ss.end()) {
return iter->second;
}
return def;
}

inline bool is_true(sstring val) {
std::transform(val.begin(), val.end(), val.begin(), ::tolower);
return val == "true" || val == "1";
}

future<> configure_tls_creds_builder(seastar::tls::credentials_builder& creds, db::config::string_map options);

}
62 changes: 10 additions & 52 deletions main.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -144,15 +144,6 @@ class stop_signal {
sharded<abort_source>& as_sharded_abort_source() { return _abort_sources; }
};

template<typename K, typename V, typename... Args, typename K2, typename V2 = V>
V get_or_default(const std::unordered_map<K, V, Args...>& ss, const K2& key, const V2& def = V()) {
const auto iter = ss.find(key);
if (iter != ss.end()) {
return iter->second;
}
return def;
}

static future<>
read_config(bpo::variables_map& opts, db::config& cfg) {
sstring file;
Expand Down Expand Up @@ -697,27 +688,6 @@ int main(int ac, char** av) {
utils::fb_utilities::set_broadcast_rpc_address(gms::inet_address::lookup(rpc_address, family, preferred).get0());
}

// TODO: lib.
auto is_true = [](sstring val) {
std::transform(val.begin(), val.end(), val.begin(), ::tolower);
return val == "true" || val == "1";
};

// The start_native_transport method is invoked by API as well, and uses the config object
// (through db) directly. Lets fixup default valued right here instead then, so it in turn can be
// kept simple
// TODO: make intrinsic part of config defaults instead
auto ceo = cfg->client_encryption_options();
if (is_true(get_or_default(ceo, "enabled", "false"))) {
ceo["enabled"] = "true";
ceo["certificate"] = get_or_default(ceo, "certificate", db::config::get_conf_sub("scylla.crt").string());
ceo["keyfile"] = get_or_default(ceo, "keyfile", db::config::get_conf_sub("scylla.key").string());
ceo["require_client_auth"] = is_true(get_or_default(ceo, "require_client_auth", "false")) ? "true" : "false";
} else {
ceo["enabled"] = "false";
}
cfg->client_encryption_options(std::move(ceo), cfg->client_encryption_options.source());

using namespace locator;
// Re-apply strict-dma after we've read the config file, this time
// to all reactors
Expand Down Expand Up @@ -798,14 +768,6 @@ int main(int ac, char** av) {
dbcfg.gossip_scheduling_group = make_sched_group("gossip", 1000);
dbcfg.available_memory = memory::stats().total_memory();

const auto& ssl_opts = cfg->server_encryption_options();
auto encrypt_what = get_or_default(ssl_opts, "internode_encryption", "none");
auto trust_store = get_or_default(ssl_opts, "truststore");
auto cert = get_or_default(ssl_opts, "certificate", db::config::get_conf_sub("scylla.crt").string());
auto key = get_or_default(ssl_opts, "keyfile", db::config::get_conf_sub("scylla.key").string());
auto prio = get_or_default(ssl_opts, "priority_string", sstring());
auto clauth = is_true(get_or_default(ssl_opts, "require_client_auth", "false"));

netw::messaging_service::config mscfg;

mscfg.ip = gms::inet_address::lookup(listen_address, family).get0();
Expand All @@ -814,19 +776,15 @@ int main(int ac, char** av) {
mscfg.listen_on_broadcast_address = cfg->listen_on_broadcast_address();
mscfg.rpc_memory_limit = std::max<size_t>(0.08 * memory::stats().total_memory(), mscfg.rpc_memory_limit);

if (encrypt_what == "all") {
mscfg.encrypt = netw::messaging_service::encrypt_what::all;
} else if (encrypt_what == "dc") {
mscfg.encrypt = netw::messaging_service::encrypt_what::dc;
} else if (encrypt_what == "rack") {
mscfg.encrypt = netw::messaging_service::encrypt_what::rack;
}

if (clauth && (mscfg.encrypt == netw::messaging_service::encrypt_what::dc || mscfg.encrypt == netw::messaging_service::encrypt_what::rack)) {
startlog.warn("Setting require_client_auth is incompatible with 'rack' and 'dc' internode_encryption values."
" To ensure that mutual TLS authentication is enforced, please set internode_encryption to 'all'. Continuing with"
" potentially insecure configuration."
);
const auto& seo = cfg->server_encryption_options();
if (utils::is_true(utils::get_or_default(seo, "require_client_auth", "false"))) {
auto encrypt = utils::get_or_default(seo, "internode_encryption", "none");
if (encrypt == "dc" || encrypt == "rack") {
startlog.warn("Setting require_client_auth is incompatible with 'rack' and 'dc' internode_encryption values."
" To ensure that mutual TLS authentication is enforced, please set internode_encryption to 'all'. Continuing with"
" potentially insecure configuration."
);
}
}

sstring compress_what = cfg->internode_compression();
Expand Down Expand Up @@ -861,7 +819,7 @@ int main(int ac, char** av) {
scfg.gossip = dbcfg.gossip_scheduling_group;

debug::the_messaging_service = &messaging;
netw::init_messaging_service(messaging, std::move(mscfg), std::move(scfg), trust_store, cert, key, prio, clauth);
netw::init_messaging_service(messaging, std::move(mscfg), std::move(scfg), *cfg);
auto stop_ms = defer_verbose_shutdown("messaging service", [&messaging] {
netw::uninit_messaging_service(messaging).get();
});
Expand Down
32 changes: 13 additions & 19 deletions message/messaging_service.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1591,32 +1591,26 @@ future<raft::read_barrier_reply> messaging_service::send_raft_execute_read_barri
}

void init_messaging_service(sharded<messaging_service>& ms,
messaging_service::config mscfg, netw::messaging_service::scheduling_config scfg,
sstring ms_trust_store, sstring ms_cert, sstring ms_key, sstring ms_tls_prio, bool ms_client_auth) {
messaging_service::config mscfg, netw::messaging_service::scheduling_config scfg, const db::config& db_config) {
using encrypt_what = messaging_service::encrypt_what;
using namespace seastar::tls;

const auto& ssl_opts = db_config.server_encryption_options();
auto encrypt = utils::get_or_default(ssl_opts, "internode_encryption", "none");

if (encrypt == "all") {
mscfg.encrypt = encrypt_what::all;
} else if (encrypt == "dc") {
mscfg.encrypt = encrypt_what::dc;
} else if (encrypt == "rack") {
mscfg.encrypt = encrypt_what::rack;
}

std::shared_ptr<credentials_builder> creds;

if (mscfg.encrypt != encrypt_what::none) {
creds = std::make_shared<credentials_builder>();
creds->set_dh_level(dh_params::level::MEDIUM);

creds->set_x509_key_file(ms_cert, ms_key, x509_crt_format::PEM).get();
if (ms_trust_store.empty()) {
creds->set_system_trust().get();
} else {
creds->set_x509_trust_file(ms_trust_store, x509_crt_format::PEM).get();
}

creds->set_priority_string(db::config::default_tls_priority);

if (!ms_tls_prio.empty()) {
creds->set_priority_string(ms_tls_prio);
}
if (ms_client_auth) {
creds->set_client_auth(seastar::tls::client_auth::REQUIRE);
}
utils::configure_tls_creds_builder(*creds, std::move(ssl_opts)).get();
}

// Init messaging_service
Expand Down
4 changes: 2 additions & 2 deletions message/messaging_service.hh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ namespace utils {

namespace db {
class seed_provider_type;
class config;
}

namespace db::view {
Expand Down Expand Up @@ -611,8 +612,7 @@ public:
};

void init_messaging_service(sharded<messaging_service>& ms,
messaging_service::config cfg, messaging_service::scheduling_config scheduling_config,
sstring ms_trust_store, sstring ms_cert, sstring ms_key, sstring ms_tls_prio, bool ms_client_auth);
messaging_service::config cfg, messaging_service::scheduling_config scheduling_config, const db::config& db_config);
future<> uninit_messaging_service(sharded<messaging_service>& ms);

} // namespace netw
19 changes: 2 additions & 17 deletions redis/service.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,24 +72,9 @@ future<> redis_service::listen(seastar::sharded<auth::service>& auth_service, db
}

// main should have made sure values are clean and neatish
if (ceo.at("enabled") == "true") {
if (utils::is_true(utils::get_or_default(ceo, "enabled", "false"))) {
auto cred = std::make_shared<seastar::tls::credentials_builder>();

cred->set_dh_level(seastar::tls::dh_params::level::MEDIUM);
cred->set_priority_string(db::config::default_tls_priority);

if (ceo.contains("priority_string")) {
cred->set_priority_string(ceo.at("priority_string"));
}
if (ceo.contains("require_client_auth") && ceo.at("require_client_auth") == "true") {
cred->set_client_auth(seastar::tls::client_auth::REQUIRE);
}

f = cred->set_x509_key_file(ceo.at("certificate"), ceo.at("keyfile"), seastar::tls::x509_crt_format::PEM);

if (ceo.contains("truststore")) {
f = f.then([cred, f = ceo.at("truststore")] { return cred->set_x509_trust_file(f, seastar::tls::x509_crt_format::PEM); });
}
f = utils::configure_tls_creds_builder(*cred, std::move(ceo));

slogger.info("Enabling encrypted REDIS connections between client and server");

Expand Down
19 changes: 2 additions & 17 deletions transport/controller.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,24 +113,9 @@ future<> controller::do_start_server() {
}

// main should have made sure values are clean and neatish
if (ceo.at("enabled") == "true") {
if (utils::is_true(utils::get_or_default(ceo, "enabled", "false"))) {
auto cred = std::make_shared<seastar::tls::credentials_builder>();

cred->set_dh_level(seastar::tls::dh_params::level::MEDIUM);
cred->set_priority_string(db::config::default_tls_priority);

if (ceo.contains("priority_string")) {
cred->set_priority_string(ceo.at("priority_string"));
}
if (ceo.contains("require_client_auth") && ceo.at("require_client_auth") == "true") {
cred->set_client_auth(seastar::tls::client_auth::REQUIRE);
}

cred->set_x509_key_file(ceo.at("certificate"), ceo.at("keyfile"), seastar::tls::x509_crt_format::PEM).get();

if (ceo.contains("truststore")) {
cred->set_x509_trust_file(ceo.at("truststore"), seastar::tls::x509_crt_format::PEM).get();
}
utils::configure_tls_creds_builder(*cred, std::move(ceo)).get();

logger.info("Enabling encrypted CQL connections between client and server");

Expand Down

0 comments on commit 705f957

Please sign in to comment.