[SSSD] Re: [PATCH SET] AD_PROVIDER: ad_enabled_domains

[Petr Cech]

Sorry, I experienced some issue with mailing list.
So I send it again.

 Forwarded Message 
Subject: Re: [SSSD] Re: [PATCH SET] AD_PROVIDER: ad_enabled_domains
Date: Tue, 9 Aug 2016 17:29:38 +0200
From: Petr Cech 
To: sssd-devel@lists.fedorahosted.org

On 08/09/2016 11:07 AM, Jakub Hrozek wrote:

On Mon, Jul 25, 2016 at 06:18:28PM +0200, Petr Cech wrote:

> Hello,
>
> there is fixed patch set attached.
>
> Segmentation fault was caused by wrong pointer :-(, sorry.
>
> This new patch set has new debug message. I am open to dissccus the
> debug_level and content of message. Any improving idea?
>
> I hit one issue during testing -- sometimes if I am connected to subdomain
> and I enable only sibling subdomain (the master is added automaticaly) and
> forest root is not enabled -- I see only master and sibling not. But if I
> added sleep for cycle (for using dbg) to function ad_subdomains_init()
> everythink is OK.
> Any idea?

Can you test that case with valgrind? This sounds like some uninitilized
variable condition.



I didn't run valgrind but I have new information.

If you clear the cache and reset sssd, first attempt to obtain 
information about user from sibling domain fails. The second and the 
other attempts runs correctly.


I see that the sibling domain is enabled. But if I look more carefully 
there is message in log (gamma.domain.bootes is sibling domain):


[sssd[be[beta.domain.bootes]]] [dp_req_new] (0x0020): Unknown domain: 
gamma.domain.bootes


First attempt should works too but you should wait nearly exactly 6 
seconds after restart sssd.


New patch set is attached.



> Anyway, I would like to ask you for testing.
>
> Regards
>
> --
> Petr^4 Čech
> From 7fd9ad8f42f274463c1d107b195d21290fd0b0f2 Mon Sep 17 00:00:00 2001
> From: Petr Cech 
> Date: Fri, 13 May 2016 05:21:07 -0400
> Subject: [PATCH 1/5] AD_PROVIDER: Add ad_enabled_domains option

ACK


Thanks.


> From dfa6e063d3ef4850a7809e2f5a6d3826ea061b27 Mon Sep 17 00:00:00 2001
> From: Petr Cech 
> Date: Tue, 21 Jun 2016 08:34:15 +0200
> Subject: [PATCH 2/5] AD_PROVIDER: Initializing of ad_enabled_domains
>
> We add ad_enabled_domains into ad_subdomains_ctx.
>
> Resolves:
> https://fedorahosted.org/sssd/ticket/2828
> ---
>  src/providers/ad/ad_subdomains.c | 82 

>  1 file changed, 82 insertions(+)
>
> diff --git a/src/providers/ad/ad_subdomains.c 
b/src/providers/ad/ad_subdomains.c
> index 
e9da04e384e598927f9c8c203a751bcccd29e895..9c0afb19418e44a3e3daa661baf1c7a82439d60d 
100644
> --- a/src/providers/ad/ad_subdomains.c
> +++ b/src/providers/ad/ad_subdomains.c
> @@ -57,6 +57,79 @@
>  /* do not refresh more often than every 5 seconds for now */
>  #define AD_SUBDOMAIN_REFRESH_LIMIT 5
>
> +static errno_t ad_get_enabled_domains(TALLOC_CTX *mem_ctx,
> +  struct ad_id_ctx *ad_id_ctx,
> +  const char *ad_domain,
> +  const char ***_ad_enabled_domains)
> +{
> +int ret;
> +const char *str;
> +const char *option_name;
> +const char **domains = NULL;
> +int count;
> +bool is_ad_in_domains;
> +TALLOC_CTX *tmp_ctx = NULL;
> +
> +tmp_ctx = talloc_new(NULL);
> +if (tmp_ctx == NULL) {
> +return ENOMEM;
> +}
> +
> +str = dp_opt_get_cstring(ad_id_ctx->ad_options->basic, 
AD_ENABLED_DOMAINS);
> +if (str == NULL) {
> +_ad_enabled_domains = NULL;

I think you wanted to dereference the pointer here? (it might also be a
good idea to return EINVAL if the caller passed NULL as the output pointer)


Right, thanks, addressed.


> +ret = EOK;
> +goto done;
> +}
> +
> From 4cd5c77f09750f9eb20c91eadc4759c3e4166093 Mon Sep 17 00:00:00 2001
> From: Petr Cech 
> Date: Tue, 21 Jun 2016 09:48:52 +0200
> Subject: [PATCH 3/5] AD_PROVIDER: ad_enabled_domains - only master
>
> We can skip looking up other domains if option ad_enabled_domains
> contains only master domain.
>
> Resolves:
> https://fedorahosted.org/sssd/ticket/2828
> ---
>  src/providers/ad/ad_subdomains.c | 16 
>  1 file changed, 16 insertions(+)
>
> diff --git a/src/providers/ad/ad_subdomains.c 
b/src/providers/ad/ad_subdomains.c
> index 
9c0afb19418e44a3e3daa661baf1c7a82439d60d..eaa85f802dbb4022dc632cc4c05d89685eccd901 
100644
> --- a/src/providers/ad/ad_subdomains.c
> +++ b/src/providers/ad/ad_subdomains.c
> @@ -1163,6 +1163,7 @@ static void ad_subdomains_refresh_connect_done(struct 
tevent_req *subreq)
>  return;
>  }
>
> +/* connect to the DC we are a member of */
>  subreq = ad_master_domain_send(state, state->ev, state->id_ctx->conn,
> state->sdap_op, 
state->sd_ctx->domain_name);
>  if (subreq == NULL) {
> @@ -1211,6 +1212,21 @@ static void ad_subdomains_refresh_master_done(struct 
tevent_req *subreq)
>  goto done;
>  }
>
> +/*
> + * If ad_enabled_domains contains

[SSSD] Re: [PATCH] dyndns-tests: Fix false positive failures

[Petr Cech]

On 08/10/2016 08:15 AM, Lukas Slebodnik wrote:

On (10/08/16 08:11), Lukas Slebodnik wrote:

On (10/08/16 08:09), Lukas Slebodnik wrote:

ehlo,

yesterdat, I build a sssd in copr for various distributions
and the dyndns-tests failed for me few time.
We had a similar failures also in CI
http://sssd-ci.duckdns.org/logs/job/50/54/fedora_rawhide/ci-build-coverage/dyndns-tests.log

Attached patch should fix it.


And now with a subject :-)


And hopefully, the last version without debug information used
for troubleshooting.

LS


Hello Lukas,

it looks good to me. I will wait for CI.

Regards

--
Petr^4 Čech
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] [PATCH] config_schema: Add ldap_user_email to schema

[Lukas Slebodnik]

ehlo,

yet another oneliner.
It reminds me that we should generate either schema or
data for python test.

LS
>From 1091e34d4d44ca0587c776769769ae05ffa673b0 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik 
Date: Wed, 10 Aug 2016 08:17:02 +0200
Subject: [PATCH] config_schema: Add ldap_user_email to schema

Resolves:
https://fedorahosted.org/sssd/ticket/3068
---
 src/config/cfg_rules.ini | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/config/cfg_rules.ini b/src/config/cfg_rules.ini
index 
635c078436e8ca47f60e8d82341cb131469fe4c9..09f53fa41eb2904f11a78af333b6d79619d2759c
 100644
--- a/src/config/cfg_rules.ini
+++ b/src/config/cfg_rules.ini
@@ -588,6 +588,7 @@ option = ldap_user_authorized_host
 option = ldap_user_authorized_service
 option = ldap_user_auth_type
 option = ldap_user_certificate
+option = ldap_user_email
 option = ldap_user_entry_usn
 option = ldap_user_extra_attrs
 option = ldap_user_fullname
-- 
2.9.2

___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] dyndns-tests: Fix false positive failures

[Lukas Slebodnik]

On (10/08/16 08:11), Lukas Slebodnik wrote:
>On (10/08/16 08:09), Lukas Slebodnik wrote:
>>ehlo,
>>
>>yesterdat, I build a sssd in copr for various distributions
>>and the dyndns-tests failed for me few time.
>>We had a similar failures also in CI
>>http://sssd-ci.duckdns.org/logs/job/50/54/fedora_rawhide/ci-build-coverage/dyndns-tests.log
>>
>>Attached patch should fix it.
>>
>And now with a subject :-)
>
And hopefully, the last version without debug information used
for troubleshooting.

LS
>From 0cfba20a97d034d75f174f83ad44fd4883e1f5b2 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik 
Date: Wed, 10 Aug 2016 07:44:28 +0200
Subject: [PATCH] dyndns-tests: Fix false positive failures

The child process finished faster then it has handled by parent
and therefore it timed out. It's the similar solution as in
b3074dca3acebd91437ef13d3329d6d65d655215

[ RUN  ] dyndns_test_error
(Fri Jul 29 16:12:00:621444 2016) [sssd] [nsupdate_child_timeout] (0x0020):
  Timeout reached for dynamic DNS update
Could not run the test - check test fixtures
[  ERROR   ] dyndns_test_error
---
 src/tests/cmocka/test_dyndns.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/tests/cmocka/test_dyndns.c b/src/tests/cmocka/test_dyndns.c
index 
b8404ef1018c1ec9720e506807bcf3f3df379a2d..fafd4d8a595cc608447d4068eeb0cee2ba819d89
 100644
--- a/src/tests/cmocka/test_dyndns.c
+++ b/src/tests/cmocka/test_dyndns.c
@@ -75,6 +75,7 @@ void __wrap_execv(const char *path, char *const argv[])
 case MOCK_NSUPDATE_ERR:
 DEBUG(SSSDBG_FUNC_DATA, "nsupdate error test case\n");
 err = 1;
+usleep(5); /* 50 miliseconds */
 break;
 case MOCK_NSUPDATE_TIMEOUT:
 DEBUG(SSSDBG_FUNC_DATA, "nsupdate timeout test case\n");
-- 
2.9.2

___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] [PATCH] dyndns-tests: Fix false positive failures

[Lukas Slebodnik]

On (10/08/16 08:09), Lukas Slebodnik wrote:
>ehlo,
>
>yesterdat, I build a sssd in copr for various distributions
>and the dyndns-tests failed for me few time.
>We had a similar failures also in CI
>http://sssd-ci.duckdns.org/logs/job/50/54/fedora_rawhide/ci-build-coverage/dyndns-tests.log
>
>Attached patch should fix it.
>
And now with a subject :-)

LS
>From 041010427e44762226e96a5d9cf4ce88af253134 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik 
Date: Wed, 10 Aug 2016 07:44:28 +0200
Subject: [PATCH] dyndns-tests: Fix false positive failures

The child process finished faster then it has handled by parent
and therefore it timed out. It's the similar solution as in
b3074dca3acebd91437ef13d3329d6d65d655215

[ RUN  ] dyndns_test_error
(Fri Jul 29 16:12:00:621444 2016) [sssd] [nsupdate_child_timeout] (0x0020):
  Timeout reached for dynamic DNS update
Could not run the test - check test fixtures
[  ERROR   ] dyndns_test_error
---
 src/tests/cmocka/test_dyndns.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/tests/cmocka/test_dyndns.c b/src/tests/cmocka/test_dyndns.c
index 
b8404ef1018c1ec9720e506807bcf3f3df379a2d..4f8706545cdb41df0d92e45e92d3279c63909695
 100644
--- a/src/tests/cmocka/test_dyndns.c
+++ b/src/tests/cmocka/test_dyndns.c
@@ -75,6 +75,7 @@ void __wrap_execv(const char *path, char *const argv[])
 case MOCK_NSUPDATE_ERR:
 DEBUG(SSSDBG_FUNC_DATA, "nsupdate error test case\n");
 err = 1;
+usleep(5); /* 50 miliseconds */
 break;
 case MOCK_NSUPDATE_TIMEOUT:
 DEBUG(SSSDBG_FUNC_DATA, "nsupdate timeout test case\n");
@@ -1059,7 +1060,7 @@ int main(int argc, const char *argv[])
 }
 }
 poptFreeContext(pc);
-
+debug_level = 8;
 DEBUG_CLI_INIT(debug_level);
 
 /* Even though normally the tests should clean up after themselves
-- 
2.9.2

___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] (no subject)

[Lukas Slebodnik]

ehlo,

yesterdat, I build a sssd in copr for various distributions
and the dyndns-tests failed for me few time.
We had a similar failures also in CI
http://sssd-ci.duckdns.org/logs/job/50/54/fedora_rawhide/ci-build-coverage/dyndns-tests.log

Attached patch should fix it.

LS
>From 041010427e44762226e96a5d9cf4ce88af253134 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik 
Date: Wed, 10 Aug 2016 07:44:28 +0200
Subject: [PATCH] dyndns-tests: Fix false positive failures

The child process finished faster then it has handled by parent
and therefore it timed out. It's the similar solution as in
b3074dca3acebd91437ef13d3329d6d65d655215

[ RUN  ] dyndns_test_error
(Fri Jul 29 16:12:00:621444 2016) [sssd] [nsupdate_child_timeout] (0x0020):
  Timeout reached for dynamic DNS update
Could not run the test - check test fixtures
[  ERROR   ] dyndns_test_error
---
 src/tests/cmocka/test_dyndns.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/tests/cmocka/test_dyndns.c b/src/tests/cmocka/test_dyndns.c
index 
b8404ef1018c1ec9720e506807bcf3f3df379a2d..4f8706545cdb41df0d92e45e92d3279c63909695
 100644
--- a/src/tests/cmocka/test_dyndns.c
+++ b/src/tests/cmocka/test_dyndns.c
@@ -75,6 +75,7 @@ void __wrap_execv(const char *path, char *const argv[])
 case MOCK_NSUPDATE_ERR:
 DEBUG(SSSDBG_FUNC_DATA, "nsupdate error test case\n");
 err = 1;
+usleep(5); /* 50 miliseconds */
 break;
 case MOCK_NSUPDATE_TIMEOUT:
 DEBUG(SSSDBG_FUNC_DATA, "nsupdate timeout test case\n");
@@ -1059,7 +1060,7 @@ int main(int argc, const char *argv[])
 }
 }
 poptFreeContext(pc);
-
+debug_level = 8;
 DEBUG_CLI_INIT(debug_level);
 
 /* Even though normally the tests should clean up after themselves
-- 
2.9.2

___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] NSS: Use correct name for invalidating memory cache

[Petr Cech]

On 08/10/2016 07:30 AM, Petr Cech wrote:

On 08/09/2016 07:10 PM, Lukas Slebodnik wrote:

On (09/08/16 10:21), Jakub Hrozek wrote:

On Mon, Aug 08, 2016 at 06:37:10PM +0200, Lukas Slebodnik wrote:

ehlo,

yet another patch which fixes issues caused by sysdb refactoring.

reproducer:
a) add user with two groups
b) call id user
c) add another group to user
d) authenticate
e) check tha id -G return 3 groups.

I am looking forward to the pam_wrapper So we can test it in upstream.

LS



From e501f791ee2e03b8eaea919a2f8ca4474773e3ba Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik 
Date: Mon, 8 Aug 2016 17:30:29 +0200
Subject: [PATCH] NSS: Use correct name for invalidating memory cache

After refactoring of sysdb, we get and internal fully qualified
name from backend in org.freedesktop.sssd.dataprovider_rev.initgrCheck
Previously we got short name and we created fq name in
nss_update_initgr_memcache. Memory cache still need to use short names
if it was specified.


In fill_pwent() we use the sized_output_name() to store the name in the
memcache, can we use the same here?

Sure,

LS


Hi Lukas, Jakub,

LGTM, I will wait for new CI.


Hi,

CI passed:
http://sssd-ci.duckdns.org/logs/job/51/22/summary.html

Manual testing passed.

=> ACK

Regards

--
Petr^4 Čech
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] NSS: Use correct name for invalidating memory cache

[Petr Cech]

On 08/09/2016 07:10 PM, Lukas Slebodnik wrote:

On (09/08/16 10:21), Jakub Hrozek wrote:

On Mon, Aug 08, 2016 at 06:37:10PM +0200, Lukas Slebodnik wrote:

ehlo,

yet another patch which fixes issues caused by sysdb refactoring.

reproducer:
a) add user with two groups
b) call id user
c) add another group to user
d) authenticate
e) check tha id -G return 3 groups.

I am looking forward to the pam_wrapper So we can test it in upstream.

LS



From e501f791ee2e03b8eaea919a2f8ca4474773e3ba Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik 
Date: Mon, 8 Aug 2016 17:30:29 +0200
Subject: [PATCH] NSS: Use correct name for invalidating memory cache

After refactoring of sysdb, we get and internal fully qualified
name from backend in org.freedesktop.sssd.dataprovider_rev.initgrCheck
Previously we got short name and we created fq name in
nss_update_initgr_memcache. Memory cache still need to use short names
if it was specified.


In fill_pwent() we use the sized_output_name() to store the name in the
memcache, can we use the same here?

Sure,

LS


Hi Lukas, Jakub,

LGTM, I will wait for new CI.

Regards

--
Petr^4 Čech
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] Add support for disabling netlink use

[Justin Stephenson]

Apologies for the duplicate mail!

The mailing list was having some issues for me.

-Justin

On 08/09/2016 04:49 PM, Justin Stephenson wrote:


Hello,

The attached patch fixes: https://fedorahosted.org/sssd/ticket/2860

This provides an option used primarily for testing purpose to ignore 
network status changes from the netlink interface(helpful when sssd 
should remaining in offline/online mode)


===

Before the patch, restarting the network triggers sssd to go back online

(Mon Aug  8 15:50:18 2016) [sssd] [route_msg_debug_print] 
(0x1000): route idx 1 flags 0 family 10 addr 2002:ac10::/28
(Mon Aug  8 15:50:18 2016) [sssd] [network_status_change_cb] 
(0x2000): A networking status change detected signaling providers to 
reset offline status
(Mon Aug  8 15:50:18 2016) [sssd] [sbus_add_timeout] (0x2000): 
0x21af1a0
(Mon Aug  8 15:50:18 2016) [sssd] [sbus_remove_timeout] (0x2000): 
0x21af1a0
(Mon Aug  8 15:50:18 2016) [sssd] [sbus_dispatch] (0x4000): dbus 
conn: 0x21ba170
(Mon Aug  8 15:50:18 2016) [sssd] [sbus_dispatch] (0x4000): 
Dispatching.
(Mon Aug  8 15:50:18 2016) [sssd] [message_type] (0x0200): netlink 
Message type: 24
(Mon Aug  8 15:50:18 2016) [sssd] [route_msg_debug_print] 
(0x1000): route idx 1 flags 0 family 10 addr 2002:c0a8::/32
(Mon Aug  8 15:50:18 2016) [sssd] [network_status_change_cb] 
(0x2000): A networking status change detected signaling providers to 
reset offline status
(Mon Aug  8 15:50:18 2016) [sssd] [sbus_add_timeout] (0x2000): 
0x21bd2a0
(Mon Aug  8 15:50:18 2016) [sssd] [sbus_remove_timeout] (0x2000): 
0x21bd2a0
(Mon Aug  8 15:50:18 2016) [sssd] [sbus_dispatch] (0x4000): dbus 
conn: 0x21ba170
(Mon Aug  8 15:50:18 2016) [sssd] [sbus_dispatch] (0x4000): 
Dispatching.
(Mon Aug  8 15:50:18 2016) [sssd] [message_type] (0x0200): netlink 
Message type: 24


After the patch,

# pkill -USR1 sssd

Restart the network, sssd remains offline
# service network restart

Test backend lookup with no cache shows backend is still offline
# id aduser@ad.domain


(Mon Aug  8 16:44:17 2016) [sssd[be[AD.JSTEPHEN]]] 
[sbus_dispatch] (0x4000): dbus conn: 0x1853d60
(Mon Aug  8 16:44:17 2016) [sssd[be[AD.JSTEPHEN]]] 
[sbus_dispatch] (0x4000): Dispatching.
(Mon Aug  8 16:44:17 2016) [sssd[be[AD.JSTEPHEN]]] 
[sbus_message_handler] (0x2000): Received SBUS method 
org.freedesktop.sssd.dataprovider.getAccountInfo on path 
/org/freedesktop/sssd/dataprovider
(Mon Aug  8 16:44:17 2016) [sssd[be[AD.JSTEPHEN]]] 
[sbus_get_sender_id_send] (0x2000): Not a sysbus message, quit
(Mon Aug  8 16:44:17 2016) [sssd[be[AD.JSTEPHEN]]] 
[dp_attach_req] (0x0400): DP Request [Account #2]: New request. Flags 
[0x0001].
(Mon Aug  8 16:44:17 2016) [sssd[be[AD.JSTEPHEN]]] 
[dp_attach_req] (0x0400): Number of active DP request: 1
(Mon Aug  8 16:44:17 2016) [sssd[be[AD.JSTEPHEN]]] 
[_dp_req_recv] (0x0400): DP Request [Account #2]: Receiving request data.
(Mon Aug  8 16:44:17 2016) [sssd[be[AD.JSTEPHEN]]] 
[dp_req_reply_gen_error] (0x0080): DP Request [Account #2]: Finished. 
Backend is currently offline.
(Mon Aug  8 16:44:17 2016) [sssd[be[AD.JSTEPHEN]]] 
[dp_table_value_destructor] (0x0400): Removing 
[0:1:0x0001:1:1::AD.JSTEPHEN:name=myaduser@ad.jstephen] from reply table
(Mon Aug  8 16:44:17 2016) [sssd[be[AD.JSTEPHEN]]] 
[dp_req_destructor] (0x0400): DP Request [Account #2]: Request removed.
(Mon Aug  8 16:44:17 2016) [sssd[be[AD.JSTEPHEN]]] 
[dp_req_destructor] (0x0400): Number of active DP request: 0
(Mon Aug  8 16:44:17 2016) [sssd[nss]] [sss_dp_get_reply] 
(0x0010): The Data Provider returned an error 
[org.freedesktop.sssd.Error.DataProvider.Offline]



Kind regards,

Justin Stephenson



___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] [PATCH] Add support for disabling netlink use

[Justin Stephenson]

Hello,

The attached patch fixes: https://fedorahosted.org/sssd/ticket/2860

This provides an option used primarily for testing purpose to ignore 
network status changes from the netlink interface(helpful when sssd 
should remaining in offline/online mode)


===

Before the patch, restarting the network triggers sssd to go back online

(Mon Aug  8 15:50:18 2016) [sssd] [route_msg_debug_print] (0x1000): 
route idx 1 flags 0 family 10 addr 2002:ac10::/28
(Mon Aug  8 15:50:18 2016) [sssd] [network_status_change_cb] 
(0x2000): A networking status change detected signaling providers to 
reset offline status
(Mon Aug  8 15:50:18 2016) [sssd] [sbus_add_timeout] (0x2000): 
0x21af1a0
(Mon Aug  8 15:50:18 2016) [sssd] [sbus_remove_timeout] (0x2000): 
0x21af1a0
(Mon Aug  8 15:50:18 2016) [sssd] [sbus_dispatch] (0x4000): dbus 
conn: 0x21ba170
(Mon Aug  8 15:50:18 2016) [sssd] [sbus_dispatch] (0x4000): 
Dispatching.
(Mon Aug  8 15:50:18 2016) [sssd] [message_type] (0x0200): netlink 
Message type: 24
(Mon Aug  8 15:50:18 2016) [sssd] [route_msg_debug_print] (0x1000): 
route idx 1 flags 0 family 10 addr 2002:c0a8::/32
(Mon Aug  8 15:50:18 2016) [sssd] [network_status_change_cb] 
(0x2000): A networking status change detected signaling providers to 
reset offline status
(Mon Aug  8 15:50:18 2016) [sssd] [sbus_add_timeout] (0x2000): 
0x21bd2a0
(Mon Aug  8 15:50:18 2016) [sssd] [sbus_remove_timeout] (0x2000): 
0x21bd2a0
(Mon Aug  8 15:50:18 2016) [sssd] [sbus_dispatch] (0x4000): dbus 
conn: 0x21ba170
(Mon Aug  8 15:50:18 2016) [sssd] [sbus_dispatch] (0x4000): 
Dispatching.
(Mon Aug  8 15:50:18 2016) [sssd] [message_type] (0x0200): netlink 
Message type: 24


After the patch,

# pkill -USR1 sssd

Restart the network, sssd remains offline
# service network restart

Test backend lookup with no cache shows backend is still offline
# id aduser@ad.domain


(Mon Aug  8 16:44:17 2016) [sssd[be[AD.JSTEPHEN]]] 
[sbus_dispatch] (0x4000): dbus conn: 0x1853d60
(Mon Aug  8 16:44:17 2016) [sssd[be[AD.JSTEPHEN]]] 
[sbus_dispatch] (0x4000): Dispatching.
(Mon Aug  8 16:44:17 2016) [sssd[be[AD.JSTEPHEN]]] 
[sbus_message_handler] (0x2000): Received SBUS method 
org.freedesktop.sssd.dataprovider.getAccountInfo on path 
/org/freedesktop/sssd/dataprovider
(Mon Aug  8 16:44:17 2016) [sssd[be[AD.JSTEPHEN]]] 
[sbus_get_sender_id_send] (0x2000): Not a sysbus message, quit
(Mon Aug  8 16:44:17 2016) [sssd[be[AD.JSTEPHEN]]] 
[dp_attach_req] (0x0400): DP Request [Account #2]: New request. Flags 
[0x0001].
(Mon Aug  8 16:44:17 2016) [sssd[be[AD.JSTEPHEN]]] 
[dp_attach_req] (0x0400): Number of active DP request: 1
(Mon Aug  8 16:44:17 2016) [sssd[be[AD.JSTEPHEN]]] 
[_dp_req_recv] (0x0400): DP Request [Account #2]: Receiving request data.
(Mon Aug  8 16:44:17 2016) [sssd[be[AD.JSTEPHEN]]] 
[dp_req_reply_gen_error] (0x0080): DP Request [Account #2]: Finished. 
Backend is currently offline.
(Mon Aug  8 16:44:17 2016) [sssd[be[AD.JSTEPHEN]]] 
[dp_table_value_destructor] (0x0400): Removing 
[0:1:0x0001:1:1::AD.JSTEPHEN:name=myaduser@ad.jstephen] from reply table
(Mon Aug  8 16:44:17 2016) [sssd[be[AD.JSTEPHEN]]] 
[dp_req_destructor] (0x0400): DP Request [Account #2]: Request removed.
(Mon Aug  8 16:44:17 2016) [sssd[be[AD.JSTEPHEN]]] 
[dp_req_destructor] (0x0400): Number of active DP request: 0
(Mon Aug  8 16:44:17 2016) [sssd[nss]] [sss_dp_get_reply] 
(0x0010): The Data Provider returned an error 
[org.freedesktop.sssd.Error.DataProvider.Offline]



Kind regards,

Justin Stephenson

>From a22f1b5f1fe0ae8bd60c238b0069a26f1360e02b Mon Sep 17 00:00:00 2001
From: Justin Stephenson 
Date: Tue, 9 Aug 2016 11:59:49 -0400
Subject: [PATCH] Add support for disabling netlink use with --disable-netlink
 option

Resolves:
https://fedorahosted.org/sssd/ticket/2860
---
 src/monitor/monitor.c | 24 +++-
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c
index 89dd0a91d1fbd41dd853cf8745de732b7059d02b..e7537d7303d4c9ef1fef7d073ba7d171a78c8087 100644
--- a/src/monitor/monitor.c
+++ b/src/monitor/monitor.c
@@ -2341,7 +2341,8 @@ static void missing_resolv_conf(struct tevent_context *ev,
 }
 
 static int monitor_process_init(struct mt_ctx *ctx,
-const char *config_file)
+const char *config_file,
+bool opt_netlinkoff)
 {
 TALLOC_CTX *tmp_ctx;
 struct tevent_signal *tes;
@@ -2472,12 +2473,14 @@ static int monitor_process_init(struct mt_ctx *ctx,
 return ret;
 }
 
-ret = setup_netlink(ctx, ctx->ev, network_status_change_cb,
-ctx, &ctx->nlctx);
-if (ret != EOK) {
-DEBUG(SSSDBG_OP_FAILURE,
-  "Cannot set up listening for network notifications\n

[SSSD] [PATCH] IPA: Parse qualified names when guessing AD user principal

[Jakub Hrozek]

The attached patch fixes issues with logging in as users without an
explicit UPN in a trust scenario. The simplest reproducer is to log in
as Administrator or configure sssd to not look up the principal
attribute by adding this to the server's sssd.conf
subdomain_inherit = ldap_user_principal
ldap_user_principal = nosuchatt

Please see the commit message for more details.
>From 80dd688eaf7a20fbf6d71768c29fb7d73b315238 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek 
Date: Tue, 9 Aug 2016 22:08:27 +0200
Subject: [PATCH] IPA: Parse qualified names when guessing AD user principal

https://fedorahosted.org/sssd/ticket/3127

Most AD users store their UPN in an attribute. If they don't, or the sssd was
configured (typically in earlier versions to work around a bug) to not look
at the principal attribute, then sssd is supposed to guess the attribute.

That currently doesn't work in 1.14, because the username is already
qualified and then we also append the realm name to it. We need to parse
the simple username from the qualified name first.

The issue can be reproduced simply by authenticating as the Administrator
account in IPA-AD trust setups.
---
 src/providers/ipa/ipa_s2n_exop.c | 14 --
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/src/providers/ipa/ipa_s2n_exop.c b/src/providers/ipa/ipa_s2n_exop.c
index 
a8c415b4c86ccd3bd3b180c8df835c75420fbb21..07bbb2b4d252c8ca9ada4d890c36c903c9f75773
 100644
--- a/src/providers/ipa/ipa_s2n_exop.c
+++ b/src/providers/ipa/ipa_s2n_exop.c
@@ -1941,6 +1941,7 @@ static errno_t ipa_s2n_save_objects(struct 
sss_domain_info *dom,
 struct sss_nss_homedir_ctx homedir_ctx;
 char *name = NULL;
 char *realm;
+char *short_name = NULL;
 char *upn = NULL;
 gid_t gid;
 gid_t orig_gid = 0;
@@ -2092,8 +2093,17 @@ static errno_t ipa_s2n_save_objects(struct 
sss_domain_info *dom,
 ret = ENOMEM;
 goto done;
 }
-upn = talloc_asprintf(tmp_ctx, "%s@%s",
-  attrs->a.user.pw_name, realm);
+
+ret = sss_parse_internal_fqname(tmp_ctx, attrs->a.user.pw_name,
+&short_name, NULL);
+if (ret != EOK) {
+DEBUG(SSSDBG_CRIT_FAILURE,
+  "Cannot parse internal name %s\n",
+  attrs->a.user.pw_name);
+goto done;
+}
+
+upn = talloc_asprintf(tmp_ctx, "%s@%s", short_name, realm);
 if (!upn) {
 DEBUG(SSSDBG_OP_FAILURE, "failed to format UPN.\n");
 ret = ENOMEM;
-- 
2.4.11

___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] intg: test nested membership

[Lukas Slebodnik]

On (09/08/16 14:54), Jakub Hrozek wrote:
>On Tue, Aug 09, 2016 at 12:36:11PM +0200, Jakub Hrozek wrote:
>> On Wed, Aug 03, 2016 at 09:56:40AM +0200, Lukas Slebodnik wrote:
>> > On (13/07/16 17:48), Lukas Slebodnik wrote:
>> > >ehlo,
>> > >
>> > >attched patch is an integration test for regression #3093.
>> > >I prepared a test and I let someone else to fix it :-)
>> > >
>> > >I will try to find more bugs in downstream tests.
>> > >
>> > >BTW we might move test from test_ts_cache somewhere else.
>> > >But it was the fastest way how to write a test without enumeration.
>> > >
>> > 
>> > As I previously wrote test_ts_cache is not the best place for this
>> > test. Attached are patches which some changes in intg tests.
>> > There is also test for grups with special characters
>> > 
>> > LS
>> 
>> Thank you, the only comment I have is that we should remove
>> ldap_auth_disable_tls_never_use_in_production option. We don't use it in
>> tests (we don't test authentication there at all at the moment) and it's
>> better to not advertise the option.
>>
I can remove it as part of "moving files" if you wish.

>> I'll give a formal ACK once the CI run finishes.
>
>For some reason, I can't get a 'green' CI run on RHEL-6. I tried three
>times, the last attempt is here:
>
> http://sssd-ci.duckdns.org/logs/job/51/19/rhel6/ci-build-debug/ci-make-intgcheck.log
>but all had the same errors.
>
>Did the patches change something in the way the server is set up on
>RHEL-6?

E SERVER_DOWN: {'desc': "Can't contact LDAP server"}

I have no idea why it should be stopped.
We do not stop openldap server. It should be done
only in cleanup (end of all tests in module)

LS
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] [PATCH] intg: Allow to test netgroups

[Lukas Slebodnik]

ehlo,

attcheck patch is a python wrapper for
netgroup lookups in sssd.

LS
>From 2b0578efa3ebc2b6710aa420b0647e7208f0e0ea Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik 
Date: Tue, 9 Aug 2016 13:50:24 +0200
Subject: [PATCH 1/2] intg: Make location of sssd nss module configurable

The path to sssd nss module (libsss_nss.so) was
relative to prefix and expected subdirectory "lib".
32bit and 64bit platforms and different distributions
use different paths. This patch allows to use python module sssd_id
even with real module and not just integration tests.
It is just required to prepare "config.py" with right path.

e.g.
  cd ~/sssd/src/tests/intg
  [~/sssd/src/tests/intg]$ echo "NSS_MODULE_DIR = '/usr/lib64'" > config.py
  [~/sssd/src/tests/intg]$ python
  Python 2.7.12 (default, Jul 18 2016, 09:57:01)
  [GCC 6.1.1 20160621 (Red Hat 6.1.1-3)] on linux2
  Type "help", "copyright", "credits" or "license" for more information.
  >>> import sssd_id
  >>> sssd_id.get_user_gids('user')
  (1, 0, [5977, 1070, 5845, 1076, 1074, 10327, 5975, 5766])
---
 src/tests/intg/config.py.m4 | 1 +
 src/tests/intg/sssd_id.py   | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/tests/intg/config.py.m4 b/src/tests/intg/config.py.m4
index 
563127c6ea895508308a4f94689cc4e26ca4cbde..77aa47b7958783217132b724159d9d3d247e1079
 100644
--- a/src/tests/intg/config.py.m4
+++ b/src/tests/intg/config.py.m4
@@ -4,6 +4,7 @@ Build configuration variables.
 
 PREFIX  = "prefix"
 SYSCONFDIR  = "sysconfdir"
+NSS_MODULE_DIR  = PREFIX + "/lib"
 SSSDCONFDIR = SYSCONFDIR + "/sssd"
 CONF_PATH   = SSSDCONFDIR + "/sssd.conf"
 DB_PATH = "dbpath"
diff --git a/src/tests/intg/sssd_id.py b/src/tests/intg/sssd_id.py
index 
500f242ecc6c890a5683d8747ac0338555ce1709..4ae41af98bad804026083b193eddfa6c2d3c924c
 100644
--- a/src/tests/intg/sssd_id.py
+++ b/src/tests/intg/sssd_id.py
@@ -45,7 +45,7 @@ def call_sssd_initgroups(user, gid):
 gids should contain user group IDs if err is NssReturnCode.SUCCESS
 otherwise errno will contain non-zero value.
 """
-libnss_sss_path = config.PREFIX + "/lib/libnss_sss.so.2"
+libnss_sss_path = config.NSS_MODULE_DIR + "/libnss_sss.so.2"
 libnss_sss = cdll.LoadLibrary(libnss_sss_path)
 
 func = libnss_sss._nss_sss_initgroups_dyn
-- 
2.9.2

>From 7675d041262bb65672877e62e0e438d28dc7dc87 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik 
Date: Tue, 9 Aug 2016 15:15:12 +0200
Subject: [PATCH 2/2] intg: Allow to test netgroups

  sh-4.2# getent netgroup -s sss QAUsers
  QAUsers   ( ,qa1,example.com) ( ,qa2,example.com) ( 
,qa3,example.com)
  sh-4.2# getent netgroup -s sss QASystems
  QASystems (qahost1.example.com,,) 
(qahost2.lab.eng.pnq.redhat.com,,)
  sh-4.2# getent netgroup -s sss test
  sh-4.2# echo $?
  2

  sh-4.2# python
  Python 2.7.5 (default, Aug  2 2016, 04:20:16)
  [GCC 4.8.5 20150623 (Red Hat 4.8.5-4)] on linux2
  Type "help", "copyright", "credits" or "license" for more information.
  >>> import sssd_netgroup
  >>> sssd_netgroup.get_sssd_netgroups('QAUsers')
  (1, 0, [(None, 'qa1', 'example.com'), (None, 'qa2', 'example.com'), (None, 
'qa3', 'example.com')])
  >>> sssd_netgroup.get_sssd_netgroups('QASystems')
  (1, 0, [('qahost1.example.com', None, None), 
('qahost2.lab.eng.pnq.redhat.com', None, None)])
  >>> sssd_netgroup.get_sssd_netgroups('test')
  (0, 0, [])
  >>>
---
 src/tests/intg/Makefile.am  |   1 +
 src/tests/intg/sssd_netgroup.py | 155 
 2 files changed, 156 insertions(+)
 create mode 100644 src/tests/intg/sssd_netgroup.py

diff --git a/src/tests/intg/Makefile.am b/src/tests/intg/Makefile.am
index 
d79f6424e0226f8b81d3a104877fa166bd885b42..b8cc5c006845f911d8518df815925455482e9f6d
 100644
--- a/src/tests/intg/Makefile.am
+++ b/src/tests/intg/Makefile.am
@@ -2,6 +2,7 @@ dist_noinst_DATA = \
 config.py.m4 \
 sssd_id.py \
 sssd_ldb.py \
+sssd_netgroup.py \
 ds.py \
 ds_openldap.py \
 ent.py \
diff --git a/src/tests/intg/sssd_netgroup.py b/src/tests/intg/sssd_netgroup.py
new file mode 100644
index 
..9f3e5204ef5b935ffdf3b3fc998e5592be3100c3
--- /dev/null
+++ b/src/tests/intg/sssd_netgroup.py
@@ -0,0 +1,155 @@
+#
+# Module for simulation of utility "getent netgroup -s sss" from coreutils
+#
+# Copyright (c) 2016 Red Hat, Inc.
+# Author: Lukas Slebodnik 
+#
+# This is free software; you can redistribute it and/or modify it
+# under the terms of the GNU General Public License as published by
+# the Free Software Foundation; version 2 only
+#
+# This program is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If 

[SSSD] Re: [PATCH] NSS: Use correct name for invalidating memory cache

[Lukas Slebodnik]

On (09/08/16 10:21), Jakub Hrozek wrote:
>On Mon, Aug 08, 2016 at 06:37:10PM +0200, Lukas Slebodnik wrote:
>> ehlo,
>> 
>> yet another patch which fixes issues caused by sysdb refactoring.
>> 
>> reproducer:
>> a) add user with two groups
>> b) call id user
>> c) add another group to user
>> d) authenticate
>> e) check tha id -G return 3 groups.
>> 
>> I am looking forward to the pam_wrapper So we can test it in upstream.
>> 
>> LS
>
>> From e501f791ee2e03b8eaea919a2f8ca4474773e3ba Mon Sep 17 00:00:00 2001
>> From: Lukas Slebodnik 
>> Date: Mon, 8 Aug 2016 17:30:29 +0200
>> Subject: [PATCH] NSS: Use correct name for invalidating memory cache
>> 
>> After refactoring of sysdb, we get and internal fully qualified
>> name from backend in org.freedesktop.sssd.dataprovider_rev.initgrCheck
>> Previously we got short name and we created fq name in
>> nss_update_initgr_memcache. Memory cache still need to use short names
>> if it was specified.
>
>In fill_pwent() we use the sized_output_name() to store the name in the
>memcache, can we use the same here?
Sure,

LS
>From 8efd54108aaa6e9f74e3ba884565a78f33d82cea Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik 
Date: Mon, 8 Aug 2016 17:30:29 +0200
Subject: [PATCH] NSS: Use correct name for invalidating memory cache

After refactoring of sysdb, we get and internal fully qualified
name from backend in org.freedesktop.sssd.dataprovider_rev.initgrCheck
Previously we got short name and we created fq name in
nss_update_initgr_memcache. Memory cache still need to use short names
if it was specified.

This patch uses right name in different places.
---
 src/responder/nss/nsssrv_cmd.c | 31 +--
 src/responder/nss/nsssrv_private.h |  2 +-
 2 files changed, 18 insertions(+), 15 deletions(-)

diff --git a/src/responder/nss/nsssrv_cmd.c b/src/responder/nss/nsssrv_cmd.c
index 
f3b6ac4afb5d1571f283933b48e0256b91c56391..573959ea76fc1277fe84f40b88dcd34093da468d
 100644
--- a/src/responder/nss/nsssrv_cmd.c
+++ b/src/responder/nss/nsssrv_cmd.c
@@ -3961,13 +3961,13 @@ done:
 }
 
 void nss_update_initgr_memcache(struct nss_ctx *nctx,
-const char *name, const char *domain,
+const char *fq_name, const char *domain,
 int gnum, uint32_t *groups)
 {
 TALLOC_CTX *tmp_ctx = NULL;
 struct sss_domain_info *dom;
 struct ldb_result *res;
-struct sized_string delete_name;
+struct sized_string *delete_name;
 bool changed = false;
 uint32_t id;
 uint32_t gids[gnum];
@@ -3987,8 +3987,19 @@ void nss_update_initgr_memcache(struct nss_ctx *nctx,
 }
 
 tmp_ctx = talloc_new(NULL);
+if (tmp_ctx == NULL) {
+return;
+}
 
-ret = sysdb_initgroups(tmp_ctx, dom, name, &res);
+ret = sized_output_name(tmp_ctx, nctx->rctx, fq_name, dom, &delete_name);
+if (ret != EOK) {
+DEBUG(SSSDBG_OP_FAILURE,
+  "sized_output_name failed for '%s': %d [%s]\n",
+  fq_name, ret, sss_strerror(ret));
+goto done;
+}
+
+ret = sysdb_initgroups(tmp_ctx, dom, fq_name, &res);
 if (ret != EOK && ret != ENOENT) {
 DEBUG(SSSDBG_CRIT_FAILURE,
   "Failed to make request to our cache! [%d][%s]\n",
@@ -4002,8 +4013,7 @@ void nss_update_initgr_memcache(struct nss_ctx *nctx,
 
 if (ret == ENOENT || res->count == 0) {
 /* The user is gone. Invalidate the mc record */
-to_sized_string(&delete_name, name);
-ret = sss_mmap_cache_pw_invalidate(nctx->pwd_mc_ctx, &delete_name);
+ret = sss_mmap_cache_pw_invalidate(nctx->pwd_mc_ctx, delete_name);
 if (ret != EOK && ret != ENOENT) {
 DEBUG(SSSDBG_CRIT_FAILURE,
   "Internal failure in memory cache code: %d [%s]\n",
@@ -4047,13 +4057,6 @@ void nss_update_initgr_memcache(struct nss_ctx *nctx,
 }
 
 if (changed) {
-char *fq_name = sss_tc_fqname(tmp_ctx, dom->names, dom, name);
-if (!fq_name) {
-DEBUG(SSSDBG_CRIT_FAILURE,
-  "Could not create fq name\n");
-goto done;
-}
-
 for (i = 0; i < gnum; i++) {
 id = groups[i];
 
@@ -4065,9 +4068,9 @@ void nss_update_initgr_memcache(struct nss_ctx *nctx,
 }
 }
 
-to_sized_string(&delete_name, fq_name);
+to_sized_string(delete_name, fq_name);
 ret = sss_mmap_cache_initgr_invalidate(nctx->initgr_mc_ctx,
-   &delete_name);
+   delete_name);
 if (ret != EOK && ret != ENOENT) {
 DEBUG(SSSDBG_CRIT_FAILURE,
   "Internal failure in memory cache code: %d [%s]\n",
diff --git a/src/responder/nss/nsssrv_private.h 
b/src/responder/nss/nsssrv_private.h
index 
79c7b7265f66f57e0ea89fe192a1da4f8992f1a3..391eaaf40f84a7436bee63fd699241e4957fdbeb
 100644
--- a/src/responder/nss/nsssrv_private.h
+++ b/src/res

[SSSD] [PATCH] Add support for disabling netlink use

[Justin Stephenson]

Hello,

The attached patch fixes: https://fedorahosted.org/sssd/ticket/2860

This provides an option to allow sssd testing to ignore network status 
changes from the netlink interface(remaining in offline mode, for example)


Kind regards,

Justin Stephenson

>From a22f1b5f1fe0ae8bd60c238b0069a26f1360e02b Mon Sep 17 00:00:00 2001
From: Justin Stephenson 
Date: Tue, 9 Aug 2016 11:59:49 -0400
Subject: [PATCH] Add support for disabling netlink use with --disable-netlink
 option

Resolves:
https://fedorahosted.org/sssd/ticket/
---
 src/monitor/monitor.c | 24 +++-
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c
index 89dd0a91d1fbd41dd853cf8745de732b7059d02b..e7537d7303d4c9ef1fef7d073ba7d171a78c8087 100644
--- a/src/monitor/monitor.c
+++ b/src/monitor/monitor.c
@@ -2341,7 +2341,8 @@ static void missing_resolv_conf(struct tevent_context *ev,
 }
 
 static int monitor_process_init(struct mt_ctx *ctx,
-const char *config_file)
+const char *config_file,
+bool opt_netlinkoff)
 {
 TALLOC_CTX *tmp_ctx;
 struct tevent_signal *tes;
@@ -2472,12 +2473,14 @@ static int monitor_process_init(struct mt_ctx *ctx,
 return ret;
 }
 
-ret = setup_netlink(ctx, ctx->ev, network_status_change_cb,
-ctx, &ctx->nlctx);
-if (ret != EOK) {
-DEBUG(SSSDBG_OP_FAILURE,
-  "Cannot set up listening for network notifications\n");
-return ret;
+if (opt_netlinkoff == false) {
+ret = setup_netlink(ctx, ctx->ev, network_status_change_cb,
+ctx, &ctx->nlctx);
+if (ret != EOK) {
+DEBUG(SSSDBG_OP_FAILURE,
+  "Cannot set up listening for network notifications\n");
+return ret;
+}
 }
 
 /* start providers */
@@ -2773,6 +2776,7 @@ int main(int argc, const char *argv[])
 int opt_interactive = 0;
 int opt_genconf = 0;
 int opt_version = 0;
+int opt_netlinkoff = 0;
 char *opt_config_file = NULL;
 char *config_file = NULL;
 int flags = 0;
@@ -2789,6 +2793,8 @@ int main(int argc, const char *argv[])
  _("Become a daemon (default)"), NULL }, \
 {"interactive", 'i', POPT_ARG_NONE, &opt_interactive, 0, \
  _("Run interactive (not a daemon)"), NULL}, \
+{"disable-netlink", '\0', POPT_ARG_NONE, &opt_netlinkoff, 0, \
+ _("Disable netlink interface"), NULL}, \
 {"config", 'c', POPT_ARG_STRING, &opt_config_file, 0, \
  _("Specify a non-default config file"), NULL}, \
 {"genconf", 'g', POPT_ARG_NONE, &opt_genconf, 0, \
@@ -2991,8 +2997,8 @@ int main(int argc, const char *argv[])
 monitor->ev = main_ctx->event_ctx;
 talloc_steal(main_ctx, monitor);
 
-ret = monitor_process_init(monitor,
-   config_file);
+ret = monitor_process_init(monitor, config_file,
+   opt_netlinkoff);
 if (ret != EOK) return 3;
 talloc_free(tmp_ctx);
 
-- 
2.7.4

___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] sssctl: use internal API to remove files

[Jakub Hrozek]

On Tue, Aug 09, 2016 at 02:49:05PM +0200, Petr Cech wrote:
> On 08/09/2016 02:10 PM, Pavel Březina wrote:
> > On 08/09/2016 01:53 PM, Petr Cech wrote:
> > > On 08/05/2016 01:48 PM, Petr Cech wrote:
> > > > On 07/13/2016 01:47 PM, Pavel Březina wrote:
> > > > > 0001-utils-add-remove_subtree.patch
> > > > > 
> > > > > 
> > > > > From 0aa39a46b707212e6487b6b537238e31bf7da1b4 Mon Sep 17 00:00:00 2001
> > > > > From: =?UTF-8?q?Pavel=20B=C5=99ezina?= 
> > > > > Date: Wed, 13 Jul 2016 12:17:58 +0200
> > > > > Subject: [PATCH 1/2] utils: add remove_subtree
> > > > > 
> > > > > Remove all entries in a directory but will
> > > > > not remove the directory itself.
> > > >   ^--- [1]
> > > > ...
> > > > 
> > > > > +if (!subtree) {
> > > >^---
> > > > Pavel, please, would you like to add the comment in meaning of [1] here.
> > > > I needed spent time to understand that the code do what is expected.
> > > > 
> > > > > +ret = unlinkat(parent_fd, dir_name, AT_REMOVEDIR);
> > > > > +if (ret == -1) {
> > > > > +ret = errno;
> > > > > +}
> > > > >  }
> > > > > 0002-sssctl-use-internal-API-to-remove-files.patch
> > > > > 
> > > > > 
> > > > > From 6413cb17138d8b24d579109454ae5dd7049b552a Mon Sep 17 00:00:00 2001
> > > > > From: =?UTF-8?q?Pavel=20B=C5=99ezina?= 
> > > > > Date: Wed, 13 Jul 2016 13:29:54 +0200
> > > > > Subject: [PATCH 2/2] sssctl: use internal API to remove files
> > > > > 
> > > > > -
> > > > 
> > > > CI passed:
> > > > http://sssd-ci.duckdns.org/logs/job/50/90/summary.html
> > > > 
> > > > ACK (and please add the comment)
> 
> > New patch is attached. I added a comment a renamed subtree to
> > keep_root_dir.
> 
> Thanks, Pavel. The new naming is also very useful.
> 
> > 0001-utils-add-remove_subtree.patch
> > 
> > 
> > From f94c68ef867327c42d873c508301bfcdc4c66801 Mon Sep 17 00:00:00 2001
> > From: =?UTF-8?q?Pavel=20B=C5=99ezina?= 
> > Date: Wed, 13 Jul 2016 12:17:58 +0200
> > Subject: [PATCH 1/2] utils: add remove_subtree
> > 
> > Remove all entries in a directory but will
> > not remove the directory itself.
> > ---
> ack
> 
> > 0002-sssctl-use-internal-API-to-remove-files.patch
> > 
> > 
> > From c58d2f6507dda79133813f215c9a4a5feec06d1b Mon Sep 17 00:00:00 2001
> > From: =?UTF-8?q?Pavel=20B=C5=99ezina?= 
> > Date: Wed, 13 Jul 2016 13:29:54 +0200
> > Subject: [PATCH 2/2] sssctl: use internal API to remove files
> > 
> > ---
> ack
> 
> => ACK

* master:
* 9c7e046cc10a834b86457844df3ba810866cad45
* 68f73e56a9b6133f8a9f4b3c0e696df6c30fec91
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] intg: test nested membership

[Jakub Hrozek]

On Tue, Aug 09, 2016 at 12:36:11PM +0200, Jakub Hrozek wrote:
> On Wed, Aug 03, 2016 at 09:56:40AM +0200, Lukas Slebodnik wrote:
> > On (13/07/16 17:48), Lukas Slebodnik wrote:
> > >ehlo,
> > >
> > >attched patch is an integration test for regression #3093.
> > >I prepared a test and I let someone else to fix it :-)
> > >
> > >I will try to find more bugs in downstream tests.
> > >
> > >BTW we might move test from test_ts_cache somewhere else.
> > >But it was the fastest way how to write a test without enumeration.
> > >
> > 
> > As I previously wrote test_ts_cache is not the best place for this
> > test. Attached are patches which some changes in intg tests.
> > There is also test for grups with special characters
> > 
> > LS
> 
> Thank you, the only comment I have is that we should remove
> ldap_auth_disable_tls_never_use_in_production option. We don't use it in
> tests (we don't test authentication there at all at the moment) and it's
> better to not advertise the option.
> 
> I'll give a formal ACK once the CI run finishes.

For some reason, I can't get a 'green' CI run on RHEL-6. I tried three
times, the last attempt is here:

http://sssd-ci.duckdns.org/logs/job/51/19/rhel6/ci-build-debug/ci-make-intgcheck.log
but all had the same errors.

Did the patches change something in the way the server is set up on
RHEL-6?
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: tasks for newcomers or non-developers

[Jakub Hrozek]

On Tue, Aug 09, 2016 at 08:18:16AM -0400, Dan Lavu wrote:
> I think it should be less formal than that, maybe on the wiki page, to have
> a simple wishlist? So they'll ask to do it, grant access, write a draft in
> the wiki, one reviewer, then publish?

We can do that, but I would stil prefer (at least for my tracking) to
have tickets as well, maybe in a special milestone that would not be
used otherwise, otherwise the wishlist might get out of sync easily..
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH 1/2] LDAP: Adding support for SIGTERM signal

[Jakub Hrozek]

On Tue, Aug 09, 2016 at 12:57:58PM +0200, Petr Cech wrote:
> On 08/09/2016 11:26 AM, Jakub Hrozek wrote:
> > On Mon, Aug 08, 2016 at 09:46:55AM +0200, Petr Cech wrote:
> > > > On 08/04/2016 05:01 PM, Petr Cech wrote:
> > > > > > On 08/04/2016 04:35 PM, Petr Cech wrote:
> > > > > > > > Hi list,
> > > > > > > >
> > > > > > > > there is the first version of patch for [1]. I need
> > > > > > > > to investigate if we have the same issue in other
> > > > > > > > *_childs.
> > > > > > > >
> > > > > > > > I tested it with sleep() added into ldap_child code.
> > > > > > > >
> > > > > > > > [1] https://fedorahosted.org/sssd/ticket/3106
> > > > > > > >
> > > > > >
> > > > > > I know, it is not perfect. For example 2 seconds between SIGTERM and
> > > > > > SIGKILL should be constant. I will send the second version with 
> > > > > > other
> > > > > > childs, but not today.
> > > > > >
> > > > > > Regards
> > > >
> > > > Hello all,
> > > >
> > > > there is fixed patch set. There is only one minor change -- constant for
> > > > time between signals.
> > > >
> > > > Is there better name for global_ccname_file_dummy? I did not want to 
> > > > break
> > > > the naming... but it looks really long.
> > I think this name is fine, but I would prefer to not use the variable in
> > the code, except for setting it after the temporary file is created and
> > then set the variable back to NULL after the temporary file is renamed
> > to the real one.
> 
> Addressed.
> 
> Thanks Jakub, it looks better. :-)

OK, now I actually tested the patch and it regresses the way we handle
responses, because the handler exits the process with success error
code, the provider then tries to parse the child output, which is not
there..

I think the easiest way would be to define a return code of ldap_child
that would be used in exit in the signal handler and the back end code
would print a nice debug message telling the admin that the child timed
out.
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] sssctl: use internal API to remove files

[Petr Cech]

On 08/09/2016 02:10 PM, Pavel Březina wrote:

On 08/09/2016 01:53 PM, Petr Cech wrote:

On 08/05/2016 01:48 PM, Petr Cech wrote:

On 07/13/2016 01:47 PM, Pavel Březina wrote:

0001-utils-add-remove_subtree.patch


From 0aa39a46b707212e6487b6b537238e31bf7da1b4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= 
Date: Wed, 13 Jul 2016 12:17:58 +0200
Subject: [PATCH 1/2] utils: add remove_subtree

Remove all entries in a directory but will
not remove the directory itself.

  ^--- [1]
...


+if (!subtree) {

   ^---
Pavel, please, would you like to add the comment in meaning of [1] here.
I needed spent time to understand that the code do what is expected.


+ret = unlinkat(parent_fd, dir_name, AT_REMOVEDIR);
+if (ret == -1) {
+ret = errno;
+}
 }
0002-sssctl-use-internal-API-to-remove-files.patch


From 6413cb17138d8b24d579109454ae5dd7049b552a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= 
Date: Wed, 13 Jul 2016 13:29:54 +0200
Subject: [PATCH 2/2] sssctl: use internal API to remove files

-


CI passed:
http://sssd-ci.duckdns.org/logs/job/50/90/summary.html

ACK (and please add the comment)



New patch is attached. I added a comment a renamed subtree to
keep_root_dir.


Thanks, Pavel. The new naming is also very useful.


0001-utils-add-remove_subtree.patch


From f94c68ef867327c42d873c508301bfcdc4c66801 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= 
Date: Wed, 13 Jul 2016 12:17:58 +0200
Subject: [PATCH 1/2] utils: add remove_subtree

Remove all entries in a directory but will
not remove the directory itself.
---

ack


0002-sssctl-use-internal-API-to-remove-files.patch


From c58d2f6507dda79133813f215c9a4a5feec06d1b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= 
Date: Wed, 13 Jul 2016 13:29:54 +0200
Subject: [PATCH 2/2] sssctl: use internal API to remove files

---

ack

=> ACK

Regards

--
Petr^4 Čech
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: tasks for newcomers or non-developers

[Dan Lavu]

I think it should be less formal than that, maybe on the wiki page, to have
a simple wishlist? So they'll ask to do it, grant access, write a draft in
the wiki, one reviewer, then publish?

On Tue, Aug 9, 2016 at 7:38 AM, Jakub Hrozek  wrote:

> On Tue, Aug 09, 2016 at 07:26:24AM -0400, Dan Lavu wrote:
> > I know it's not code related, but documentation and guides come to mind.
> > Maybe let people contribute more freely to the upstream wiki as they work
> > on tickets?
>
> Yes, how should we let potential contributors let about what's needed?
> File tickets as well?
> ___
> sssd-devel mailing list
> sssd-devel@lists.fedorahosted.org
> https://lists.fedorahosted.org/admin/lists/sssd-devel@
> lists.fedorahosted.org
>
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] SYSDB: Check changed virtual attributes before modified timestamp

[Jakub Hrozek]

On Fri, Aug 05, 2016 at 01:28:36PM +0200, Lukas Slebodnik wrote:
> and does not apply anymore on current master

ACK (CI pending).

I did some performance tests with systemtap and the performance hit was
there, but very small (the user lookup went from 2-3ms to 4-6ms).

nonetheless, Simo had a good idea recently on IRC which I captured in
this ticket:
https://fedorahosted.org/sssd/ticket/3126
but that's larger effort than what we can accomplish in 1.14.1.
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] sssctl: use internal API to remove files

[Pavel Březina]

On 08/09/2016 01:53 PM, Petr Cech wrote:

On 08/05/2016 01:48 PM, Petr Cech wrote:

On 07/13/2016 01:47 PM, Pavel Březina wrote:

0001-utils-add-remove_subtree.patch


From 0aa39a46b707212e6487b6b537238e31bf7da1b4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= 
Date: Wed, 13 Jul 2016 12:17:58 +0200
Subject: [PATCH 1/2] utils: add remove_subtree

Remove all entries in a directory but will
not remove the directory itself.

  ^--- [1]
...


+if (!subtree) {

   ^---
Pavel, please, would you like to add the comment in meaning of [1] here.
I needed spent time to understand that the code do what is expected.


+ret = unlinkat(parent_fd, dir_name, AT_REMOVEDIR);
+if (ret == -1) {
+ret = errno;
+}
 }
0002-sssctl-use-internal-API-to-remove-files.patch


From 6413cb17138d8b24d579109454ae5dd7049b552a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= 
Date: Wed, 13 Jul 2016 13:29:54 +0200
Subject: [PATCH 2/2] sssctl: use internal API to remove files

-


CI passed:
http://sssd-ci.duckdns.org/logs/job/50/90/summary.html

ACK (and please add the comment)


bump


New patch is attached. I added a comment a renamed subtree to keep_root_dir.

From f94c68ef867327c42d873c508301bfcdc4c66801 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= 
Date: Wed, 13 Jul 2016 12:17:58 +0200
Subject: [PATCH 1/2] utils: add remove_subtree

Remove all entries in a directory but will
not remove the directory itself.
---
 src/tests/files-tests.c | 53 +
 src/tools/files.c   | 35 +---
 src/tools/tools_util.h  |  1 +
 3 files changed, 82 insertions(+), 7 deletions(-)

diff --git a/src/tests/files-tests.c b/src/tests/files-tests.c
index 596069e2858e07953b3d48f6b8015ce66e2dd423..e96a60af1817b5f7a2e99d8b09ebc91c1a52667b 100644
--- a/src/tests/files-tests.c
+++ b/src/tests/files-tests.c
@@ -153,6 +153,58 @@ START_TEST(test_remove_tree)
 }
 END_TEST
 
+START_TEST(test_remove_subtree)
+{
+int ret;
+char origpath[PATH_MAX+1];
+
+errno = 0;
+fail_unless(getcwd(origpath, PATH_MAX) == origpath, "Cannot getcwd\n");
+fail_unless(errno == 0, "Cannot getcwd\n");
+
+DEBUG(SSSDBG_FUNC_DATA, "About to delete %s\n", dir_path);
+
+/* create a file */
+ret = chdir(dir_path);
+fail_if(ret == -1, "Cannot chdir1\n");
+
+ret = create_simple_file("bar", "bar");
+fail_if(ret == -1, "Cannot create file1\n");
+
+/* create a subdir and file inside it */
+ret = mkdir("subdir", 0700);
+fail_if(ret == -1, "Cannot create subdir\n");
+
+ret = chdir("subdir");
+fail_if(ret == -1, "Cannot chdir\n");
+
+ret = create_simple_file("foo", "foo");
+fail_if(ret == -1, "Cannot create file\n");
+
+/* create another subdir, empty this time */
+ret = mkdir("subdir2", 0700);
+fail_if(ret == -1, "Cannot create subdir\n");
+
+ret = chdir(origpath);
+fail_if(ret == -1, "Cannot chdir2\n");
+
+/* go back */
+ret = chdir(origpath);
+fail_if(ret == -1, "Cannot chdir\n");
+
+/* and finally wipe it out.. */
+ret = remove_subtree(dir_path);
+fail_unless(ret == EOK, "remove_subtree failed\n");
+
+/* check if really gone */
+ret = access(dir_path, F_OK);
+fail_unless(ret == 0, "directory was deleted\n");
+
+ret = rmdir(dir_path);
+fail_unless(ret == 0, "unable to delete root directory\n");
+}
+END_TEST
+
 START_TEST(test_simple_copy)
 {
 int ret;
@@ -337,6 +389,7 @@ static Suite *files_suite(void)
   teardown_files_test);
 
 tcase_add_test(tc_files, test_remove_tree);
+tcase_add_test(tc_files, test_remove_subtree);
 tcase_add_test(tc_files, test_simple_copy);
 tcase_add_test(tc_files, test_copy_file);
 tcase_add_test(tc_files, test_copy_symlink);
diff --git a/src/tools/files.c b/src/tools/files.c
index 8f1aa68beeb2676b56733f49550de170b404c789..9f4e7caa7257144702c417c39bc1643f0be8661a 100644
--- a/src/tools/files.c
+++ b/src/tools/files.c
@@ -137,7 +137,8 @@ static int sss_futime_set(int fd, const struct stat *statp)
 static int remove_tree_with_ctx(TALLOC_CTX *mem_ctx,
 int parent_fd,
 const char *dir_name,
-dev_t parent_dev);
+dev_t parent_dev,
+bool keep_root_dir);
 
 int remove_tree(const char *root)
 {
@@ -149,7 +150,22 @@ int remove_tree(const char *root)
 return ENOMEM;
 }
 
-ret = remove_tree_with_ctx(tmp_ctx, AT_FDCWD, root, 0);
+ret = remove_tree_with_ctx(tmp_ctx, AT_FDCWD, root, 0, false);
+talloc_free(tmp_ctx);
+return ret;
+}
+
+int remove_subtree(const char *root)
+{
+TALLOC_CTX *tmp_ctx = NULL;
+int ret;
+
+tmp_ctx = talloc_new(NULL);
+if (!tmp_ctx) {
+return ENOMEM;
+}
+
+ret = remove_tree_with_ctx(tmp_ctx, AT_FDCWD

[SSSD] Re: [PATCH] sssctl: use internal API to remove files

[Petr Cech]

On 08/05/2016 01:48 PM, Petr Cech wrote:

On 07/13/2016 01:47 PM, Pavel Březina wrote:

0001-utils-add-remove_subtree.patch


From 0aa39a46b707212e6487b6b537238e31bf7da1b4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= 
Date: Wed, 13 Jul 2016 12:17:58 +0200
Subject: [PATCH 1/2] utils: add remove_subtree

Remove all entries in a directory but will
not remove the directory itself.

  ^--- [1]
...


+if (!subtree) {

   ^---
Pavel, please, would you like to add the comment in meaning of [1] here.
I needed spent time to understand that the code do what is expected.


+ret = unlinkat(parent_fd, dir_name, AT_REMOVEDIR);
+if (ret == -1) {
+ret = errno;
+}
 }
0002-sssctl-use-internal-API-to-remove-files.patch


From 6413cb17138d8b24d579109454ae5dd7049b552a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= 
Date: Wed, 13 Jul 2016 13:29:54 +0200
Subject: [PATCH 2/2] sssctl: use internal API to remove files

-


CI passed:
http://sssd-ci.duckdns.org/logs/job/50/90/summary.html

ACK (and please add the comment)


bump


Regards


--
Petr^4 Čech
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: tasks for newcomers or non-developers

[Jakub Hrozek]

On Tue, Aug 09, 2016 at 07:26:24AM -0400, Dan Lavu wrote:
> I know it's not code related, but documentation and guides come to mind.
> Maybe let people contribute more freely to the upstream wiki as they work
> on tickets?

Yes, how should we let potential contributors let about what's needed?
File tickets as well?
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: tasks for newcomers or non-developers

[Dan Lavu]

I know it's not code related, but documentation and guides come to mind.
Maybe let people contribute more freely to the upstream wiki as they work
on tickets?

On Tue, Aug 9, 2016 at 6:34 AM, Jakub Hrozek  wrote:

> On Thu, Aug 04, 2016 at 10:20:37AM +0200, Jakub Hrozek wrote:
> > Hi,
> >
> > Over the last couple of weeks, I've talked to several people, mostly
> > engineers with not too much development experience, who said they would
> > like to start contributing little fixes to SSSD. I would like to find
> some
> > tasks that they can easily handle without making their had spin from
> > looking at harder stuff like asynchronous tevent code :-)
> >
> > In the past, we used the 'easyfix' tag in the sssd tickets. We still had
> > some, but not enough.  I tagged quite a few today, but if anyone knows
> > about more easy fixes, please tag them as well.
> > https://fedorahosted.org/sssd/report/34
> >
> > Can anybody think of more tasks to ask newcomes to do? I know adding
> > tests would be great, tests don't require C knowledge, but only Python.
> > But we would have to document how to write new tests, currently the
> > process is very convoluted.
> >
> > We also need to improve and fix documentation on the wiki.
> >
> > Anything else?
>
> btw I amended the Contribute page with so that the tasks for newcomers
> are better visible:
> https://fedorahosted.org/sssd/wiki/Contribute?action=diff&;
> version=46&old_version=45
> ___
> sssd-devel mailing list
> sssd-devel@lists.fedorahosted.org
> https://lists.fedorahosted.org/admin/lists/sssd-devel@
> lists.fedorahosted.org
>
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] intg: test nested membership

[Jakub Hrozek]

On Tue, Aug 09, 2016 at 12:36:11PM +0200, Jakub Hrozek wrote:
> On Wed, Aug 03, 2016 at 09:56:40AM +0200, Lukas Slebodnik wrote:
> > On (13/07/16 17:48), Lukas Slebodnik wrote:
> > >ehlo,
> > >
> > >attched patch is an integration test for regression #3093.
> > >I prepared a test and I let someone else to fix it :-)
> > >
> > >I will try to find more bugs in downstream tests.
> > >
> > >BTW we might move test from test_ts_cache somewhere else.
> > >But it was the fastest way how to write a test without enumeration.
> > >
> > 
> > As I previously wrote test_ts_cache is not the best place for this
> > test. Attached are patches which some changes in intg tests.
> > There is also test for grups with special characters
> > 
> > LS
> 
> Thank you, the only comment I have is that we should remove
> ldap_auth_disable_tls_never_use_in_production option. We don't use it in
> tests (we don't test authentication there at all at the moment) and it's
> better to not advertise the option.
> 
> I'll give a formal ACK once the CI run finishes.

btw while reading the patches I again realized the big amount of code
duplication we have between tests. Most of the fixtures are just
copy-pasted. This is not related to this patchset per-se, but I think
that during the 1.14.2 development, we should focus on making the
integration tests easier to run and debug. So we should have tickets to
track the issues in tests and reducing code duplication should be one of
them.
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH 1/2] LDAP: Adding support for SIGTERM signal

[Petr Cech]

On 08/09/2016 11:26 AM, Jakub Hrozek wrote:

On Mon, Aug 08, 2016 at 09:46:55AM +0200, Petr Cech wrote:

> On 08/04/2016 05:01 PM, Petr Cech wrote:

> > On 08/04/2016 04:35 PM, Petr Cech wrote:

> > > Hi list,
> > >
> > > there is the first version of patch for [1]. I need
> > > to investigate if we have the same issue in other
> > > *_childs.
> > >
> > > I tested it with sleep() added into ldap_child code.
> > >
> > > [1] https://fedorahosted.org/sssd/ticket/3106
> > >

> >
> > I know, it is not perfect. For example 2 seconds between SIGTERM and
> > SIGKILL should be constant. I will send the second version with other
> > childs, but not today.
> >
> > Regards

>
> Hello all,
>
> there is fixed patch set. There is only one minor change -- constant for
> time between signals.
>
> Is there better name for global_ccname_file_dummy? I did not want to break
> the naming... but it looks really long.

I think this name is fine, but I would prefer to not use the variable in
the code, except for setting it after the temporary file is created and
then set the variable back to NULL after the temporary file is renamed
to the real one.


Addressed.

Thanks Jakub, it looks better. :-)


> I looked at code of others children. I saw the same schema at
> src/providers/ad/ad_gpo_child.c:361 (temporary file renamed to solid one).
>
> In my opinion we could fix ad_qpo_child in next patch (not the highest
> priority for now) and make SIGTERM/SIGKILL more general. Maybe all child
> processes could have it.
>
> Regards
>
> --
> Petr^4 Čech


--
Petr^4 Čech
>From 4a79f2911168e4cafe78bc201f9961eea846d15d Mon Sep 17 00:00:00 2001
From: Petr Cech 
Date: Thu, 4 Aug 2016 16:25:28 +0200
Subject: [PATCH 1/2] LDAP: Adding support for SIGTERM signal

We add support for handling SIGTERM signal. If ldap_child receives
SIGTERM signal it removes temporary file.

Resolves:
https://fedorahosted.org/sssd/ticket/3106
---
 src/providers/ldap/ldap_child.c | 29 +
 1 file changed, 29 insertions(+)

diff --git a/src/providers/ldap/ldap_child.c b/src/providers/ldap/ldap_child.c
index 52c271f36cb57b774e6091e7322e4b1394c78969..2edfc30ec7b4a3ff03d86ae41f086a6128451a9b 100644
--- a/src/providers/ldap/ldap_child.c
+++ b/src/providers/ldap/ldap_child.c
@@ -33,6 +33,30 @@
 #include "providers/backend.h"
 #include "providers/krb5/krb5_common.h"
 
+char *global_ccname_file_dummy = NULL;
+
+static void sig_term_handler(int sig)
+{
+int ret;
+
+DEBUG(SSSDBG_FATAL_FAILURE, "Received signal [%s] [%i], shutting down\n",
+strsignal(sig), sig);
+
+if (global_ccname_file_dummy != NULL) {
+ret = unlink(global_ccname_file_dummy);
+if (ret != 0) {
+DEBUG(SSSDBG_FATAL_FAILURE, "Unlink file [%s] failed [%i][%s]\n",
+global_ccname_file_dummy,
+errno, strerror(errno));
+} else {
+DEBUG(SSSDBG_FATAL_FAILURE, "Unlink file [%s]\n",
+global_ccname_file_dummy);
+}
+}
+
+_exit(0);
+}
+
 static krb5_context krb5_error_ctx;
 #define LDAP_CHILD_DEBUG(level, error) KRB5_DEBUG(level, krb5_error_ctx, error)
 
@@ -405,6 +429,7 @@ static krb5_error_code ldap_child_get_tgt_sync(TALLOC_CTX *memctx,
   strerror(krberr), krberr);
 goto done;
 }
+global_ccname_file_dummy = ccname_file_dummy;
 
 ret = sss_unique_filename(tmp_ctx, ccname_file_dummy);
 if (ret != EOK) {
@@ -490,6 +515,7 @@ static krb5_error_code ldap_child_get_tgt_sync(TALLOC_CTX *memctx,
   "rename failed [%d][%s].\n", ret, strerror(ret));
 goto done;
 }
+global_ccname_file_dummy = NULL;
 
 krberr = 0;
 *ccname_out = talloc_steal(memctx, ccname);
@@ -631,6 +657,9 @@ int main(int argc, const char *argv[])
 }
 }
 
+BlockSignals(false, SIGTERM);
+CatchSignal(SIGTERM, sig_term_handler);
+
 DEBUG(SSSDBG_TRACE_FUNC, "ldap_child started.\n");
 
 main_ctx = talloc_new(NULL);
-- 
2.7.4

>From 221bf0bea9293a0192426144aaa7d39682300bb9 Mon Sep 17 00:00:00 2001
From: Petr Cech 
Date: Thu, 4 Aug 2016 16:27:40 +0200
Subject: [PATCH 2/2] LDAP: Adding SIGTERM signal before SIGKILL

We add better termination of ldap_child. If ldap_child reaches
the timeout for termination parent sents SIGTERM signal. Child
has 2 seconds for removing temporary file and exit.
If it is not sufficient there is SIGKILL send to the child.

Resolves:
https://fedorahosted.org/sssd/ticket/3106
---
 src/providers/ldap/sdap_child_helpers.c | 39 +
 src/util/child_common.h |  2 ++
 2 files changed, 37 insertions(+), 4 deletions(-)

diff --git a/src/providers/ldap/sdap_child_helpers.c b/src/providers/ldap/sdap_child_helpers.c
index 92642e8e47e530e5aef5f78bf1c7a2427979275e..9f9c9b57a7ca5e1217d56ab560cc941432084df8 100644
--- a/src/providers/ldap/sdap_child_helpers.c
+++ b/src/prov

[SSSD] Re: [PATCH] gpo: gPCMachineExtensionNames with just whitespaces

[Michal Židek]

Summary for Alexander (in CC):
- Regarding processing GPOs on the client.
- If groupPolicyContainer in AD has attribute
  gPCMachineExtensionNames that contains only whitespaces, SSSD
  fails to process GPOs and denies access to users
- if the gPCMachineExtensionNames is missing, it is Ok and
  SSSD skips such GPO (because we are only interested in
  Machine extensions)
- We have customer that has thousands of GPOs stored in AD and
  some of them have just ' ' (space) in the gPCMachineExtensionNames
  attribute. The AD administrators say that they created the GPOs
  using the GUI provided by AD.
- Treating the gPCMachineExtensionNames with just whitespaces the
  same way as if the gpcMachineExtensionNames was missing completely
  fixed the issue for the customer.

- Now, it would be good to support the fix with some links to
  documentation.

- I believe we should go with that fix, but could not find any
  documentation that would explicitly say something about just
  whitespaces  in the gpcMachineExtensionNames
- Gunter could also not find documentation that would say something
  about just whitespaces in that attribute, but believes that we should
  use the fix and skip such attributes.

Alexander, can you try to find something in the MSDN documentation,
that would support our fix? If not, then just what is your opinion?

Thanks (the original conversation is below - does not include Gunter
because that was on IRC).

On 08/09/2016 11:50 AM, Lukas Slebodnik wrote:

On (09/08/16 11:48), Jakub Hrozek wrote:

On Fri, Jul 29, 2016 at 05:40:44PM +0200, Michal Židek wrote:

Hi,

the attached patch fixes:
https://fedorahosted.org/sssd/ticket/3114

We have a user that can not login with
enforced GPO because of this. I do not
think it is a common issue, I could not
create groupPolicyContainer with gPCMachineExtensionNames
containing only whitespaces, but you can
create it with a script and the blank
value is not an error so we should handle it
properly (and maybe thre is a way in the
GUI maze to create such GPOs as well, I just
could not find it).

I was able to reproduce the same "sssd log path"
the user has and this patch fixed the issue.


If the user tested the patch, then I would say we should accept it.

Did you ask someone from the Samba team if this is the right thing to
do?


I asked Gunter and he said that we should ignore this
attribute if it contains just whitespaces. Btw. I can
confirm that this fixed the issue for the customer.
He is now hitting this:
https://fedorahosted.org/sssd/ticket/2751

which is already fixed in master.


It would be good to add link to the MSDN documentation.
Try to ask ab. He is quite familiar with the documentation.



I tried to find some MSDN documentation,
but nothing explicitly mentioned if the
attribute can contain just whitespaces or not.
Gunter could not find anything either.

However if the attribute is missing completely, it
is a valid GPO (we already ignore such GPOs).
So having it containing just whitespaces
is not too distant from that. I can ask Alexander
if he can find something in the documentation though.


LS
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org


___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: which tickets do we need to close before releasing 1.14.1?

[Pavel Březina]

On 08/09/2016 11:58 AM, Jakub Hrozek wrote:

I would say we want to include:
 https://fedorahosted.org/sssd/ticket/3110 - Access denied after
 activating user in 389ds

 https://fedorahosted.org/sssd/ticket/3120 - SSSD fails to start
 when ldap_user_extra_attrs contains mail

 https://fedorahosted.org/sssd/ticket/3101 - sssd does not start if
 sub-domain user is used with simple access provider

When these are included, I would like to tag 1.14.1. Any other opinions?


Also https://fedorahosted.org/sssd/ticket/3069
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] intg: test nested membership

[Jakub Hrozek]

On Wed, Aug 03, 2016 at 09:56:40AM +0200, Lukas Slebodnik wrote:
> On (13/07/16 17:48), Lukas Slebodnik wrote:
> >ehlo,
> >
> >attched patch is an integration test for regression #3093.
> >I prepared a test and I let someone else to fix it :-)
> >
> >I will try to find more bugs in downstream tests.
> >
> >BTW we might move test from test_ts_cache somewhere else.
> >But it was the fastest way how to write a test without enumeration.
> >
> 
> As I previously wrote test_ts_cache is not the best place for this
> test. Attached are patches which some changes in intg tests.
> There is also test for grups with special characters
> 
> LS

Thank you, the only comment I have is that we should remove
ldap_auth_disable_tls_never_use_in_production option. We don't use it in
tests (we don't test authentication there at all at the moment) and it's
better to not advertise the option.

I'll give a formal ACK once the CI run finishes.
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: tasks for newcomers or non-developers

[Jakub Hrozek]

On Thu, Aug 04, 2016 at 10:20:37AM +0200, Jakub Hrozek wrote:
> Hi,
> 
> Over the last couple of weeks, I've talked to several people, mostly
> engineers with not too much development experience, who said they would
> like to start contributing little fixes to SSSD. I would like to find some
> tasks that they can easily handle without making their had spin from
> looking at harder stuff like asynchronous tevent code :-)
> 
> In the past, we used the 'easyfix' tag in the sssd tickets. We still had
> some, but not enough.  I tagged quite a few today, but if anyone knows
> about more easy fixes, please tag them as well.
> https://fedorahosted.org/sssd/report/34
> 
> Can anybody think of more tasks to ask newcomes to do? I know adding
> tests would be great, tests don't require C knowledge, but only Python.
> But we would have to document how to write new tests, currently the
> process is very convoluted.
> 
> We also need to improve and fix documentation on the wiki.
> 
> Anything else?

btw I amended the Contribute page with so that the tasks for newcomers
are better visible:

https://fedorahosted.org/sssd/wiki/Contribute?action=diff&version=46&old_version=45
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] which tickets do we need to close before releasing 1.14.1?

[Jakub Hrozek]

I would say we want to include:
https://fedorahosted.org/sssd/ticket/3110 - Access denied after
activating user in 389ds

https://fedorahosted.org/sssd/ticket/3120 - SSSD fails to start
when ldap_user_extra_attrs contains mail 

https://fedorahosted.org/sssd/ticket/3101 - sssd does not start if
sub-domain user is used with simple access provider

When these are included, I would like to tag 1.14.1. Any other opinions?
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] gpo: gPCMachineExtensionNames with just whitespaces

[Lukas Slebodnik]

On (09/08/16 11:48), Jakub Hrozek wrote:
>On Fri, Jul 29, 2016 at 05:40:44PM +0200, Michal Židek wrote:
>> Hi,
>> 
>> the attached patch fixes:
>> https://fedorahosted.org/sssd/ticket/3114
>> 
>> We have a user that can not login with
>> enforced GPO because of this. I do not
>> think it is a common issue, I could not
>> create groupPolicyContainer with gPCMachineExtensionNames
>> containing only whitespaces, but you can
>> create it with a script and the blank
>> value is not an error so we should handle it
>> properly (and maybe thre is a way in the
>> GUI maze to create such GPOs as well, I just
>> could not find it).
>> 
>> I was able to reproduce the same "sssd log path"
>> the user has and this patch fixed the issue.
>
>If the user tested the patch, then I would say we should accept it.
>
>Did you ask someone from the Samba team if this is the right thing to
>do?
It would be good to add link to the MSDN documentation.
Try to ask ab. He is quite familiar with the documentation.

LS
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] gpo: gPCMachineExtensionNames with just whitespaces

[Jakub Hrozek]

On Fri, Jul 29, 2016 at 05:40:44PM +0200, Michal Židek wrote:
> Hi,
> 
> the attached patch fixes:
> https://fedorahosted.org/sssd/ticket/3114
> 
> We have a user that can not login with
> enforced GPO because of this. I do not
> think it is a common issue, I could not
> create groupPolicyContainer with gPCMachineExtensionNames
> containing only whitespaces, but you can
> create it with a script and the blank
> value is not an error so we should handle it
> properly (and maybe thre is a way in the
> GUI maze to create such GPOs as well, I just
> could not find it).
> 
> I was able to reproduce the same "sssd log path"
> the user has and this patch fixed the issue.

If the user tested the patch, then I would say we should accept it.

Did you ask someone from the Samba team if this is the right thing to
do?
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] Remove old DP interface

[Lukas Slebodnik]

On (09/08/16 10:50), Pavel Březina wrote:
>On 08/09/2016 10:47 AM, Lukas Slebodnik wrote:
>> On (09/08/16 10:35), Pavel Březina wrote:
>> > On 07/20/2016 11:10 AM, Pavel Březina wrote:
>> > > CI: http://sssd-ci.duckdns.org/logs/job/50/01/summary.html
>> > > 
>> > > The failure is about missing dependencies, unrelated to these patches.
>> > > 
>> > > It depends on the sssctl failover patches due to changes in attaching
>> > > dbus message to a talloc context. Now it is possible to also free the
>> > > message with both dbus_message_unref() and talloc_free(). Since the
>> > > sssctl patches are already in late review process I didn't want to
>> > > change them.
>> > 
>> > Bump.
>> I had a plan to test these patches
>> but there are still some regressions in sssd
>> and I would like to avoid introducing new ones without testing.
>> 
>> Do these patches block you or other patches?
>> 
>> LS
>
>I would like to build on top of those patches further improvements to sssctl,
>such as talloc memory report. That said, I'd like to get them reviewed sooner
>that later, it doesn't need to be pushed though.
OK, I will try to run tests as soon as possible

LS
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] Warn if ad_server contains IP address

[Jakub Hrozek]

On Fri, Aug 05, 2016 at 12:09:27PM -0400, Justin Stephenson wrote:
> Hi Lukas,
> 
> I sent a response on July 6th but perhaps there was an issue with the
> mailing list or some reason it did not go through.

Yes, we had issues with the mailing list back then (it was a Fedora
mailman bug that was fixed in the meantime)

> 
>Updated patch attached.
> 
>I moved the resolv_is_address() function declaration into the
>async_resolv.h file(so that it could be included in the
>cmocka/test_resolv_fake.c test but I am not sure if this is the
>correct approach). I also made the assumption of including my tests
>in the already existing test_resolv_fake.c file instead of a
>different file.
> 
>Also, I wasn't sure whether to use SSSDBG_MINOR_FAILURE or
>SSSDBG_CONF_SETTINGS debug log level.

I would say SSSDBG_IMPORTANT_INFO, we want to be loud here.

> 
>I am sure there are some corrections to make so I appreciate any
>feedback.

I haven't tested the patch yet, just read the diff, but the code looks
OK to me. I would just suggest to split the patch into two, one that
makes the resolv function public and adds the test and the other that
uses the function in the providers.
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] UTILS: Fixing duplication of pid file declaration

[Jakub Hrozek]

On Tue, Aug 09, 2016 at 11:27:12AM +0200, Jakub Hrozek wrote:
> On Fri, Aug 05, 2016 at 02:49:59PM +0200, Petr Cech wrote:
> > On 08/04/2016 11:06 AM, Jakub Hrozek wrote:
> > > On Thu, Aug 04, 2016 at 08:41:34AM +0200, Fabiano Fidêncio wrote:
> > > > On Tue, Jul 26, 2016 at 4:01 PM, Petr Cech  wrote:
> > > > > Hello list,
> > > > > 
> > > > > attached patch fixes duplication of pid file declaration. I hope that 
> > > > > the
> > > > > util/util.h is the right place for it. Another opinion are welcome.
> > > > 
> > > > LGTM!
> > > > 
> > > > I'd wait till someone else reviews the patch, in case you want to be
> > > > sure that util/util.h is the right place for the defines.
> > > 
> > > It is, but I don't think that MONITOR_NAME belongs there. I think the
> > > pidfile declaration can just use hardcoded sssd (or just #define
> > > PIDFILE_BASENAME to "sssd") and MONITOR_NAME should stay in monitor.c
> > 
> > Right, thank you. Fixed patch attached :-)
> 
> ACK
> 
> CI: http://sssd-ci.duckdns.org/logs/job/51/12/summary.html

* master: 08cd034c8584b6f058cf565ce66f7f9f7120622f
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] NSS: Do not check local users with disabled local_negative_timeout

[Jakub Hrozek]

On Tue, Aug 09, 2016 at 07:54:15AM +0200, Petr Cech wrote:
> On 08/08/2016 04:20 PM, Petr Cech wrote:
> > On 08/08/2016 04:09 PM, Lukas Slebodnik wrote:
> > > On (08/08/16 15:36), Petr Cech wrote:
> > > > On 08/08/2016 03:14 PM, Petr Cech wrote:
> > > > > On 08/08/2016 02:40 PM, Lukas Slebodnik wrote:
> > > > > > ehlo,
> > > > > > 
> > > > > > The simple reproducer is to use getent passwd,group with 
> > > > > > non-existing
> > > > > > entry.
> > > > > > Without the patch you will see that "/var/lib/sss/mc/passwd" is 
> > > > > > opened
> > > > > > twice. Onece with mode "rw" opened by sssd_nss and the 2nd time
> > > > > > with "r-" opened by sssd-client (getpwnam, getpwuid, getgrnam,
> > > > > > getgrgid)
> > > > > > 
> > > > > > LS
> > > > > 
> > > > > Hello Lukas,
> > > > > 
> > > > > it looks good to me. I am waiting for CI tests.
> > > > 
> > > > Hi Lukas,
> > > > 
> > > > CI failed:
> > > > http://sssd-ci.duckdns.org/logs/job/51/05/summary.html
> > > > 
> > > > On F24 during valgrind negcache-tests
> > > > 
> > > > http://sssd-ci.duckdns:.org/logs/job/51/05/fedora24/ci-build-debug/ci-make-check-valgrind.log
> > > > 
> > > > 
> > > > I didn't find more descriptive log. Is there any?
> > > > 
> > > Yes,
> > > http://sssd-ci.duckdns.org/logs/job/51/05/fedora24/ci-build-debug/src/tests/cwrap/negcache-tests.log
> > > 
> > > http://sssd-ci.duckdns.org/logs/job/51/05/fedora24/ci-build-debug/src/tests/cwrap/responder_common-tests.1122.valgrind.log
> > > 
> > > 
> > > 
> > > Anyway,
> > > attached is a patch which does not require changes in unit test.
> > > 
> > > LS
> > 
> > Great,
> > 
> > this looks better to me. (LBTM?)
> > 
> > I am waiting for CI tests.
> > 
> > Regards
> > 
> 
> CI nearly passed:
> http://sssd-ci.duckdns.org/logs/job/51/07/summary.html
> The failure is not connected.
> 
> => ACK

* master: 950716d2087446205c84f00b371f468d6ead1ec2
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] UTILS: Fixing duplication of pid file declaration

[Jakub Hrozek]

On Fri, Aug 05, 2016 at 02:49:59PM +0200, Petr Cech wrote:
> On 08/04/2016 11:06 AM, Jakub Hrozek wrote:
> > On Thu, Aug 04, 2016 at 08:41:34AM +0200, Fabiano Fidêncio wrote:
> > > On Tue, Jul 26, 2016 at 4:01 PM, Petr Cech  wrote:
> > > > Hello list,
> > > > 
> > > > attached patch fixes duplication of pid file declaration. I hope that 
> > > > the
> > > > util/util.h is the right place for it. Another opinion are welcome.
> > > 
> > > LGTM!
> > > 
> > > I'd wait till someone else reviews the patch, in case you want to be
> > > sure that util/util.h is the right place for the defines.
> > 
> > It is, but I don't think that MONITOR_NAME belongs there. I think the
> > pidfile declaration can just use hardcoded sssd (or just #define
> > PIDFILE_BASENAME to "sssd") and MONITOR_NAME should stay in monitor.c
> 
> Right, thank you. Fixed patch attached :-)

ACK

CI: http://sssd-ci.duckdns.org/logs/job/51/12/summary.html
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH 1/2] LDAP: Adding support for SIGTERM signal

[Jakub Hrozek]

On Mon, Aug 08, 2016 at 09:46:55AM +0200, Petr Cech wrote:
> On 08/04/2016 05:01 PM, Petr Cech wrote:
> > On 08/04/2016 04:35 PM, Petr Cech wrote:
> > > Hi list,
> > > 
> > > there is the first version of patch for [1]. I need
> > > to investigate if we have the same issue in other
> > > *_childs.
> > > 
> > > I tested it with sleep() added into ldap_child code.
> > > 
> > > [1] https://fedorahosted.org/sssd/ticket/3106
> > > 
> > 
> > I know, it is not perfect. For example 2 seconds between SIGTERM and
> > SIGKILL should be constant. I will send the second version with other
> > childs, but not today.
> > 
> > Regards
> 
> Hello all,
> 
> there is fixed patch set. There is only one minor change -- constant for
> time between signals.
> 
> Is there better name for global_ccname_file_dummy? I did not want to break
> the naming... but it looks really long.

I think this name is fine, but I would prefer to not use the variable in
the code, except for setting it after the temporary file is created and
then set the variable back to NULL after the temporary file is renamed
to the real one.

> 
> I looked at code of others children. I saw the same schema at
> src/providers/ad/ad_gpo_child.c:361 (temporary file renamed to solid one).
> 
> In my opinion we could fix ad_qpo_child in next patch (not the highest
> priority for now) and make SIGTERM/SIGKILL more general. Maybe all child
> processes could have it.
> 
> Regards
> 
> -- 
> Petr^4 Čech
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCHES] sssctl: print active server and server list

[Jakub Hrozek]

On Fri, Aug 05, 2016 at 12:02:19PM +0200, Pavel Březina wrote:
> On 07/25/2016 12:55 PM, Pavel Březina wrote:
> > On 07/20/2016 03:03 PM, Jakub Hrozek wrote:
> > > On Wed, Jul 20, 2016 at 03:00:14PM +0200, Jakub Hrozek wrote:
> > > > On Tue, Jul 19, 2016 at 12:20:16PM +0200, Pavel Březina wrote:
> > > > > On 07/18/2016 03:23 PM, Jakub Hrozek wrote:
> > > > > > On Thu, Jul 14, 2016 at 11:39:40AM +0200, Pavel Březina wrote:
> > > > > > >   From 6af064012844f3e61b9fa5b502bebe44e4620ffd Mon Sep 17
> > > > > > > 00:00:00 2001
> > > > > > > From: =?UTF-8?q?Pavel=20B=C5=99ezina?= 
> > > > > > > Date: Tue, 28 Jun 2016 11:40:16 +0200
> > > > > > > Subject: [PATCH 1/7] rdp: add ability to forward reply to the
> > > > > > > client request
> > > > > > 
> > > > > > LGTM, but I didn't test yet.
> > > > > > 
> > > > > > >   From 2ad0b97f9e435ac5f57caf37479304c16d0424c9 Mon Sep 17
> > > > > > > 00:00:00 2001
> > > > > > > From: =?UTF-8?q?Pavel=20B=C5=99ezina?= 
> > > > > > > Date: Tue, 28 Jun 2016 15:29:39 +0200
> > > > > > > Subject: [PATCH 2/7] sbus: add sbus_request_reply_error()
> > > > > > 
> > > > > > OK, but what is the plan on the other calls? I wouldn't like us to 
> > > > > > end
> > > > > > up with two competing implementations of the same (again)..
> > > > > 
> > > > > It is sometimes desirable to pass DBusError as an argument but in
> > > > > much more
> > > > > cases it is simpler to just use this call. The plan is to change the
> > > > > calls
> > > > > where it is desirable, I'll send separate patch later for this. Here I
> > > > > changed it only for sssctl related code.
> > > > > 
> > > > > > >   From 0f508095599b3e04573a21a600b4f42e172cc304 Mon Sep 17
> > > > > > > 00:00:00 2001
> > > > > > > From: =?UTF-8?q?Pavel=20B=C5=99ezina?= 
> > > > > > > Date: Wed, 29 Jun 2016 12:35:59 +0200
> > > > > > > Subject: [PATCH 3/7] sbus: add utility function to simplify
> > > > > > > message and reply
> > > > > > >handling
> > > > > > > 
> > > > > > > This patch adds the ability to hook DBusMessage to a talloc 
> > > > > > > context
> > > > > > > to remove the need of calling dbus_message_unref(). It also 
> > > > > > > provides
> > > > > > > an automatical way to detect error in a reply so the caller does
> > > > > > > not need to parse it manually and the whole code around DBusError
> > > > > > > can be avoided.
> > > > > > 
> > > > > > One code readability nitpick..
> > > > > > 
> > > > > > > +errno_t sbus_check_reply(DBusMessage *reply)
> > > > > > > +{
> > > > > > > +dbus_bool_t bret;
> > > > > > > +DBusError error;
> > > > > > > +errno_t ret;
> > > > > > > +
> > > > > > > +dbus_error_init(&error);
> > > > > > > +
> > > > > > > +switch (dbus_message_get_type(reply)) {
> > > > > > > +case DBUS_MESSAGE_TYPE_METHOD_RETURN:
> > > > > > > +ret = EOK;
> > > > > > > +goto done;
> > > > > > 
> > > > > > Can you use break in the non-failure cases? I would say this would 
> > > > > > be
> > > > > > more readable..
> > > > > 
> > > > > Done.
> > > > > 
> > > > > > > +
> > > > > > > +case DBUS_MESSAGE_TYPE_ERROR:
> > > > > > > +bret = dbus_set_error_from_message(&error, reply);
> > > > > > > +if (bret == false) {
> > > > > > > +DEBUG(SSSDBG_CRIT_FAILURE, "Unable to read error from
> > > > > > > message\n");
> > > > > > > +ret = EIO;
> > > > > > > +goto done;
> > > > > > > +}
> > > > > > > +
> > > > > > > +DEBUG(SSSDBG_CRIT_FAILURE, "D-Bus error [%s]: %s\n",
> > > > > > > +  error.name, (error.message == NULL ? "(null)" :
> > > > > > > error.message));
> > > > > > > +ret = sbus_error_to_errno(&error);
> > > > > > > +goto done;
> > > > > > > +default:
> > > > > > > +DEBUG(SSSDBG_CRIT_FAILURE, "Unexpected D-Bus message
> > > > > > > type?\n");
> > > > > > > +ret = ERR_INTERNAL;
> > > > > > > +goto done;
> > > > > > > +}
> > > > > > > +
> > > > > > > +done:
> > > > > > > +dbus_error_free(&error);
> > > > > > > +
> > > > > > > +return ret;
> > > > > > > +}
> > > > > > 
> > > > > > The rest looks good to me.
> > > > > > 
> > > > > > >   From 93b4f2e9077f1959d563ba8e4d3f41a9764cc8c6 Mon Sep 17
> > > > > > > 00:00:00 2001
> > > > > > > From: =?UTF-8?q?Pavel=20B=C5=99ezina?= 
> > > > > > > Date: Wed, 29 Jun 2016 14:03:38 +0200
> > > > > > > Subject: [PATCH 4/7] sssctl: use talloc with sifp
> > > > > > 
> > > > > > LGTM
> > > > > > 
> > > > > > >   From 33f5fd436a0fc3d5d1eac970b66c174bb2da6189 Mon Sep 17
> > > > > > > 00:00:00 2001
> > > > > > > From: =?UTF-8?q?Pavel=20B=C5=99ezina?= 
> > > > > > > Date: Wed, 29 Jun 2016 14:58:37 +0200
> > > > > > > Subject: [PATCH 5/7] failover: mark subdomain service with sd_ 
> > > > > > > prefix
> > > > > > 
> > > > > > Did you test the failover with subdomain? I wonder if we rely on the
> > > > > > service name somewhere, but I can't remember.
> > > > > 
> > > > > Yes. The name is generated and stored in service con

[SSSD] Re: [PATCH] DP: Initialize D-Bus as soon as possible

[Jakub Hrozek]

On Tue, Aug 09, 2016 at 11:15:29AM +0200, Pavel Březina wrote:
> On 08/09/2016 11:08 AM, Jakub Hrozek wrote:
> > On Tue, Aug 09, 2016 at 11:03:43AM +0200, Pavel Březina wrote:
> > > On 08/09/2016 10:53 AM, Jakub Hrozek wrote:
> > > > On Tue, Aug 09, 2016 at 10:39:25AM +0200, Jakub Hrozek wrote:
> > > > > On Mon, Aug 08, 2016 at 10:28:41AM +0200, Petr Cech wrote:
> > > > > > 
> > > > > > 
> > > > > > On 08/08/2016 10:26 AM, Pavel Březina wrote:
> > > > > > > On 08/05/2016 01:02 PM, Petr Cech wrote:
> > > > > > > > On 08/05/2016 12:19 PM, Petr Cech wrote:
> > > > > > > > > On 08/05/2016 11:41 AM, Pavel Březina wrote:
> > > > > > > > > > https://fedorahosted.org/sssd/ticket/3111
> > > > > > > > > 
> > > > > > > > > Hi list,
> > > > > > > > > 
> > > > > > > > > LGTM
> > > > > > > > > 
> > > > > > > > > I am waiting or CI.
> > > > > > > > 
> > > > > > > > CI:
> > > > > > > > http://sssd-ci.duckdns.org/logs/job/50/88/summary.html
> > > > > > > > 
> > > > > > > > It failed on F24 on dyndns:
> > > > > > > > http://sssd-ci.duckdns.org/logs/job/50/88/fedora24/ci-build-coverage/dyndns-tests.log
> > > > > > > > 
> > > > > > > > 
> > > > > > > > 
> > > > > > > > It failed by reaching timeout. I think, it is not connected. 
> > > > > > > > What do you
> > > > > > > > think, Pavel?
> > > > > > > > 
> > > > > > > > Regards
> > > > > > > > 
> > > > > > > 
> > > > > > > Not connected.
> > > > > > 
> > > > > > ACK
> > > > > 
> > > > > * master: a16e7a370d0b564a5edad7791d2421d175c0787a
> > > > 
> > > > By the way, even though sssd started, so the bug is fixed, the time it
> > > > takes for the first lookup is IMO just too long. We should abort the
> > > > identity lookups sooner and just go to the cache, otherwise lookups on
> > > > system startup might delay the init process.
> > > > 
> > > > Do you want me to open another bug?
> > > 
> > > Code hangs in gethostbyname in ldap. Timeouts have the same value as in
> > > previous versions.
> > 
> > Yes, but can the responder abort the request sooner?
> 
> Currently the timeout on responder side is 150 seconds. This can be
> decreased, but is it safe to descrease it to lower value than 10? (I think
> 10 seconds is the timeout in gethostbyname).

For user lookups, I think it can, I don't think a user lookup should
take that long. but initgroups and group requests shold have a longer
timeout. It might also be nice to use a different timeout for cases
where there is something in the cache (and we have something to return)
and where the cache is totally empty.

But this is not that important, unless someone hits a case where this
breaks some scenario.
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] DP: Initialize D-Bus as soon as possible

[Pavel Březina]

On 08/09/2016 11:08 AM, Jakub Hrozek wrote:

On Tue, Aug 09, 2016 at 11:03:43AM +0200, Pavel Březina wrote:

On 08/09/2016 10:53 AM, Jakub Hrozek wrote:

On Tue, Aug 09, 2016 at 10:39:25AM +0200, Jakub Hrozek wrote:

On Mon, Aug 08, 2016 at 10:28:41AM +0200, Petr Cech wrote:



On 08/08/2016 10:26 AM, Pavel Březina wrote:

On 08/05/2016 01:02 PM, Petr Cech wrote:

On 08/05/2016 12:19 PM, Petr Cech wrote:

On 08/05/2016 11:41 AM, Pavel Březina wrote:

https://fedorahosted.org/sssd/ticket/3111


Hi list,

LGTM

I am waiting or CI.


CI:
http://sssd-ci.duckdns.org/logs/job/50/88/summary.html

It failed on F24 on dyndns:
http://sssd-ci.duckdns.org/logs/job/50/88/fedora24/ci-build-coverage/dyndns-tests.log



It failed by reaching timeout. I think, it is not connected. What do you
think, Pavel?

Regards



Not connected.


ACK


* master: a16e7a370d0b564a5edad7791d2421d175c0787a


By the way, even though sssd started, so the bug is fixed, the time it
takes for the first lookup is IMO just too long. We should abort the
identity lookups sooner and just go to the cache, otherwise lookups on
system startup might delay the init process.

Do you want me to open another bug?


Code hangs in gethostbyname in ldap. Timeouts have the same value as in
previous versions.


Yes, but can the responder abort the request sooner?


Currently the timeout on responder side is 150 seconds. This can be 
decreased, but is it safe to descrease it to lower value than 10? (I 
think 10 seconds is the timeout in gethostbyname).

___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] DP: Initialize D-Bus as soon as possible

[Jakub Hrozek]

On Tue, Aug 09, 2016 at 11:03:43AM +0200, Pavel Březina wrote:
> On 08/09/2016 10:53 AM, Jakub Hrozek wrote:
> > On Tue, Aug 09, 2016 at 10:39:25AM +0200, Jakub Hrozek wrote:
> > > On Mon, Aug 08, 2016 at 10:28:41AM +0200, Petr Cech wrote:
> > > > 
> > > > 
> > > > On 08/08/2016 10:26 AM, Pavel Březina wrote:
> > > > > On 08/05/2016 01:02 PM, Petr Cech wrote:
> > > > > > On 08/05/2016 12:19 PM, Petr Cech wrote:
> > > > > > > On 08/05/2016 11:41 AM, Pavel Březina wrote:
> > > > > > > > https://fedorahosted.org/sssd/ticket/3111
> > > > > > > 
> > > > > > > Hi list,
> > > > > > > 
> > > > > > > LGTM
> > > > > > > 
> > > > > > > I am waiting or CI.
> > > > > > 
> > > > > > CI:
> > > > > > http://sssd-ci.duckdns.org/logs/job/50/88/summary.html
> > > > > > 
> > > > > > It failed on F24 on dyndns:
> > > > > > http://sssd-ci.duckdns.org/logs/job/50/88/fedora24/ci-build-coverage/dyndns-tests.log
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > It failed by reaching timeout. I think, it is not connected. What 
> > > > > > do you
> > > > > > think, Pavel?
> > > > > > 
> > > > > > Regards
> > > > > > 
> > > > > 
> > > > > Not connected.
> > > > 
> > > > ACK
> > > 
> > > * master: a16e7a370d0b564a5edad7791d2421d175c0787a
> > 
> > By the way, even though sssd started, so the bug is fixed, the time it
> > takes for the first lookup is IMO just too long. We should abort the
> > identity lookups sooner and just go to the cache, otherwise lookups on
> > system startup might delay the init process.
> > 
> > Do you want me to open another bug?
> 
> Code hangs in gethostbyname in ldap. Timeouts have the same value as in
> previous versions.

Yes, but can the responder abort the request sooner?
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH SET] AD_PROVIDER: ad_enabled_domains

[Jakub Hrozek]

On Mon, Jul 25, 2016 at 06:18:28PM +0200, Petr Cech wrote:
> Hello,
> 
> there is fixed patch set attached.
> 
> Segmentation fault was caused by wrong pointer :-(, sorry.
> 
> This new patch set has new debug message. I am open to dissccus the
> debug_level and content of message. Any improving idea?
> 
> I hit one issue during testing -- sometimes if I am connected to subdomain
> and I enable only sibling subdomain (the master is added automaticaly) and
> forest root is not enabled -- I see only master and sibling not. But if I
> added sleep for cycle (for using dbg) to function ad_subdomains_init()
> everythink is OK.
> Any idea?

Can you test that case with valgrind? This sounds like some uninitilized
variable condition.

> 
> Anyway, I would like to ask you for testing.
> 
> Regards
> 
> -- 
> Petr^4 Čech

> From 7fd9ad8f42f274463c1d107b195d21290fd0b0f2 Mon Sep 17 00:00:00 2001
> From: Petr Cech 
> Date: Fri, 13 May 2016 05:21:07 -0400
> Subject: [PATCH 1/5] AD_PROVIDER: Add ad_enabled_domains option

ACK

> From dfa6e063d3ef4850a7809e2f5a6d3826ea061b27 Mon Sep 17 00:00:00 2001
> From: Petr Cech 
> Date: Tue, 21 Jun 2016 08:34:15 +0200
> Subject: [PATCH 2/5] AD_PROVIDER: Initializing of ad_enabled_domains
> 
> We add ad_enabled_domains into ad_subdomains_ctx.
> 
> Resolves:
> https://fedorahosted.org/sssd/ticket/2828
> ---
>  src/providers/ad/ad_subdomains.c | 82 
> 
>  1 file changed, 82 insertions(+)
> 
> diff --git a/src/providers/ad/ad_subdomains.c 
> b/src/providers/ad/ad_subdomains.c
> index 
> e9da04e384e598927f9c8c203a751bcccd29e895..9c0afb19418e44a3e3daa661baf1c7a82439d60d
>  100644
> --- a/src/providers/ad/ad_subdomains.c
> +++ b/src/providers/ad/ad_subdomains.c
> @@ -57,6 +57,79 @@
>  /* do not refresh more often than every 5 seconds for now */
>  #define AD_SUBDOMAIN_REFRESH_LIMIT 5
>  
> +static errno_t ad_get_enabled_domains(TALLOC_CTX *mem_ctx,
> +  struct ad_id_ctx *ad_id_ctx,
> +  const char *ad_domain,
> +  const char ***_ad_enabled_domains)
> +{
> +int ret;
> +const char *str;
> +const char *option_name;
> +const char **domains = NULL;
> +int count;
> +bool is_ad_in_domains;
> +TALLOC_CTX *tmp_ctx = NULL;
> +
> +tmp_ctx = talloc_new(NULL);
> +if (tmp_ctx == NULL) {
> +return ENOMEM;
> +}
> +
> +str = dp_opt_get_cstring(ad_id_ctx->ad_options->basic, 
> AD_ENABLED_DOMAINS);
> +if (str == NULL) {
> +_ad_enabled_domains = NULL;

I think you wanted to dereference the pointer here? (it might also be a
good idea to return EINVAL if the caller passed NULL as the output pointer)

> +ret = EOK;
> +goto done;
> +}
> +

> From 4cd5c77f09750f9eb20c91eadc4759c3e4166093 Mon Sep 17 00:00:00 2001
> From: Petr Cech 
> Date: Tue, 21 Jun 2016 09:48:52 +0200
> Subject: [PATCH 3/5] AD_PROVIDER: ad_enabled_domains - only master
> 
> We can skip looking up other domains if option ad_enabled_domains
> contains only master domain.
> 
> Resolves:
> https://fedorahosted.org/sssd/ticket/2828
> ---
>  src/providers/ad/ad_subdomains.c | 16 
>  1 file changed, 16 insertions(+)
> 
> diff --git a/src/providers/ad/ad_subdomains.c 
> b/src/providers/ad/ad_subdomains.c
> index 
> 9c0afb19418e44a3e3daa661baf1c7a82439d60d..eaa85f802dbb4022dc632cc4c05d89685eccd901
>  100644
> --- a/src/providers/ad/ad_subdomains.c
> +++ b/src/providers/ad/ad_subdomains.c
> @@ -1163,6 +1163,7 @@ static void ad_subdomains_refresh_connect_done(struct 
> tevent_req *subreq)
>  return;
>  }
>  
> +/* connect to the DC we are a member of */
>  subreq = ad_master_domain_send(state, state->ev, state->id_ctx->conn,
> state->sdap_op, 
> state->sd_ctx->domain_name);
>  if (subreq == NULL) {
> @@ -1211,6 +1212,21 @@ static void ad_subdomains_refresh_master_done(struct 
> tevent_req *subreq)
>  goto done;
>  }
>  
> +/*
> + * If ad_enabled_domains contains only master domain
> + * we shouldn't lookup other domains.
> + */
> +if (state->sd_ctx->ad_enabled_domains != NULL) {
> +if (talloc_array_length(state->sd_ctx->ad_enabled_domains) == 2) {
> +if (strcmp(state->sd_ctx->ad_enabled_domains[0],
> +   state->be_ctx->domain->name) == 0) {

I only notices this now, but IIRC the domains are case insensitive,
shouldn't we use strcasecmp here?

> +DEBUG(SSSDBG_TRACE_FUNC,
> +  "No other enabled domain than master.\n");
> +goto done;
> +}
> +}
> +}
> +
>  subreq = ad_get_root_domain_send(state, state->ev, forest,
>   sdap_id_op_handle(state->sdap_op),
>   state->sd_ctx);
> -- 
> 2.7.4
> 

> From 0d2b0c67875663d0e614b8361152ec7edc14c37e Mo

[SSSD] Re: [PATCH] DP: Initialize D-Bus as soon as possible

[Pavel Březina]

On 08/09/2016 10:53 AM, Jakub Hrozek wrote:

On Tue, Aug 09, 2016 at 10:39:25AM +0200, Jakub Hrozek wrote:

On Mon, Aug 08, 2016 at 10:28:41AM +0200, Petr Cech wrote:



On 08/08/2016 10:26 AM, Pavel Březina wrote:

On 08/05/2016 01:02 PM, Petr Cech wrote:

On 08/05/2016 12:19 PM, Petr Cech wrote:

On 08/05/2016 11:41 AM, Pavel Březina wrote:

https://fedorahosted.org/sssd/ticket/3111


Hi list,

LGTM

I am waiting or CI.


CI:
http://sssd-ci.duckdns.org/logs/job/50/88/summary.html

It failed on F24 on dyndns:
http://sssd-ci.duckdns.org/logs/job/50/88/fedora24/ci-build-coverage/dyndns-tests.log



It failed by reaching timeout. I think, it is not connected. What do you
think, Pavel?

Regards



Not connected.


ACK


* master: a16e7a370d0b564a5edad7791d2421d175c0787a


By the way, even though sssd started, so the bug is fixed, the time it
takes for the first lookup is IMO just too long. We should abort the
identity lookups sooner and just go to the cache, otherwise lookups on
system startup might delay the init process.

Do you want me to open another bug?


Code hangs in gethostbyname in ldap. Timeouts have the same value as in 
previous versions.

___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] DP: Initialize D-Bus as soon as possible

[Jakub Hrozek]

On Tue, Aug 09, 2016 at 10:39:25AM +0200, Jakub Hrozek wrote:
> On Mon, Aug 08, 2016 at 10:28:41AM +0200, Petr Cech wrote:
> > 
> > 
> > On 08/08/2016 10:26 AM, Pavel Březina wrote:
> > > On 08/05/2016 01:02 PM, Petr Cech wrote:
> > > > On 08/05/2016 12:19 PM, Petr Cech wrote:
> > > > > On 08/05/2016 11:41 AM, Pavel Březina wrote:
> > > > > > https://fedorahosted.org/sssd/ticket/3111
> > > > > 
> > > > > Hi list,
> > > > > 
> > > > > LGTM
> > > > > 
> > > > > I am waiting or CI.
> > > > 
> > > > CI:
> > > > http://sssd-ci.duckdns.org/logs/job/50/88/summary.html
> > > > 
> > > > It failed on F24 on dyndns:
> > > > http://sssd-ci.duckdns.org/logs/job/50/88/fedora24/ci-build-coverage/dyndns-tests.log
> > > > 
> > > > 
> > > > 
> > > > It failed by reaching timeout. I think, it is not connected. What do you
> > > > think, Pavel?
> > > > 
> > > > Regards
> > > > 
> > > 
> > > Not connected.
> > 
> > ACK
> 
> * master: a16e7a370d0b564a5edad7791d2421d175c0787a

By the way, even though sssd started, so the bug is fixed, the time it
takes for the first lookup is IMO just too long. We should abort the
identity lookups sooner and just go to the cache, otherwise lookups on
system startup might delay the init process.

Do you want me to open another bug?
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] Remove old DP interface

[Pavel Březina]

On 08/09/2016 10:47 AM, Lukas Slebodnik wrote:

On (09/08/16 10:35), Pavel Březina wrote:

On 07/20/2016 11:10 AM, Pavel Březina wrote:

CI: http://sssd-ci.duckdns.org/logs/job/50/01/summary.html

The failure is about missing dependencies, unrelated to these patches.

It depends on the sssctl failover patches due to changes in attaching
dbus message to a talloc context. Now it is possible to also free the
message with both dbus_message_unref() and talloc_free(). Since the
sssctl patches are already in late review process I didn't want to
change them.


Bump.

I had a plan to test these patches
but there are still some regressions in sssd
and I would like to avoid introducing new ones without testing.

Do these patches block you or other patches?

LS


I would like to build on top of those patches further improvements to 
sssctl, such as talloc memory report. That said, I'd like to get them 
reviewed sooner that later, it doesn't need to be pushed though.

___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] Remove old DP interface

[Lukas Slebodnik]

On (09/08/16 10:35), Pavel Březina wrote:
>On 07/20/2016 11:10 AM, Pavel Březina wrote:
>> CI: http://sssd-ci.duckdns.org/logs/job/50/01/summary.html
>> 
>> The failure is about missing dependencies, unrelated to these patches.
>> 
>> It depends on the sssctl failover patches due to changes in attaching
>> dbus message to a talloc context. Now it is possible to also free the
>> message with both dbus_message_unref() and talloc_free(). Since the
>> sssctl patches are already in late review process I didn't want to
>> change them.
>
>Bump.
I had a plan to test these patches
but there are still some regressions in sssd
and I would like to avoid introducing new ones without testing.

Do these patches block you or other patches?

LS
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] sssctl: Generic help for cache-upgrade and config-check

[Jakub Hrozek]

On Mon, Aug 08, 2016 at 10:45:32AM +0200, Pavel Březina wrote:
> On 08/05/2016 12:47 PM, Michal Židek wrote:
> > On 08/05/2016 12:01 PM, Pavel Březina wrote:
> > > On 07/26/2016 04:43 PM, Michal Židek wrote:
> > > > Hi!
> > > > 
> > > > Attached is patch for ticket:
> > > > https://fedorahosted.org/sssd/ticket/3086
> > > > 
> > > > This patch applies on top of the patches from
> > > > thread:
> > > > [SSSD] [PATCH] sssctl: Consistent commands naming
> > > > 
> > > > Michal
> > > 
> > > Hi, I believe you can use NULL instead of options.
> > 
> > Yes. New patch attached.
> > 
> > Michal
> 
> Ack.

* master: 55857e924977dbc66958f8033c6b38d6262ee631
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] DP: Initialize D-Bus as soon as possible

[Jakub Hrozek]

On Mon, Aug 08, 2016 at 10:28:41AM +0200, Petr Cech wrote:
> 
> 
> On 08/08/2016 10:26 AM, Pavel Březina wrote:
> > On 08/05/2016 01:02 PM, Petr Cech wrote:
> > > On 08/05/2016 12:19 PM, Petr Cech wrote:
> > > > On 08/05/2016 11:41 AM, Pavel Březina wrote:
> > > > > https://fedorahosted.org/sssd/ticket/3111
> > > > 
> > > > Hi list,
> > > > 
> > > > LGTM
> > > > 
> > > > I am waiting or CI.
> > > 
> > > CI:
> > > http://sssd-ci.duckdns.org/logs/job/50/88/summary.html
> > > 
> > > It failed on F24 on dyndns:
> > > http://sssd-ci.duckdns.org/logs/job/50/88/fedora24/ci-build-coverage/dyndns-tests.log
> > > 
> > > 
> > > 
> > > It failed by reaching timeout. I think, it is not connected. What do you
> > > think, Pavel?
> > > 
> > > Regards
> > > 
> > 
> > Not connected.
> 
> ACK

* master: a16e7a370d0b564a5edad7791d2421d175c0787a
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] Remove old DP interface

[Pavel Březina]

On 07/20/2016 11:10 AM, Pavel Březina wrote:

CI: http://sssd-ci.duckdns.org/logs/job/50/01/summary.html

The failure is about missing dependencies, unrelated to these patches.

It depends on the sssctl failover patches due to changes in attaching
dbus message to a talloc context. Now it is possible to also free the
message with both dbus_message_unref() and talloc_free(). Since the
sssctl patches are already in late review process I didn't want to
change them.


Bump.
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] PROVIDERS: Default values in debug

[Pavel Březina]

On 08/08/2016 01:47 PM, Petr Cech wrote:

On 08/05/2016 03:58 PM, Lukas Slebodnik wrote:

On (29/07/16 10:46), Petr Cech wrote:

On 07/29/2016 10:17 AM, Lukas Slebodnik wrote:

On (29/07/16 10:05), Petr Cech wrote:

On 07/29/2016 09:52 AM, Lukas Slebodnik wrote:

On (29/07/16 09:41), Petr Cech wrote:

On 07/29/2016 09:23 AM, Lukas Slebodnik wrote:

What is a benefit of string "default ".


Benefit is that we provide more information.


How does it improve reading debug messages with very high debug
level?


This patch doesn't improve reading in such way.


Why should we care wheter it's default or not?


I was in situation during debug that I needed to know if value
was default or
not.


It does not explain why we should care wheter it's default or not.



This debug messages have so high level that it will not visible
in standard
deployment. Is there reason why we would like not to say what is
default?


The default values are described in man page and from sssd point
of view
it does not matter whether the value is default or or it was
explicitly
set to default value in configuration file or it has non-default
value.

I'm sorry I asked first and I didn't get a sufficient answer.
So one more time
Why should we care whether it's default or not?


From SSSD point of view we don't care if value is default or not.
It doesn't
influence logic of sssd.

From developer point of view it might be relevant. And that's my
motivation
for sending it.


Would you be so kind and could you explain why it is importatn from
developers
point of view?

The default values are described in man page and moreover developer can
see default in header/implementation files.

And I still miss a benefit of seeing the default value in log.
Neither for developer not for other users.
Could you elaborate a little bit. Or could you give few examples
when is such
information useful?


The patch don't show default values as such. It just tags the default
values.
I was in situation that I needed to know what values comes from
config file
and what values are from default 'automagic'. Yes I could study every
option
one by one in man page or in code. But I just added this tag.
That's all. I don't have any other reason or something like that.

If you think that I use wrong way how to obtain information about
defaults/non-defaults -- just say it.

On the other hand, I think it might be a little shortcut for less
experienced
to tag the default options in debug message.

I am sorry that's all that I have to this topic. And if you think
that such
improvement is needless, just nack it.



I'm sorry but I am still not persuaded about benefit of this patch.
As I previously wrote the string "default" or "non-defaults".
The value is already in log file and it's enought for debugging.

I'm sorry but I cannot give an ACK

LS


OK, Lukas, thank you for review.


Even though I don't have anything against this patch I do not see the 
need for this either. We already have sssd.conf available, so it is easy 
to check which options are set and which are not. But as you said, it 
doesn't influence sssd in any way so it doesn't matter.


Maybe it will help if you describe the situation where this information 
was relevant.


___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] NSS: Use correct name for invalidating memory cache

[Jakub Hrozek]

On Mon, Aug 08, 2016 at 06:37:10PM +0200, Lukas Slebodnik wrote:
> ehlo,
> 
> yet another patch which fixes issues caused by sysdb refactoring.
> 
> reproducer:
> a) add user with two groups
> b) call id user
> c) add another group to user
> d) authenticate
> e) check tha id -G return 3 groups.
> 
> I am looking forward to the pam_wrapper So we can test it in upstream.
> 
> LS

> From e501f791ee2e03b8eaea919a2f8ca4474773e3ba Mon Sep 17 00:00:00 2001
> From: Lukas Slebodnik 
> Date: Mon, 8 Aug 2016 17:30:29 +0200
> Subject: [PATCH] NSS: Use correct name for invalidating memory cache
> 
> After refactoring of sysdb, we get and internal fully qualified
> name from backend in org.freedesktop.sssd.dataprovider_rev.initgrCheck
> Previously we got short name and we created fq name in
> nss_update_initgr_memcache. Memory cache still need to use short names
> if it was specified.

In fill_pwent() we use the sized_output_name() to store the name in the
memcache, can we use the same here?
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] Unit tests for pam_sss using pam_wrapper (need help with CI..)

[Jakub Hrozek]

On Tue, Aug 09, 2016 at 08:04:38AM +0200, Lukas Slebodnik wrote:
> On (09/05/16 10:07), Jakub Hrozek wrote:
> >On Wed, May 04, 2016 at 11:36:57PM +0200, Lukas Slebodnik wrote:
> >> On (27/04/16 10:51), Jakub Hrozek wrote:
> >> >Hi,
> >> >
> >> >the attached patches implement unit tests for the pam_sss module using
> >> >pam_wrapper and libpamtest. In my testing, the coverage is around 75%
> >> >with mostly the parts that require running as root being untested.
> >> >
> >> >I worked on this patchset even though the features for 1.14 are in full
> >> >swing because there are several tickets that will require us to patch
> >> >pam_sss, so it's important to have the code that changes tested. In
> >> >addition, when we merge Dan's patches to use TLS with integration tests,
> >> >then we'll be able to also test authentication in integration tests
> >> >easily using libpamtest-python.
> >> >
> >> >However, our CI fails for me constantly:
> >> >http://sssd-ci.duckdns.org/logs/job/42/75/fedora_rawhide/ci.html
> >> >The strange thing is that running CI locally works fine and so does make
> >> >check. Can anyone help point me in the right direction as to what should
> >> >I check next? I suspect some of the environment variables might not be
> >> >set correctly, but I don't see why..
> >> 
> >> Are you sure it pass locally with valgrind?
> >> 
> >> It failed because there are valgrind errors.
> >> I can see then on fedora 22 and fedora 23
> >> http://sssd-ci.duckdns.org/logs-test/job/2/70/fedora22/ci-build-debug/src/tests/cwrap/pam_sss-tests.sh.1444.valgrind.log
> >> http://sssd-ci.duckdns.org/logs-test/job/2/70/fedora22/ci-build-debug/src/tests/cwrap/pam_sss-tests.sh.log
> >> 
> >> I cannot see them on fedora 24 or fedora rawhide
> >> http://sssd-ci.duckdns.org/logs-test/job/2/70/fedora_rawhide/ci-build-debug/src/tests/cwrap/pam_sss-tests.sh.10504.valgrind.log
> >> but it fails as well
> >> http://sssd-ci.duckdns.org/logs-test/job/2/70/fedora_rawhide/ci-build-debug/src/tests/cwrap/pam_sss-tests.sh.log
> >> 
> >> If the problem is with missing environment variables
> >> then you might log all environment variables in main function
> >
> >just a quick update: the issues in tests were resolved. I hit two bugs
> >in pam_wrapper:
> >
> > https://github.com/jhrozek/pam_wrapper/commit/ff7ec1c5ea7ed2360cbc59bd58f9caae7c9fa53d
> >
> > https://github.com/jhrozek/pam_wrapper/commit/c9b11ac8947bc0ff2ec3c4140b4d7413bfaadb37
> >so once Andreas approves them, I will build a new pam_wrapper RPM for
> >Fedora and EPEL so that we can formally review the pam_sss test patches.
> Bump

I only had time to rebase the patches:
https://github.com/jhrozek/sssd/tree/pwrap
But now the tests fail, so I did something wrong, somewhere. I don't
think I will have time to look into this until next week, sorry.
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] NSS: Use correct name for invalidating memory cache

[Petr Cech]

On 08/09/2016 09:04 AM, Petr Cech wrote:

On 08/08/2016 06:37 PM, Lukas Slebodnik wrote:

ehlo,

yet another patch which fixes issues caused by sysdb refactoring.

reproducer:
a) add user with two groups
b) call id user
c) add another group to user
d) authenticate
e) check tha id -G return 3 groups.

I am looking forward to the pam_wrapper So we can test it in upstream.

LS


Hello Lukas,

CI nearly passed:
http://sssd-ci.duckdns.org/logs/job/51/10/summary.html
RHEL7 didn't work at all. This is not connected.

Code: LGTM

Tests: Please, file ticket on writting tests for this issue with
pam_wrapper.

I try your reproducer manually, after this I give you ack for this patch.


Manually reproducer before and after patch works how we expected.

=> ACK

--
Petr^4 Čech
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] NSS: Use correct name for invalidating memory cache

[Petr Cech]

On 08/08/2016 06:37 PM, Lukas Slebodnik wrote:

ehlo,

yet another patch which fixes issues caused by sysdb refactoring.

reproducer:
a) add user with two groups
b) call id user
c) add another group to user
d) authenticate
e) check tha id -G return 3 groups.

I am looking forward to the pam_wrapper So we can test it in upstream.

LS


Hello Lukas,

CI nearly passed:
http://sssd-ci.duckdns.org/logs/job/51/10/summary.html
RHEL7 didn't work at all. This is not connected.

Code: LGTM

Tests: Please, file ticket on writting tests for this issue with 
pam_wrapper.


I try your reproducer manually, after this I give you ack for this patch.

Regards

--
Petr^4 Čech
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org