Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CORE-3168 support p12 #21313

Merged
merged 6 commits into from
Sep 9, 2024
Merged

CORE-3168 support p12 #21313

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
cb10bc4
configuration: Add support for P12 files
michael-redpanda Jul 2, 2024
28a09c8
dt: Cleanup of tls.py
michael-redpanda Jul 2, 2024
5151053
dt: Added ability to generate P12 files
michael-redpanda Jul 2, 2024
c50a2c5
dt: Added saving of PKCS12 file to node
michael-redpanda Jul 2, 2024
18ddeb1
dt: Added plumbing for PKCS12 file
michael-redpanda Jul 2, 2024
90d4bdb
dt: Added pkcs12 test
michael-redpanda Jul 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 15 additions & 5 deletions src/v/config/rjson_serialization.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

#include "config/rjson_serialization.h"

#include "config/tls_config.h"
#include "config/types.h"

namespace json {
Expand Down Expand Up @@ -51,11 +52,20 @@ void rjson_serialize_impl(
w.Bool(v.get_require_client_auth());

if (v.get_key_cert_files()) {
w.Key("key_file");
w.String(v.get_key_cert_files()->key_file.c_str());

w.Key("cert_file");
w.String(v.get_key_cert_files()->cert_file.c_str());
ss::visit(
v.get_key_cert_files().value(),
[&w](const config::key_cert& k) {
w.Key("key_file");
w.String(k.key_file.c_str());
w.Key("cert_file");
w.String(k.cert_file.c_str());
},
[&w](const config::p12_container& p) {
w.Key("p12_file");
w.String(p.p12_path.c_str());
w.Key("p12_password");
w.String("REDACTED");
});
}

if (v.get_truststore_file()) {
Expand Down
93 changes: 85 additions & 8 deletions src/v/config/tests/tls_config_convert_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,13 @@
// by the Apache License, Version 2.0

#include "config/configuration.h"
#include "config/tls_config.h"
#include "utils/to_string.h"

#include <seastar/core/thread.hh>
#include <seastar/testing/thread_test_case.hh>

#include <yaml-cpp/exceptions.h>
#include <yaml-cpp/yaml.h>

config::tls_config read_from_yaml(ss::sstring yaml_string) {
Expand Down Expand Up @@ -40,10 +42,10 @@ SEASTAR_THREAD_TEST_CASE(test_decode_full_abs_path) {
" require_client_auth: true\n";
auto full_cfg = read_from_yaml(with_values);
BOOST_TEST(full_cfg.is_enabled());
BOOST_TEST(
(*full_cfg.get_key_cert_files()).key_file == "/fake/key_file.key");
BOOST_TEST(
(*full_cfg.get_key_cert_files()).cert_file == "/fake/cret_file.crt");
const auto& key_cert = std::get<config::key_cert>(
*full_cfg.get_key_cert_files());
BOOST_TEST(key_cert.key_file == "/fake/key_file.key");
BOOST_TEST(key_cert.cert_file == "/fake/cret_file.crt");
BOOST_TEST(*full_cfg.get_truststore_file() == "/fake/truststore");
BOOST_TEST(*full_cfg.get_crl_file() == "/fake/crl");
BOOST_TEST(full_cfg.get_require_client_auth());
Expand All @@ -59,8 +61,10 @@ SEASTAR_THREAD_TEST_CASE(test_decode_full_rel_path) {
" require_client_auth: true\n";
auto full_cfg = read_from_yaml(with_values);
BOOST_TEST(full_cfg.is_enabled());
BOOST_TEST((*full_cfg.get_key_cert_files()).key_file != "./key_file.key");
BOOST_TEST((*full_cfg.get_key_cert_files()).cert_file != "./cret_file.crt");
const auto& key_cert = std::get<config::key_cert>(
*full_cfg.get_key_cert_files());
BOOST_TEST(key_cert.key_file != "./key_file.key");
BOOST_TEST(key_cert.cert_file != "./cret_file.crt");
BOOST_TEST(*full_cfg.get_truststore_file() != "./truststore");
BOOST_TEST(*full_cfg.get_crl_file() != "./crl");
BOOST_TEST(full_cfg.get_require_client_auth());
Expand Down Expand Up @@ -92,9 +96,82 @@ SEASTAR_THREAD_TEST_CASE(test_decode_enabled_but_contains_empty_path) {
" require_client_auth: false\n";
auto full_cfg = read_from_yaml(with_values);
BOOST_TEST(full_cfg.is_enabled());
BOOST_TEST((*full_cfg.get_key_cert_files()).key_file == "");
BOOST_TEST((*full_cfg.get_key_cert_files()).cert_file == "");
const auto& key_cert = std::get<config::key_cert>(
*full_cfg.get_key_cert_files());
BOOST_TEST(key_cert.key_file == "");
BOOST_TEST(key_cert.cert_file == "");
BOOST_TEST(*full_cfg.get_truststore_file() == "");
BOOST_TEST(*full_cfg.get_crl_file() == "");
BOOST_TEST(!full_cfg.get_require_client_auth());
}

SEASTAR_THREAD_TEST_CASE(test_decode_p12_file) {
auto with_values = "tls_config:\n"
" enabled: true\n"
" truststore_file: /fake/truststore\n"
" crl_file: /fake/crl\n"
" require_client_auth: true\n"
" p12_file: /fake/temp.pfx\n"
" p12_password: test\n";
auto full_cfg = read_from_yaml(with_values);
BOOST_TEST(full_cfg.is_enabled());
const auto& p12_bag = std::get<config::p12_container>(
*full_cfg.get_key_cert_files());
BOOST_TEST(p12_bag.p12_path == "/fake/temp.pfx");
BOOST_TEST(p12_bag.p12_password == "test");
BOOST_TEST(*full_cfg.get_truststore_file() == "/fake/truststore");
BOOST_TEST(*full_cfg.get_crl_file() == "/fake/crl");
BOOST_TEST(full_cfg.get_require_client_auth());
}

SEASTAR_THREAD_TEST_CASE(test_decode_p12_full_rel_path) {
auto with_values = "tls_config:\n"
" enabled: true\n"
" truststore_file: ./truststore\n"
" crl_file: ./crl\n"
" require_client_auth: true\n"
" p12_file: ./temp.pfx\n"
" p12_password: test\n";
auto full_cfg = read_from_yaml(with_values);
BOOST_TEST(full_cfg.is_enabled());
const auto& p12_bag = std::get<config::p12_container>(
*full_cfg.get_key_cert_files());
BOOST_TEST(p12_bag.p12_path != "./temp.pfx");
BOOST_TEST(p12_bag.p12_password == "test");
BOOST_TEST(*full_cfg.get_truststore_file() != "./truststore");
BOOST_TEST(*full_cfg.get_crl_file() != "./crl");
BOOST_TEST(full_cfg.get_require_client_auth());
}

SEASTAR_THREAD_TEST_CASE(test_decode_p12_and_key_cert) {
auto with_values = "tls_config:\n"
" enabled: true\n"
" cert_file: /fake/cret_file.crt\n"
" key_file: /fake/key_file.key\n"
" truststore_file: /fake/truststore\n"
" crl_file: /fake/crl\n"
" require_client_auth: true\n"
" p12_file: /fake/temp.pfx\n"
" p12_password: test\n";
try {
read_from_yaml(with_values);
BOOST_REQUIRE(false);
} catch (const YAML::TypedBadConversion<config::tls_config>&) {
// good!
}
}

SEASTAR_THREAD_TEST_CASE(test_decode_p12_missing_password) {
auto with_values = "tls_config:\n"
" enabled: true\n"
" truststore_file: /fake/truststore\n"
" crl_file: /fake/crl\n"
" require_client_auth: true\n"
" p12_file: /fake/temp.pfx\n";
try {
read_from_yaml(with_values);
BOOST_REQUIRE(false);
} catch (const YAML::TypedBadConversion<config::tls_config>&) {
// good!
}
}
83 changes: 68 additions & 15 deletions src/v/config/tls_config.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@

#include <seastar/core/do_with.hh>
#include <seastar/core/shared_ptr.hh>
#include <seastar/net/tls.hh>
#include <seastar/util/variant_utils.hh>

namespace config {
ss::future<std::optional<ss::tls::credentials_builder>>
Expand Down Expand Up @@ -47,10 +49,20 @@ tls_config::get_credentials_builder() const& {

if (_key_cert) {
f = f.then([this, &builder] {
return builder.set_x509_key_file(
(*_key_cert).cert_file,
(*_key_cert).key_file,
ss::tls::x509_crt_format::PEM);
return ss::visit(
_key_cert.value(),
[&builder](const key_cert& c) {
return builder.set_x509_key_file(
c.cert_file,
c.key_file,
ss::tls::x509_crt_format::PEM);
},
[&builder](const p12_container& p) {
return builder.set_simple_pkcs12_file(
p.p12_path,
ss::tls::x509_crt_format::PEM,
p.p12_password);
});
});
}

Expand All @@ -71,14 +83,28 @@ tls_config::get_credentials_builder() && {
}

std::optional<ss::sstring> tls_config::validate(const tls_config& c) {
if (c.get_require_client_auth() && !c.get_truststore_file()) {
return "Trust store is required when client authentication is "
"enabled";
const auto contains_p12_file = [&c]() {
if (c.get_key_cert_files()) {
return std::holds_alternative<config::p12_container>(
(*c.get_key_cert_files()));
}
return false;
};
if (
c.get_require_client_auth() && !c.get_truststore_file()
&& !contains_p12_file()) {
return "Trust store or P12 file is required when client authentication "
"is enabled";
}

return std::nullopt;
}

std::ostream& operator<<(std::ostream& o, const config::p12_container& p) {
fmt::print(o, "{{ p12 file: {}, p12 password: REDACTED }}", p.p12_password);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@michael-redpanda did you intend to print the file, rather than the password?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

presumably, yeah, but Mike can confirm. PR because it's a trivial: #23249

return o;
}

std::ostream& operator<<(std::ostream& o, const config::key_cert& c) {
o << "{ "
<< "key_file: " << c.key_file << " "
Expand Down Expand Up @@ -123,8 +149,16 @@ Node convert<config::tls_config>::encode(const config::tls_config& rhs) {
node["require_client_auth"] = rhs.get_require_client_auth();

if (rhs.get_key_cert_files()) {
node["cert_file"] = (*rhs.get_key_cert_files()).key_file;
node["key_file"] = (*rhs.get_key_cert_files()).cert_file;
ss::visit(
rhs.get_key_cert_files().value(),
[&node](const config::key_cert& c) {
node["cert_file"] = c.cert_file;
node["key_file"] = c.key_file;
},
[&node](const config::p12_container& p12) {
node["p12_file"] = p12.p12_path;
node["p12_password"] = "REDACTED";
});
}

if (rhs.get_truststore_file()) {
Expand All @@ -150,19 +184,38 @@ bool convert<config::tls_config>::decode(
^ static_cast<bool>(node["cert_file"])) {
return false;
}

// Must have either both or neither for PKCS#12 files
if (
static_cast<bool>(node["p12_file"])
^ static_cast<bool>(node["p12_password"])) {
return false;
}

// Cannot have both key/cert and P12 file
if (
static_cast<bool>(node["key_file"])
&& static_cast<bool>(node["p12_file"])) {
return false;
}
auto enabled = node["enabled"] && node["enabled"].as<bool>();
if (!enabled) {
rhs = config::tls_config(
false, std::nullopt, std::nullopt, std::nullopt, false);
} else {
auto key_cert = node["key_file"] ? std::make_optional<config::key_cert>(
config::key_cert{
to_absolute(node["key_file"].as<ss::sstring>()),
to_absolute(node["cert_file"].as<ss::sstring>())})
: std::nullopt;
std::optional<config::key_cert_container> container;
if (node["key_file"]) {
container.emplace(config::key_cert{
to_absolute(node["key_file"].as<ss::sstring>()),
to_absolute(node["cert_file"].as<ss::sstring>())});
} else if (node["p12_file"]) {
container.emplace(config::p12_container{
to_absolute(node["p12_file"].as<ss::sstring>()),
node["p12_password"].as<ss::sstring>()});
}
rhs = config::tls_config(
enabled,
key_cert,
container,
to_absolute(read_optional(node, "truststore_file")),
to_absolute(read_optional(node, "crl_file")),
node["require_client_auth"]
Expand Down
19 changes: 16 additions & 3 deletions src/v/config/tls_config.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include <yaml-cpp/yaml.h>

#include <optional>
#include <variant>

namespace config {

Expand All @@ -34,6 +35,18 @@ struct key_cert {
friend std::ostream& operator<<(std::ostream& o, const key_cert& c);
};

struct p12_container {
ss::sstring p12_path;
ss::sstring p12_password;

friend bool operator==(const p12_container&, const p12_container&)
= default;

friend std::ostream& operator<<(std::ostream& os, const p12_container& p);
};

using key_cert_container = std::variant<key_cert, p12_container>;

inline constexpr std::string_view tlsv1_2_cipher_string
= "ECDHE-RSA-AES128-GCM-SHA256:AES128-GCM-SHA256:ECDHE-RSA-AES256-GCM-SHA384:"
"AES256-GCM-SHA384:ECDHE-RSA-CHACHA20-POLY1305:ECDHE-RSA-AES128-SHA:AES128-"
Expand All @@ -51,7 +64,7 @@ class tls_config {

tls_config(
bool enabled,
std::optional<key_cert> key_cert,
std::optional<key_cert_container> key_cert,
std::optional<ss::sstring> truststore,
std::optional<ss::sstring> crl,
bool require_client_auth)
Expand All @@ -63,7 +76,7 @@ class tls_config {

bool is_enabled() const { return _enabled; }

const std::optional<key_cert>& get_key_cert_files() const {
const std::optional<key_cert_container>& get_key_cert_files() const {
return _key_cert;
}

Expand Down Expand Up @@ -91,7 +104,7 @@ class tls_config {

private:
bool _enabled{false};
std::optional<key_cert> _key_cert;
std::optional<key_cert_container> _key_cert;
std::optional<ss::sstring> _truststore_file;
std::optional<ss::sstring> _crl_file;
bool _require_client_auth{false};
Expand Down
Loading