Skip to content

Commit

Permalink
ldap: Add new "allow" value for ldap_id_use_start_tls
Browse files Browse the repository at this point in the history
Allow can be used to attempt startTLS first but fallback to
not using TLS if this operation fails.

Resolves: #6681

(cherry picked from commit ca61ae6)
  • Loading branch information
justin-stephenson committed Oct 5, 2023
1 parent df709da commit 7143c5a
Show file tree
Hide file tree
Showing 12 changed files with 142 additions and 21 deletions.
8 changes: 4 additions & 4 deletions src/config/SSSDConfigTest.py
Original file line number Diff line number Diff line change
Expand Up @@ -1758,7 +1758,7 @@ def testGetDomain(self):
self.assertFalse(domain.active)

# TODO verify the contents of this domain
self.assertTrue(domain.get_option('ldap_id_use_start_tls'))
self.assertEqual(domain.get_option('ldap_id_use_start_tls'), 'true')
self.assertTrue(domain.get_option('ldap_sudo_include_regexp'))
self.assertTrue(domain.get_option('ldap_autofs_map_master_name'))

Expand Down Expand Up @@ -1912,11 +1912,11 @@ def testSaveDomain(self):

# Positive test - Ensure that saved domains retain values
domain.set_option('ldap_krb5_init_creds', True)
domain.set_option('ldap_id_use_start_tls', False)
domain.set_option('ldap_id_use_start_tls', 'allow')
domain.set_option('ldap_user_search_base',
'cn=accounts, dc=example, dc=com')
self.assertTrue(domain.get_option('ldap_krb5_init_creds'))
self.assertFalse(domain.get_option('ldap_id_use_start_tls'))
self.assertEqual(domain.get_option('ldap_id_use_start_tls'), 'allow')
self.assertEqual(domain.get_option('ldap_user_search_base'),
'cn=accounts, dc=example, dc=com')

Expand Down Expand Up @@ -1945,7 +1945,7 @@ def testSaveDomain(self):

domain2 = sssdconfig.get_domain('example.com2')
self.assertTrue(domain2.get_option('ldap_krb5_init_creds'))
self.assertFalse(domain2.get_option('ldap_id_use_start_tls'))
self.assertEqual(domain.get_option('ldap_id_use_start_tls'), 'allow')

def testActivateDomain(self):
sssdconfig = SSSDConfig.SSSDConfig(srcdir + "/etc/sssd.api.conf",
Expand Down
2 changes: 1 addition & 1 deletion src/config/etc/sssd.api.d/sssd-ad.conf
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ wildcard_limit = int, None, false
ldap_search_timeout = int, None, false
ldap_enumeration_refresh_timeout = int, None, false
ldap_purge_cache_timeout = int, None, false
ldap_id_use_start_tls = bool, None, false
ldap_id_use_start_tls = str, None, false
ldap_id_mapping = bool, None, false
ldap_user_search_base = str, None, false
ldap_user_search_scope = str, None, false
Expand Down
2 changes: 1 addition & 1 deletion src/config/etc/sssd.api.d/sssd-ipa.conf
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ wildcard_limit = int, None, false
ldap_search_timeout = int, None, false
ldap_enumeration_refresh_timeout = int, None, false
ldap_purge_cache_timeout = int, None, false
ldap_id_use_start_tls = bool, None, false
ldap_id_use_start_tls = str, None, false
ldap_id_mapping = bool, None, false
ldap_user_search_base = str, None, false
ldap_user_search_scope = str, None, false
Expand Down
2 changes: 1 addition & 1 deletion src/config/etc/sssd.api.d/sssd-ldap.conf
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ ldap_search_timeout = int, None, false
ldap_enumeration_search_timeout = int, None, false
ldap_enumeration_refresh_timeout = int, None, false
ldap_purge_cache_timeout = int, None, false
ldap_id_use_start_tls = bool, None, false
ldap_id_use_start_tls = str, None, false
ldap_id_mapping = bool, None, false
ldap_user_search_base = str, None, false
ldap_user_search_scope = str, None, false
Expand Down
2 changes: 1 addition & 1 deletion src/config/testconfigs/sssd-invalid-badbool.conf
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ debug_level = 0

[domain/IPA]
id_provider = ldap
ldap_id_use_start_tls = Fal
ldap_id_mapping = Fal
auth_provider = krb5
debug_level = 0

Expand Down
28 changes: 28 additions & 0 deletions src/man/sssd-ldap.5.xml
Original file line number Diff line number Diff line change
Expand Up @@ -913,9 +913,37 @@
Specifies that the id_provider connection must also
use <systemitem class="protocol">tls</systemitem> to protect the channel.
</para>
<para>
Three modes are currently supported:
<itemizedlist>
<listitem>
<para>
allow - Attempt startTLS first then fallback
to connecting without if it fails.
</para>
</listitem>
<listitem>
<para>
true - TLS must be used or the connection
will fail.
</para>
</listitem>
<listitem>
<para>
false - startTLS will not be attempted, Not
recommended for security reasons.
</para>
</listitem>
</itemizedlist>
</para>
<para>
Default: false
</para>
<para>
Used as a default for compatibility reasons, please
check that TLS is really used (can check SSSD logs for
warnings about fallback).
</para>
</listitem>
</varlistentry>

Expand Down
2 changes: 1 addition & 1 deletion src/providers/ad/ad_opts.c
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ struct dp_option ad_def_ldap_opts[] = {
{ "ldap_tls_cert", DP_OPT_STRING, NULL_STRING, NULL_STRING },
{ "ldap_tls_key", DP_OPT_STRING, NULL_STRING, NULL_STRING },
{ "ldap_tls_cipher_suite", DP_OPT_STRING, NULL_STRING, NULL_STRING },
{ "ldap_id_use_start_tls", DP_OPT_BOOL, BOOL_FALSE, BOOL_FALSE },
{ "ldap_id_use_start_tls", DP_OPT_STRING,{ "false" }, NULL_STRING },
{ "ldap_id_mapping", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE },
{ "ldap_sasl_mech", DP_OPT_STRING, { "GSS-SPNEGO" }, NULL_STRING },
{ "ldap_sasl_authid", DP_OPT_STRING, NULL_STRING, NULL_STRING },
Expand Down
2 changes: 1 addition & 1 deletion src/providers/ipa/ipa_opts.c
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ struct dp_option ipa_def_ldap_opts[] = {
{ "ldap_tls_cert", DP_OPT_STRING, NULL_STRING, NULL_STRING },
{ "ldap_tls_key", DP_OPT_STRING, NULL_STRING, NULL_STRING },
{ "ldap_tls_cipher_suite", DP_OPT_STRING, NULL_STRING, NULL_STRING },
{ "ldap_id_use_start_tls", DP_OPT_BOOL, BOOL_FALSE, BOOL_FALSE },
{ "ldap_id_use_start_tls", DP_OPT_STRING, { "false" }, NULL_STRING },
{ "ldap_id_mapping", DP_OPT_BOOL, BOOL_FALSE, BOOL_FALSE },
{ "ldap_sasl_mech", DP_OPT_STRING, { "GSSAPI" } , NULL_STRING },
{ "ldap_sasl_authid", DP_OPT_STRING, NULL_STRING, NULL_STRING },
Expand Down
2 changes: 1 addition & 1 deletion src/providers/ldap/ldap_opts.c
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ struct dp_option default_basic_opts[] = {
{ "ldap_tls_cert", DP_OPT_STRING, NULL_STRING, NULL_STRING },
{ "ldap_tls_key", DP_OPT_STRING, NULL_STRING, NULL_STRING },
{ "ldap_tls_cipher_suite", DP_OPT_STRING, NULL_STRING, NULL_STRING },
{ "ldap_id_use_start_tls", DP_OPT_BOOL, BOOL_FALSE, BOOL_FALSE },
{ "ldap_id_use_start_tls", DP_OPT_STRING, { "false" }, NULL_STRING },
{ "ldap_id_mapping", DP_OPT_BOOL, BOOL_FALSE, BOOL_FALSE },
{ "ldap_sasl_mech", DP_OPT_STRING, NULL_STRING, NULL_STRING },
{ "ldap_sasl_authid", DP_OPT_STRING, NULL_STRING, NULL_STRING },
Expand Down
3 changes: 2 additions & 1 deletion src/providers/ldap/sdap_async.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ struct tevent_req *sdap_connect_send(TALLOC_CTX *memctx,
const char *uri,
struct sockaddr *sockaddr,
socklen_t sockaddr_len,
bool use_start_tls);
bool use_start_tls,
bool retry_no_tls);
int sdap_connect_recv(struct tevent_req *req,
TALLOC_CTX *memctx,
struct sdap_handle **sh);
Expand Down
78 changes: 69 additions & 9 deletions src/providers/ldap/sdap_async_connection.c
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ struct sdap_connect_state {
struct sdap_handle *sh;
const char *uri;
bool use_start_tls;
bool retry_no_tls;

struct sdap_op *op;

Expand All @@ -67,7 +68,8 @@ struct tevent_req *sdap_connect_send(TALLOC_CTX *memctx,
const char *uri,
struct sockaddr *sockaddr,
socklen_t sockaddr_len,
bool use_start_tls)
bool use_start_tls,
bool retry_no_tls)
{
struct tevent_req *req;
struct tevent_req *subreq;
Expand All @@ -93,6 +95,7 @@ struct tevent_req *sdap_connect_send(TALLOC_CTX *memctx,
state->ev = ev;
state->opts = opts;
state->use_start_tls = use_start_tls;
state->retry_no_tls = retry_no_tls;

state->uri = talloc_asprintf(state, "%s", uri);
if (!state->uri) {
Expand Down Expand Up @@ -319,6 +322,9 @@ static void sdap_sys_connect_done(struct tevent_req *subreq)

DEBUG(SSSDBG_CONF_SETTINGS, "Executing START TLS\n");

/* If ldap_id_use_start_tls is "allow", retry without TLS next */
state->retry_no_tls = true;

lret = ldap_start_tls(state->sh->ldap, NULL, NULL, &msgid);
if (lret != LDAP_SUCCESS) {
optret = sss_ldap_get_diagnostic_msg(state, state->sh->ldap,
Expand Down Expand Up @@ -460,6 +466,7 @@ struct sdap_connect_host_state {
char *host;
int port;
bool use_start_tls;
bool retry_no_tls;

struct sdap_handle *sh;
};
Expand Down Expand Up @@ -566,7 +573,7 @@ static void sdap_connect_host_resolv_done(struct tevent_req *subreq)

subreq = sdap_connect_send(state, state->ev, state->opts,
state->uri, sockaddr, sockaddr_len,
state->use_start_tls);
state->use_start_tls, state->retry_no_tls);
if (subreq == NULL) {
ret = ENOMEM;
goto done;
Expand Down Expand Up @@ -1486,6 +1493,7 @@ struct sdap_cli_connect_state {
enum connect_tls force_tls;
bool do_auth;
bool use_tls;
bool retry_no_tls;

int retry_attempts;
};
Expand All @@ -1506,13 +1514,27 @@ static void sdap_cli_rootdse_auth_done(struct tevent_req *subreq);

static errno_t
decide_tls_usage(enum connect_tls force_tls, struct dp_option *basic,
const char *uri, bool *_use_tls)
const char *uri, bool *_use_tls, bool *_retry_no_tls)
{
bool use_tls = true;
bool retry_no_tls = false;
const char *id_tls;

switch (force_tls) {
case CON_TLS_DFL:
use_tls = dp_opt_get_bool(basic, SDAP_ID_TLS);
id_tls = dp_opt_get_string(basic, SDAP_ID_TLS);
if (strcasecmp(id_tls, "allow") == 0) {
use_tls = true;
retry_no_tls = true;
} else if (strcasecmp(id_tls, "true") == 0) {
use_tls = true;
} else if (strcasecmp(id_tls, "false") == 0) {
use_tls = false;
} else {
DEBUG(SSSDBG_OP_FAILURE, "Invalid ldap_id_use_start_tls value " \
": [%s]\n", id_tls);
return EINVAL;
}
break;
case CON_TLS_ON:
use_tls = true;
Expand All @@ -1531,6 +1553,7 @@ decide_tls_usage(enum connect_tls force_tls, struct dp_option *basic,
use_tls = false;
}

*_retry_no_tls = retry_no_tls;
*_use_tls = use_tls;
return EOK;
}
Expand Down Expand Up @@ -1610,7 +1633,8 @@ static void sdap_cli_resolve_done(struct tevent_req *subreq)
}

ret = decide_tls_usage(state->force_tls, state->opts->basic,
state->service->uri, &state->use_tls);
state->service->uri, &state->use_tls,
&state->retry_no_tls);

if (ret != EOK) {
tevent_req_error(req, EINVAL);
Expand All @@ -1621,7 +1645,8 @@ static void sdap_cli_resolve_done(struct tevent_req *subreq)
state->service->uri,
state->service->sockaddr,
state->service->sockaddr_len,
state->use_tls);
state->use_tls,
state->retry_no_tls);
if (!subreq) {
tevent_req_error(req, ENOMEM);
return;
Expand All @@ -1641,6 +1666,12 @@ static void sdap_cli_connect_done(struct tevent_req *subreq)
talloc_zfree(state->sh);
ret = sdap_connect_recv(subreq, state, &state->sh);
talloc_zfree(subreq);

/* We don't need to retry */
if (ret == EOK) {
state->retry_no_tls = false;
}

if (ret == ERR_TLS_HANDSHAKE_INTERRUPTED &&
state->retry_attempts < MAX_RETRY_ATTEMPTS) {
DEBUG(SSSDBG_OP_FAILURE,
Expand All @@ -1650,7 +1681,34 @@ static void sdap_cli_connect_done(struct tevent_req *subreq)
state->service->uri,
state->service->sockaddr,
state->service->sockaddr_len,
state->use_tls);
state->use_tls,
state->retry_no_tls);

if (!subreq) {
tevent_req_error(req, ENOMEM);
return;
}

tevent_req_set_callback(subreq, sdap_cli_connect_done, req);
return;
} else if (state->retry_no_tls) {
DEBUG(SSSDBG_IMPORTANT_INFO, "StartTLS operation failed! Falling back to " \
"connecting with startTLS disabled. Warning: " \
"this means network communication is NOT " \
"protected with TLS.\n");
sss_log(SSS_LOG_INFO, "StartTLS operation failed! Falling back to " \
"connecting with startTLS disabled. Warning: " \
"this means network communication is NOT " \
"protected with TLS.\n");
state->use_tls = false;
state->retry_no_tls = false;

subreq = sdap_connect_send(state, state->ev, state->opts,
state->service->uri,
state->service->sockaddr,
state->service->sockaddr_len,
state->use_tls,
state->retry_no_tls);

if (!subreq) {
tevent_req_error(req, ENOMEM);
Expand Down Expand Up @@ -1990,7 +2048,8 @@ static errno_t sdap_cli_auth_reconnect(struct tevent_req *req)
state = tevent_req_data(req, struct sdap_cli_connect_state);

ret = decide_tls_usage(state->force_tls, state->opts->basic,
state->service->uri, &state->use_tls);
state->service->uri, &state->use_tls,
&state->retry_no_tls);
if (ret != EOK) {
goto done;
}
Expand All @@ -1999,7 +2058,8 @@ static errno_t sdap_cli_auth_reconnect(struct tevent_req *req)
state->service->uri,
state->service->sockaddr,
state->service->sockaddr_len,
state->use_tls);
state->use_tls,
state->retry_no_tls);

if (subreq == NULL) {
ret = ENOMEM;
Expand Down
32 changes: 32 additions & 0 deletions src/tests/system/tests/test_ldap.py
Original file line number Diff line number Diff line change
Expand Up @@ -144,3 +144,35 @@ def test_ldap__change_password_wrong_current(client: Client, ldap: LDAP, modify_
client.sssd.start()

assert not client.auth.passwd.password("user1", "wrong123", "Newpass123"), "Password change did not fail"


@pytest.mark.topology(KnownTopology.LDAP)
def test_ldap__use_start_tls_allow_fallback(client: Client, ldap: LDAP):
"""
:title: Check that 'allow' start_tls option works
:setup:
1. Add user to SSSD
2. Set incorrect TLS configuration with "ldap_tls_cacert"
3. Set ldap_id_use_start_tls to "true"
3. Start SSSD
4. Set ldap_id_use_start_tls to "allow"
5. Restart SSSD
:steps:
1. Attempt to lookup 'tuser'
2. Attempt to lookup 'tuser' again
:expectedresults:
1. User lookup should fail
2. User lookup should succeed
:customerscenario: False
"""
ldap.user("tuser").add()
client.sssd.domain["ldap_tls_cacert"] = "badpath.crt"
client.sssd.domain["ldap_id_use_start_tls"] = "true"
client.sssd.start()
result = client.tools.id("tuser")
assert result is None
client.sssd.domain["ldap_id_use_start_tls"] = "allow"
client.sssd.restart()
result = client.tools.id("tuser")
assert result is not None
assert result.user.name == "tuser"

0 comments on commit 7143c5a

Please sign in to comment.