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.

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

Send SNI information when connecting via TLS #7

Open
wants to merge 8 commits into
base: vast/0.18.x
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions libcaf_core/caf/actor_system_config.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@ class CAF_CORE_EXPORT actor_system_config {
std::string openssl_passphrase;
std::string openssl_capath;
std::string openssl_cafile;
bool hostname_validation;

// -- factories --------------------------------------------------------------

Expand Down
1 change: 1 addition & 0 deletions libcaf_core/src/actor_system_config.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,7 @@ settings actor_system_config::dump_content() const {
put_missing(openssl_group, "passphrase", std::string{});
put_missing(openssl_group, "capath", std::string{});
put_missing(openssl_group, "cafile", std::string{});
put_missing(openssl_group, "hostname-validation", true);
return result;
}

Expand Down
4 changes: 3 additions & 1 deletion libcaf_openssl/caf/openssl/session.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ class CAF_OPENSSL_EXPORT session {
rw_state read_some(size_t& result, native_socket fd, void* buf, size_t len);
rw_state
write_some(size_t& result, native_socket fd, const void* buf, size_t len);
bool try_connect(native_socket fd);
bool try_connect(native_socket fd, const std::string& sni_servername);
bool try_accept(native_socket fd);

bool must_read_more(native_socket, size_t threshold);
Expand All @@ -61,13 +61,15 @@ class CAF_OPENSSL_EXPORT session {
std::string openssl_passphrase_;
bool connecting_;
bool accepting_;
bool hostname_validation_;
};

/// @relates session
using session_ptr = std::unique_ptr<session>;

/// @relates session
CAF_OPENSSL_EXPORT session_ptr make_session(actor_system& sys, native_socket fd,
const std::string& servername,
bool from_accepted_socket);

} // namespace caf::openssl
4 changes: 3 additions & 1 deletion libcaf_openssl/src/openssl/manager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,9 @@ void manager::add_module_options(actor_system_config& cfg) {
"path to an OpenSSL-style directory of trusted certificates")
.add<std::string>(
cfg.openssl_cafile, "cafile",
"path to a file of concatenated PEM-formatted certificates");
"path to a file of concatenated PEM-formatted certificates")
.add<bool>(cfg.hostname_validation, "enable-hostname-validation",
"explicitly toggle hostname validation on outgoing connections");
}

actor_system::module* manager::make(actor_system& sys, detail::type_list<>) {
Expand Down
4 changes: 2 additions & 2 deletions libcaf_openssl/src/openssl/middleman_actor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ class doorman_impl : public io::network::doorman_impl {
auto fd = acceptor_.accepted_socket();
detail::socket_guard sguard{fd};
io::network::nonblocking(fd, true);
auto sssn = make_session(parent()->system(), fd, true);
auto sssn = make_session(parent()->system(), fd, "", true);
if (sssn == nullptr) {
CAF_LOG_ERROR("Unable to create SSL session for accepted socket");
return false;
Expand Down Expand Up @@ -245,7 +245,7 @@ class middleman_actor_impl : public io::middleman_actor_impl {
if (!fd)
return std::move(fd.error());
io::network::nonblocking(*fd, true);
auto sssn = make_session(system(), *fd, false);
auto sssn = make_session(system(), *fd, host, false);
if (!sssn) {
CAF_LOG_ERROR("Unable to create SSL session for connection");
return sec::cannot_connect_to_node;
Expand Down
107 changes: 99 additions & 8 deletions libcaf_openssl/src/openssl/session.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,11 @@

#include "caf/openssl/session.hpp"

#include <optional>

CAF_PUSH_WARNINGS
#include <openssl/err.h>
#include <openssl/x509v3.h>
CAF_POP_WARNINGS

#include "caf/actor_system_config.hpp"
Expand Down Expand Up @@ -56,6 +59,58 @@ int pem_passwd_cb(char* buf, int size, int, void* ptr) {
return static_cast<int>(strlen(buf));
}

std::optional<std::string> contents_from_direct_envvar(const std::string& str) {
return str;
}

// If the input is a string like `env:SOME_VARIABLE` and the environment variable
// `SOME_VARIABLE` exists, returns a string with the value of `SOME_VARIABLE`.
// Otherwise, returns `std::nullopt`.
std::optional<std::string> contents_from_indirect_envvar(const std::string& str) {
if (str.find("env:") != 0)
return std::nullopt;
auto var = str.substr(4);
auto const* contents = ::getenv(var.c_str());
if (contents == nullptr)
return std::nullopt;
return std::string{contents};
}

// If the input is a string like `env:SOME_VARIABLE` and the environment variable
// `SOME_VARIABLE` exists, returns a string with the value of `SOME_VARIABLE`.
std::optional<std::string> tmpfile_from_string(const std::string& content) {
const char* base = ::getenv("TMPDIR");
if (!base)
base = "/tmp";
auto filename = base + std::string{"/caf-openssl.XXXXXX"};
::mkstemp(&filename[0]);
std::ofstream ofs(filename);
ofs << content;
ofs.close();
return filename;
}
Copy link
Member Author

Choose a reason for hiding this comment

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

I just realized that this approach will only work for docker-compose, there's no good way to inject environment variables when the user uses his own vast and a vast.yaml.

However adding the value without the indirection has the disadvantage that the secrets will appear everywhere the VAST config is printed.

Copy link
Member

Choose a reason for hiding this comment

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

Can we make it possible to pass the file path in the env variable for the second case?

Copy link
Member Author

Choose a reason for hiding this comment

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

A file path is how it already works, before our patches.

I think the cleanest solution is to not pass this via config at all but to provide a /get-client-certificates endpoint where we can download everything before starting CAF. (and to only use this mechanism for the manager nodes) But for the short term I just added support for putting the PEM directly into the option.


// Returns a string in PEM format, ie. base64-encoded data surrounded
// by `-----BEGIN XXX` and `-----END XXX` blocks.
std::optional<std::string> pem_from_envvar(const std::string& str) {
if (str.find("env:") == 0)
return contents_from_indirect_envvar(str);
// The PEM format isn't very strictly defined, some implementations
// write the header as `---- BEGIN`. However, we probably don't want to
// keep supporting this in the long run anyways, so we're fine with just
// detecting exactly those certificates that we create ourselves.
if (str.find("-----BEGIN") == 0)
return contents_from_direct_envvar(str);
return std::nullopt;
}

std::optional<std::string> pem_file_from_envvar(const std::string& str) {
if (auto contents = pem_from_envvar(str))
return tmpfile_from_string(*contents);
return std::nullopt;
}


} // namespace

session::session(actor_system& sys)
Expand Down Expand Up @@ -154,11 +209,22 @@ rw_state session::write_some(size_t& result, native_socket, const void* buf,
return do_some(wr_fun, result, const_cast<void*>(buf), len, "write_some");
}

bool session::try_connect(native_socket fd) {
bool session::try_connect(native_socket fd, const std::string& sni_servername) {
CAF_LOG_TRACE(CAF_ARG(fd));
CAF_BLOCK_SIGPIPE();
SSL_set_fd(ssl_, fd);
SSL_set_connect_state(ssl_);
#if OPENSSL_VERSION_NUMBER >= 0x10100000L
// Enable hostname validation.
if (hostname_validation_) {
SSL_set_hostflags(ssl_, X509_CHECK_FLAG_NO_PARTIAL_WILDCARDS);
if (SSL_set1_host(ssl_, sni_servername.c_str()) != 1)
return false;
}
#endif
// Send SNI when connecting.
if (SSL_set_tlsext_host_name(ssl_, sni_servername.c_str()) != 1)
return false;
auto ret = SSL_connect(ssl_);
if (ret == 1)
return true;
Expand Down Expand Up @@ -198,25 +264,49 @@ SSL_CTX* session::create_ssl_context() {
if (sys_.openssl_manager().authentication_enabled()) {
// Require valid certificates on both sides.
auto& cfg = sys_.config();
if (!cfg.openssl_certificate.empty()
enable_hostname_validation_ = cfg.enable_hostname_validation;
// OpenSSL doesn't expose an API to read a certificate chain PEM from
// memory and the implementation of `SSL_CTX_use_certificate_chain_file`
// is huge, so instead of reproducing that we write into a temporary file.
if (auto filename = pem_file_from_envvar(cfg.openssl_certificate)) {
if (SSL_CTX_use_certificate_chain_file(ctx, filename->c_str())
!= 1)
CAF_RAISE_ERROR("cannot load certificate from environment variable");
} else if (!cfg.openssl_certificate.empty()
&& SSL_CTX_use_certificate_chain_file(ctx,
cfg.openssl_certificate.c_str())
!= 1)
!= 1) {
CAF_RAISE_ERROR("cannot load certificate");
}
if (!cfg.openssl_passphrase.empty()) {
openssl_passphrase_ = cfg.openssl_passphrase;
SSL_CTX_set_default_passwd_cb(ctx, pem_passwd_cb);
SSL_CTX_set_default_passwd_cb_userdata(ctx, this);
}
if (!cfg.openssl_key.empty()
&& SSL_CTX_use_PrivateKey_file(ctx, cfg.openssl_key.c_str(),
SSL_FILETYPE_PEM)
!= 1)
if (auto var = pem_from_envvar(cfg.openssl_key)) {
// BIO_new_mem_buf just creates a read-only view, so we don't need
// to free it afterwards.
auto* kbio = BIO_new_mem_buf(var->data(), -1);
if (kbio == nullptr)
CAF_RAISE_ERROR("failed to construct OpenSSL BIO");
// Starting from OpenSSL 3.0, the OSSL_DECODER API is the suggested
// alternative for this.
// TODO: Pass the pem_passwd_cb here if a passphrase was configured.
auto* pkey = PEM_read_bio_PrivateKey(kbio, nullptr, nullptr, nullptr);
if (pkey == nullptr)
CAF_RAISE_ERROR("invalid private key");
SSL_CTX_use_PrivateKey(ctx, pkey);
} else if (!cfg.openssl_key.empty()
&& SSL_CTX_use_PrivateKey_file(ctx, cfg.openssl_key.c_str(),
SSL_FILETYPE_PEM) != 1)
CAF_RAISE_ERROR("cannot load private key");
auto cafile = (!cfg.openssl_cafile.empty() ? cfg.openssl_cafile.c_str()
: nullptr);
auto capath = (!cfg.openssl_capath.empty() ? cfg.openssl_capath.c_str()
: nullptr);
auto tmpfile = pem_file_from_envvar(cafile);
if (tmpfile)
cafile = tmpfile->c_str();
if (cafile || capath) {
if (SSL_CTX_load_verify_locations(ctx, cafile, capath) != 1)
CAF_RAISE_ERROR("cannot load trusted CA certificates");
Expand Down Expand Up @@ -285,6 +375,7 @@ bool session::handle_ssl_result(int ret) {
}

session_ptr make_session(actor_system& sys, native_socket fd,
const std::string& servername,
bool from_accepted_socket) {
session_ptr ptr{new session(sys)};
if (!ptr->init())
Expand All @@ -293,7 +384,7 @@ session_ptr make_session(actor_system& sys, native_socket fd,
if (!ptr->try_accept(fd))
return nullptr;
} else {
if (!ptr->try_connect(fd))
if (!ptr->try_connect(fd, servername))
return nullptr;
}
return ptr;
Expand Down