Skip to content
Merged
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
21 changes: 17 additions & 4 deletions src/iocore/net/P_SSLCertLookup.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
#include "iocore/eventsystem/ConfigProcessor.h"
#include "iocore/net/SSLTypes.h"
#include "records/RecCore.h"
#include "tsutil/Bravo.h"
#include <mutex>
#include <shared_mutex>

#include <set>
Expand Down Expand Up @@ -134,8 +136,7 @@ struct SSLCertLookup : public ConfigInfo {
std::unique_ptr<SSLContextStorage> ssl_storage;
std::unique_ptr<SSLContextStorage> ec_storage;

shared_SSL_CTX ssl_default;
bool is_valid = true;
bool is_valid = true;

int insert(const char *name, SSLCertContext const &cc);
int insert(const IpEndpoint &address, SSLCertContext const &cc);
Expand All @@ -146,6 +147,7 @@ struct SSLCertLookup : public ConfigInfo {
@return @c A pointer to the matched context, @c nullptr if no match is found.
*/
SSLCertContext *find(const IpEndpoint &address) const;
SSLCertContext *find(const IpEndpoint &address, [[maybe_unused]] SSLCertContextType ctxType) const;

/** Find certificate context by name (FQDN).
Exact matches have priority, then wildcards. Only destination based matches are checked.
Expand All @@ -154,10 +156,18 @@ struct SSLCertLookup : public ConfigInfo {
SSLCertContext *find(const std::string &name, SSLCertContextType ctxType = SSLCertContextType::GENERIC) const;

// Return the last-resort default TLS context if there is no name or address match.
SSL_CTX *
shared_SSL_CTX
defaultContext() const
Comment thread
bneradt marked this conversation as resolved.
{
return ssl_default.get();
ts::bravo::shared_lock<ts::bravo::shared_mutex> lock(default_ctx_mutex);
return ssl_default;
}
Comment thread
bneradt marked this conversation as resolved.

void
setDefaultContext(shared_SSL_CTX ctx) const
{
std::lock_guard<ts::bravo::shared_mutex> lock(default_ctx_mutex);
ssl_default = std::move(ctx);
}

unsigned count(SSLCertContextType ctxType = SSLCertContextType::GENERIC) const;
Expand All @@ -170,6 +180,9 @@ struct SSLCertLookup : public ConfigInfo {
~SSLCertLookup() override;

private:
mutable ts::bravo::shared_mutex default_ctx_mutex;
mutable shared_SSL_CTX ssl_default;

// Map cert_secret name to lookup keys
std::unordered_map<std::string, std::vector<std::string>> cert_secret_registry;
};
Expand Down
3 changes: 2 additions & 1 deletion src/iocore/net/QUICPacketHandler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,8 @@ QUICPacketHandlerIn::_recv_packet(int /* event ATS_UNUSED */, UDPPacket *udp_pac
QUICConnectionId new_cid;

QUICCertConfig::scoped_config server_cert;
SSL *ssl = SSL_new(server_cert->defaultContext());
auto default_ctx = server_cert->defaultContext();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not for this PR, but worth a note in the description or a follow-up issue: QUICCertConfig builds its own SSLCertLookup that only a full reload rebuilds, so secret updates still leave QUIC serving the stale default cert.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Agreed. This change updates the SSLConfig lookup used by TCP TLS; QUICCertConfig maintains a separate lookup and still requires a full reload. I’m treating QUIC secret-update support as follow-up work rather than expanding this fix into that separate lifecycle.

SSL *ssl = SSL_new(default_ctx.get());

quiche_conn *quiche_con = quiche_conn_new_with_tls(
new_cid, new_cid.length(), retry_token.original_dcid(), retry_token.original_dcid().length(), &udp_packet->to.sa,
Expand Down
46 changes: 29 additions & 17 deletions src/iocore/net/SSLCertLookup.cc
Original file line number Diff line number Diff line change
Expand Up @@ -273,8 +273,8 @@ SSLCertContext::setCtx(shared_SSL_CTX sc)
SSLCertLookup::SSLCertLookup()
: ssl_storage(std::make_unique<SSLContextStorage>()),
ec_storage(std::make_unique<SSLContextStorage>()),
ssl_default(nullptr),
is_valid(true)
is_valid(true),
ssl_default(nullptr)
{
}

Expand All @@ -299,36 +299,48 @@ SSLCertLookup::find(const std::string &address, [[maybe_unused]] SSLCertContextT
SSLCertContext *
SSLCertLookup::find(const IpEndpoint &address) const
{
SSLCertContext *cc;
SSLAddressLookupKey key(address);

#ifdef OPENSSL_IS_BORINGSSL
// If the context is EC supportable, try finding that first.
if ((cc = this->ec_storage->lookup(key.get()))) {
if (auto *cc = this->find(address, SSLCertContextType::EC)) {
return cc;
}
#endif

// If that failed, try the address without the port.
if (address.network_order_port()) {
key.split();
if ((cc = this->ec_storage->lookup(key.get()))) {
return cc;
}
}
return this->find(address, SSLCertContextType::RSA);
}

SSLCertContext *
SSLCertLookup::find(const IpEndpoint &address, [[maybe_unused]] SSLCertContextType ctxType) const
{
SSLAddressLookupKey key(address);

// reset for search across RSA
key = SSLAddressLookupKey(address);
#ifdef OPENSSL_IS_BORINGSSL
SSLContextStorage *storage = nullptr;
switch (ctxType) {
case SSLCertContextType::GENERIC:
case SSLCertContextType::RSA:
storage = ssl_storage.get();
break;
case SSLCertContextType::EC:
storage = ec_storage.get();
break;
default:
ink_assert(false);
return nullptr;
}
#else
SSLContextStorage *storage = ssl_storage.get();
#endif

// First try the full address.
if ((cc = this->ssl_storage->lookup(key.get()))) {
if (auto *cc = storage->lookup(key.get())) {
return cc;
}

// If that failed, try the address without the port.
if (address.network_order_port()) {
key.split();
return this->ssl_storage->lookup(key.get());
return storage->lookup(key.get());
}

return nullptr;
Expand Down
9 changes: 5 additions & 4 deletions src/iocore/net/SSLNetVConnection.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1108,7 +1108,8 @@ SSLNetVConnection::_sslStartHandShake(int event, int &err)
}
ats_ip_nptop(&dst, ipb1, sizeof(ipb1));
ats_ip_nptop(&src, ipb2, sizeof(ipb2));
DbgPrint(dbg_ctl_ssl, "IP context is %p for [%s] -> [%s], default context %p", cc, ipb2, ipb1, lookup->defaultContext());
auto default_ctx = lookup->defaultContext();
DbgPrint(dbg_ctl_ssl, "IP context is %p for [%s] -> [%s], default context %p", cc, ipb2, ipb1, default_ctx.get());
}

// Escape if this is marked to be a tunnel.
Expand All @@ -1130,7 +1131,7 @@ SSLNetVConnection::_sslStartHandShake(int event, int &err)
// Attach the default SSL_CTX to this SSL session. The default context is never going to be able
// to negotiate a SSL session, but it's enough to trampoline us into the SNI callback where we
// can select the right server certificate.
this->_make_ssl_connection(lookup->defaultContext());
this->_make_ssl_connection(lookup->defaultContext().get());
}

if (this->ssl == nullptr) {
Expand Down Expand Up @@ -2005,8 +2006,8 @@ SSLNetVConnection::_lookupContextByIP()
return nullptr;
}
ats_ip_nptop(&src, ipb2, sizeof(ipb2));
DbgPrint(dbg_ctl_proxyprotocol, "IP context is %p for [%s] -> [%s], default context %p", cc, ipb2, ipb1,
lookup->defaultContext());
auto default_ctx = lookup->defaultContext();
DbgPrint(dbg_ctl_proxyprotocol, "IP context is %p for [%s] -> [%s], default context %p", cc, ipb2, ipb1, default_ctx.get());
}
} else if (0 == safe_getsockname(this->get_socket(), &ip.sa, &namelen)) {
cc = lookup->find(ip);
Expand Down
5 changes: 3 additions & 2 deletions src/iocore/net/SSLStats.cc
Original file line number Diff line number Diff line change
Expand Up @@ -273,12 +273,13 @@ SSLInitializeStatistics()
// Acquire the loaded SSL certificate configuration to enumerate ciphers and groups.
// This must be called AFTER SSLCertificateConfig::startup().
SSLCertificateConfig::scoped_config lookup;
if (!lookup || !lookup->ssl_default) {
auto default_ctx = lookup ? lookup->defaultContext() : nullptr;
if (!default_ctx) {
Dbg(dbg_ctl_ssl, "No SSL configuration, skipping cipher/group statistics initialization");
return;
}

SSL_CTX *ctx = lookup->ssl_default.get();
SSL_CTX *ctx = default_ctx.get();
SSL *ssl = SSL_new(ctx);
STACK_OF(SSL_CIPHER) *ciphers = SSL_get_ciphers(ssl);

Expand Down
27 changes: 24 additions & 3 deletions src/iocore/net/SSLUtils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1733,6 +1733,27 @@ SSLMultiCertConfigLoader::update_ssl_ctx(const std::string &secret_name)
if (!ctx) {
retval = false;
} else {
if ((*policy_iter)->addr) {
SSLCertContext *cc = nullptr;
if (strcmp((*policy_iter)->addr, "*") == 0) {
this->_set_handshake_callbacks(ctx.get());
cc = lookup->find("*", loadingctx.ctx_type);
} else {
IpEndpoint ep;
if (ats_ip_pton((*policy_iter)->addr, &ep) == 0) {
cc = lookup->find(ep, loadingctx.ctx_type);
} else {
Error("'%s' is not a valid IPv4 or IPv6 address", (const char *)(*policy_iter)->addr);
retval = false;
}
}
if (cc && cc->userconfig.get() == policy_iter->get()) {
cc->setCtx(ctx);
if (strcmp((*policy_iter)->addr, "*") == 0) {
lookup->setDefaultContext(ctx);
}
}
}
for (auto const &name : common_names) {
SSLCertContext *cc = lookup->find(name, loadingctx.ctx_type);
if (cc && cc->userconfig.get() == policy_iter->get()) {
Expand Down Expand Up @@ -1787,8 +1808,8 @@ SSLMultiCertConfigLoader::_store_single_ssl_ctx(SSLCertLookup *lookup, const sha
if (strcmp(sslMultCertSettings->addr, "*") == 0) {
Dbg(dbg_ctl_ssl_load, "Addr is '*'; setting %p to default", ctx.get());
if (lookup->insert(sslMultCertSettings->addr, SSLCertContext(ctx, ctx_type, sslMultCertSettings, keyblock)) >= 0) {
inserted = true;
lookup->ssl_default = ctx;
inserted = true;
lookup->setDefaultContext(ctx);
this->_set_handshake_callbacks(ctx.get());
}
} else {
Expand Down Expand Up @@ -1890,7 +1911,7 @@ SSLMultiCertConfigLoader::load(SSLCertLookup *lookup, bool firstLoad)
// We *must* have a default context even if it can't possibly work. The default context is used to
// bootstrap the SSL handshake so that we can subsequently do the SNI lookup to switch to the real
// context.
if (lookup->ssl_default == nullptr) {
if (lookup->defaultContext() == nullptr) {
shared_SSLMultiCertConfigParams sslMultiCertSettings(new SSLMultiCertConfigParams);
sslMultiCertSettings->addr = ats_strdup("*");
if (!this->_store_ssl_ctx(lookup, sslMultiCertSettings)) {
Expand Down
Loading