[SSSD] [sssd PR#455][comment] mmap_cache: make checks independent of input size
URL: https://github.com/SSSD/sssd/pull/455 Title: #455: mmap_cache: make checks independent of input size jhrozek commented: """ OK, good. There is a downstream request for a release that is based on the sssd-1-13 branch, so I'm quite sure I won't forget about the backport. btw it's not needed to run Coverity this week (unless you're bored), it's fine to do that when you do some other work in the stable branch. """ See the full comment at https://github.com/SSSD/sssd/pull/455#issuecomment-347345125 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#455][comment] mmap_cache: make checks independent of input size
URL: https://github.com/SSSD/sssd/pull/455 Title: #455: mmap_cache: make checks independent of input size fidencio commented: """ I haven't ran coverity on sssd-1-13 branch. I'd say no. Let me run coverity at some point this week and then I can submit a PR with all the fixes. """ See the full comment at https://github.com/SSSD/sssd/pull/455#issuecomment-347334819 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#455][comment] mmap_cache: make checks independent of input size
URL: https://github.com/SSSD/sssd/pull/455 Title: #455: mmap_cache: make checks independent of input size jhrozek commented: """ @fidencio do you know if the Coverity patch should be backported to sssd-1-13 as well? """ See the full comment at https://github.com/SSSD/sssd/pull/455#issuecomment-347327875 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#450][+Pushed] sysdb: do not use objectClass for users and groups
URL: https://github.com/SSSD/sssd/pull/450 Title: #450: sysdb: do not use objectClass for users and groups Label: +Pushed ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#450][closed] sysdb: do not use objectClass for users and groups
URL: https://github.com/SSSD/sssd/pull/450 Author: sumit-bose Title: #450: sysdb: do not use objectClass for users and groups Action: closed To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/450/head:pr450 git checkout pr450 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#450][comment] sysdb: do not use objectClass for users and groups
URL: https://github.com/SSSD/sssd/pull/450 Title: #450: sysdb: do not use objectClass for users and groups jhrozek commented: """ * master: * 2927da49dd8a16fff6312d89ad43cc355655800c * 98195e591c4d97caa6125e8214879660b740973f * 0e238c259c066cf997aaa940d33d6bda96c15925 * 0cce3d3ad14caf406303cf2ce6bf80171b708a93 """ See the full comment at https://github.com/SSSD/sssd/pull/450#issuecomment-347321263 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#455][comment] mmap_cache: make checks independent of input size
URL: https://github.com/SSSD/sssd/pull/455 Title: #455: mmap_cache: make checks independent of input size lslebodn commented: """ http://vm-031.$ABC/logs/job/81/85/summary.html """ See the full comment at https://github.com/SSSD/sssd/pull/455#issuecomment-347320388 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#455][comment] mmap_cache: make checks independent of input size
URL: https://github.com/SSSD/sssd/pull/455 Title: #455: mmap_cache: make checks independent of input size lslebodn commented: """ master: * ec13304e0c4dc1ffe8c6abe6d0741fa7f6da3ceb * c4027058427d79d81404331ad0717907e56e87c4 """ See the full comment at https://github.com/SSSD/sssd/pull/455#issuecomment-347319559 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#455][+Pushed] mmap_cache: make checks independent of input size
URL: https://github.com/SSSD/sssd/pull/455 Title: #455: mmap_cache: make checks independent of input size Label: +Pushed ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#455][comment] mmap_cache: make checks independent of input size
URL: https://github.com/SSSD/sssd/pull/455 Title: #455: mmap_cache: make checks independent of input size jhrozek commented: """ * master: * 1d88a0591ce8445ea3b6a88845a5997d61c915b4 * 4382047490dd4f80b407cc1e618da048f13e5f8f """ See the full comment at https://github.com/SSSD/sssd/pull/455#issuecomment-347319569 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#455][closed] mmap_cache: make checks independent of input size
URL: https://github.com/SSSD/sssd/pull/455 Author: sumit-bose Title: #455: mmap_cache: make checks independent of input size Action: closed To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/455/head:pr455 git checkout pr455 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#455][comment] mmap_cache: make checks independent of input size
URL: https://github.com/SSSD/sssd/pull/455 Title: #455: mmap_cache: make checks independent of input size lslebodn commented: """ master: * ec13304e0c4dc1ffe8c6abe6d0741fa7f6da3ceb * c4027058427d79d81404331ad0717907e56e87c4 """ See the full comment at https://github.com/SSSD/sssd/pull/455#issuecomment-347319559 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#450][+Accepted] sysdb: do not use objectClass for users and groups
URL: https://github.com/SSSD/sssd/pull/450 Title: #450: sysdb: do not use objectClass for users and groups Label: +Accepted ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#450][comment] sysdb: do not use objectClass for users and groups
URL: https://github.com/SSSD/sssd/pull/450 Title: #450: sysdb: do not use objectClass for users and groups jhrozek commented: """ CI: http://vm-031.$ABC/logs/job/81/84/summary.html """ See the full comment at https://github.com/SSSD/sssd/pull/450#issuecomment-347314051 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#459][synchronized] krb5: show error message for krb5_init_context() failures
URL: https://github.com/SSSD/sssd/pull/459 Author: sumit-bose Title: #459: krb5: show error message for krb5_init_context() failures Action: synchronized To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/459/head:pr459 git checkout pr459 From eafbbbd33f9682f18dd71f3ec020c15897536447 Mon Sep 17 00:00:00 2001 From: Sumit BoseDate: Mon, 27 Nov 2017 13:45:14 +0100 Subject: [PATCH] krb5: show error message for krb5_init_context() failures If there are typos in /etc/krb5.conf (or one of the included config snippets) krb5_init_context(), the initial call always needed to do any other operation with libkrb5, fails because /etc/krb5.conf cannot be parsed. Currently the related debug/syslog messages might be misleading, e.g. failed to read keytab. This is because SSSD does not use a global krb5 context but creates a fresh one for every new request or operation (to always use the latest settings from /etc/krb5.conf) and typically there is an error message indicating that the related operation failed but not giving more details. Since krb5_init_context() is fundamental for Kerberos support this patch tries to add as much details as libkrb5 provides in the logs if the call fails. Resolves https://pagure.io/SSSD/sssd/issue/3586 --- src/providers/krb5/krb5_ccache.c | 6 +++--- src/providers/krb5/krb5_common.c | 2 +- src/providers/ldap/ldap_child.c | 2 +- src/providers/ldap/ldap_common.c | 2 +- src/util/sss_krb5.c | 25 ++--- src/util/sss_krb5.h | 2 ++ 6 files changed, 30 insertions(+), 9 deletions(-) diff --git a/src/providers/krb5/krb5_ccache.c b/src/providers/krb5/krb5_ccache.c index f9bb25efd..2e28276b7 100644 --- a/src/providers/krb5/krb5_ccache.c +++ b/src/providers/krb5/krb5_ccache.c @@ -299,7 +299,7 @@ static errno_t sss_open_ccache_as_user(TALLOC_CTX *mem_ctx, goto done; } -kerr = krb5_init_context(>context); +kerr = sss_krb5_init_context(>context); if (kerr) { ret = EIO; goto done; @@ -565,9 +565,9 @@ errno_t get_ccache_file_data(const char *ccache_file, const char *client_name, const char *realm_name; int realm_length; -kerr = krb5_init_context(); +kerr = sss_krb5_init_context(); if (kerr != 0) { -DEBUG(SSSDBG_CRIT_FAILURE, "krb5_init_context failed.\n"); +DEBUG(SSSDBG_CRIT_FAILURE, "sss_krb5_init_context failed.\n"); goto done; } diff --git a/src/providers/krb5/krb5_common.c b/src/providers/krb5/krb5_common.c index 0b32da94d..520e7591c 100644 --- a/src/providers/krb5/krb5_common.c +++ b/src/providers/krb5/krb5_common.c @@ -106,7 +106,7 @@ static errno_t sss_get_system_ccname_template(TALLOC_CTX *mem_ctx, *ccname = NULL; -ret = krb5_init_context(); +ret = sss_krb5_init_context(); if (ret) return ret; ret = krb5_get_profile(ctx, ); diff --git a/src/providers/ldap/ldap_child.c b/src/providers/ldap/ldap_child.c index c0618d6d8..4558fd7c4 100644 --- a/src/providers/ldap/ldap_child.c +++ b/src/providers/ldap/ldap_child.c @@ -574,7 +574,7 @@ static krb5_error_code privileged_krb5_setup(struct input_buffer *ibuf) krb5_error_code kerr; char *keytab_name; -kerr = krb5_init_context(>context); +kerr = sss_krb5_init_context(>context); if (kerr != 0) { DEBUG(SSSDBG_CRIT_FAILURE, "Failed to init kerberos context\n"); return kerr; diff --git a/src/providers/ldap/ldap_common.c b/src/providers/ldap/ldap_common.c index 0597e91f7..4ec36584a 100644 --- a/src/providers/ldap/ldap_common.c +++ b/src/providers/ldap/ldap_common.c @@ -364,7 +364,7 @@ sdap_gssapi_get_default_realm(TALLOC_CTX *mem_ctx) krb5_error_code krberr; krb5_context context = NULL; -krberr = krb5_init_context(); +krberr = sss_krb5_init_context(); if (krberr) { DEBUG(SSSDBG_OP_FAILURE, "Failed to init kerberos context\n"); goto done; diff --git a/src/util/sss_krb5.c b/src/util/sss_krb5.c index a702a8b57..7c2577b34 100644 --- a/src/util/sss_krb5.c +++ b/src/util/sss_krb5.c @@ -113,7 +113,7 @@ errno_t select_principal_from_keytab(TALLOC_CTX *mem_ctx, return ENOMEM; } -kerr = krb5_init_context(_ctx); +kerr = sss_krb5_init_context(_ctx); if (kerr) { DEBUG(SSSDBG_OP_FAILURE, "Failed to init kerberos context\n"); ret = EFAULT; @@ -1096,9 +1096,9 @@ bool sss_krb5_realm_has_proxy(const char *realm) return false; } -kerr = krb5_init_context(); +kerr = sss_krb5_init_context(); if (kerr != 0) { -DEBUG(SSSDBG_OP_FAILURE, "krb5_init_context failed.\n"); +DEBUG(SSSDBG_OP_FAILURE, "sss_krb5_init_context failed.\n"); return false; } @@ -1330,3 +1330,22 @@ krb5_error_code sss_krb5_marshal_princ(krb5_principal princ, } return EOK; } + +krb5_error_code sss_krb5_init_context(krb5_context *context) +{ +
[SSSD] [sssd PR#459][opened] krb5: show error message for krb5_init_context() failures
URL: https://github.com/SSSD/sssd/pull/459 Author: sumit-bose Title: #459: krb5: show error message for krb5_init_context() failures Action: opened PR body: """ If there are typos in /etc/krb5.conf (or one of the included config snippets) krb5_init_context(), the initial call always needed to do any other operation with libkrb5, fails because /etc/krb5.conf cannot be parsed. Currently the related debug/syslog messages might be misleading, e.g. failed to read keytab. This is because SSSD does not use a global krb5 context but creates a fresh one for every new request or operation (to always use the latest settings from /etc/krb5.conf) and typically there is an error message indicating that the related operation failed but not giving more details. Since krb5_init_context() is fundamental for Kerberos support this patch tries to add as much details as libkrb5 provides in the logs if the call fails. Resolves https://pagure.io/SSSD/sssd/issue/3586 """ To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/459/head:pr459 git checkout pr459 From ae64265c8531e8ea4e902cda8c44d1c5803ca091 Mon Sep 17 00:00:00 2001 From: Sumit BoseDate: Mon, 27 Nov 2017 13:45:14 +0100 Subject: [PATCH] krb5: show error message for krb5_init_context() failures If there are typos in /etc/krb5.conf (or one of the included config snippets) krb5_init_context(), the initial call always needed to do any other operation with libkrb5, fails because /etc/krb5.conf cannot be parsed. Currently the related debug/syslog messages might be misleading, e.g. failed to read keytab. This is because SSSD does not use a global krb5 context but creates a fresh one for every new request or operation (to always use the latest settings from /etc/krb5.conf) and typically there is an error message indicating that the related operation failed but not giving more details. Since krb5_init_context() is fundamental for Kerberos support this patch tries to add as much details as libkrb5 provides in the logs if the call fails. Resolves https://pagure.io/SSSD/sssd/issue/3586 --- src/providers/krb5/krb5_ccache.c | 6 +++--- src/providers/krb5/krb5_common.c | 2 +- src/providers/ldap/ldap_child.c | 2 +- src/providers/ldap/ldap_common.c | 2 +- src/util/sss_krb5.c | 25 ++--- src/util/sss_krb5.h | 2 ++ 6 files changed, 30 insertions(+), 9 deletions(-) diff --git a/src/providers/krb5/krb5_ccache.c b/src/providers/krb5/krb5_ccache.c index f9bb25efd..2e28276b7 100644 --- a/src/providers/krb5/krb5_ccache.c +++ b/src/providers/krb5/krb5_ccache.c @@ -299,7 +299,7 @@ static errno_t sss_open_ccache_as_user(TALLOC_CTX *mem_ctx, goto done; } -kerr = krb5_init_context(>context); +kerr = sss_krb5_init_context(>context); if (kerr) { ret = EIO; goto done; @@ -565,9 +565,9 @@ errno_t get_ccache_file_data(const char *ccache_file, const char *client_name, const char *realm_name; int realm_length; -kerr = krb5_init_context(); +kerr = sss_krb5_init_context(); if (kerr != 0) { -DEBUG(SSSDBG_CRIT_FAILURE, "krb5_init_context failed.\n"); +DEBUG(SSSDBG_CRIT_FAILURE, "sss_krb5_init_context failed.\n"); goto done; } diff --git a/src/providers/krb5/krb5_common.c b/src/providers/krb5/krb5_common.c index 0b32da94d..520e7591c 100644 --- a/src/providers/krb5/krb5_common.c +++ b/src/providers/krb5/krb5_common.c @@ -106,7 +106,7 @@ static errno_t sss_get_system_ccname_template(TALLOC_CTX *mem_ctx, *ccname = NULL; -ret = krb5_init_context(); +ret = sss_krb5_init_context(); if (ret) return ret; ret = krb5_get_profile(ctx, ); diff --git a/src/providers/ldap/ldap_child.c b/src/providers/ldap/ldap_child.c index c0618d6d8..4558fd7c4 100644 --- a/src/providers/ldap/ldap_child.c +++ b/src/providers/ldap/ldap_child.c @@ -574,7 +574,7 @@ static krb5_error_code privileged_krb5_setup(struct input_buffer *ibuf) krb5_error_code kerr; char *keytab_name; -kerr = krb5_init_context(>context); +kerr = sss_krb5_init_context(>context); if (kerr != 0) { DEBUG(SSSDBG_CRIT_FAILURE, "Failed to init kerberos context\n"); return kerr; diff --git a/src/providers/ldap/ldap_common.c b/src/providers/ldap/ldap_common.c index 0597e91f7..4ec36584a 100644 --- a/src/providers/ldap/ldap_common.c +++ b/src/providers/ldap/ldap_common.c @@ -364,7 +364,7 @@ sdap_gssapi_get_default_realm(TALLOC_CTX *mem_ctx) krb5_error_code krberr; krb5_context context = NULL; -krberr = krb5_init_context(); +krberr = sss_krb5_init_context(); if (krberr) { DEBUG(SSSDBG_OP_FAILURE, "Failed to init kerberos context\n"); goto done; diff --git a/src/util/sss_krb5.c b/src/util/sss_krb5.c index a702a8b57..b50311db2 100644 --- a/src/util/sss_krb5.c +++ b/src/util/sss_krb5.c @@ -113,7 +113,7 @@ errno_t
[SSSD] [sssd PR#455][+Accepted] mmap_cache: make checks independent of input size
URL: https://github.com/SSSD/sssd/pull/455 Title: #455: mmap_cache: make checks independent of input size Label: +Accepted ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#450][comment] sysdb: do not use objectClass for users and groups
URL: https://github.com/SSSD/sssd/pull/450 Title: #450: sysdb: do not use objectClass for users and groups jhrozek commented: """ So far I approved the patches in the github review tool, because the code looks good and my tests all passed. I'll add the Accepted label once CI and Coverity finish so that we have everything "by the book". """ See the full comment at https://github.com/SSSD/sssd/pull/450#issuecomment-347204007 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#455][comment] mmap_cache: make checks independent of input size
URL: https://github.com/SSSD/sssd/pull/455 Title: #455: mmap_cache: make checks independent of input size lslebodn commented: """ On (27/11/17 14:23), sumit-bose wrote: >I think it has to be at least > >- if (key->len > strs_len) { >+ if (key->len + name_ptr - strs_offset > strs_len) { > >because name_ptr is relative to rec->data and includes some other uint32_t >values the different record structs starts with which are not include in >strs_len/data_len. But I think being more strict here is not needed because >also for the passwd and group case strs_len includes other strings as well >making the whole check only a rough estimate. > Probably yes, it was from top of my head. >> IMHO it's faster then trying to find '\0'. > >I already removed the check in the latest version. > I noticed that and thank you. I'm fine even with current version. ACK++ LS """ See the full comment at https://github.com/SSSD/sssd/pull/455#issuecomment-347198606 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#455][comment] mmap_cache: make checks independent of input size
URL: https://github.com/SSSD/sssd/pull/455 Title: #455: mmap_cache: make checks independent of input size sumit-bose commented: """ I think it has to be at least - if (key->len > strs_len) { + if (key->len + name_ptr - strs_offset > strs_len) { because name_ptr is relative to rec->data and includes some other uint32_t values the different record structs starts with which are not include in strs_len/data_len. But I think being more strict here is not needed because also for the passwd and group case strs_len includes other strings as well making the whole check only a rough estimate. About > IMHO it's faster then trying to find '\0'. I already removed the check in the latest version. """ See the full comment at https://github.com/SSSD/sssd/pull/455#issuecomment-347196012 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#455][comment] mmap_cache: make checks independent of input size
URL: https://github.com/SSSD/sssd/pull/455 Title: #455: mmap_cache: make checks independent of input size lslebodn commented: """ On (24/11/17 15:42), sumit-bose wrote: >I agree this is nitpicking and artificial, but I think strcmp() can run >outside the data table. > >If the last slot in the data table is the last of an initgr record with many >GIDs for a user with a short user name, e.g. 'user' and the slot is fully used >so that the user name is at the end of the slot. data_len of in the initgr >record (which is used for strs_len in sss_mc_find_record() in this case) will >be large keys much longer then 'user' will pass until the strcmp. If now due >to a corruption the '0' at the end of the user name (and the unique name) are >replaced by a '-' with a key like 'user-user-x' strcmp() will read the first >byte of the free table when trying to compare the 'x'. But I agree that even >then it would not do any harm because we are still reading from our own memory. > Ahh that's true because GIDs are stored before strings for initgtoup and strs_len (sss_mc_get_strs_len) is actually data_len for initgroups. That case could be covered by stricter check for length of input string ``` -if (key->len > strs_len) { +if (key->len + name_ptr > strs_len) { /* The string cannot be in current record */ slot = sss_mc_next_slot_with_hash(rec, hash); continue; } ``` But such check should be probably after checking the value of `name_ptr`. So probably inside "if" which would backup "corrupted" memory cache. IMHO it's faster then trying to find '\0'. LS """ See the full comment at https://github.com/SSSD/sssd/pull/455#issuecomment-347185728 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#457][comment] ipa: Removal of umask(0) in selinux_child
URL: https://github.com/SSSD/sssd/pull/457 Title: #457: ipa: Removal of umask(0) in selinux_child simo5 commented: """ You need to detect we have the correct libsemanage and ifdef those out. In a few years (when RHEL6 goes away) we can remove the code entirely. Also add minimum required libsemanage version in sssd spec file in contrib if not there yet. """ See the full comment at https://github.com/SSSD/sssd/pull/457#issuecomment-347182064 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#450][comment] sysdb: do not use objectClass for users and groups
URL: https://github.com/SSSD/sssd/pull/450 Title: #450: sysdb: do not use objectClass for users and groups sumit-bose commented: """ Thank you for the rigid review. I think all your comments are valid and I included them in the latest version. """ See the full comment at https://github.com/SSSD/sssd/pull/450#issuecomment-347149753 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#450][synchronized] sysdb: do not use objectClass for users and groups
URL: https://github.com/SSSD/sssd/pull/450 Author: sumit-bose Title: #450: sysdb: do not use objectClass for users and groups Action: synchronized To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/450/head:pr450 git checkout pr450 From 14a145072ea5e203bf1612dc56a64bbe60e0d453 Mon Sep 17 00:00:00 2001 From: Sumit BoseDate: Wed, 8 Nov 2017 14:04:40 +0100 Subject: [PATCH 1/4] sysdb: be_refresh_get_values_ex() remove unused option The objectclass argument is not used in be_refresh_get_values_ex() anymore. Related to https://pagure.io/SSSD/sssd/issue/3503 --- src/providers/be_refresh.c | 7 +-- 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/providers/be_refresh.c b/src/providers/be_refresh.c index 81f2c5d1d..e8cf5da75 100644 --- a/src/providers/be_refresh.c +++ b/src/providers/be_refresh.c @@ -32,7 +32,6 @@ static errno_t be_refresh_get_values_ex(TALLOC_CTX *mem_ctx, struct sss_domain_info *domain, time_t period, -const char *objectclass, struct ldb_dn *base_dn, const char *attr, char ***_values) @@ -96,21 +95,17 @@ static errno_t be_refresh_get_values(TALLOC_CTX *mem_ctx, char ***_values) { struct ldb_dn *base_dn = NULL; -const char *class = NULL; errno_t ret; switch (type) { case BE_REFRESH_TYPE_USERS: base_dn = sysdb_user_base_dn(mem_ctx, domain); -class = SYSDB_USER_CLASS; break; case BE_REFRESH_TYPE_GROUPS: base_dn = sysdb_group_base_dn(mem_ctx, domain); -class = SYSDB_GROUP_CLASS; break; case BE_REFRESH_TYPE_NETGROUPS: base_dn = sysdb_netgroup_base_dn(mem_ctx, domain); -class = SYSDB_NETGROUP_CLASS; break; case BE_REFRESH_TYPE_SENTINEL: return ERR_INTERNAL; @@ -121,7 +116,7 @@ static errno_t be_refresh_get_values(TALLOC_CTX *mem_ctx, return ENOMEM; } -ret = be_refresh_get_values_ex(mem_ctx, domain, period, class, +ret = be_refresh_get_values_ex(mem_ctx, domain, period, base_dn, SYSDB_NAME, _values); talloc_free(base_dn); From d8adc7a2e2125c7aeaf180376c35a23843b10562 Mon Sep 17 00:00:00 2001 From: Sumit Bose Date: Wed, 8 Nov 2017 15:14:58 +0100 Subject: [PATCH 2/4] sysdb: do not use objectClass for users and groups The majority of the object in the SSSD cache are users and groups. If there are many user and groups in the cache the index objects of the objectclass attributes 'user' and 'group' become large because the must hold references to all objects of those object classes. As a result the management of these index objects becomes costly because they must be parsed and split apart quite often. Additionally they are mostly useless because user and groups are lookup up by more specific attributes in general. Only when enumerating all user or groups this kind of index might be useful. There are two way of removing this kind of index from the user and group objects. Either by removing objectClass from the list of indexes and add a new attribute to all other type of object we want and index for. Or by replacing objectClass with a different attribute for the user and group objects. After some testing I think the latter one is the more reliable one and implemented it in this patch. Related to https://pagure.io/SSSD/sssd/issue/3503 --- src/db/sysdb.h | 10 ++-- src/db/sysdb_init.c| 5 +- src/db/sysdb_ops.c | 6 +-- src/db/sysdb_search.c | 11 +++-- src/db/sysdb_upgrade.c | 4 ++ src/ldb_modules/memberof.c | 6 +-- src/providers/ad/ad_pac.c | 2 +- src/providers/ipa/ipa_id.c | 9 ++-- src/providers/ipa/ipa_subdomains_ext_groups.c | 2 +- src/providers/ipa/ipa_subdomains_id.c | 2 +- src/providers/krb5/krb5_renew_tgt.c| 3 +- src/providers/ldap/ldap_id_cleanup.c | 2 +- src/providers/ldap/sdap_async_groups.c | 8 ++-- src/providers/ldap/sdap_async_initgroups.c | 2 +- src/providers/ldap/sdap_async_initgroups_ad.c | 2 +- src/providers/ldap/sdap_async_nested_groups.c | 2 +- .../common/cache_req/plugins/cache_req_common.c| 2 +- src/responder/ifp/ifp_cache.c | 4 +- src/responder/ifp/ifp_groups.c | 4 +- src/responder/ifp/ifp_users.c | 2 +- src/responder/nss/nss_cmd.c| 2 +-