Thursday, April 2, 2020

[389-commits] [389-ds-base] branch 389-ds-base-1.4.1 updated: Issue 50869 - Setting nsslapd-allowed-sasl-mechanisms truncates the value

This is an automated email from the git hooks/post-receive script.

spichugi pushed a commit to branch 389-ds-base-1.4.1
in repository 389-ds-base.

The following commit(s) were added to refs/heads/389-ds-base-1.4.1 by this push:
new 9a2beaf Issue 50869 - Setting nsslapd-allowed-sasl-mechanisms truncates the value
9a2beaf is described below

commit 9a2beafcec02dd6b5a67fc2da8c5b9e6b05573cb
Author: Simon Pichugin <spichugi@redhat.com>
AuthorDate: Tue Mar 31 01:52:02 2020 +0200

Issue 50869 - Setting nsslapd-allowed-sasl-mechanisms truncates the value

Bug Description: Adding multiple mechanisms to nsslapd-allowed-sasl-mechanisms ignores all but one of the mechanisms specified.

Fix Description: The issue happens because we use the same memory address
for 'char *' for slapdFrontendConfig->allowed_sasl_mechs and
for slapdFrontendConfig->allowed_sasl_mechs_array.
So when we split the 'char *' into the 'char **' with ' ' delimetr,
allowed_sasl_mechs has only the first element becuase ' ' is set to 0 now.

Define a separate 'char *' for the array.
Add a test for the issue.

https://pagure.io/389-ds-base/issue/50869

Reviewed by: mreynolds, firstyear, tbordaz (Thanks!!)
---
.../tests/suites/sasl/allowed_mechs_test.py | 26 ++++++++++++++++++++++
ldap/servers/slapd/charray.c | 2 ++
ldap/servers/slapd/libglobs.c | 7 +++++-
3 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/dirsrvtests/tests/suites/sasl/allowed_mechs_test.py b/dirsrvtests/tests/suites/sasl/allowed_mechs_test.py
index d43a581..311ccb3 100644
--- a/dirsrvtests/tests/suites/sasl/allowed_mechs_test.py
+++ b/dirsrvtests/tests/suites/sasl/allowed_mechs_test.py
@@ -176,6 +176,32 @@ def test_basic_feature(topology_st):
assert(set(final_mechs) == set(orig_mechs))


+@pytest.mark.bz1816854
+@pytest.mark.ds50869
+def test_config_set_few_mechs(topology_st):
+ """Test that we can successfully set multiple values to nsslapd-allowed-sasl-mechanisms
+
+ :id: d7c3c58b-4fbe-42ab-a8d4-9dd362916d5f
+ :setup: Standalone instance
+ :steps:
+ 1. Set nsslapd-allowed-sasl-mechanisms to "PLAIN GSSAPI"
+ 2. Verify nsslapd-allowed-sasl-mechanisms has the values
+ :expectedresults:
+ 1. Operation should be successful
+ 2. Operation should be successful
+ """
+
+ standalone = topology_st.standalone
+
+ standalone.log.info("Set nsslapd-allowed-sasl-mechanisms to 'PLAIN GSSAPI'")
+ standalone.config.set('nsslapd-allowed-sasl-mechanisms', 'PLAIN GSSAPI')
+
+ standalone.log.info("Verify nsslapd-allowed-sasl-mechanisms has the values")
+ allowed_mechs = standalone.config.get_attr_val_utf8('nsslapd-allowed-sasl-mechanisms')
+ assert('PLAIN' in allowed_mechs)
+ assert('GSSAPI' in allowed_mechs)
+
+
if __name__ == '__main__':
# Run isolated
# -s for DEBUG mode
diff --git a/ldap/servers/slapd/charray.c b/ldap/servers/slapd/charray.c
index 6175491..c27c5da 100644
--- a/ldap/servers/slapd/charray.c
+++ b/ldap/servers/slapd/charray.c
@@ -325,6 +325,8 @@ slapi_str2charray(char *str, char *brkstr)
/*
* extended version of str2charray lets you disallow
* duplicate values into the array.
+ * Also, "char *str" should be a temporary string which is freed afterwards.
+ * the string is changed during this function execution
*/
char **
slapi_str2charray_ext(char *str, char *brkstr, int allow_dups)
diff --git a/ldap/servers/slapd/libglobs.c b/ldap/servers/slapd/libglobs.c
index 848e088..d1d8801 100644
--- a/ldap/servers/slapd/libglobs.c
+++ b/ldap/servers/slapd/libglobs.c
@@ -7272,9 +7272,12 @@ config_set_allowed_sasl_mechs(const char *attrname, char *value, char *errorbuf
/* During a reset, the value is "", so we have to handle this case. */
if (strcmp(value, "") != 0) {
char *nval = slapi_ch_strdup(value);
+ /* A separate variable is used because slapi_str2charray_ext can change it and nval'd become corrupted */
+ char *tmp_array_nval;

/* cyrus sasl doesn't like comma separated lists */
remove_commas(nval);
+ tmp_array_nval = slapi_ch_strdup(nval);

if (invalid_sasl_mech(nval)) {
slapi_log_err(SLAPI_LOG_ERR, "config_set_allowed_sasl_mechs",
@@ -7283,13 +7286,15 @@ config_set_allowed_sasl_mechs(const char *attrname, char *value, char *errorbuf
"digits, hyphens, or underscores\n",
nval);
slapi_ch_free_string(&nval);
+ slapi_ch_free_string(&tmp_array_nval);
return LDAP_UNWILLING_TO_PERFORM;
}
CFG_LOCK_WRITE(slapdFrontendConfig);
slapi_ch_free_string(&slapdFrontendConfig->allowed_sasl_mechs);
slapi_ch_array_free(slapdFrontendConfig->allowed_sasl_mechs_array);
slapdFrontendConfig->allowed_sasl_mechs = nval;
- slapdFrontendConfig->allowed_sasl_mechs_array = slapi_str2charray_ext(nval, " ", 0);
+ slapdFrontendConfig->allowed_sasl_mechs_array = slapi_str2charray_ext(tmp_array_nval, " ", 0);
+ slapi_ch_free_string(&tmp_array_nval);
CFG_UNLOCK_WRITE(slapdFrontendConfig);
} else {
/* If this value is "", we need to set the list to *all* possible mechs */

--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
_______________________________________________
389-commits mailing list -- 389-commits@lists.fedoraproject.org
To unsubscribe send an email to 389-commits-leave@lists.fedoraproject.org
Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: https://lists.fedoraproject.org/archives/list/389-commits@lists.fedoraproject.org

No comments:

Post a Comment