URL: https://github.com/SSSD/sssd/pull/374 Author: justin-stephenson Title: #374: IPA: Add threshold for sudo command and command group searches Action: opened
PR body: """ In large IPA environments where a high number of sudo commands and command groups are used, retrieval of sudo data can lead to SSSD constructing an overly large search filter which is not handled well on the ns-slapd side.
This PR implements a threshold by adding the `ipa_sudo_command_threshold` option(defaults to 50) which is used to prevent the large search filter from being created, an idea similar to https://github.com/SSSD/sssd/pull/319
Additionally, a commit was added to rename the **sudo_threshold** option to **sudo_rules_threshold**. This can be dropped if it seems unnecessary but I thought I would add it.
This can be reproduced by adding more sudo commands/command groups than the defined `ipa_sudo_command_threshold` in sssd.conf and checking the search filter used in the domain log or dirsrv access logs. If the threshold is not exceeded, the searches will still include the sudo command groups and command groups in the search filter.
I tested this on both the IPA server and IPA client by verifying sudo commands work as expected when the threshold is exceeded for IPA and AD trust users, and `sudo -l` command is the same as before the patch. """
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/374/head:pr374 git checkout pr374
URL: https://github.com/SSSD/sssd/pull/374 Title: #374: IPA: Add threshold for sudo command and command group searches
centos-ci commented: """ Can one of the admins verify this patch? """
See the full comment at https://github.com/SSSD/sssd/pull/374#issuecomment-328103428
URL: https://github.com/SSSD/sssd/pull/374 Title: #374: IPA: Add threshold for sudo command and command group searches
centos-ci commented: """ Can one of the admins verify this patch? """
See the full comment at https://github.com/SSSD/sssd/pull/374#issuecomment-328103433
URL: https://github.com/SSSD/sssd/pull/374 Title: #374: IPA: Add threshold for sudo command and command group searches
sumit-bose commented: """ ok to test """
See the full comment at https://github.com/SSSD/sssd/pull/374#issuecomment-328104624
URL: https://github.com/SSSD/sssd/pull/374 Title: #374: IPA: Add threshold for sudo command and command group searches
jhrozek commented: """ Sorry my patch that fixes another issue was just pushed and causes a conflict.
@justin-stephenson can you rebase, please?
@pbrezina can you review? """
See the full comment at https://github.com/SSSD/sssd/pull/374#issuecomment-328136267
URL: https://github.com/SSSD/sssd/pull/374 Author: justin-stephenson Title: #374: IPA: Add threshold for sudo command and command group searches Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/374/head:pr374 git checkout pr374
URL: https://github.com/SSSD/sssd/pull/374 Title: #374: IPA: Add threshold for sudo command and command group searches
jhrozek commented: """ ping @pbrezina do you have time to review this PR? """
See the full comment at https://github.com/SSSD/sssd/pull/374#issuecomment-331259509
URL: https://github.com/SSSD/sssd/pull/374 Title: #374: IPA: Add threshold for sudo command and command group searches
sumit-bose commented: """ @justin-stephenson, I think the rename patch should be dropped because the original patch introducing 'sudo_threshold' is already published in some Fedora releases. """
See the full comment at https://github.com/SSSD/sssd/pull/374#issuecomment-331388083
URL: https://github.com/SSSD/sssd/pull/374 Title: #374: IPA: Add threshold for sudo command and command group searches
sumit-bose commented: """ @justin-stephenson, I wonder if 'sudo_threshold' can be used for the commands as well or if there is a specific use case which requires to be able to set the threshold for rules and commands differently? """
See the full comment at https://github.com/SSSD/sssd/pull/374#issuecomment-331389632
URL: https://github.com/SSSD/sssd/pull/374 Title: #374: IPA: Add threshold for sudo command and command group searches
pbrezina commented: """ I agree with Summit with both case. Case I also ask you to move the thresholding logic
```c + if (ipa_sudo_cmds_exceed_threshold(state->conv, state->cmd_threshold)) { + DEBUG(SSSDBG_TRACE_FUNC, + "Command threshold [%d] exceeded, retrieving all sudo commands\n", + state->cmd_threshold); + filter = talloc_asprintf(state, "(objectClass=%s)", + state->map_cmd->name); + } else { + filter = ipa_sudo_conv_cmd_filter(state, state->conv); + } ``` inside the conversion functions (`ipa_sudo_conv_cmd_filter`, ipa_sudo_conv_cmdgoup_filter)? """
See the full comment at https://github.com/SSSD/sssd/pull/374#issuecomment-331395398
URL: https://github.com/SSSD/sssd/pull/374 Title: #374: IPA: Add threshold for sudo command and command group searches
Label: +Changes requested
URL: https://github.com/SSSD/sssd/pull/374 Author: justin-stephenson Title: #374: IPA: Add threshold for sudo command and command group searches Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/374/head:pr374 git checkout pr374
URL: https://github.com/SSSD/sssd/pull/374 Title: #374: IPA: Add threshold for sudo command and command group searches
justin-stephenson commented: """ @sumit-bose @pbrezina Thank you for the review, changes made and rebased.
Everything is working in my testing, but I am sure I did not use the best method to use the `sudo_threshold` option from the sudo responder to the IPA provider. Please let me know how this can be improved and if there is any need to document this option in the `sssd-ipa man` page. """
See the full comment at https://github.com/SSSD/sssd/pull/374#issuecomment-332233896
URL: https://github.com/SSSD/sssd/pull/374 Title: #374: IPA: Add threshold for sudo command and command group searches
Label: -Changes requested
URL: https://github.com/SSSD/sssd/pull/374 Title: #374: IPA: Add threshold for sudo command and command group searches
jhrozek commented: """ @pbrezina can you check this version, please? """
See the full comment at https://github.com/SSSD/sssd/pull/374#issuecomment-336864039
URL: https://github.com/SSSD/sssd/pull/374 Title: #374: IPA: Add threshold for sudo command and command group searches
pbrezina commented: """ I'm reviewing it right now. """
See the full comment at https://github.com/SSSD/sssd/pull/374#issuecomment-336864134
URL: https://github.com/SSSD/sssd/pull/374 Title: #374: IPA: Add threshold for sudo command and command group searches
pbrezina commented: """ @justin-stephenson Thank you for the changes. Please, can you also remove the ipa option completely since it can't be really set in sssd.conf (it is always overridden with sudo_threshold). """
See the full comment at https://github.com/SSSD/sssd/pull/374#issuecomment-336865319
URL: https://github.com/SSSD/sssd/pull/374 Title: #374: IPA: Add threshold for sudo command and command group searches
justin-stephenson commented: """ @pbrezina Yes, but I am having trouble determining how to retrieve the sudo_threshold value from inside the file `ipa_sudo_async.c`
I don't see the confdb structure exposed in `ipa_sudo_async.c` to be able to use `confdb_get_int(cdb, CONFDB_SUDO_THRESHOLD, ...`
Currently, the way I have it working is inside `ipa_init.c` I am retrieving the value from `be_ctx->cdb` and copying the value to ipa_sudo_command_threshold in `ipa_options->basic`, then retrieving the value where I need to use it with `dp_opt_get_int` """
See the full comment at https://github.com/SSSD/sssd/pull/374#issuecomment-337016827
URL: https://github.com/SSSD/sssd/pull/374 Title: #374: IPA: Add threshold for sudo command and command group searches
pbrezina commented: """ You can load it in `ipa_sudo_init_ipa_schema()` and store it inside `sudo_ctx`. """
See the full comment at https://github.com/SSSD/sssd/pull/374#issuecomment-337161165
URL: https://github.com/SSSD/sssd/pull/374 Author: justin-stephenson Title: #374: IPA: Add threshold for sudo command and command group searches Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/374/head:pr374 git checkout pr374
URL: https://github.com/SSSD/sssd/pull/374 Author: justin-stephenson Title: #374: IPA: Add threshold for sudo command and command group searches Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/374/head:pr374 git checkout pr374
URL: https://github.com/SSSD/sssd/pull/374 Title: #374: IPA: Add threshold for sudo command and command group searches
justin-stephenson commented: """ @pbrezina thanks, PR updated. I also added one brief sentence about this under the sudo_threshold definition in `sssd.conf.5.xml` """
See the full comment at https://github.com/SSSD/sssd/pull/374#issuecomment-337434783
URL: https://github.com/SSSD/sssd/pull/374 Title: #374: IPA: Add threshold for sudo command and command group searches
pbrezina commented: """ Ack. Thank you for your contribution. """
See the full comment at https://github.com/SSSD/sssd/pull/374#issuecomment-337849400
URL: https://github.com/SSSD/sssd/pull/374 Title: #374: IPA: Add threshold for sudo command and command group searches
Label: +Accepted
URL: https://github.com/SSSD/sssd/pull/374 Title: #374: IPA: Add threshold for sudo command and command group searches
lslebodn commented: """ master: * bc854800cc67271205d63136daaf68d7863cea6b """
See the full comment at https://github.com/SSSD/sssd/pull/374#issuecomment-337881338
URL: https://github.com/SSSD/sssd/pull/374 Author: justin-stephenson Title: #374: IPA: Add threshold for sudo command and command group searches Action: closed
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/374/head:pr374 git checkout pr374
sssd-devel@lists.fedorahosted.org
-
centos-ci -
jhrozek
-
justin-stephenson
-
lslebodn
-
pbrezina
-
sumit-bose