Skip to content

Commit

Permalink
fix LDAP in case of many roles on user
Browse files Browse the repository at this point in the history 
- Raised default value of search_limit to 256
- Added option to change that to arbitrary value
- using SipHash for computing hash of LDAP server parameters
- other minor changes
  • Loading branch information
@Enmk
Enmk committed Oct 20, 2022
1 parent 0d07aee commit 1ed7ad5
Show file tree
Hide file tree
Showing 4 changed files with 89 additions and 68 deletions.
50 changes: 32 additions & 18 deletions src/Access/ExternalAuthenticators.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
#include <Access/LDAPClient.h>
#include <Common/Exception.h>
#include <Common/quoteString.h>
#include <Common/SipHash.h>

#include <Poco/Util/AbstractConfiguration.h>
#include <boost/algorithm/string/case_conv.hpp>

Expand Down Expand Up @@ -73,6 +75,7 @@ void parseLDAPServer(LDAPClient::Params & params, const Poco::Util::AbstractConf
const bool has_tls_ca_cert_file = config.has(ldap_server_config + ".tls_ca_cert_file");
const bool has_tls_ca_cert_dir = config.has(ldap_server_config + ".tls_ca_cert_dir");
const bool has_tls_cipher_suite = config.has(ldap_server_config + ".tls_cipher_suite");
const bool has_search_limit = config.has(ldap_server_config + ".search_limit");

if (!has_host)
throw Exception("Missing 'host' entry", ErrorCodes::BAD_ARGUMENTS);
Expand Down Expand Up @@ -184,6 +187,17 @@ void parseLDAPServer(LDAPClient::Params & params, const Poco::Util::AbstractConf
}
else
params.port = (params.enable_tls == LDAPClient::Params::TLSEnable::YES ? 636 : 389);

if (has_search_limit)
{
const auto search_limit = config.getUInt(ldap_server_config + ".search_limit");
// Arbitrary large number.
// Previously default value was 100, but in practice one client reported to have 120 roles for a user.
if (search_limit >= 2048)
throw Exception("Bad value for 'search_limit' entry", ErrorCodes::BAD_ARGUMENTS);

params.search_limit = search_limit;
}
}

void parseKerberosParams(GSSAcceptorContext::Params & params, const Poco::Util::AbstractConfiguration & config)
Expand Down Expand Up @@ -313,11 +327,26 @@ void ExternalAuthenticators::setConfiguration(const Poco::Util::AbstractConfigur
}
}

UInt128 computeParamsHash(const LDAPClient::Params & params, const LDAPClient::RoleSearchParamsList * role_search_params)
{
SipHash hash;
params.updateHash(hash);
if (role_search_params)
{
for (const auto & params_instance : *role_search_params)
{
params_instance.updateHash(hash);
}
}

return hash.get128();
}

bool ExternalAuthenticators::checkLDAPCredentials(const String & server, const BasicCredentials & credentials,
const LDAPClient::RoleSearchParamsList * role_search_params, LDAPClient::SearchResultsList * role_search_results) const
{
std::optional<LDAPClient::Params> params;
std::size_t params_hash = 0;
UInt128 params_hash = 0;

{
std::scoped_lock lock(mutex);
Expand All @@ -331,14 +360,7 @@ bool ExternalAuthenticators::checkLDAPCredentials(const String & server, const B
params->user = credentials.getUserName();
params->password = credentials.getPassword();

params->combineCoreHash(params_hash);
if (role_search_params)
{
for (const auto & params_instance : *role_search_params)
{
params_instance.combineHash(params_hash);
}
}
params_hash = computeParamsHash(*params, role_search_params);

// Check the cache, but only if the caching is enabled at all.
if (params->verification_cooldown > std::chrono::seconds{0})
Expand Down Expand Up @@ -408,15 +430,7 @@ bool ExternalAuthenticators::checkLDAPCredentials(const String & server, const B
new_params.user = credentials.getUserName();
new_params.password = credentials.getPassword();

std::size_t new_params_hash = 0;
new_params.combineCoreHash(new_params_hash);
if (role_search_params)
{
for (const auto & params_instance : *role_search_params)
{
params_instance.combineHash(new_params_hash);
}
}
const UInt128 new_params_hash = computeParamsHash(new_params, role_search_params);

// If the critical server params have changed while we were checking the password, we discard the current result.
if (params_hash != new_params_hash)
Expand Down
84 changes: 42 additions & 42 deletions src/Access/LDAPClient.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@
#include <Common/Exception.h>
#include <base/scope_guard.h>
#include <Common/logger_useful.h>
#include <Common/SipHash.h>

#include <Poco/Logger.h>
#include <boost/algorithm/string/predicate.hpp>
#include <boost/container_hash/hash.hpp>

#include <mutex>
#include <utility>
Expand All @@ -26,30 +26,30 @@ namespace ErrorCodes
extern const int LDAP_ERROR;
}

void LDAPClient::SearchParams::combineHash(std::size_t & seed) const
void LDAPClient::SearchParams::updateHash(SipHash & hash) const
{
boost::hash_combine(seed, base_dn);
boost::hash_combine(seed, static_cast<int>(scope));
boost::hash_combine(seed, search_filter);
boost::hash_combine(seed, attribute);
hash.update(base_dn);
hash.update(static_cast<int>(scope));
hash.update(search_filter);
hash.update(attribute);
}

void LDAPClient::RoleSearchParams::combineHash(std::size_t & seed) const
void LDAPClient::RoleSearchParams::updateHash(SipHash & hash) const
{
SearchParams::combineHash(seed);
boost::hash_combine(seed, prefix);
SearchParams::updateHash(hash);
hash.update(prefix);
}

void LDAPClient::Params::combineCoreHash(std::size_t & seed) const
void LDAPClient::Params::updateHash(SipHash & hash) const
{
boost::hash_combine(seed, host);
boost::hash_combine(seed, port);
boost::hash_combine(seed, bind_dn);
boost::hash_combine(seed, user);
boost::hash_combine(seed, password);
hash.update(host);
hash.update(port);
hash.update(bind_dn);
hash.update(user);
hash.update(password);

if (user_dn_detection)
user_dn_detection->combineHash(seed);
user_dn_detection->updateHash(hash);
}

LDAPClient::LDAPClient(const Params & params_)
Expand Down Expand Up @@ -153,13 +153,13 @@ namespace

}

void LDAPClient::diag(int rc, String text)
void LDAPClient::handleError(int result_code, String text)
{
std::scoped_lock lock(ldap_global_mutex);

if (rc != LDAP_SUCCESS)
if (result_code != LDAP_SUCCESS)
{
const char * raw_err_str = ldap_err2string(rc);
const char * raw_err_str = ldap_err2string(result_code);
if (raw_err_str && *raw_err_str != '\0')
{
if (!text.empty())
Expand Down Expand Up @@ -214,7 +214,7 @@ bool LDAPClient::openConnection()

SCOPE_EXIT({ ldap_memfree(uri); });

diag(ldap_initialize(&handle, uri));
handleError(ldap_initialize(&handle, uri));
if (!handle)
throw Exception("ldap_initialize() failed", ErrorCodes::LDAP_ERROR);
}
Expand All @@ -226,21 +226,21 @@ bool LDAPClient::openConnection()
case LDAPClient::Params::ProtocolVersion::V2: value = LDAP_VERSION2; break;
case LDAPClient::Params::ProtocolVersion::V3: value = LDAP_VERSION3; break;
}
diag(ldap_set_option(handle, LDAP_OPT_PROTOCOL_VERSION, &value));
handleError(ldap_set_option(handle, LDAP_OPT_PROTOCOL_VERSION, &value));
}

diag(ldap_set_option(handle, LDAP_OPT_RESTART, LDAP_OPT_ON));
handleError(ldap_set_option(handle, LDAP_OPT_RESTART, LDAP_OPT_ON));

#ifdef LDAP_OPT_KEEPCONN
diag(ldap_set_option(handle, LDAP_OPT_KEEPCONN, LDAP_OPT_ON));
handleError(ldap_set_option(handle, LDAP_OPT_KEEPCONN, LDAP_OPT_ON));
#endif

#ifdef LDAP_OPT_TIMEOUT
{
::timeval operation_timeout;
operation_timeout.tv_sec = params.operation_timeout.count();
operation_timeout.tv_usec = 0;
diag(ldap_set_option(handle, LDAP_OPT_TIMEOUT, &operation_timeout));
handleError(ldap_set_option(handle, LDAP_OPT_TIMEOUT, &operation_timeout));
}
#endif

Expand All @@ -249,18 +249,18 @@ bool LDAPClient::openConnection()
::timeval network_timeout;
network_timeout.tv_sec = params.network_timeout.count();
network_timeout.tv_usec = 0;
diag(ldap_set_option(handle, LDAP_OPT_NETWORK_TIMEOUT, &network_timeout));
handleError(ldap_set_option(handle, LDAP_OPT_NETWORK_TIMEOUT, &network_timeout));
}
#endif

{
const int search_timeout = params.search_timeout.count();
diag(ldap_set_option(handle, LDAP_OPT_TIMELIMIT, &search_timeout));
handleError(ldap_set_option(handle, LDAP_OPT_TIMELIMIT, &search_timeout));
}

{
const int size_limit = params.search_limit;
diag(ldap_set_option(handle, LDAP_OPT_SIZELIMIT, &size_limit));
handleError(ldap_set_option(handle, LDAP_OPT_SIZELIMIT, &size_limit));
}

#ifdef LDAP_OPT_X_TLS_PROTOCOL_MIN
Expand All @@ -274,7 +274,7 @@ bool LDAPClient::openConnection()
case LDAPClient::Params::TLSProtocolVersion::TLS1_1: value = LDAP_OPT_X_TLS_PROTOCOL_TLS1_1; break;
case LDAPClient::Params::TLSProtocolVersion::TLS1_2: value = LDAP_OPT_X_TLS_PROTOCOL_TLS1_2; break;
}
diag(ldap_set_option(handle, LDAP_OPT_X_TLS_PROTOCOL_MIN, &value));
handleError(ldap_set_option(handle, LDAP_OPT_X_TLS_PROTOCOL_MIN, &value));
}
#endif

Expand All @@ -288,44 +288,44 @@ bool LDAPClient::openConnection()
case LDAPClient::Params::TLSRequireCert::TRY: value = LDAP_OPT_X_TLS_TRY; break;
case LDAPClient::Params::TLSRequireCert::DEMAND: value = LDAP_OPT_X_TLS_DEMAND; break;
}
diag(ldap_set_option(handle, LDAP_OPT_X_TLS_REQUIRE_CERT, &value));
handleError(ldap_set_option(handle, LDAP_OPT_X_TLS_REQUIRE_CERT, &value));
}
#endif

#ifdef LDAP_OPT_X_TLS_CERTFILE
if (!params.tls_cert_file.empty())
diag(ldap_set_option(handle, LDAP_OPT_X_TLS_CERTFILE, params.tls_cert_file.c_str()));
handleError(ldap_set_option(handle, LDAP_OPT_X_TLS_CERTFILE, params.tls_cert_file.c_str()));
#endif

#ifdef LDAP_OPT_X_TLS_KEYFILE
if (!params.tls_key_file.empty())
diag(ldap_set_option(handle, LDAP_OPT_X_TLS_KEYFILE, params.tls_key_file.c_str()));
handleError(ldap_set_option(handle, LDAP_OPT_X_TLS_KEYFILE, params.tls_key_file.c_str()));
#endif

#ifdef LDAP_OPT_X_TLS_CACERTFILE
if (!params.tls_ca_cert_file.empty())
diag(ldap_set_option(handle, LDAP_OPT_X_TLS_CACERTFILE, params.tls_ca_cert_file.c_str()));
handleError(ldap_set_option(handle, LDAP_OPT_X_TLS_CACERTFILE, params.tls_ca_cert_file.c_str()));
#endif

#ifdef LDAP_OPT_X_TLS_CACERTDIR
if (!params.tls_ca_cert_dir.empty())
diag(ldap_set_option(handle, LDAP_OPT_X_TLS_CACERTDIR, params.tls_ca_cert_dir.c_str()));
handleError(ldap_set_option(handle, LDAP_OPT_X_TLS_CACERTDIR, params.tls_ca_cert_dir.c_str()));
#endif

#ifdef LDAP_OPT_X_TLS_CIPHER_SUITE
if (!params.tls_cipher_suite.empty())
diag(ldap_set_option(handle, LDAP_OPT_X_TLS_CIPHER_SUITE, params.tls_cipher_suite.c_str()));
handleError(ldap_set_option(handle, LDAP_OPT_X_TLS_CIPHER_SUITE, params.tls_cipher_suite.c_str()));
#endif

#ifdef LDAP_OPT_X_TLS_NEWCTX
{
const int i_am_a_server = 0;
diag(ldap_set_option(handle, LDAP_OPT_X_TLS_NEWCTX, &i_am_a_server));
handleError(ldap_set_option(handle, LDAP_OPT_X_TLS_NEWCTX, &i_am_a_server));
}
#endif

if (params.enable_tls == LDAPClient::Params::TLSEnable::YES_STARTTLS)
diag(ldap_start_tls_s(handle, nullptr, nullptr));
handleError(ldap_start_tls_s(handle, nullptr, nullptr));

final_user_name = escapeForDN(params.user);
final_bind_dn = replacePlaceholders(params.bind_dn, { {"{user_name}", final_user_name} });
Expand All @@ -346,7 +346,7 @@ bool LDAPClient::openConnection()
if (rc == LDAP_INVALID_CREDENTIALS)
return false;

diag(rc);
handleError(rc);
}

// Once bound, run the user DN search query and update the default value, if asked.
Expand Down Expand Up @@ -425,7 +425,7 @@ LDAPClient::SearchResults LDAPClient::search(const SearchParams & search_params)
}
});

diag(ldap_search_ext_s(handle, final_base_dn.c_str(), scope, final_search_filter.c_str(), attrs, 0, nullptr, nullptr, &timeout, params.search_limit, &msgs));
handleError(ldap_search_ext_s(handle, final_base_dn.c_str(), scope, final_search_filter.c_str(), attrs, 0, nullptr, nullptr, &timeout, params.search_limit, &msgs));

for (
auto * msg = ldap_first_message(handle, msgs);
Expand All @@ -452,7 +452,7 @@ LDAPClient::SearchResults LDAPClient::search(const SearchParams & search_params)

::berval bv;

diag(ldap_get_dn_ber(handle, msg, &ber, &bv));
handleError(ldap_get_dn_ber(handle, msg, &ber, &bv));

if (bv.bv_val && bv.bv_len > 0)
result.emplace(bv.bv_val, bv.bv_len);
Expand Down Expand Up @@ -504,7 +504,7 @@ LDAPClient::SearchResults LDAPClient::search(const SearchParams & search_params)
case LDAP_RES_SEARCH_REFERENCE:
{
char ** referrals = nullptr;
diag(ldap_parse_reference(handle, msg, &referrals, nullptr, 0));
handleError(ldap_parse_reference(handle, msg, &referrals, nullptr, 0));

if (referrals)
{
Expand All @@ -528,7 +528,7 @@ LDAPClient::SearchResults LDAPClient::search(const SearchParams & search_params)
char * matched_msg = nullptr;
char * error_msg = nullptr;

diag(ldap_parse_result(handle, msg, &rc, &matched_msg, &error_msg, nullptr, nullptr, 0));
handleError(ldap_parse_result(handle, msg, &rc, &matched_msg, &error_msg, nullptr, nullptr, 0));

if (rc != LDAP_SUCCESS)
{
Expand Down Expand Up @@ -610,7 +610,7 @@ bool LDAPSimpleAuthClient::authenticate(const RoleSearchParamsList * role_search

#else // USE_LDAP

void LDAPClient::diag(const int, String)
void LDAPClient::handleError(const int, String)
{
throw Exception("ClickHouse was built without LDAP support", ErrorCodes::FEATURE_IS_NOT_ENABLED_AT_BUILD_TIME);
}
Expand Down
11 changes: 6 additions & 5 deletions src/Access/LDAPClient.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include <set>
#include <vector>

class SipHash;

namespace DB
{
Expand All @@ -38,15 +39,15 @@ class LDAPClient
String search_filter;
String attribute = "cn";

void combineHash(std::size_t & seed) const;
void updateHash(SipHash & hash) const;
};

struct RoleSearchParams
: public SearchParams
{
String prefix;

void combineHash(std::size_t & seed) const;
void updateHash(SipHash & hash) const;
};

using RoleSearchParamsList = std::vector<RoleSearchParams>;
Expand Down Expand Up @@ -119,9 +120,9 @@ class LDAPClient
std::chrono::seconds operation_timeout{40};
std::chrono::seconds network_timeout{30};
std::chrono::seconds search_timeout{20};
std::uint32_t search_limit = 100;
std::uint32_t search_limit = 256;

void combineCoreHash(std::size_t & seed) const;
void updateHash(SipHash & hash) const;
};

explicit LDAPClient(const Params & params_);
Expand All @@ -133,7 +134,7 @@ class LDAPClient
LDAPClient & operator= (LDAPClient &&) = delete;

protected:
MAYBE_NORETURN void diag(int rc, String text = "");
MAYBE_NORETURN void handleError(int result_code, String text = "");
MAYBE_NORETURN bool openConnection();
void closeConnection() noexcept;
SearchResults search(const SearchParams & search_params);
Expand Down

0 comments on commit 1ed7ad5

Please sign in to comment.