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

fix: tls replication without ca verification #4539

Open
wants to merge 2 commits into
base: main
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
2 changes: 1 addition & 1 deletion src/facade/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ cxx_link(dfly_parser_lib base strings_lib)

add_library(dfly_facade conn_context.cc dragonfly_listener.cc dragonfly_connection.cc facade.cc
memcache_parser.cc reply_builder.cc op_status.cc service_interface.cc
reply_capture.cc cmd_arg_parser.cc tls_error.cc)
reply_capture.cc cmd_arg_parser.cc tls_helpers.cc)

if (DF_USE_SSL)
set(TLS_LIB tls_lib)
Expand Down
72 changes: 8 additions & 64 deletions src/facade/dragonfly_listener.cc
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright 2022, DragonflyDB authors. All rights reserved.
// Copyright 2025, DragonflyDB authors. All rights reserved.
// See LICENSE for licensing terms.
//

Expand All @@ -9,7 +9,7 @@
#include <memory>

#include "absl/functional/bind_front.h"
#include "facade/tls_error.h"
#include "facade/tls_helpers.h"

#ifdef DFLY_USE_SSL
#include <openssl/ssl.h>
Expand All @@ -29,12 +29,11 @@ ABSL_FLAG(bool, conn_use_incoming_cpu, false,
"If true uses incoming cpu of a socket in order to distribute"
" incoming connections");

ABSL_FLAG(string, tls_cert_file, "", "cert file for tls connections");
ABSL_FLAG(string, tls_key_file, "", "key file for tls connections");
ABSL_FLAG(string, tls_ca_cert_file, "", "ca signed certificate to validate tls connections");
ABSL_FLAG(string, tls_ca_cert_dir, "",
"ca signed certificates directory. Use c_rehash before, read description in "
"https://www.openssl.org/docs/man3.0/man1/c_rehash.html");
ABSL_DECLARE_FLAG(std::string, tls_cert_file);
ABSL_DECLARE_FLAG(std::string, tls_key_file);
ABSL_DECLARE_FLAG(std::string, tls_ca_cert_file);
ABSL_DECLARE_FLAG(std::string, tls_ca_cert_dir);

ABSL_FLAG(uint32_t, tcp_keepalive, 300,
"the period in seconds of inactivity after which keep-alives are triggerred,"
"the duration until an inactive connection is terminated is twice the specified time");
Expand Down Expand Up @@ -71,61 +70,6 @@ using absl::GetFlag;

namespace {

#ifdef DFLY_USE_SSL

// Creates the TLS context. Returns nullptr if the TLS configuration is invalid.
// To connect: openssl s_client -state -crlf -connect 127.0.0.1:6380
SSL_CTX* CreateSslServerCntx() {
const auto& tls_key_file = GetFlag(FLAGS_tls_key_file);
if (tls_key_file.empty()) {
LOG(ERROR) << "To use TLS, a server certificate must be provided with the --tls_key_file flag!";
return nullptr;
}

SSL_CTX* ctx = SSL_CTX_new(TLS_server_method());
unsigned mask = SSL_VERIFY_NONE;

if (SSL_CTX_use_PrivateKey_file(ctx, tls_key_file.c_str(), SSL_FILETYPE_PEM) != 1) {
LOG(ERROR) << "Failed to load TLS key";
return nullptr;
}
const auto& tls_cert_file = GetFlag(FLAGS_tls_cert_file);

if (!tls_cert_file.empty()) {
// TO connect with redis-cli you need both tls-key-file and tls-cert-file
// loaded. Use `redis-cli --tls -p 6380 --insecure PING` to test
if (SSL_CTX_use_certificate_chain_file(ctx, tls_cert_file.c_str()) != 1) {
LOG(ERROR) << "Failed to load TLS certificate";
return nullptr;
}
}

const auto tls_ca_cert_file = GetFlag(FLAGS_tls_ca_cert_file);
const auto tls_ca_cert_dir = GetFlag(FLAGS_tls_ca_cert_dir);
if (!tls_ca_cert_file.empty() || !tls_ca_cert_dir.empty()) {
const auto* file = tls_ca_cert_file.empty() ? nullptr : tls_ca_cert_file.data();
const auto* dir = tls_ca_cert_dir.empty() ? nullptr : tls_ca_cert_dir.data();
if (SSL_CTX_load_verify_locations(ctx, file, dir) != 1) {
LOG(ERROR) << "Failed to load TLS verify locations (CA cert file or CA cert dir)";
return nullptr;
}
mask = SSL_VERIFY_PEER | SSL_VERIFY_FAIL_IF_NO_PEER_CERT;
}

DFLY_SSL_CHECK(1 == SSL_CTX_set_cipher_list(ctx, "DEFAULT"));

SSL_CTX_set_min_proto_version(ctx, TLS1_2_VERSION);

SSL_CTX_set_options(ctx, SSL_OP_DONT_INSERT_EMPTY_FRAGMENTS);

SSL_CTX_set_verify(ctx, mask, NULL);

DFLY_SSL_CHECK(1 == SSL_CTX_set_dh_auto(ctx, 1));

return ctx;
}
#endif

bool ConfigureKeepAlive(int fd) {
int val = 1;
if (setsockopt(fd, SOL_SOCKET, SO_KEEPALIVE, &val, sizeof(val)) < 0)
Expand Down Expand Up @@ -260,7 +204,7 @@ error_code Listener::ConfigureServerSocket(int fd) {
bool Listener::ReconfigureTLS() {
SSL_CTX* prev_ctx = ctx_;
if (GetFlag(FLAGS_tls)) {
SSL_CTX* ctx = CreateSslServerCntx();
SSL_CTX* ctx = CreateSslCntx(facade::TlsContextRole::SERVER);
if (!ctx) {
return false;
}
Expand Down
2 changes: 1 addition & 1 deletion src/facade/dragonfly_listener.h
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright 2022, DragonflyDB authors. All rights reserved.
// Copyright 2025, DragonflyDB authors. All rights reserved.
// See LICENSE for licensing terms.
//

Expand Down
25 changes: 0 additions & 25 deletions src/facade/tls_error.cc

This file was deleted.

98 changes: 98 additions & 0 deletions src/facade/tls_helpers.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
// Copyright 2025, DragonflyDB authors. All rights reserved.
// See LICENSE for licensing terms.
//
#include "tls_helpers.h"

#include <openssl/err.h>

#ifdef DFLY_USE_SSL
#include <openssl/ssl.h>
#endif

#include <string>

#include "absl/functional/bind_front.h"
#include "base/flags.h"
#include "base/logging.h"

ABSL_FLAG(std::string, tls_cert_file, "", "cert file for tls connections");
ABSL_FLAG(std::string, tls_key_file, "", "key file for tls connections");
ABSL_FLAG(std::string, tls_ca_cert_file, "", "ca signed certificate to validate tls connections");
ABSL_FLAG(std::string, tls_ca_cert_dir, "",
"ca signed certificates directory. Use c_rehash before, read description in "
"https://www.openssl.org/docs/man3.0/man1/c_rehash.html");

namespace facade {

#ifdef DFLY_USE_SSL

// Creates the TLS context. Returns nullptr if the TLS configuration is invalid.
// To connect: openssl s_client -state -crlf -connect 127.0.0.1:6380
SSL_CTX* CreateSslCntx(TlsContextRole role) {
using absl::GetFlag;
const auto& tls_key_file = GetFlag(FLAGS_tls_key_file);
if (tls_key_file.empty()) {
LOG(ERROR) << "To use TLS, a server certificate must be provided with the --tls_key_file flag!";
return nullptr;
}

SSL_CTX* ctx;

if (role == TlsContextRole::SERVER) {
ctx = SSL_CTX_new(TLS_server_method());
} else {
ctx = SSL_CTX_new(TLS_client_method());
}
unsigned mask = SSL_VERIFY_NONE;

if (SSL_CTX_use_PrivateKey_file(ctx, tls_key_file.c_str(), SSL_FILETYPE_PEM) != 1) {
LOG(ERROR) << "Failed to load TLS key";
return nullptr;
}
const auto& tls_cert_file = GetFlag(FLAGS_tls_cert_file);

if (!tls_cert_file.empty()) {
// TO connect with redis-cli you need both tls-key-file and tls-cert-file
// loaded. Use `redis-cli --tls -p 6380 --insecure PING` to test
if (SSL_CTX_use_certificate_chain_file(ctx, tls_cert_file.c_str()) != 1) {
LOG(ERROR) << "Failed to load TLS certificate";
return nullptr;
}
}

const auto tls_ca_cert_file = GetFlag(FLAGS_tls_ca_cert_file);
const auto tls_ca_cert_dir = GetFlag(FLAGS_tls_ca_cert_dir);
if (!tls_ca_cert_file.empty() || !tls_ca_cert_dir.empty()) {
const auto* file = tls_ca_cert_file.empty() ? nullptr : tls_ca_cert_file.data();
const auto* dir = tls_ca_cert_dir.empty() ? nullptr : tls_ca_cert_dir.data();
if (SSL_CTX_load_verify_locations(ctx, file, dir) != 1) {
LOG(ERROR) << "Failed to load TLS verify locations (CA cert file or CA cert dir)";
return nullptr;
}
mask = SSL_VERIFY_PEER | SSL_VERIFY_FAIL_IF_NO_PEER_CERT;
}

DFLY_SSL_CHECK(1 == SSL_CTX_set_cipher_list(ctx, "DEFAULT"));

SSL_CTX_set_min_proto_version(ctx, TLS1_2_VERSION);

SSL_CTX_set_options(ctx, SSL_OP_DONT_INSERT_EMPTY_FRAGMENTS);

SSL_CTX_set_verify(ctx, mask, NULL);

DFLY_SSL_CHECK(1 == SSL_CTX_set_dh_auto(ctx, 1));

return ctx;
}

void PrintSSLError() {
ERR_print_errors_cb(
[](const char* str, size_t len, void* u) {
LOG(ERROR) << std::string_view(str, len);
return 1;
},
nullptr);
}

#endif
} // namespace facade
17 changes: 14 additions & 3 deletions src/facade/tls_error.h → src/facade/tls_helpers.h
Original file line number Diff line number Diff line change
@@ -1,18 +1,29 @@
// Copyright 2023, DragonflyDB authors. All rights reserved.
// Copyright 2025, DragonflyDB authors. All rights reserved.
// See LICENSE for licensing terms.
//

#pragma once

#ifdef DFLY_USE_SSL
#include <openssl/ssl.h>
#endif

namespace facade {

void PrintSSLError();
#ifdef DFLY_USE_SSL
enum class TlsContextRole { SERVER, CLIENT };

SSL_CTX* CreateSslCntx(TlsContextRole role);

}
void PrintSSLError();

#define DFLY_SSL_CHECK(condition) \
if (!(condition)) { \
LOG(ERROR) << "OpenSSL Error: " #condition; \
PrintSSLError(); \
exit(17); \
}

#endif

} // namespace facade
46 changes: 4 additions & 42 deletions src/server/protocol_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
//
#include "server/protocol_client.h"

#include "facade/tls_error.h"
#include "facade/tls_helpers.h"

extern "C" {
#include "redis/rdb.h"
Expand Down Expand Up @@ -54,46 +54,6 @@ using absl::StrCat;

namespace {

#ifdef DFLY_USE_SSL

static ProtocolClient::SSL_CTX* CreateSslClientCntx() {
ProtocolClient::SSL_CTX* ctx = SSL_CTX_new(TLS_client_method());
const auto& tls_key_file = GetFlag(FLAGS_tls_key_file);
unsigned mask = SSL_VERIFY_PEER | SSL_VERIFY_FAIL_IF_NO_PEER_CERT;

// Load client certificate if given.
if (!tls_key_file.empty()) {
DFLY_SSL_CHECK(1 == SSL_CTX_use_PrivateKey_file(ctx, tls_key_file.c_str(), SSL_FILETYPE_PEM));
// We checked that the flag is non empty in ValidateClientTlsFlags.
const auto& tls_cert_file = GetFlag(FLAGS_tls_cert_file);

DFLY_SSL_CHECK(1 == SSL_CTX_use_certificate_chain_file(ctx, tls_cert_file.c_str()));
}

// Load custom certificate validation if given.
const auto& tls_ca_cert_file = GetFlag(FLAGS_tls_ca_cert_file);
const auto& tls_ca_cert_dir = GetFlag(FLAGS_tls_ca_cert_dir);

const auto* file = tls_ca_cert_file.empty() ? nullptr : tls_ca_cert_file.data();
const auto* dir = tls_ca_cert_dir.empty() ? nullptr : tls_ca_cert_dir.data();
if (file || dir) {
DFLY_SSL_CHECK(1 == SSL_CTX_load_verify_locations(ctx, file, dir));
} else {
DFLY_SSL_CHECK(1 == SSL_CTX_set_default_verify_paths(ctx));
}

DFLY_SSL_CHECK(1 == SSL_CTX_set_cipher_list(ctx, "DEFAULT"));
SSL_CTX_set_min_proto_version(ctx, TLS1_2_VERSION);

SSL_CTX_set_options(ctx, SSL_OP_DONT_INSERT_EMPTY_FRAGMENTS);

SSL_CTX_set_verify(ctx, mask, NULL);

DFLY_SSL_CHECK(1 == SSL_CTX_set_dh_auto(ctx, 1));
return ctx;
}
#endif

error_code Recv(FiberSocketBase* input, base::IoBuf* dest) {
auto buf = dest->AppendBuffer();
io::Result<size_t> exp_size = input->Recv(buf);
Expand Down Expand Up @@ -136,9 +96,11 @@ void ValidateClientTlsFlags() {
}

void ProtocolClient::MaybeInitSslCtx() {
#ifdef DFLY_USE_SSL
if (absl::GetFlag(FLAGS_tls_replication)) {
ssl_ctx_ = CreateSslClientCntx();
ssl_ctx_ = CreateSslCntx(facade::TlsContextRole::CLIENT);
}
#endif
}

ProtocolClient::ProtocolClient(string host, uint16_t port) {
Expand Down
29 changes: 29 additions & 0 deletions tests/dragonfly/replication_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -1491,6 +1491,35 @@ async def test_tls_replication(
await proxy.close(proxy_task)


@dfly_args({"proactor_threads": 2})
async def test_tls_replication_without_ca(
df_factory,
df_seeder_factory,
with_tls_server_args,
with_ca_tls_client_args,
):
# 1. Spin up dragonfly tls enabled, debug populate
master = df_factory.create(tls_replication="true", **with_tls_server_args, requirepass="hi")
master.start()
# Somehow redis-py forces to verify the certificate and it fails
Copy link
Contributor Author

Choose a reason for hiding this comment

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

🤷‍♂️ 🤷‍♂️

# TODO investigate why and remove with_ca_tls_clients_args
c_master = master.client(password="hi", **with_ca_tls_client_args)
await c_master.execute_command("DEBUG POPULATE 100")

# 2. Spin up a replica and initiate a REPLICAOF
replica = df_factory.create(
tls_replication="true", **with_tls_server_args, masterauth="hi", requirepass="hi"
)
replica.start()

c_replica = replica.client(password="hi", **with_ca_tls_client_args)

res = await c_replica.execute_command("REPLICAOF localhost " + str(master.port))
assert "OK" == res
await check_all_replicas_finished([c_replica], c_master)
assert 100 == await c_replica.execute_command("dbsize")


@pytest.mark.exclude_epoll
async def test_ipv6_replication(df_factory: DflyInstanceFactory):
"""Test that IPV6 addresses work for replication, ::1 is 127.0.0.1 localhost"""
Expand Down
Loading