[SSSD] [sssd PR#455][comment] mmap_cache: make checks independent of input size

2017-11-27 Thread jhrozek
  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

2017-11-27 Thread fidencio
  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

2017-11-27 Thread jhrozek
  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

2017-11-27 Thread jhrozek
  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

2017-11-27 Thread jhrozek
   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

2017-11-27 Thread jhrozek
  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

2017-11-27 Thread lslebodn
  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

2017-11-27 Thread lslebodn
  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

2017-11-27 Thread jhrozek
  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

2017-11-27 Thread jhrozek
  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

2017-11-27 Thread lslebodn
   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

2017-11-27 Thread lslebodn
  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

2017-11-27 Thread jhrozek
  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

2017-11-27 Thread jhrozek
  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

2017-11-27 Thread sumit-bose
   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 Bose 
Date: 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

2017-11-27 Thread sumit-bose
   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 Bose 
Date: 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

2017-11-27 Thread jhrozek
  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

2017-11-27 Thread jhrozek
  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

2017-11-27 Thread lslebodn
  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

2017-11-27 Thread sumit-bose
  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

2017-11-27 Thread lslebodn
  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

2017-11-27 Thread simo5
  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

2017-11-27 Thread sumit-bose
  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

2017-11-27 Thread sumit-bose
   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 Bose 
Date: 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 +-