[SSSD] [sssd PR#248][comment] IPA: Improve s2n debug message for missing ipaNTSecurityIdentifier

2017-04-26 Thread justin-stephenson 
  URL: https://github.com/SSSD/sssd/pull/248
Title: #248: IPA: Improve s2n debug message for missing ipaNTSecurityIdentifier

justin-stephenson commented:
"""
I think the patch from **PR 242** is okay. This patch is inside the very large 
`ipa_s2n_save_objects` function containing a case statement performing 
conditional code based on the value of `attrs->response_type`. As I understand 
it, this can lead to `attrs->a.name` not being set in certain cases.
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/248#issuecomment-297547990
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#249][comment] DP: Reduce Data Provider log level noise

2017-04-26 Thread jhrozek 
  URL: https://github.com/SSSD/sssd/pull/249
Title: #249: DP: Reduce Data Provider log level noise

jhrozek commented:
"""
ok to test
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/249#issuecomment-297544662
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#248][comment] IPA: Improve s2n debug message for missing ipaNTSecurityIdentifier

2017-04-26 Thread jhrozek 
  URL: https://github.com/SSSD/sssd/pull/248
Title: #248: IPA: Improve s2n debug message for missing ipaNTSecurityIdentifier

jhrozek commented:
"""
By the way is the first patch where we added this message OK or does it need 
the same treatment as well?
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/248#issuecomment-297544535
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#249][comment] DP: Reduce Data Provider log level noise

2017-04-26 Thread centos-ci 
  URL: https://github.com/SSSD/sssd/pull/249
Title: #249: DP: Reduce Data Provider log level noise

centos-ci commented:
"""
Can one of the admins verify this patch?
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/249#issuecomment-297539057
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#249][opened] DP: Reduce Data Provider log level noise

2017-04-26 Thread justin-stephenson 
   URL: https://github.com/SSSD/sssd/pull/249
Author: justin-stephenson
 Title: #249: DP: Reduce Data Provider log level noise
Action: opened

PR body:
"""

Certain operations are not supported with certain providers
causing informational Data Provider log messages to be logged as
errors or failures. This patch lowers the log level to reduce overall
log noise and ensure only critical log messages are logged when
a low debug_level value is used.

Resolves:
https://pagure.io/SSSD/sssd/issue/3287
https://pagure.io/SSSD/sssd/issue/3278

Tested with the LDAP provider changing ``debug_level``
between 7 and 1 then checking for log messages:

* Before patch with **debug_level=1**
```
# egrep 'dp_find_method|dp_target_init|Data Provider returned' /var/log/sssd/*
[dp_target_init] (0x0020): Target [selinux] is not supported by module [ldap].
[dp_target_init] (0x0020): Target [hostid] is not supported by module [ldap].
[dp_target_init] (0x0020): Target [subdomains] is not supported by module 
[ldap].
[dp_find_method] (0x0020): Target [subdomains] is not initialized
[dp_find_method] (0x0020): Target [subdomains] is not initialized
[dp_find_method] (0x0020): Target [subdomains] is not initialized
[dp_find_method] (0x0020): Target [subdomains] is not initialized
[sss_dp_get_reply] (0x0010): The Data Provider returned an error 
[org.freedesktop.sssd.Error.DataProvider.NotSupported]
[sss_dp_get_reply] (0x0010): The Data Provider returned an error 
[org.freedesktop.sssd.Error.DataProvider.NotSupported]
[sss_dp_get_reply] (0x0010): The Data Provider returned an error 
[org.freedesktop.sssd.Error.DataProvider.NotSupported]
[sss_dp_get_reply] (0x0010): The Data Provider returned an error 
[org.freedesktop.sssd.Error.DataProvider.NotSupported]
```

* After patch with **debug_level=1**
```
# egrep 'dp_find_method|dp_target_init|Data Provider returned' /var/log/sssd/*
#

```
"""

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/249/head:pr249
git checkout pr249
From 2c27113fbe5af56340cd9487f83c68c5e0c55cc3 Mon Sep 17 00:00:00 2001
From: Justin Stephenson 
Date: Wed, 26 Apr 2017 15:45:33 -0400
Subject: [PATCH] DP: Reduce Data Provider log level noise

Certain operations are not supported with certain providers
causing informational Data Provider log messages to be logged as
errors or failures. This patch lowers the log level to reduce overall
log noise and ensure only critical log messages are logged when
a low debug_level value is used.

Resolves:
https://pagure.io/SSSD/sssd/issue/3287
https://pagure.io/SSSD/sssd/issue/3278
---
 src/providers/data_provider/dp_methods.c |  2 +-
 src/providers/data_provider/dp_targets.c |  2 +-
 src/responder/common/responder_dp.c  | 13 +++--
 3 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/src/providers/data_provider/dp_methods.c b/src/providers/data_provider/dp_methods.c
index 498676d..9e49c5f 100644
--- a/src/providers/data_provider/dp_methods.c
+++ b/src/providers/data_provider/dp_methods.c
@@ -109,7 +109,7 @@ errno_t dp_find_method(struct data_provider *provider,
 }
 
 if (!dp_target_initialized(provider->targets, target)) {
-DEBUG(SSSDBG_CRIT_FAILURE, "Target [%s] is not initialized\n",
+DEBUG(SSSDBG_CONF_SETTINGS, "Target [%s] is not initialized\n",
   dp_target_to_string(target));
 return ERR_MISSING_DP_TARGET;
 }
diff --git a/src/providers/data_provider/dp_targets.c b/src/providers/data_provider/dp_targets.c
index 26d20a8..e2a45bb 100644
--- a/src/providers/data_provider/dp_targets.c
+++ b/src/providers/data_provider/dp_targets.c
@@ -284,7 +284,7 @@ static errno_t dp_target_init(struct be_ctx *be_ctx,
 if (!target->explicitly_configured && (ret == ELIBBAD || ret == ENOTSUP)) {
 /* Target not found but it wasn't explicitly
  * configured so we shall just continue. */
-DEBUG(SSSDBG_CRIT_FAILURE, "Target [%s] is not supported by "
+DEBUG(SSSDBG_CONF_SETTINGS, "Target [%s] is not supported by "
   "module [%s].\n", target->name, target->module_name);
 ret = EOK;
 goto done;
diff --git a/src/responder/common/responder_dp.c b/src/responder/common/responder_dp.c
index 080f70f..d8021d1 100644
--- a/src/responder/common/responder_dp.c
+++ b/src/responder/common/responder_dp.c
@@ -218,8 +218,17 @@ static int sss_dp_get_reply(DBusPendingCall *pending,
 err = ETIME;
 goto done;
 }
-DEBUG(SSSDBG_FATAL_FAILURE,"The Data Provider returned an error [%s]\n",
- dbus_message_get_error_name(reply));
+
+if (strcmp(dbus_message_get_error_name(reply),
+   SBUS_ERROR_DP_NOTSUP) == 0) {
+   DEBUG(SSSDBG_MINOR_FAILURE,
+   "Error: Data Provider does not support this operation.\n");
+} else {
+DEBUG(SSSDBG_FATAL_FAILURE,
+"The Data Provider returned an error

[SSSD] [sssd PR#243][closed] IPA: Use search bases from sdap_domain instead of inferring search base from IPA domain structure

2017-04-26 Thread jhrozek 
   URL: https://github.com/SSSD/sssd/pull/243
Author: jhrozek
 Title: #243: IPA: Use search bases from sdap_domain instead of inferring 
search base from IPA domain structure
Action: closed

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/243/head:pr243
git checkout pr243
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#243][-Accepted] IPA: Use search bases from sdap_domain instead of inferring search base from IPA domain structure

2017-04-26 Thread jhrozek 
  URL: https://github.com/SSSD/sssd/pull/243
Title: #243: IPA: Use search bases from sdap_domain instead of inferring search 
base from IPA domain structure

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#243][+Pushed] IPA: Use search bases from sdap_domain instead of inferring search base from IPA domain structure

2017-04-26 Thread jhrozek 
  URL: https://github.com/SSSD/sssd/pull/243
Title: #243: IPA: Use search bases from sdap_domain instead of inferring search 
base from IPA domain structure

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#243][comment] IPA: Use search bases from sdap_domain instead of inferring search base from IPA domain structure

2017-04-26 Thread jhrozek 
  URL: https://github.com/SSSD/sssd/pull/243
Title: #243: IPA: Use search bases from sdap_domain instead of inferring search 
base from IPA domain structure

jhrozek commented:
"""
* master:
 * 337dd8a87cd774ac20d15c16ec3d9a6c4d2defc7
 * 53e9a5aef4a688f7c81a4a1e77013e05313e5f9a
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/243#issuecomment-297517954
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#235][+Pushed] Allow using the "shortnames" feature without requiring any configuration from the client side

2017-04-26 Thread jhrozek 
  URL: https://github.com/SSSD/sssd/pull/235
Title: #235: Allow using the "shortnames" feature without requiring any 
configuration from the client side

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#235][comment] Allow using the "shortnames" feature without requiring any configuration from the client side

2017-04-26 Thread jhrozek 
  URL: https://github.com/SSSD/sssd/pull/235
Title: #235: Allow using the "shortnames" feature without requiring any 
configuration from the client side

jhrozek commented:
"""
* b78febe4c579f86f8007a27599605d1eb9f97a62
* 213048fd9a5e800deb74cb5b7f0eaf465945c640
* f9bac02756aa05cc9c6ac07ae581dba67240c1a4
* dae798231fc2c575f213785768bc24ed765ba243
* ed518f61f1a5d4cf5d87eec492c158725a73d6a1
* a3faad0e4dc1ca4473746c3822ecfc5aed876e6d

"""

See the full comment at 
https://github.com/SSSD/sssd/pull/235#issuecomment-297515948
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#235][closed] Allow using the "shortnames" feature without requiring any configuration from the client side

2017-04-26 Thread jhrozek 
   URL: https://github.com/SSSD/sssd/pull/235
Author: fidencio
 Title: #235: Allow using the "shortnames" feature without requiring any 
configuration from the client side
Action: closed

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/235/head:pr235
git checkout pr235
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#235][-Accepted] Allow using the "shortnames" feature without requiring any configuration from the client side

2017-04-26 Thread jhrozek 
  URL: https://github.com/SSSD/sssd/pull/235
Title: #235: Allow using the "shortnames" feature without requiring any 
configuration from the client side

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#246][comment] filter_users and filter_groups stop working properly in v 1.15

2017-04-26 Thread fidencio 
  URL: https://github.com/SSSD/sssd/pull/246
Title: #246: filter_users and filter_groups stop working properly in v 1.15

fidencio commented:
"""
On Wed, Apr 26, 2017 at 6:34 PM, Fabiano Fidêncio 
wrote:

>
>
> On Wed, Apr 26, 2017 at 11:34 AM, Pavel Březina 
> wrote:
>
>>
>>-
>>
>>*NSS: Use fqnames when performing a ncache check*
>>This should be done in the ncache module, not in the callers.
>>
>>
> Well, I'm not exactly sure about this.
> Currently we the situation we have in the code is that we call ncache
> check using both fully qualified names and non fully qualified names ... so
> patching it in ncache module itself may end up causing more issues.
>
> Any input here is appreciated :-)
>

Nevermind, got it :-)


>
>
>>
>>-
>>-
>>
>>*RESPONDER: Make nss_get_name_from_msg() part of responder_utils*
>>Nikolai already did the same for his tlog integration and he need
>>this function also in providers. Can you cherry-pick this commit instead 
>> so
>>Nikolai doesn't have to solve conflicts?
>>7465d48
>>
>> 
>>-
>>
>>*CACHE_REQ: Add a new cache_req_ncache_filter_fn() plugin function*
>>
>>  /**+ * Filter the result through the negative cache.+ *+ * This is useful 
>> for plugins which don't use name as an input+ * takes but can be affected by 
>> filter_users and filter_groups ^ token+ * options.+ */
>> +typedef errno_t
>> +(*cache_req_ncache_filter_fn)(struct sss_nc_ctx *ncache,
>> +  struct sss_domain_info *domain,
>> +  char *name);
>>
>> * **CACHE_REQ: Make use of cache_req_ncache_filter_fn()**
>> ```c
>> static errno_t
>> cache_req_search_get_name_from_msg(TALLOC_CTX *mem_ctx,
>>struct ldb_message *msg,
>>struct sss_domain_info *domain,
>>bool override_space,
>>char **_name)
>> {
>> TALLOC_CTX *tmp_ctx;
>> const char *name;
>> char *output_name;
>> char *fqname;
>> errno_t ret;
>>
>> tmp_ctx = talloc_new(NULL);
>> if (tmp_ctx == NULL) {
>> return ENOMEM;
>> }
>>
>> name = sss_get_name_from_msg(domain, msg);
>>
>> ^^^ name can be NULL and we don't want to fail with EINVAL in this case
>>
>>
@pbrezina ...

What do we want to do in this case? It would fail with EINVAL later on when
filling the grent/pwent anyways, no?


> output_name = sss_output_name(tmp_ctx, name, domain->case_preserve,   
>override_space);if (output_name == NULL) { 
>DEBUG(SSSDBG_CRIT_FAILURE, "sss_output_name() failed\n");ret = 
> ENOMEM;goto done;}fqname = 
> sss_create_internal_fqname(tmp_ctx, output_name, domain->name);if (fqname 
> == NULL) {DEBUG(SSSDBG_CRIT_FAILURE, "sss_create_internal_fqname() 
> failed\n");ret = ENOMEM;goto done;}*_name = 
> talloc_steal(mem_ctx, fqname);ret = EOK;done:talloc_free(tmp_ctx);
> return ret;}
>>
>> Besides these small things, the approach you've taken is good. I would
>> just like you to do more thing -- move code that deals with creating the
>> new result into separate function(s) into cache_req_result.c and enable
>> this also for enumeration so you can remove the check (and the first patch)
>> from nss responder.
>>
>
And last comment ... the introduced functions are not dealing with
cache_req_result at all. What they do is changing the ldb_result and moving
this part of the code to the cache_req_result.c does seem a little bit
weird to me.

—
>> You are receiving this because you authored the thread.
>> Reply to this email directly, view it on GitHub
>> , or mute
>> the thread
>> 
>> .
>>
>
>
Best Regards,

"""

See the full comment at 
https://github.com/SSSD/sssd/pull/246#issuecomment-297515332
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#246][comment] filter_users and filter_groups stop working properly in v 1.15

2017-04-26 Thread fidencio 
  URL: https://github.com/SSSD/sssd/pull/246
Title: #246: filter_users and filter_groups stop working properly in v 1.15

fidencio commented:
"""
On Wed, Apr 26, 2017 at 11:34 AM, Pavel Březina 
wrote:

>
>-
>
>*NSS: Use fqnames when performing a ncache check*
>This should be done in the ncache module, not in the callers.
>
>
Well, I'm not exactly sure about this.
Currently we the situation we have in the code is that we call ncache check
using both fully qualified names and non fully qualified names ... so
patching it in ncache module itself may end up causing more issues.

Any input here is appreciated :-)


>
>-
>-
>
>*RESPONDER: Make nss_get_name_from_msg() part of responder_utils*
>Nikolai already did the same for his tlog integration and he need this
>function also in providers. Can you cherry-pick this commit instead so
>Nikolai doesn't have to solve conflicts?
>7465d48
>
> 
>-
>
>*CACHE_REQ: Add a new cache_req_ncache_filter_fn() plugin function*
>
>  /**+ * Filter the result through the negative cache.+ *+ * This is useful 
> for plugins which don't use name as an input+ * takes but can be affected by 
> filter_users and filter_groups ^ token+ * options.+ */
> +typedef errno_t
> +(*cache_req_ncache_filter_fn)(struct sss_nc_ctx *ncache,
> +  struct sss_domain_info *domain,
> +  char *name);
>
> * **CACHE_REQ: Make use of cache_req_ncache_filter_fn()**
> ```c
> static errno_t
> cache_req_search_get_name_from_msg(TALLOC_CTX *mem_ctx,
>struct ldb_message *msg,
>struct sss_domain_info *domain,
>bool override_space,
>char **_name)
> {
> TALLOC_CTX *tmp_ctx;
> const char *name;
> char *output_name;
> char *fqname;
> errno_t ret;
>
> tmp_ctx = talloc_new(NULL);
> if (tmp_ctx == NULL) {
> return ENOMEM;
> }
>
> name = sss_get_name_from_msg(domain, msg);
>
> ^^^ name can be NULL and we don't want to fail with EINVAL in this case
> output_name = sss_output_name(tmp_ctx, name, domain->case_preserve,   
>override_space);if (output_name == NULL) {
> DEBUG(SSSDBG_CRIT_FAILURE, "sss_output_name() failed\n");ret = 
> ENOMEM;goto done;}fqname = 
> sss_create_internal_fqname(tmp_ctx, output_name, domain->name);if (fqname 
> == NULL) {DEBUG(SSSDBG_CRIT_FAILURE, "sss_create_internal_fqname() 
> failed\n");ret = ENOMEM;goto done;}*_name = 
> talloc_steal(mem_ctx, fqname);ret = EOK;done:talloc_free(tmp_ctx);
> return ret;}
>
> Besides these small things, the approach you've taken is good. I would
> just like you to do more thing -- move code that deals with creating the
> new result into separate function(s) into cache_req_result.c and enable
> this also for enumeration so you can remove the check (and the first patch)
> from nss responder.
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> , or mute
> the thread
> 
> .
>

"""

See the full comment at 
https://github.com/SSSD/sssd/pull/246#issuecomment-297468578
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#248][comment] IPA: Improve s2n debug message for missing ipaNTSecurityIdentifier

2017-04-26 Thread justin-stephenson 
  URL: https://github.com/SSSD/sssd/pull/248
Title: #248: IPA: Improve s2n debug message for missing ipaNTSecurityIdentifier

justin-stephenson commented:
"""
Thank you for the review, patch updated.
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/248#issuecomment-297458904
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#248][synchronized] IPA: Improve s2n debug message for missing ipaNTSecurityIdentifier

2017-04-26 Thread justin-stephenson 
   URL: https://github.com/SSSD/sssd/pull/248
Author: justin-stephenson
 Title: #248: IPA: Improve s2n debug message for missing ipaNTSecurityIdentifier
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/248/head:pr248
git checkout pr248
From fb178cd4d7df1ad95ee35805df3d87f73b5b9267 Mon Sep 17 00:00:00 2001
From: Justin Stephenson 
Date: Tue, 25 Apr 2017 13:02:10 -0400
Subject: [PATCH] IPA: Improve s2n debug message for missing
 ipaNTSecurityIdentifier

This patch improves the log message to be more information for
the SSSD user troubleshooting issues.

If the IDM POSIX group used for AD trust HBAC/SUDO operation is missing
the ipaNTSecurityIdentifier it can cause client s2n operations failures
resolving the group which resulted in the inability to login for the AD
user.
---
 src/providers/ipa/ipa_s2n_exop.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/providers/ipa/ipa_s2n_exop.c b/src/providers/ipa/ipa_s2n_exop.c
index 55ec904..f5f4401 100644
--- a/src/providers/ipa/ipa_s2n_exop.c
+++ b/src/providers/ipa/ipa_s2n_exop.c
@@ -2580,7 +2580,13 @@ static errno_t ipa_s2n_save_objects(struct sss_domain_info *dom,
 ret = sysdb_attrs_get_string(attrs->sysdb_attrs, SYSDB_SID_STR, _str);
 if (ret != EOK) {
 DEBUG(SSSDBG_CRIT_FAILURE,
-  "Cannot find SID of object with override.\n");
+  "Cannot find SID of object.\n");
+if (name != NULL) {
+DEBUG(SSSDBG_CRIT_FAILURE,
+  "Object [%s] has no SID, please check the "
+  "ipaNTSecurityIdentifier attribute on the server-side.\n",
+  name);
+}
 goto done;
 }
 
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#235][comment] Allow using the "shortnames" feature without requiring any configuration from the client side

2017-04-26 Thread fidencio 
  URL: https://github.com/SSSD/sssd/pull/235
Title: #235: Allow using the "shortnames" feature without requiring any 
configuration from the client side

fidencio commented:
"""
Added the "Accepted" label according to @pbrezina's ACK,
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/235#issuecomment-297432010
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#235][+Accepted] Allow using the "shortnames" feature without requiring any configuration from the client side

2017-04-26 Thread fidencio 
  URL: https://github.com/SSSD/sssd/pull/235
Title: #235: Allow using the "shortnames" feature without requiring any 
configuration from the client side

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#136][comment] Tlog integration

2017-04-26 Thread spbnick 
  URL: https://github.com/SSSD/sssd/pull/136
Title: #136: Tlog integration

spbnick commented:
"""
Alright, I'll work on the manpages as well.

Another thing: should we also make a configure option for enabling session 
recording, so that we can have this enabled and have an RPM dependency in 
Fedora, where tlog is already available, and disabled in RHEL and everywhere 
else, until it is available there?
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/136#issuecomment-297404214
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#246][+Changes requested] filter_users and filter_groups stop working properly in v 1.15

2017-04-26 Thread fidencio 
  URL: https://github.com/SSSD/sssd/pull/246
Title: #246: filter_users and filter_groups stop working properly in v 1.15

Label: +Changes requested
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#246][comment] filter_users and filter_groups stop working properly in v 1.15

2017-04-26 Thread fidencio 
  URL: https://github.com/SSSD/sssd/pull/246
Title: #246: filter_users and filter_groups stop working properly in v 1.15

fidencio commented:
"""
On Wed, Apr 26, 2017 at 11:34 AM, Pavel Březina 
wrote:

>
>-
>
>*NSS: Use fqnames when performing a ncache check*
>This should be done in the ncache module, not in the callers.
>
>
Okay.

>
>-
>
>*RESPONDER: Make nss_get_name_from_msg() part of responder_utils*
>Nikolai already did the same for his tlog integration and he need this
>function also in providers. Can you cherry-pick this commit instead so
>Nikolai doesn't have to solve conflicts?
>7465d48
>
> 
>
>
Okay, but I really would prefer to use have the function names as
sss_get_name_from_msg instead of sss_nss_get_name_from_msg.
I'll sync with Nikolai about that.



>
>-
>-
>
>*CACHE_REQ: Add a new cache_req_ncache_filter_fn() plugin function*
>
>  /**+ * Filter the result through the negative cache.+ *+ * This is useful 
> for plugins which don't use name as an input+ * takes but can be affected by 
> filter_users and filter_groups ^ token+ * options.+ */
> +typedef errno_t
> +(*cache_req_ncache_filter_fn)(struct sss_nc_ctx *ncache,
> +  struct sss_domain_info *domain,
> +  char *name);
>
> * **CACHE_REQ: Make use of cache_req_ncache_filter_fn()**
> ```c
> static errno_t
> cache_req_search_get_name_from_msg(TALLOC_CTX *mem_ctx,
>struct ldb_message *msg,
>struct sss_domain_info *domain,
>bool override_space,
>char **_name)
> {
> TALLOC_CTX *tmp_ctx;
> const char *name;
> char *output_name;
> char *fqname;
> errno_t ret;
>
> tmp_ctx = talloc_new(NULL);
> if (tmp_ctx == NULL) {
> return ENOMEM;
> }
>
> name = sss_get_name_from_msg(domain, msg);
>
> ^^^ name can be NULL and we don't want to fail with EINVAL in this case
>
>
Sure!


> output_name = sss_output_name(tmp_ctx, name, domain->case_preserve,   
>override_space);if (output_name == NULL) { 
>DEBUG(SSSDBG_CRIT_FAILURE, "sss_output_name() failed\n");ret = 
> ENOMEM;goto done;}fqname = 
> sss_create_internal_fqname(tmp_ctx, output_name, domain->name);if (fqname 
> == NULL) {DEBUG(SSSDBG_CRIT_FAILURE, "sss_create_internal_fqname() 
> failed\n");ret = ENOMEM;goto done;}*_name = 
> talloc_steal(mem_ctx, fqname);ret = EOK;done:talloc_free(tmp_ctx);
> return ret;}
>
> Besides these small things, the approach you've taken is good. I would
> just like you to do more thing -- move code that deals with creating the
> new result into separate function(s) into cache_req_result.c and enable
> this also for enumeration so you can remove the check (and the first patch)
> from nss responder.
>

Okay, super!

Thanks for the review!


> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> , or mute
> the thread
> 
> .
>

"""

See the full comment at 
https://github.com/SSSD/sssd/pull/246#issuecomment-297336143
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#246][comment] filter_users and filter_groups stop working properly in v 1.15

2017-04-26 Thread pbrezina 
  URL: https://github.com/SSSD/sssd/pull/246
Title: #246: filter_users and filter_groups stop working properly in v 1.15

pbrezina commented:
"""
* **NSS: Use fqnames when performing a ncache check**
This should be done in the ncache module, not in the callers.

* **RESPONDER: Make nss_get_name_from_msg() part of responder_utils**
Nikolai already did the same for his `tlog` integration and he need this 
function also in providers. Can you cherry-pick this commit instead so Nikolai 
doesn't have to solve conflicts?
https://github.com/SSSD/sssd/pull/136/commits/7465d487ef52fd1cc704e2e67c62d427143f0af1

* **CACHE_REQ: Add a new cache_req_ncache_filter_fn() plugin function**
```c
 /**
+ * Filter the result through the negative cache.
+ *
+ * This is useful for plugins which don't use name as an input
+ * takes but can be affected by filter_users and filter_groups
 ^ token
+ * options.
+ */
+typedef errno_t
+(*cache_req_ncache_filter_fn)(struct sss_nc_ctx *ncache,
+  struct sss_domain_info *domain,
+  char *name);

* **CACHE_REQ: Make use of cache_req_ncache_filter_fn()**
```c
static errno_t
cache_req_search_get_name_from_msg(TALLOC_CTX *mem_ctx,
   struct ldb_message *msg,
   struct sss_domain_info *domain,
   bool override_space,
   char **_name)
{
TALLOC_CTX *tmp_ctx;
const char *name;
char *output_name;
char *fqname;
errno_t ret;

tmp_ctx = talloc_new(NULL);
if (tmp_ctx == NULL) {
return ENOMEM;
}

name = sss_get_name_from_msg(domain, msg);

^^^ name can be NULL and we don't want to fail with EINVAL in this case

output_name = sss_output_name(tmp_ctx, name, domain->case_preserve,
  override_space);
if (output_name == NULL) {
DEBUG(SSSDBG_CRIT_FAILURE, "sss_output_name() failed\n");
ret = ENOMEM;
goto done;
}

fqname = sss_create_internal_fqname(tmp_ctx, output_name, domain->name);
if (fqname == NULL) {
DEBUG(SSSDBG_CRIT_FAILURE, "sss_create_internal_fqname() failed\n");
ret = ENOMEM;
goto done;
}

*_name = talloc_steal(mem_ctx, fqname);
ret = EOK;

done:
talloc_free(tmp_ctx);
return ret;
}
```

Besides these small things, the approach you've taken is good. I would just 
like you to do more thing -- move code that deals with creating the new result 
into separate function(s) into `cache_req_result.c`  and enable this also for 
enumeration so you can remove the check (and the first patch) from `nss` 
responder.
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/246#issuecomment-297319252
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#246][comment] filter_users and filter_groups stop working properly in v 1.15

2017-04-26 Thread pbrezina 
  URL: https://github.com/SSSD/sssd/pull/246
Title: #246: filter_users and filter_groups stop working properly in v 1.15

pbrezina commented:
"""
* **NSS: Use fqnames when performing a ncache check**
This should be done in the ncache module, not in the callers.

* **RESPONDER: Make nss_get_name_from_msg() part of responder_utils**
Nikolai already did the same for his `tlog` integration and he need this 
function also in providers. Can you cherry-pick this commit instead so Nikolai 
doesn't have to solve conflicts?
https://github.com/SSSD/sssd/pull/136/commits/7465d487ef52fd1cc704e2e67c62d427143f0af1

* **CACHE_REQ: Add a new cache_req_ncache_filter_fn() plugin function**
```c
 /**
+ * Filter the result through the negative cache.
+ *
+ * This is useful for plugins which don't use name as an input
+ * takes but can be affected by filter_users and filter_groups
 ^ token
+ * options.
+ */
+typedef errno_t
+(*cache_req_ncache_filter_fn)(struct sss_nc_ctx *ncache,
+  struct sss_domain_info *domain,
+  char *name);

* **CACHE_REQ: Make use of cache_req_ncache_filter_fn()**
```c
static errno_t
cache_req_search_get_name_from_msg(TALLOC_CTX *mem_ctx,
   struct ldb_message *msg,
   struct sss_domain_info *domain,
   bool override_space,
   char **_name)
{
TALLOC_CTX *tmp_ctx;
const char *name;
char *output_name;
char *fqname;
errno_t ret;

tmp_ctx = talloc_new(NULL);
if (tmp_ctx == NULL) {
return ENOMEM;
}

name = sss_get_name_from_msg(domain, msg);

^^^ name can be NULL and we don't want to fail with EINVAL in this case

output_name = sss_output_name(tmp_ctx, name, domain->case_preserve,
  override_space);
if (output_name == NULL) {
DEBUG(SSSDBG_CRIT_FAILURE, "sss_output_name() failed\n");
ret = ENOMEM;
goto done;
}

fqname = sss_create_internal_fqname(tmp_ctx, output_name, domain->name);
if (fqname == NULL) {
DEBUG(SSSDBG_CRIT_FAILURE, "sss_create_internal_fqname() failed\n");
ret = ENOMEM;
goto done;
}

*_name = talloc_steal(mem_ctx, fqname);
ret = EOK;

done:
talloc_free(tmp_ctx);
return ret;
}
```

Besides these small things, the approach you've taken is good. I would just 
like you to do more thing -- move code that deals with creating the new result 
into separate function(s) into `cache_req_result.c`  and enable this also for 
enumeration so you can remove the check (and the first patch) from `nss` 
responder.
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/246#issuecomment-297319252
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#248][comment] IPA: Improve s2n debug message for missing ipaNTSecurityIdentifier

2017-04-26 Thread pbrezina 
  URL: https://github.com/SSSD/sssd/pull/248
Title: #248: IPA: Improve s2n debug message for missing ipaNTSecurityIdentifier

pbrezina commented:
"""
Using just local variable `name` seems to be a better idea, but please check if 
it is always initialized.
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/248#issuecomment-297313544
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#248][comment] IPA: Improve s2n debug message for missing ipaNTSecurityIdentifier

2017-04-26 Thread pbrezina 
  URL: https://github.com/SSSD/sssd/pull/248
Title: #248: IPA: Improve s2n debug message for missing ipaNTSecurityIdentifier

pbrezina commented:
"""
Nack. `attrs->a.name` is set only when `attrs->response_type == RESP_NAME` 
which is not always true here.
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/248#issuecomment-297312715
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#248][comment] IPA: Improve s2n debug message for missing ipaNTSecurityIdentifier

2017-04-26 Thread pbrezina 
  URL: https://github.com/SSSD/sssd/pull/248
Title: #248: IPA: Improve s2n debug message for missing ipaNTSecurityIdentifier

pbrezina commented:
"""
ok to test
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/248#issuecomment-297310590
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#136][comment] Tlog integration

2017-04-26 Thread pbrezina 
  URL: https://github.com/SSSD/sssd/pull/136
Title: #136: Tlog integration

pbrezina commented:
"""
Yes, that is a good idea. And although the configuration itself is rather 
simple, it may be a good thing to also create sssd-tlog or 
sssd-session-recording manpage with few paragraphs describing the future and 
how to use it in more details just to promote it.
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/136#issuecomment-297309335
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#235][comment] Allow using the "shortnames" feature without requiring any configuration from the client side

2017-04-26 Thread pbrezina 
  URL: https://github.com/SSSD/sssd/pull/235
Title: #235: Allow using the "shortnames" feature without requiring any 
configuration from the client side

pbrezina commented:
"""
Ack. Thank you for your patience.
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/235#issuecomment-297304243
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#246][synchronized] filter_users and filter_groups stop working properly in v 1.15

2017-04-26 Thread fidencio 
   URL: https://github.com/SSSD/sssd/pull/246
Author: fidencio
 Title: #246: filter_users and filter_groups stop working properly in v 1.15
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/246/head:pr246
git checkout pr246
From 6a6c9866301ed6bd7b35f5799e6c058ac81eea15 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= 
Date: Mon, 24 Apr 2017 12:11:46 +0200
Subject: [PATCH 1/5] NSS: Use fqnames when performing a ncache check
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The names stored in the negative cache are fully qualified, so we have
to use fully qualified names when checking whether a user/group is part
of negative cache or not.

Related:
https://pagure.io/SSSD/sssd/issue/3362

Signed-off-by: Fabiano Fidêncio 
---
 src/responder/nss/nss_protocol_grent.c | 12 +++-
 src/responder/nss/nss_protocol_pwent.c | 12 +++-
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/src/responder/nss/nss_protocol_grent.c b/src/responder/nss/nss_protocol_grent.c
index fae1d47..6f6ad9e 100644
--- a/src/responder/nss/nss_protocol_grent.c
+++ b/src/responder/nss/nss_protocol_grent.c
@@ -205,6 +205,7 @@ nss_protocol_fill_grent(struct nss_ctx *nss_ctx,
 uint32_t num_results;
 uint32_t num_members;
 char *members;
+char *fqname;
 size_t members_size;
 size_t rp;
 size_t rp_members;
@@ -243,8 +244,17 @@ nss_protocol_fill_grent(struct nss_ctx *nss_ctx,
 
 /* Check negative cache during enumeration. */
 if (cmd_ctx->enumeration) {
+fqname = sss_create_internal_fqname(tmp_ctx, name->str,
+result->domain->name);
+if (fqname == NULL) {
+DEBUG(SSSDBG_OP_FAILURE,
+  "sss_create_internal_fqname() failed\n");
+ret = ENOMEM;
+goto done;
+}
+
 ret = sss_ncache_check_group(nss_ctx->rctx->ncache,
- result->domain, name->str);
+ result->domain, fqname);
 if (ret == EEXIST) {
 DEBUG(SSSDBG_TRACE_FUNC,
   "User [%s] filtered out! (negative cache)\n",
diff --git a/src/responder/nss/nss_protocol_pwent.c b/src/responder/nss/nss_protocol_pwent.c
index edda9d3..e781352 100644
--- a/src/responder/nss/nss_protocol_pwent.c
+++ b/src/responder/nss/nss_protocol_pwent.c
@@ -273,6 +273,7 @@ nss_protocol_fill_pwent(struct nss_ctx *nss_ctx,
 struct sized_string gecos;
 struct sized_string homedir;
 struct sized_string shell;
+char *fqname;
 uint32_t gid;
 uint32_t uid;
 uint32_t num_results;
@@ -311,8 +312,17 @@ nss_protocol_fill_pwent(struct nss_ctx *nss_ctx,
 
 /* Check negative cache during enumeration. */
 if (cmd_ctx->enumeration) {
+fqname = sss_create_internal_fqname(tmp_ctx, name->str,
+result->domain->name);
+if (fqname == NULL) {
+DEBUG(SSSDBG_OP_FAILURE,
+  "sss_create_internal_fqname() failed\n");
+ret = ENOMEM;
+goto done;
+}
+
 ret = sss_ncache_check_user(nss_ctx->rctx->ncache,
-result->domain, name->str);
+result->domain, fqname);
 if (ret == EEXIST) {
 DEBUG(SSSDBG_TRACE_FUNC,
   "User [%s] filtered out! (negative cache)\n", name->str);

From ff661ea6ecbcd93deb7dc9d039f6bb3be4f41c1f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= 
Date: Tue, 25 Apr 2017 16:41:56 +0200
Subject: [PATCH 2/5] RESPONDER: Make nss_get_name_from_msg() part of
 responder_utils
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

In order to do so, the function got renamed to sss_get_name_from_msg()
and will be used as part of cache_req as well in the follow up patches.

Related:
https://pagure.io/SSSD/sssd/issue/3362

Signed-off-by: Fabiano Fidêncio 
---
 src/responder/common/responder.h   |  3 +++
 src/responder/common/responder_utils.c | 26 ++
 src/responder/nss/nss_private.h|  4 
 src/responder/nss/nss_protocol_grent.c |  2 +-
 src/responder/nss/nss_protocol_pwent.c |  2 +-
 src/responder/nss/nss_protocol_sid.c   |  2 +-
 src/responder/nss/nss_utils.c  | 27 ---
 src/tests/cwrap/Makefile.am|  1 +
 8 files changed, 33 insertions(+), 34 deletions(-)

diff --git a/src/responder/common/responder.h b/src/responder/common/responder.h
index dfe1ec4..e5b9c5b 100644
--- a/src/responder/common/responder.h
+++