diff --git a/src/config/SSSDConfigTest.py b/src/config/SSSDConfigTest.py index b160be2b128..cad79914578 100755 --- a/src/config/SSSDConfigTest.py +++ b/src/config/SSSDConfigTest.py @@ -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')) @@ -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') @@ -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", diff --git a/src/config/etc/sssd.api.d/sssd-ad.conf b/src/config/etc/sssd.api.d/sssd-ad.conf index 3bf89ed54f9..5a33fde5cc6 100644 --- a/src/config/etc/sssd.api.d/sssd-ad.conf +++ b/src/config/etc/sssd.api.d/sssd-ad.conf @@ -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 diff --git a/src/config/etc/sssd.api.d/sssd-ipa.conf b/src/config/etc/sssd.api.d/sssd-ipa.conf index b28281c2a52..e03ea0a530b 100644 --- a/src/config/etc/sssd.api.d/sssd-ipa.conf +++ b/src/config/etc/sssd.api.d/sssd-ipa.conf @@ -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 diff --git a/src/config/etc/sssd.api.d/sssd-ldap.conf b/src/config/etc/sssd.api.d/sssd-ldap.conf index 237cf40ccd5..f7cec2a7071 100644 --- a/src/config/etc/sssd.api.d/sssd-ldap.conf +++ b/src/config/etc/sssd.api.d/sssd-ldap.conf @@ -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 diff --git a/src/config/testconfigs/sssd-invalid-badbool.conf b/src/config/testconfigs/sssd-invalid-badbool.conf index a0b8f6d7799..80fa6288a36 100644 --- a/src/config/testconfigs/sssd-invalid-badbool.conf +++ b/src/config/testconfigs/sssd-invalid-badbool.conf @@ -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 diff --git a/src/man/sssd-ldap.5.xml b/src/man/sssd-ldap.5.xml index c094a6125df..3837147c40e 100644 --- a/src/man/sssd-ldap.5.xml +++ b/src/man/sssd-ldap.5.xml @@ -913,9 +913,37 @@ Specifies that the id_provider connection must also use tls to protect the channel. + + Three modes are currently supported: + + + + allow - Attempt startTLS first then fallback + to connecting without if it fails. + + + + + true - TLS must be used or the connection + will fail. + + + + + false - startTLS will not be attempted, Not + recommended for security reasons. + + + + Default: false + + Used as a default for compatibility reasons, please + check that TLS is really used (can check SSSD logs for + warnings about fallback). + diff --git a/src/providers/ad/ad_opts.c b/src/providers/ad/ad_opts.c index 9dd2f03871a..3cd01608ae6 100644 --- a/src/providers/ad/ad_opts.c +++ b/src/providers/ad/ad_opts.c @@ -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 }, diff --git a/src/providers/ipa/ipa_opts.c b/src/providers/ipa/ipa_opts.c index 97cddb1d70d..71d88e5da97 100644 --- a/src/providers/ipa/ipa_opts.c +++ b/src/providers/ipa/ipa_opts.c @@ -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 }, diff --git a/src/providers/ldap/ldap_opts.c b/src/providers/ldap/ldap_opts.c index bcf029920c8..a66a85d989f 100644 --- a/src/providers/ldap/ldap_opts.c +++ b/src/providers/ldap/ldap_opts.c @@ -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 }, diff --git a/src/providers/ldap/sdap_async.h b/src/providers/ldap/sdap_async.h index a7b0f691283..22ef69f8557 100644 --- a/src/providers/ldap/sdap_async.h +++ b/src/providers/ldap/sdap_async.h @@ -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); diff --git a/src/providers/ldap/sdap_async_connection.c b/src/providers/ldap/sdap_async_connection.c index e8638725c78..991f565d9ad 100644 --- a/src/providers/ldap/sdap_async_connection.c +++ b/src/providers/ldap/sdap_async_connection.c @@ -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; @@ -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; @@ -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) { @@ -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, @@ -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; }; @@ -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; @@ -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; }; @@ -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; @@ -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; } @@ -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); @@ -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; @@ -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, @@ -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); @@ -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; } @@ -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; diff --git a/src/tests/system/tests/test_ldap.py b/src/tests/system/tests/test_ldap.py index b94a7c6a96f..df05c2c4fcd 100644 --- a/src/tests/system/tests/test_ldap.py +++ b/src/tests/system/tests/test_ldap.py @@ -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"