Re: [SSSD] sss_cache flush ssh hosts list.

2014-07-16 Thread Lukas Slebodnik 
On (16/07/14 11:46), William wrote:
On Tue, 2014-07-15 at 15:57 +0200, Jan Cholasta wrote:
 On 11.7.2014 03:35, William wrote:
 
 
  Thanks. Could you please rename the option to
  entry_cache_ssh_host_timeout, so that it's consistent with the rest of
  the cache timeout options?
 
  However, I can't quite work out how to access confdb
  inside of ipa_hostid.c when it calls sysdb_store_ssh_host.
 
  I guess you can store the value in sss_domain_info, like the rest of the
  cache timeouts. See confdb_get_domain_internal.
 
 
 
  Helps if I attach the patch.
 
  It certainly does :)
 
 
  Again, I have taken your advice and implemented these changes. I don't
  see any dbus related changes in my patch, so I hope that this is too
  your requirements.
 
 I do see some dbus changes: src/responder/ifp/ifp_iface_generated.c
 
 As Pavel and Lukáš pointed out earlier, these changes should not be 
 included in the patch, as they are a result of a bug in dbus codegen script.
 
 
  Any comments and advice welcome.
 
 The confdb argument in sysdb_store_ssh_host is not needed anymore.
 

Ignore that last patch, I messed up and didn't include a .h file. Here
is the fixed patch. 

I also rebased onto master, and I get:

/bin/sh ./libtool  --tag=CC   --mode=link gcc  -Wall -Wshadow
-Wstrict-prototypes -Wpointer-arith -Wcast-qual -Wcast-align
-Wwrite-strings -Wundef -Werror-implicit-function-declaration
-Winit-self -fno-strict-aliasing -std=gnu99 -g -O2
-D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -module
-avoid-version
-Wl,--version-script,./src/sss_client/autofs/sss_autofs.exports  -o
libsss_autofs.la -rpath /srv/sssd/lib/sssd/modules
src/sss_client/common.lo src/sss_client/autofs/sss_autofs.lo  -lpthread
-ldl 
libtool: link: gcc -shared  -fPIC -DPIC  src/sss_client/.libs/common.o
src/sss_client/autofs/.libs/sss_autofs.o   -lpthread -ldl  -O2
-Wl,--version-script -Wl,./src/sss_client/autofs/sss_autofs.exports
-Wl,-soname -Wl,libsss_autofs.so -o .libs/libsss_autofs.so
gcc: error: src/sss_client/.libs/common.o: No such file or directory
make[2]: *** [libsss_autofs.la] Error 1
make[2]: Leaving directory `/home/a1176360/development/sssd'
make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory `/home/a1176360/development/sssd'
make: *** [all] Error 2
 
Doesn't appear to be related to anything I have changed I don't
think ... 

You forgot to change usage of sysdb_store_ssh_host in sysdb_ssh-tests.
tests cannot be compiled. (make check)


  CC   src/tests/sysdb_ssh_tests-sysdb_ssh-tests.o
src/tests/sysdb_ssh-tests.c:179:43: error: too few arguments to function call,
 expected 6, have 5
   data-attrs);
  ^
src/db/sysdb_ssh.h:32:1: note: 'sysdb_store_ssh_host' declared here
errno_t
^
1 error generated.

From 29cdcbd9a20cfbd72c5a8d103f58e3f153887d73 Mon Sep 17 00:00:00 2001
From: William B will...@adelaide.edu.au
Date: Wed, 16 Jul 2014 11:45:02 +0930
Subject: [PATCH] Allow sss_cache tool to flush known hosts cache

---
 src/confdb/confdb.h|  2 ++
 src/config/etc/sssd.api.conf   |  1 +
 src/db/sysdb_ssh.c | 58 +++---
 src/db/sysdb_ssh.h | 17 -
 src/man/po/sssd-docs.pot   | 17 +
 src/providers/ipa/ipa_hostid.c |  2 +-
 src/tools/sss_cache.c  | 54 +++
 7 files changed, 140 insertions(+), 11 deletions(-)


//snip

diff --git a/src/man/po/sssd-docs.pot b/src/man/po/sssd-docs.pot
index df0456d..a4fce38 100644
--- a/src/man/po/sssd-docs.pot
+++ b/src/man/po/sssd-docs.pot
@@ -1140,6 +1140,23 @@ msgstr 
 msgid Default: 180
 msgstr 
 
+#. type: Content of: 
referencerefentryrefsect1refsect2variablelistvarlistentryterm
+#: sssd.conf.5.xml:878
+msgid ssh_known_hosts_expire (integer)
+msgstr 
+
+#. type: Content of: 
referencerefentryrefsect1refsect2variablelistvarlistentrylistitempara
+#: sssd.conf.5.xml:881
+msgid 
+How many seconds to keep a host ssh key after refresh. IE how long to cache 
+the host key for.
+msgstr 
+
+#. type: Content of: 
referencerefentryrefsect1refsect2variablelistvarlistentrylistitempara
+#: sssd.conf.5.xml:885
+msgid Default: 31536000 (1 Year)
+msgstr 
+
 #. type: Content of: referencerefentryrefsect1refsect2title
 #: sssd.conf.5.xml:893
 msgid PAC responder configuration options

We do not update pot files directly.
Could you edit xml file src/man/sssd.conf.5.xml ?

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

[SSSD] sss_client: thread safe initialisation of sss_nss_mc_get_ctx

2014-07-16 Thread Lukas Slebodnik 
ehlo,

attached patches fix problems with mmap cache in client code.
The 1st patch is at least 5th version, because I found few problems in my
previous versions myself. I hope you will not find anything else :-)

Patches change client code. It will be good to have at least 2 ACKs.

How to test?
You can use simple program form ticket description #2380.
Program call getuid, therefore user should be from sssd and not /etc/passwd.
It needn't crash at first time, but you can use simple for loop
for i in {1..100}; do ./a1; done

You can also use 3th patch to see code flow. e.g.

bash-4.2$ ./a1
more [2] threads try to init mem ctc
init.done
seed or table size do not match
sss_nss_check_header end:Invalid argument
more [3] threads try to init mem ctc
calling munmap
calling close
Segmentation fault (core dumped)


bash-4.2$ ./a1
init.done
more [2] threads try to init mem ctc
more [3] threads try to init mem ctc
seed or table size do not match
sss_nss_check_header end:Invalid argument
seed or table size do not match
seed or table size do not match
calling munmap
calling close
sss_nss_check_header beg:Invalid argument
calling close
init.done
Floating point exception (core dumped)

If you apply 3rd patch you can simulate old version with simple diff
diff --git a/src/sss_client/nss_mc_common.c b/src/sss_client/nss_mc_common.c
index 768763f..cea3a78 100644
--- a/src/sss_client/nss_mc_common.c
+++ b/src/sss_client/nss_mc_common.c
@@ -131,7 +131,7 @@ errno_t sss_nss_mc_get_ctx(const char *name, struct 
sss_cli_mc_ctx *ctx)
  */
 fprintf(stderr, more [%d] threads try to init mem ctc\n,
 ctx-init_count);
-return EAGAIN;
+//return EAGAIN;
 }
 
 ret = asprintf(file, %s/%s, SSS_NSS_MCACHE_DIR, name);

LS
From 79e23d5ad87ec454fe2f3f2b7e79694897db10bd Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik lsleb...@redhat.com
Date: Mon, 14 Jul 2014 16:02:22 +0200
Subject: [PATCH 1/3] sss_client: thread safe initialisation of
 sss_nss_mc_get_ctx

In multi threaded application, it may happen that more threads will call
function getpwuid(or similar) and sss client will not have initialized
structure for fast memory cache. This structure is initialized just once.
There isn't any problem with multi threaded application after successful
initialisation.

The race condition will happen if more threads try to initialise structure
sss_cli_mc_ctx in function sss_nss_mc_get_ctx (ctx-initialized is false)
It takes some time to initialise mmap cache: open file, get file size, mmap
file, initialize structure sss_cli_mc_ctx. One of problems is that file with
memory cache can be opened more times (file descriptor leak), but the race
condition is with initialising structure sss_cli_mc_ctx. One tread will start
to initialise this structure; another thread will think that structure is
already initialised and will check consistency of this structure. It will fail
because 1st thread did not finish initialisation. Therefore 2nd thread will
return EINVAL and will do clean up in done section: munmap, close file and
reset structure data. The 1st thread will finish an try to use memory cache,
but structure was zero initialised by 2nd thread and it will cause dereference
of NULL pointer in 1st thread (SIGSEGV) or dividing by zero in murmurhash
function(SIGFPE)

This patch does not allow to parallel initialisation.

Resolves:
https://fedorahosted.org/sssd/ticket/2380
---
 src/sss_client/nss_mc.h|  1 +
 src/sss_client/nss_mc_common.c | 10 ++
 src/sss_client/nss_mc_group.c  |  3 ++-
 src/sss_client/nss_mc_passwd.c |  3 ++-
 4 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/src/sss_client/nss_mc.h b/src/sss_client/nss_mc.h
index 
685cc41c0530750d890050f0917dc88be14d96ea..a77b86261bfbafc83b0e65315ec478f7f40b57e8
 100644
--- a/src/sss_client/nss_mc.h
+++ b/src/sss_client/nss_mc.h
@@ -37,6 +37,7 @@ typedef int errno_t;
 struct sss_cli_mc_ctx {
 bool initialized;
 int fd;
+volatile int init_count; /* numbers of threads trying to initialize ctx */
 
 uint32_t seed;  /* seed from the tables header */
 
diff --git a/src/sss_client/nss_mc_common.c b/src/sss_client/nss_mc_common.c
index 
db9be94b442b1b5a97ff9074fb1d98a1feea5e1a..d918b697f9cf159e6dbea46ea0f670993dc14bbf
 100644
--- a/src/sss_client/nss_mc_common.c
+++ b/src/sss_client/nss_mc_common.c
@@ -118,6 +118,15 @@ errno_t sss_nss_mc_get_ctx(const char *name, struct 
sss_cli_mc_ctx *ctx)
 goto done;
 }
 
+if (__sync_add_and_fetch(ctx-init_count, 1) != 1) {
+/*
+ * More threads try to init sss_cli_mc_ctx. There can be raise
+ * condition and we do not want to call clean up in case of error
+ * in done section. Therefore return EAGAIN is used.
+ */
+return EAGAIN;
+}
+
 ret = asprintf(file, %s/%s, SSS_NSS_MCACHE_DIR, name);
 if (ret == -1) {
 ret = ENOMEM;
@@ -154,6 +163,7 @@ errno_t sss_nss_mc_get_ctx(const char *name,

Re: [SSSD] [PATCH] sss_sifp: set output parameters if attribute is NULL

2014-07-16 Thread Pavel Březina 

On 07/10/2014 02:08 PM, Lukas Slebodnik wrote:

On (02/07/14 13:09), Pavel Březina wrote:

First patch is a minor bug in unit test I found when I was writing new tests.
The rest is described in commit message.



From de3ed7bf0e9784058241e4b532c72e324e3dd635 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= pbrez...@redhat.com
Date: Tue, 1 Jul 2014 11:55:41 +0200
Subject: [PATCH 2/4] sss_sifp: set output parameters if attribute is NULL

There are two cases that may happen when a user calls Get or GetAll:
1) the attribute is missing
2) the attribute is empty

sss_sifp has two error code to distinguish between those two cases:
1) SSS_SIFP_ATTR_MISSING
2) SSS_SIFP_ATTR_NULL

Usually the caller is not interested on situations when the attribute
is empty and it can be considered as error. Having it as a separate
error code instead of setting the output value to NULL is necesarry
since attribute does not have to be a pointer.

This patch however sets pointer type attributes to NULL since it may
simplify the code path when the caller is actually interested in
this information (e. g. empty server list on domain objects).

It is not possible to send a NULL string over a D-Bus nor it is
possible to have hash table NULL with current code so these two
scenarios are not tested. However, it is handled in sss_sifp_attr
code for completeness.
---
src/lib/sifp/sss_sifp_attrs.c|  63 +---
src/tests/cmocka/test_sss_sifp.c | 338 +++
2 files changed, 380 insertions(+), 21 deletions(-)

diff --git a/src/lib/sifp/sss_sifp_attrs.c b/src/lib/sifp/sss_sifp_attrs.c
index 
6d10c46119989b42e79cc80f251fa275d07b1cd0..5a0241f85f734891f5273d1f04e5721be859b263
 100644
--- a/src/lib/sifp/sss_sifp_attrs.c
+++ b/src/lib/sifp/sss_sifp_attrs.c
@@ -41,23 +41,31 @@
 out = attr-data.field[0];  \
} while (0)

-#define GET_ATTR_ARRAY(attrs, name, rtype, field, out_num, out_val) do {\
+#define GET_ATTR_ARRAY(attrs, name, rtype, field, out_num, out_val, ret)\
+do {\
 sss_sifp_attr *attr = sss_sifp_find_attr(attrs, name);  \
 \
 if (attr == NULL) { \
-return SSS_SIFP_ATTR_MISSING;   \
+ret = SSS_SIFP_ATTR_MISSING;\
+break;  \
 }   \
 \
 if (attr-type != rtype) {  \
-return SSS_SIFP_INCORRECT_TYPE; \
+ret = SSS_SIFP_INCORRECT_TYPE;  \
+break;  \
 }   \
 \
 if (attr-data.field == NULL) { \
-return SSS_SIFP_ATTR_NULL;  \
+out_num = 0;\
+out_val = NULL; \
+ret = SSS_SIFP_ATTR_NULL;   \
+break;  \
 }   \
 \
 out_num = attr-num_values; \
 out_val = attr-data.field; \
+\
+ret = SSS_SIFP_OK;  \
} while (0)

@@ -151,6 +159,7 @@ sss_sifp_find_attr_as_string(sss_sifp_attr **attrs,
 GET_ATTR(attrs, name, SSS_SIFP_ATTR_TYPE_STRING, str, value);



function sss_sifp_find_attr_as_string can return SSS_SIFP_ATTR_NULL
also from code generated by macro GET_ATTR.
Do we want to set value to NULL there?

Other wise patches looks good.

LS


Thanks, fixed.
From 31d1a1595d35cff6fa5aac6c8d3fd8b908747faa Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= pbrez...@redhat.com
Date: Wed, 2 Jul 2014 11:15:23 +0200
Subject: [PATCH 1/3] sss_sifp test: fix object path array test

---
 src/tests/cmocka/test_sss_sifp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/tests/cmocka/test_sss_sifp.c b/src/tests/cmocka/test_sss_sifp.c
index

Re: [SSSD] [PATCH] sss_sifp: set output parameters if attribute is NULL

2014-07-16 Thread Pavel Březina 

On 07/16/2014 01:37 PM, Pavel Březina wrote:

On 07/10/2014 02:08 PM, Lukas Slebodnik wrote:

On (02/07/14 13:09), Pavel Březina wrote:

First patch is a minor bug in unit test I found when I was writing
new tests.
The rest is described in commit message.



From de3ed7bf0e9784058241e4b532c72e324e3dd635 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= pbrez...@redhat.com
Date: Tue, 1 Jul 2014 11:55:41 +0200
Subject: [PATCH 2/4] sss_sifp: set output parameters if attribute is
NULL

There are two cases that may happen when a user calls Get or GetAll:
1) the attribute is missing
2) the attribute is empty

sss_sifp has two error code to distinguish between those two cases:
1) SSS_SIFP_ATTR_MISSING
2) SSS_SIFP_ATTR_NULL

Usually the caller is not interested on situations when the attribute
is empty and it can be considered as error. Having it as a separate
error code instead of setting the output value to NULL is necesarry
since attribute does not have to be a pointer.

This patch however sets pointer type attributes to NULL since it may
simplify the code path when the caller is actually interested in
this information (e. g. empty server list on domain objects).

It is not possible to send a NULL string over a D-Bus nor it is
possible to have hash table NULL with current code so these two
scenarios are not tested. However, it is handled in sss_sifp_attr
code for completeness.
---
src/lib/sifp/sss_sifp_attrs.c|  63 +---
src/tests/cmocka/test_sss_sifp.c | 338
+++
2 files changed, 380 insertions(+), 21 deletions(-)

diff --git a/src/lib/sifp/sss_sifp_attrs.c
b/src/lib/sifp/sss_sifp_attrs.c
index
6d10c46119989b42e79cc80f251fa275d07b1cd0..5a0241f85f734891f5273d1f04e5721be859b263
100644
--- a/src/lib/sifp/sss_sifp_attrs.c
+++ b/src/lib/sifp/sss_sifp_attrs.c
@@ -41,23 +41,31 @@
 out =
attr-data.field[0];  \
} while (0)

-#define GET_ATTR_ARRAY(attrs, name, rtype, field, out_num, out_val)
do {\
+#define GET_ATTR_ARRAY(attrs, name, rtype, field, out_num, out_val,
ret)\
+do
{
\
 sss_sifp_attr *attr = sss_sifp_find_attr(attrs,
name);  \

\
 if (attr == NULL)
{ \
-return
SSS_SIFP_ATTR_MISSING;   \
+ret =
SSS_SIFP_ATTR_MISSING;\
+
break;  \

}
\

\
 if (attr-type != rtype)
{  \
-return
SSS_SIFP_INCORRECT_TYPE; \
+ret =
SSS_SIFP_INCORRECT_TYPE;  \
+
break;  \

}
\

\
 if (attr-data.field == NULL)
{ \
-return
SSS_SIFP_ATTR_NULL;  \
+out_num =
0;\
+out_val =
NULL; \
+ret =
SSS_SIFP_ATTR_NULL;   \
+
break;  \

}
\

\
 out_num =
attr-num_values; \
 out_val =
attr-data.field; \
+
\
+ret =
SSS_SIFP_OK;  \
} while (0)

@@ -151,6 +159,7 @@ sss_sifp_find_attr_as_string(sss_sifp_attr **attrs,
 GET_ATTR(attrs, name, SSS_SIFP_ATTR_TYPE_STRING, str, value);



function sss_sifp_find_attr_as_string can return SSS_SIFP_ATTR_NULL
also from code generated by macro GET_ATTR.
Do we want to set value to NULL there?

Other wise patches looks good.

LS


Thanks, fixed.


Sorry, I did not commit the changes. So one more time.

Is there any way to make git format-patch to warn if there are unstaged 
changes?


From 31d1a1595d35cff6fa5aac6c8d3fd8b908747faa Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= pbrez...@redhat.com
Date: Wed, 2 Jul 2014 11:15:23 +0200
Subject: [PATCH 1/3] sss_sifp test: fix object path array test

---
 src/tests/cmocka/test_sss_sifp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/tests/cmocka/test_sss_sifp.c b/src/tests/cmocka/test_sss_sifp.c
index 54f2152075d860edc3a5438b4cf5cdd1a88d8f92..384bae416a8c6b3b98c8db00b9b3400aa6884b23 100644
--- a/src/tests/cmocka/test_sss_sifp.c
+++ b/src/tests/cmocka/test_sss_sifp.c
@@ -1039,7 +1039,7 @@ void test_sss_sifp_parse_attr_object_path_array(void **state)
 unsigned int i;
 
 /* prepare message */
-reply_variant_array(reply, DBUS_TYPE_STRING_AS_STRING, num_values,
+reply_variant_array(reply, DBUS_TYPE_OBJECT_PATH_AS_STRING, num_values,
 (uint8_t*)in, sizeof(const char*));
 
 /* test */
-- 
1.7.11.7

Re: [SSSD] sss_client: thread safe initialisation of sss_nss_mc_get_ctx

2014-07-16 Thread Sumit Bose 
On Wed, Jul 16, 2014 at 11:32:53AM +0200, Lukas Slebodnik wrote:
 ehlo,
 
 attached patches fix problems with mmap cache in client code.
 The 1st patch is at least 5th version, because I found few problems in my
 previous versions myself. I hope you will not find anything else :-)
 
 Patches change client code. It will be good to have at least 2 ACKs.

Currently we use sss_nss_lock() and sss_nss_unlock() to protect
sss_nss_make_request() in multi-threaded clients (there is sss_pam_lock()
and sss_pam_lock() for pam_sss as well). I wonder if a new pair like
sss_mc_init_lock() sss_mc_init_unlock() might help to protect
sss_nss_mc_get_ctx() as well?

bye,
Sumit

 
 How to test?
 You can use simple program form ticket description #2380.
 Program call getuid, therefore user should be from sssd and not /etc/passwd.
 It needn't crash at first time, but you can use simple for loop
 for i in {1..100}; do ./a1; done
 
 You can also use 3th patch to see code flow. e.g.
 
 bash-4.2$ ./a1
 more [2] threads try to init mem ctc
 init.done
 seed or table size do not match
 sss_nss_check_header end:Invalid argument
 more [3] threads try to init mem ctc
 calling munmap
 calling close
 Segmentation fault (core dumped)
 
 
 bash-4.2$ ./a1
 init.done
 more [2] threads try to init mem ctc
 more [3] threads try to init mem ctc
 seed or table size do not match
 sss_nss_check_header end:Invalid argument
 seed or table size do not match
 seed or table size do not match
 calling munmap
 calling close
 sss_nss_check_header beg:Invalid argument
 calling close
 init.done
 Floating point exception (core dumped)
 
 If you apply 3rd patch you can simulate old version with simple diff
 diff --git a/src/sss_client/nss_mc_common.c b/src/sss_client/nss_mc_common.c
 index 768763f..cea3a78 100644
 --- a/src/sss_client/nss_mc_common.c
 +++ b/src/sss_client/nss_mc_common.c
 @@ -131,7 +131,7 @@ errno_t sss_nss_mc_get_ctx(const char *name, struct 
 sss_cli_mc_ctx *ctx)
   */
  fprintf(stderr, more [%d] threads try to init mem ctc\n,
  ctx-init_count);
 -return EAGAIN;
 +//return EAGAIN;
  }
  
  ret = asprintf(file, %s/%s, SSS_NSS_MCACHE_DIR, name);
 
 LS
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCH] sss_sifp: set output parameters if attribute is NULL

2014-07-16 Thread Lukas Slebodnik 
On (16/07/14 13:46), Pavel Březina wrote:
On 07/16/2014 01:37 PM, Pavel Březina wrote:
On 07/10/2014 02:08 PM, Lukas Slebodnik wrote:
On (02/07/14 13:09), Pavel Březina wrote:
First patch is a minor bug in unit test I found when I was writing
new tests.
The rest is described in commit message.

From de3ed7bf0e9784058241e4b532c72e324e3dd635 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= pbrez...@redhat.com
Date: Tue, 1 Jul 2014 11:55:41 +0200
Subject: [PATCH 2/4] sss_sifp: set output parameters if attribute is
NULL

There are two cases that may happen when a user calls Get or GetAll:
1) the attribute is missing
2) the attribute is empty

sss_sifp has two error code to distinguish between those two cases:
1) SSS_SIFP_ATTR_MISSING
2) SSS_SIFP_ATTR_NULL

Usually the caller is not interested on situations when the attribute
is empty and it can be considered as error. Having it as a separate
error code instead of setting the output value to NULL is necesarry
since attribute does not have to be a pointer.

This patch however sets pointer type attributes to NULL since it may
simplify the code path when the caller is actually interested in
this information (e. g. empty server list on domain objects).

It is not possible to send a NULL string over a D-Bus nor it is
possible to have hash table NULL with current code so these two
scenarios are not tested. However, it is handled in sss_sifp_attr
code for completeness.
---
src/lib/sifp/sss_sifp_attrs.c|  63 +---
src/tests/cmocka/test_sss_sifp.c | 338
+++
2 files changed, 380 insertions(+), 21 deletions(-)

diff --git a/src/lib/sifp/sss_sifp_attrs.c
b/src/lib/sifp/sss_sifp_attrs.c
index
6d10c46119989b42e79cc80f251fa275d07b1cd0..5a0241f85f734891f5273d1f04e5721be859b263
100644
--- a/src/lib/sifp/sss_sifp_attrs.c
+++ b/src/lib/sifp/sss_sifp_attrs.c
@@ -41,23 +41,31 @@
 out =
attr-data.field[0];  \
} while (0)

-#define GET_ATTR_ARRAY(attrs, name, rtype, field, out_num, out_val)
do {\
+#define GET_ATTR_ARRAY(attrs, name, rtype, field, out_num, out_val,
ret)\
+do
{
\
 sss_sifp_attr *attr = sss_sifp_find_attr(attrs,
name);  \

\
 if (attr == NULL)
{ \
-return
SSS_SIFP_ATTR_MISSING;   \
+ret =
SSS_SIFP_ATTR_MISSING;\
+
break;  \

}
\

\
 if (attr-type != rtype)
{  \
-return
SSS_SIFP_INCORRECT_TYPE; \
+ret =
SSS_SIFP_INCORRECT_TYPE;  \
+
break;  \

}
\

\
 if (attr-data.field == NULL)
{ \
-return
SSS_SIFP_ATTR_NULL;  \
+out_num =
0;\
+out_val =
NULL; \
+ret =
SSS_SIFP_ATTR_NULL;   \
+
break;  \

}
\

\
 out_num =
attr-num_values; \
 out_val =
attr-data.field; \
+
\
+ret =
SSS_SIFP_OK;  \
} while (0)

@@ -151,6 +159,7 @@ sss_sifp_find_attr_as_string(sss_sifp_attr **attrs,
 GET_ATTR(attrs, name, SSS_SIFP_ATTR_TYPE_STRING, str, value);


function sss_sifp_find_attr_as_string can return SSS_SIFP_ATTR_NULL
also from code generated by macro GET_ATTR.
Do we want to set value to NULL there?

Other wise patches looks good.

LS

Thanks, fixed.

Sorry, I did not commit the changes. So one more time.

Is there any way to make git format-patch to warn if there are unstaged
changes?


Thank you.

ACK to all

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

Re: [SSSD] [PATCH] sss_sifp: set output parameters if attribute is NULL

2014-07-16 Thread Jakub Hrozek 
On Wed, Jul 16, 2014 at 02:10:16PM +0200, Lukas Slebodnik wrote:
 Thank you.
 
 ACK to all
 
 LS

Should we push all three at once?

Normally we only bump the version number before the release. I suggest
we create a blocker ticket for 1.12.1 to remind us to push the last
patch instead, in case we had to do some more changes..
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

[SSSD] [PATCH] sudo: replace asterisk with escape sequence in host filter

2014-07-16 Thread Pavel Březina 

https://fedorahosted.org/sssd/ticket/2377
From 7dec3a954f1d300122723916d9ba21c7677ee313 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= pbrez...@redhat.com
Date: Wed, 16 Jul 2014 14:17:24 +0200
Subject: [PATCH] sudo: replace asterisk with escape sequence in host filter

Resolves:
https://fedorahosted.org/sssd/ticket/2377
---
 src/providers/ldap/sdap_sudo.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/providers/ldap/sdap_sudo.c b/src/providers/ldap/sdap_sudo.c
index 6809f743c7bca2036b554fd9f1a94f678efbc28f..edef44208b69af454da0fc906e608b91d5634e90 100644
--- a/src/providers/ldap/sdap_sudo.c
+++ b/src/providers/ldap/sdap_sudo.c
@@ -386,7 +386,7 @@ static char *sdap_sudo_build_host_filter(TALLOC_CTX *mem_ctx,
  */
 if (regexp) {
 filter = talloc_asprintf_append_buffer(filter,
-   (|(%s=**)(%s=*?*)(%s=*\\**)
+   (|(%s=**)(%s=*?*)(%s=*\\2A*)
  (%s=*[*]*)),
map[SDAP_AT_SUDO_HOST].name,
map[SDAP_AT_SUDO_HOST].name,
-- 
1.7.11.7

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

Re: [SSSD] [PATCH] sudo: replace asterisk with escape sequence in host filter

2014-07-16 Thread Jakub Hrozek 
On Wed, Jul 16, 2014 at 02:49:01PM +0200, Pavel Březina wrote:
 https://fedorahosted.org/sssd/ticket/2377

On IRC you mentioned some problem with this patch and 389DS, is is still
the case?
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCH] sudo: replace asterisk with escape sequence in host filter

2014-07-16 Thread Pavel Březina 

On 07/16/2014 02:59 PM, Jakub Hrozek wrote:

On Wed, Jul 16, 2014 at 02:49:01PM +0200, Pavel Březina wrote:

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


On IRC you mentioned some problem with this patch and 389DS, is is still
the case?


Nope.

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

[SSSD] [PATCHES] sss_case = preserving

2014-07-16 Thread Michal Židek 

Hi,

patches for ticket
https://fedorahosted.org/sssd/ticket/2367
are in attachment.

Michal

From 071b3ca8cd9a194c8cb287f9abca2fe7c58323a2 Mon Sep 17 00:00:00 2001
From: Michal Zidek mzi...@redhat.com
Date: Tue, 15 Jul 2014 12:00:36 -0400
Subject: [PATCH 1/3] Add function confdb_set_string.

---
 src/confdb/confdb.c | 70 +
 src/confdb/confdb.h |  6 +
 2 files changed, 76 insertions(+)

diff --git a/src/confdb/confdb.c b/src/confdb/confdb.c
index 15de961..79c89b7 100644
--- a/src/confdb/confdb.c
+++ b/src/confdb/confdb.c
@@ -369,6 +369,76 @@ done:
 return ret;
 }
 
+int confdb_set_string(struct confdb_ctx *cdb,
+  const char *section,
+  const char *attribute,
+  char *val)
+{
+TALLOC_CTX *tmp_ctx;
+struct ldb_dn *dn;
+char *secdn;
+struct ldb_message *msg;
+int ret, lret;
+
+tmp_ctx = talloc_new(NULL);
+if (!tmp_ctx)
+return ENOMEM;
+
+ret = parse_section(tmp_ctx, section, secdn, NULL);
+if (ret != EOK) {
+goto done;
+}
+
+dn = ldb_dn_new(tmp_ctx, cdb-ldb, secdn);
+if (!dn) {
+ret = EIO;
+goto done;
+}
+
+msg = ldb_msg_new(tmp_ctx);
+if (!msg) {
+ret = ENOMEM;
+goto done;
+}
+
+msg-dn = dn;
+
+lret = ldb_msg_add_empty(msg, attribute, LDB_FLAG_MOD_REPLACE, NULL);
+if (lret != LDB_SUCCESS) {
+DEBUG(SSSDBG_MINOR_FAILURE,
+  ldb_msg_add_empty failed: [%s]\n, ldb_strerror(lret));
+ret = EIO;
+goto done;
+}
+
+lret = ldb_msg_add_string(msg, attribute, val);
+if (lret != LDB_SUCCESS) {
+DEBUG(SSSDBG_MINOR_FAILURE,
+  ldb_msg_add_string failed: [%s]\n, ldb_strerror(lret));
+ret = EIO;
+goto done;
+}
+
+
+lret = ldb_modify(cdb-ldb, msg);
+if (lret != LDB_SUCCESS) {
+DEBUG(SSSDBG_MINOR_FAILURE,
+  ldb_modify failed: [%s]\n, ldb_strerror(lret));
+ret = EIO;
+goto done;
+}
+
+ret = EOK;
+
+done:
+talloc_free(tmp_ctx);
+if (ret != EOK) {
+DEBUG(SSSDBG_CRIT_FAILURE,
+  Failed to set [%s] from [%s], error [%d] (%s)\n,
+   attribute, section, ret, strerror(ret));
+}
+return ret;
+}
 int confdb_get_string(struct confdb_ctx *cdb, TALLOC_CTX *ctx,
   const char *section, const char *attribute,
   const char *defstr, char **result)
diff --git a/src/confdb/confdb.h b/src/confdb/confdb.h
index ba33ea5..8a642b3 100644
--- a/src/confdb/confdb.h
+++ b/src/confdb/confdb.h
@@ -217,6 +217,7 @@ struct sss_domain_info {
 bool cache_credentials;
 bool legacy_passwords;
 bool case_sensitive;
+bool case_preserve;
 
 gid_t override_gid;
 const char *override_homedir;
@@ -459,6 +460,11 @@ int confdb_set_bool(struct confdb_ctx *cdb,
  const char *attribute,
  bool val);
 
+int confdb_set_string(struct confdb_ctx *cdb,
+  const char *section,
+  const char *attribute,
+  char *val);
+
 /**
  * @brief Convenience function to retrieve a single-valued attribute as a
  * null-terminated array of strings
-- 
1.9.3

From 06c624bc81b64ae35ddcb03d05f59b808209d9ec Mon Sep 17 00:00:00 2001
From: Michal Zidek mzi...@redhat.com
Date: Tue, 15 Jul 2014 12:10:34 -0400
Subject: [PATCH 2/3] case_sensitivity = preserving

If case_sensitivity is set to 'preserving', getXXnam
returns name attribute in the same format as
stored in LDAP.
---
 src/confdb/confdb.c | 31 +++
 src/providers/ad/ad_common.c| 30 +++---
 src/providers/ipa/ipa_selinux.c |  2 +-
 src/responder/nss/nsssrv_cmd.c  |  4 ++--
 4 files changed, 53 insertions(+), 14 deletions(-)

diff --git a/src/confdb/confdb.c b/src/confdb/confdb.c
index 79c89b7..674a2c4 100644
--- a/src/confdb/confdb.c
+++ b/src/confdb/confdb.c
@@ -1217,15 +1217,30 @@ static int confdb_get_domain_internal(struct confdb_ctx *cdb,
 }
 }
 
-ret = get_entry_as_bool(res-msgs[0], domain-case_sensitive,
-CONFDB_DOMAIN_CASE_SENSITIVE, true);
-if(ret != EOK) {
-DEBUG(SSSDBG_FATAL_FAILURE,
-  Invalid value for %s\n, CONFDB_DOMAIN_CASE_SENSITIVE);
-goto done;
+tmp = ldb_msg_find_attr_as_string(res-msgs[0],
+  CONFDB_DOMAIN_CASE_SENSITIVE, true);
+if (tmp != NULL) {
+if (!strcasecmp(tmp, true)) {
+domain-case_sensitive = true;
+domain-case_preserve = true;
+} else if (!strcasecmp(tmp, false)) {
+domain-case_sensitive = false;
+domain-case_preserve = false;
+} else if (!strcasecmp(tmp, preserving)) {
+domain-case_sensitive = false;
+domain-case_preserve = true;
+}

Re: [SSSD] [PATCHES] sss_case = preserving

2014-07-16 Thread Michal Židek 

On 07/16/2014 03:40 PM, Michal Židek wrote:

Hi,

patches for ticket
https://fedorahosted.org/sssd/ticket/2367
are in attachment.

Michal



I forgot to add reference to the ticket in the
patch description. New patches are attached.

Michal

From 5f7094e93988d6aa475ea33ceb87d380737b4795 Mon Sep 17 00:00:00 2001
From: Michal Zidek mzi...@redhat.com
Date: Tue, 15 Jul 2014 12:00:36 -0400
Subject: [PATCH 1/3] Add function confdb_set_string.

Part of fix for:
https://fedorahosted.org/sssd/ticket/2367
---
 src/confdb/confdb.c | 70 +
 src/confdb/confdb.h |  6 +
 2 files changed, 76 insertions(+)

diff --git a/src/confdb/confdb.c b/src/confdb/confdb.c
index 15de961..79c89b7 100644
--- a/src/confdb/confdb.c
+++ b/src/confdb/confdb.c
@@ -369,6 +369,76 @@ done:
 return ret;
 }
 
+int confdb_set_string(struct confdb_ctx *cdb,
+  const char *section,
+  const char *attribute,
+  char *val)
+{
+TALLOC_CTX *tmp_ctx;
+struct ldb_dn *dn;
+char *secdn;
+struct ldb_message *msg;
+int ret, lret;
+
+tmp_ctx = talloc_new(NULL);
+if (!tmp_ctx)
+return ENOMEM;
+
+ret = parse_section(tmp_ctx, section, secdn, NULL);
+if (ret != EOK) {
+goto done;
+}
+
+dn = ldb_dn_new(tmp_ctx, cdb-ldb, secdn);
+if (!dn) {
+ret = EIO;
+goto done;
+}
+
+msg = ldb_msg_new(tmp_ctx);
+if (!msg) {
+ret = ENOMEM;
+goto done;
+}
+
+msg-dn = dn;
+
+lret = ldb_msg_add_empty(msg, attribute, LDB_FLAG_MOD_REPLACE, NULL);
+if (lret != LDB_SUCCESS) {
+DEBUG(SSSDBG_MINOR_FAILURE,
+  ldb_msg_add_empty failed: [%s]\n, ldb_strerror(lret));
+ret = EIO;
+goto done;
+}
+
+lret = ldb_msg_add_string(msg, attribute, val);
+if (lret != LDB_SUCCESS) {
+DEBUG(SSSDBG_MINOR_FAILURE,
+  ldb_msg_add_string failed: [%s]\n, ldb_strerror(lret));
+ret = EIO;
+goto done;
+}
+
+
+lret = ldb_modify(cdb-ldb, msg);
+if (lret != LDB_SUCCESS) {
+DEBUG(SSSDBG_MINOR_FAILURE,
+  ldb_modify failed: [%s]\n, ldb_strerror(lret));
+ret = EIO;
+goto done;
+}
+
+ret = EOK;
+
+done:
+talloc_free(tmp_ctx);
+if (ret != EOK) {
+DEBUG(SSSDBG_CRIT_FAILURE,
+  Failed to set [%s] from [%s], error [%d] (%s)\n,
+   attribute, section, ret, strerror(ret));
+}
+return ret;
+}
 int confdb_get_string(struct confdb_ctx *cdb, TALLOC_CTX *ctx,
   const char *section, const char *attribute,
   const char *defstr, char **result)
diff --git a/src/confdb/confdb.h b/src/confdb/confdb.h
index ba33ea5..8a642b3 100644
--- a/src/confdb/confdb.h
+++ b/src/confdb/confdb.h
@@ -217,6 +217,7 @@ struct sss_domain_info {
 bool cache_credentials;
 bool legacy_passwords;
 bool case_sensitive;
+bool case_preserve;
 
 gid_t override_gid;
 const char *override_homedir;
@@ -459,6 +460,11 @@ int confdb_set_bool(struct confdb_ctx *cdb,
  const char *attribute,
  bool val);
 
+int confdb_set_string(struct confdb_ctx *cdb,
+  const char *section,
+  const char *attribute,
+  char *val);
+
 /**
  * @brief Convenience function to retrieve a single-valued attribute as a
  * null-terminated array of strings
-- 
1.7.11.2

From 3ebc6f3755f8f24056ef99634cc0cab79d8b0f74 Mon Sep 17 00:00:00 2001
From: Michal Zidek mzi...@redhat.com
Date: Tue, 15 Jul 2014 12:10:34 -0400
Subject: [PATCH 2/3] case_sensitivity = preserving

If case_sensitivity is set to 'preserving', getXXnam
returns name attribute in the same format as
stored in LDAP.

Fixes:
https://fedorahosted.org/sssd/ticket/2367
---
 src/confdb/confdb.c | 31 +++
 src/providers/ad/ad_common.c| 30 +++---
 src/providers/ipa/ipa_selinux.c |  2 +-
 src/responder/nss/nsssrv_cmd.c  |  4 ++--
 4 files changed, 53 insertions(+), 14 deletions(-)

diff --git a/src/confdb/confdb.c b/src/confdb/confdb.c
index 79c89b7..674a2c4 100644
--- a/src/confdb/confdb.c
+++ b/src/confdb/confdb.c
@@ -1217,15 +1217,30 @@ static int confdb_get_domain_internal(struct confdb_ctx *cdb,
 }
 }
 
-ret = get_entry_as_bool(res-msgs[0], domain-case_sensitive,
-CONFDB_DOMAIN_CASE_SENSITIVE, true);
-if(ret != EOK) {
-DEBUG(SSSDBG_FATAL_FAILURE,
-  Invalid value for %s\n, CONFDB_DOMAIN_CASE_SENSITIVE);
-goto done;
+tmp = ldb_msg_find_attr_as_string(res-msgs[0],
+  CONFDB_DOMAIN_CASE_SENSITIVE, true);
+if (tmp != NULL) {
+if (!strcasecmp(tmp, true)) {
+domain-case_sensitive = true;
+domain-case_preserve = true;
+} else if

Re: [SSSD] [PATCH] sss_sifp: set output parameters if attribute is NULL

2014-07-16 Thread Lukas Slebodnik 
On (16/07/14 14:38), Jakub Hrozek wrote:
On Wed, Jul 16, 2014 at 02:10:16PM +0200, Lukas Slebodnik wrote:
 Thank you.
 
 ACK to all
 
 LS

Should we push all three at once?

Normally we only bump the version number before the release. I suggest
we create a blocker ticket for 1.12.1 to remind us to push the last
patch instead, in case we had to do some more changes..
It is your decision :-)
You will push patches. I just ACK-ed patches.

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

Re: [SSSD] sss_client: thread safe initialisation of sss_nss_mc_get_ctx

2014-07-16 Thread Lukas Slebodnik 
On (16/07/14 14:02), Sumit Bose wrote:
On Wed, Jul 16, 2014 at 11:32:53AM +0200, Lukas Slebodnik wrote:
 ehlo,
 
 attached patches fix problems with mmap cache in client code.
 The 1st patch is at least 5th version, because I found few problems in my
 previous versions myself. I hope you will not find anything else :-)
 
 Patches change client code. It will be good to have at least 2 ACKs.

Currently we use sss_nss_lock() and sss_nss_unlock() to protect
sss_nss_make_request() in multi-threaded clients (there is sss_pam_lock()
and sss_pam_lock() for pam_sss as well). I wonder if a new pair like
sss_mc_init_lock() sss_mc_init_unlock() might help to protect
sss_nss_mc_get_ctx() as well?

New version with sss_nss_{lock,unlock} is attached.
I didn't want call lock each invocation of sss_nss_mc_get_ctx,
because it should be fast memory cache and locking is useless with already
initialised structure. You can also compare attached patch with first version
Any comments are welcomed.

Function sss_nss_mc_get_ctx was split into two parts for simplification
of locking and unlocking. The locking is used only in new static function
sss_nss_mc_init_ctx. This function will not be called very often therefore the
same mutex is used as in other nss functions.

Testing was already described in 1st mail.
You just need different diff to see logging without locking.
--- a/src/sss_client/nss_mc_common.c
+++ b/src/sss_client/nss_mc_common.c
@@ -113,7 +113,7 @@ static errno_t sss_nss_mc_init_ctx(const char *name,
 int ret;
 
 fprintf(stderr, before nss lock\n);
-sss_nss_lock();
+//sss_nss_lock();
 fprintf(stderr, after nss lock\n);
 /* check if ctx is initialised by previous thread. */
 if (ctx-initialized) {
@@ -183,7 +183,7 @@ done:
 memset(ctx, 0, sizeof(struct sss_cli_mc_ctx));
 }
 free(file);
-sss_nss_unlock();
+//sss_nss_unlock();
 fprintf(stderr, after nss unlock\n);
 
 return ret;

LS
From 032ce79028cb044485d8836d8290adb7ac33ec08 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik lsleb...@redhat.com
Date: Wed, 16 Jul 2014 14:32:04 +0200
Subject: [PATCH 1/3] sss_client: thread safe initialisation of sss_cli_mc_ctx

In multi threaded application, it may happen that more threads will call
function getpwuid(or similar) and sss client will not have initialized
structure for fast memory cache. This structure is initialized just once.
There isn't any problem with multi threaded application after successful
initialisation.

The race condition will happen if more threads try to initialise structure
sss_cli_mc_ctx in function sss_nss_mc_get_ctx (ctx-initialized is false)
It takes some time to initialise mmap cache: open file, get file size, mmap
file, initialize structure sss_cli_mc_ctx. One of problems is that file with
memory cache can be opened more times (file descriptor leak), but the race
condition is with initialising structure sss_cli_mc_ctx. One tread will start
to initialise this structure; another thread will think that structure is
already initialised and will check consistency of this structure. It will fail
because 1st thread did not finish initialisation. Therefore 2nd thread will
return EINVAL and will do clean up in done section: munmap, close file and
reset structure data. The 1st thread will finish an try to use memory cache,
but structure was zero initialised by 2nd thread and it will cause dereference
of NULL pointer in 1st thread (SIGSEGV) or dividing by zero in murmurhash
function(SIGFPE)

Function sss_nss_mc_get_ctx was split into two parts for simplification
of locking and unlocking. The locking is used only in new static function
sss_nss_mc_init_ctx. This function will not be called very often therefore the
same mutex is used as in other nss functions.

Resolves:
https://fedorahosted.org/sssd/ticket/2380
---
 src/sss_client/nss_mc_common.c | 44 +++---
 1 file changed, 37 insertions(+), 7 deletions(-)

diff --git a/src/sss_client/nss_mc_common.c b/src/sss_client/nss_mc_common.c
index 
db9be94b442b1b5a97ff9074fb1d98a1feea5e1a..cd1ac42da66a2546cb8dec1e8e7903a870a1
 100644
--- a/src/sss_client/nss_mc_common.c
+++ b/src/sss_client/nss_mc_common.c
@@ -31,6 +31,7 @@
 #include string.h
 #include stdlib.h
 #include nss_mc.h
+#include sss_cli.h
 #include util/io.h
 
 /* FIXME: hook up to library destructor to avoid leaks */
@@ -101,18 +102,15 @@ errno_t sss_nss_check_header(struct sss_cli_mc_ctx *ctx)
 return 0;
 }
 
-errno_t sss_nss_mc_get_ctx(const char *name, struct sss_cli_mc_ctx *ctx)
+static errno_t sss_nss_mc_init_ctx(const char *name,
+   struct sss_cli_mc_ctx *ctx)
 {
 struct stat fdstat;
 char *file = NULL;
-char *envval;
 int ret;
 
-envval = getenv(SSS_NSS_USE_MEMCACHE);
-if (envval  strcasecmp(envval, NO) == 0) {
-return EPERM;
-}
-
+sss_nss_lock();
+/* check if ctx is initialised by previous thread. */
 if (ctx-initialized) {

Re: [SSSD] [PATCH] Use sss_strerror instead of strerror

2014-07-16 Thread Lukas Slebodnik 
On (16/07/14 16:37), Michal Židek wrote:
On 05/11/2014 11:10 PM, Jakub Hrozek wrote:
I haven't tested this patch to be honest, but then again, there's not
so much to test except compiling and a bit of sanity testing..

I prefer Michal's version of using sss_strerror explicitly instead of
the route Simo proposed to #define strerror as sss_strerror simply
because I prefer to see what function gets called right away. The
drawback of mass-converting to sss_strerror is that we lose git-blame
history somewhat...but given that we have just recently converted the
DEBUG messages and strerror is mostly used only around DEBUG messages,
I don't think we'd lose much metadata.

But I'd like to hear other opinions as well.

On Thu, Apr 24, 2014 at 7:03 PM, Michal Židek mzi...@redhat.com wrote:
Hello,

this patch replaces strerror with sss_strerror on some places.
I think it would be OK to use always sss_strerror, but to keep
the patch relatively small I left strerror on places where
we directly print value of errno or return value of some third
party functions that do not (and never will) return our specific
error codes.

Patch is attached. It may look big (111 files changed), but
there are only few insertions and deletions in each.

Thanks,
Michal


Rebased version attached.

Michal


From fce8ba028a10087c2e71781653b65cad733aef25 Mon Sep 17 00:00:00 2001
From: Michal Zidek mzi...@redhat.com
Date: Thu, 24 Apr 2014 17:26:37 +0200
Subject: [PATCH] Use sss_strerror instead of strerror.

---
 Makefile.am|  9 +--

//snip

 111 files changed, 479 insertions(+), 446 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 8d3d366..c9defa8 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1290,7 +1290,7 @@ resolv_tests_LDADD = \
 $(SSSD_LIBS) \
 $(CHECK_LIBS) \
 $(CARES_LIBS) \
-libsss_debug.la \
+$(SSSD_INTERNAL_LTLIBS) \
 libsss_test_common.la
 
 refcount_tests_SOURCES = \
@@ -2347,12 +2347,12 @@ krb5_child_CFLAGS = \
 $(POPT_CFLAGS) \
 $(KRB5_CFLAGS)
 krb5_child_LDADD = \
-libsss_debug.la \
 $(TALLOC_LIBS) \
 $(POPT_LIBS) \
 $(DHASH_LIBS) \
 $(KRB5_LIBS) \
-$(CLIENT_LIBS)
+$(CLIENT_LIBS) \
+$(SSSD_INTERNAL_LTLIBS)

We decided to use $(NULL) at the end of lists in Makefile.am. So you will not
change two lines with adding new library next time.
see 1746e8b8399da2a7a8da4aace186f66055ccfec1

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

[SSSD] [PATCH] IPA: incorrect expansion of group membership when encountering a non-POSIX group

2014-07-16 Thread Pavel Reichl 
Hello,

please see attached patches.

Regards,

Pavel Reichl
From b0bb8006c024046ae3aca2f5837489cd12fe2cd7 Mon Sep 17 00:00:00 2001
From: Pavel Reichl prei...@redhat.com
Date: Wed, 16 Jul 2014 13:33:58 +0100
Subject: [PATCH 1/2] IPA: new attribute map for non-posix groups

Create new set of attributes to be used when processing non-posix groups.

Resolves:
https://fedorahosted.org/sssd/ticket/2343
---
 src/providers/ipa/ipa_common.c |  9 +
 src/providers/ipa/ipa_opts.h   |  8 
 src/providers/ldap/ldap_id.c   |  8 +++-
 src/providers/ldap/sdap.h  | 11 +++
 src/providers/ldap/sdap_async.h|  3 ++-
 src/providers/ldap/sdap_async_initgroups.c | 12 +---
 6 files changed, 46 insertions(+), 5 deletions(-)

diff --git a/src/providers/ipa/ipa_common.c b/src/providers/ipa/ipa_common.c
index f594de27a65ab1ff702d0c593a57e89bfd469532..54d0ecf3b5fa58f4bcf2ec144ecad578bf9c894b 100644
--- a/src/providers/ipa/ipa_common.c
+++ b/src/providers/ipa/ipa_common.c
@@ -568,6 +568,15 @@ int ipa_get_id_options(struct ipa_options *ipa_opts,
 
 ret = sdap_get_map(ipa_opts-id,
cdb, conf_path,
+   ipa_np_group_map,
+   SDAP_OPTS_NP_GROUP,
+   ipa_opts-id-np_group_map);
+if (ret != EOK) {
+goto done;
+}
+
+ret = sdap_get_map(ipa_opts-id,
+   cdb, conf_path,
ipa_netgroup_map,
IPA_OPTS_NETGROUP,
ipa_opts-id-netgroup_map);
diff --git a/src/providers/ipa/ipa_opts.h b/src/providers/ipa/ipa_opts.h
index d7c2b189fad57cb60c29b56c5e351a46070349e2..4dd4077544cb9cd0ae26b14d5eb51cc7f89dfe84 100644
--- a/src/providers/ipa/ipa_opts.h
+++ b/src/providers/ipa/ipa_opts.h
@@ -217,6 +217,14 @@ struct sdap_attr_map ipa_group_map[] = {
 SDAP_ATTR_MAP_TERMINATOR
 };
 
+/* map for non-posix groups */
+struct sdap_attr_map ipa_np_group_map[] = {
+{ ldap_group_object_class, nestedgroup, SYSDB_GROUP_CLASS, NULL },
+{ ldap_group_name, cn, SYSDB_NAME, NULL },
+{ ldap_group_member, member, SYSDB_MEMBER, NULL },
+SDAP_ATTR_MAP_TERMINATOR
+};
+
 struct sdap_attr_map ipa_netgroup_map[] = {
 { ipa_netgroup_object_class, ipaNisNetgroup, SYSDB_NETGROUP_CLASS, NULL },
 { ipa_netgroup_name, cn, SYSDB_NAME, NULL },
diff --git a/src/providers/ldap/ldap_id.c b/src/providers/ldap/ldap_id.c
index c788b6bdd6235f5b940d99382b115a2534dbb1d9..e164cde4cd551b80b95edaed477ca64bf3ea0011 100644
--- a/src/providers/ldap/ldap_id.c
+++ b/src/providers/ldap/ldap_id.c
@@ -919,6 +919,7 @@ struct groups_by_user_state {
 
 const char *name;
 const char **attrs;
+const char **np_attrs;
 
 int dp_error;
 int sdap_ret;
@@ -966,6 +967,10 @@ static struct tevent_req *groups_by_user_send(TALLOC_CTX *memctx,
NULL, state-attrs, NULL);
 if (ret != EOK) goto fail;
 
+ret = build_attrs_from_map(state, ctx-opts-np_group_map, SDAP_OPTS_NP_GROUP,
+   NULL, state-np_attrs, NULL);
+if (ret != EOK) goto fail;
+
 ret = groups_by_user_retry(req);
 if (ret != EOK) {
 goto fail;
@@ -1020,7 +1025,8 @@ static void groups_by_user_connect_done(struct tevent_req *subreq)
   state-ctx,
   state-conn,
   state-name,
-  state-attrs);
+  state-attrs,
+  state-np_attrs);
 if (!subreq) {
 tevent_req_error(req, ENOMEM);
 return;
diff --git a/src/providers/ldap/sdap.h b/src/providers/ldap/sdap.h
index 87f2131ae6e8eea68e4db81b7de6e70a4c0636a7..397e0d6875ddd8977fc29dcdb02d3987bf3ef1ec 100644
--- a/src/providers/ldap/sdap.h
+++ b/src/providers/ldap/sdap.h
@@ -304,6 +304,16 @@ enum sdap_group_attrs {
 SDAP_OPTS_GROUP /* attrs counter */
 };
 
+/* the objectclass must be the first attribute.
+ * Functions depend on this */
+enum sdap_np_group_attrs {
+SDAP_OC_NP_GROUP = 0,
+SDAP_AT_NP_GROUP_NAME,
+SDAP_AT_NP_GROUP_MEMBER,
+
+SDAP_OPTS_NP_GROUP /* attrs counter */
+};
+
 enum sdap_netgroup_attrs {
 SDAP_OC_NETGROUP = 0,
 SDAP_AT_NETGROUP_NAME,
@@ -416,6 +426,7 @@ struct sdap_options {
 struct sdap_attr_map *user_map;
 size_t user_map_cnt;
 struct sdap_attr_map *group_map;
+struct sdap_attr_map *np_group_map;
 struct sdap_attr_map *netgroup_map;
 struct sdap_attr_map *service_map;
 
diff --git a/src/providers/ldap/sdap_async.h b/src/providers/ldap/sdap_async.h
index 808254a249a3758d3f2ac257b7701c3b73526047..2ed7cb7ea38ff8a6108b5470b5b720e63c218eb5 100644
--- a/src/providers/ldap/sdap_async.h
+++ b/src/providers/ldap/sdap_async.h
@@ -134,7 +134,8 @@ struct tevent_req *sdap_get_initgr_send(TALLOC_CTX *memctx,

Re: [SSSD] [PATCH] Retry system bus connection once messagebus is up

2014-07-16 Thread Jakub Hrozek 
On Tue, Jul 15, 2014 at 02:47:21PM +0200, Pavel Březina wrote:
 On 07/14/2014 11:54 AM, Jakub Hrozek wrote:
 On Tue, Jul 08, 2014 at 08:42:43PM +0200, Jakub Hrozek wrote:
 The 1.11 version will be sent out separately.
 
 Here are the 1.11 patches. There are no functional differences, just
 some minor rebasing.
 
 Ack.

sssd-1-11:
* 80af7e9daed52b283af037864bcdd86d96695618
* 42b0c3442815c0374735377c7f5ced4fe1a00e97
* 87d3c7d23885b0b6dca3d7cf0494c7b93225429c
* fbc3f000ca0672bb18797201768bd13e5611eaad
* 3e57c78c8163f6ee395bdf34b1e2c550cd8467f1
* 727f4bf4829f2405c978a4c9b960bef3ad86b002
* 906177a2666bf360a3d85fec55fc942cf9b33163
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCH] sss_sifp: set output parameters if attribute is NULL

2014-07-16 Thread Jakub Hrozek 
On Wed, Jul 16, 2014 at 03:49:37PM +0200, Lukas Slebodnik wrote:
 On (16/07/14 14:38), Jakub Hrozek wrote:
 On Wed, Jul 16, 2014 at 02:10:16PM +0200, Lukas Slebodnik wrote:
  Thank you.
  
  ACK to all
  
  LS
 
 Should we push all three at once?
 
 Normally we only bump the version number before the release. I suggest
 we create a blocker ticket for 1.12.1 to remind us to push the last
 patch instead, in case we had to do some more changes..
 It is your decision :-)

I will only push the first two patches and filed
https://fedorahosted.org/sssd/ticket/2382 to push the last one.

I filed it directly to 1.12.1, I hope it's fine to bypass the triage
with a simple reminder to push a patch.

 You will push patches. I just ACK-ed patches.
 
 LS
 ___
 sssd-devel mailing list
 sssd-devel@lists.fedorahosted.org
 https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCH] sss_sifp: set output parameters if attribute is NULL

2014-07-16 Thread Jakub Hrozek 
On Wed, Jul 16, 2014 at 05:46:07PM +0200, Jakub Hrozek wrote:
 I will only push the first two patches

master:
973be642f3d33ba21ea9c06791295f09efcdba46
b7080f1a2c6c97224c41f6347ca3743e1054faec
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] sss_cache flush ssh hosts list.

2014-07-16 Thread William 

  
 Doesn't appear to be related to anything I have changed I don't
 think ... 
 
 You forgot to change usage of sysdb_store_ssh_host in sysdb_ssh-tests.
 tests cannot be compiled. (make check)
 
 
   CC   src/tests/sysdb_ssh_tests-sysdb_ssh-tests.o
 src/tests/sysdb_ssh-tests.c:179:43: error: too few arguments to function call,
  expected 6, have 5
data-attrs);
   ^
 src/db/sysdb_ssh.h:32:1: note: 'sysdb_store_ssh_host' declared here
 errno_t
 ^
 1 error generated.
 

Did I? I'll check that, and fix it if that's the case.

 From 29cdcbd9a20cfbd72c5a8d103f58e3f153887d73 Mon Sep 17 00:00:00 2001
 From: William B will...@adelaide.edu.au
 Date: Wed, 16 Jul 2014 11:45:02 +0930
 Subject: [PATCH] Allow sss_cache tool to flush known hosts cache
 
 ---
  src/confdb/confdb.h|  2 ++
  src/config/etc/sssd.api.conf   |  1 +
  src/db/sysdb_ssh.c | 58 
  +++---
  src/db/sysdb_ssh.h | 17 -
  src/man/po/sssd-docs.pot   | 17 +
  src/providers/ipa/ipa_hostid.c |  2 +-
  src/tools/sss_cache.c  | 54 +++
  7 files changed, 140 insertions(+), 11 deletions(-)
 
 
 //snip
 
 diff --git a/src/man/po/sssd-docs.pot b/src/man/po/sssd-docs.pot
 index df0456d..a4fce38 100644
 --- a/src/man/po/sssd-docs.pot
 +++ b/src/man/po/sssd-docs.pot
 @@ -1140,6 +1140,23 @@ msgstr 
  msgid Default: 180
  msgstr 
  
 +#. type: Content of: 
 referencerefentryrefsect1refsect2variablelistvarlistentryterm
 +#: sssd.conf.5.xml:878
 +msgid ssh_known_hosts_expire (integer)
 +msgstr 
 +
 +#. type: Content of: 
 referencerefentryrefsect1refsect2variablelistvarlistentrylistitempara
 +#: sssd.conf.5.xml:881
 +msgid 
 +How many seconds to keep a host ssh key after refresh. IE how long to 
 cache 
 +the host key for.
 +msgstr 
 +
 +#. type: Content of: 
 referencerefentryrefsect1refsect2variablelistvarlistentrylistitempara
 +#: sssd.conf.5.xml:885
 +msgid Default: 31536000 (1 Year)
 +msgstr 
 +
  #. type: Content of: referencerefentryrefsect1refsect2title
  #: sssd.conf.5.xml:893
  msgid PAC responder configuration options
 
 We do not update pot files directly.
 Could you edit xml file src/man/sssd.conf.5.xml ?
 

Okay, I'll revert the pot changes, and update the xml. Thanks for that.


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

Re: [SSSD] [PATCH] LDAP: tokengroups do not work with id_provider=ldap

2014-07-16 Thread Jakub Hrozek 

On 10 Jul 2014, at 16:38, Pavel Reichl prei...@redhat.com wrote:

 Hello,
 
 please see attached patches. 
 
 I found out that if we go with approach introduced in previous version
 (in case of LDAP provider assume SID comes from default domain) this can
 lead to resolutions of SIDs like S-1-5-32-545 and also SIDs of non-posix
 groups which in case of disabled id mapping leads to failure which will
 end request prematurely. 2nd patch should make SID resolution more
 resilient to handle this.
 

The only complaint I have about the code is that the domain NULL check is not 
needed, I actually think we should fail if there is a NULL domain in the 
provider code, the provider handler should already take care of finding the 
right domain. If not, we’re in big trouble anyway.

Otherwise the code solves the problem — I can’t say it looks great. That’s not 
your fault Pavel, just yet another reminder that we need to work on the LDAP 
provider refactoring no later than 1.13.

I tested with both id_provider=ldap and =ad using both POSIX attributes and ID 
mapping. Both worked fine in my testing.

Can you re-send the patches without the domain check? Then I’ll ack.

 Regards,
 Pavel Reichl 
 
 On Tue, 2014-07-08 at 09:57 +0200, Sumit Bose wrote:
 On Mon, Jul 07, 2014 at 10:25:23PM +0200, Jakub Hrozek wrote:
 On Mon, Jul 07, 2014 at 07:03:01PM +0200, Pavel Reichl wrote:
 On Mon, 2014-06-23 at 11:41 +0200, Sumit Bose wrote:
 On Mon, Jun 23, 2014 at 11:20:52AM +0200, Jakub Hrozek wrote:
 On Tue, Jun 17, 2014 at 06:30:31PM +0200, Pavel Reichl wrote:
 Hello,
 
 please see attached patch.
 
 Regards,
 
 PR
 
 The patch solves the problem, but I think one part should be improved:
 
 @@ -875,7 +893,13 @@ static void 
 sdap_ad_tokengroups_initgr_mapping_done(struct tevent_req *subreq)
 domain = find_subdomain_by_sid(get_domains_head(state-domain), 
 sid);
 if (domain == NULL) {
 DEBUG(SSSDBG_MINOR_FAILURE, Domain not found for SID 
 %s\n, sid);
 -continue;
 +if (state-domain-parent == NULL 
 +state-domain-subdomains == NULL) {
 +domain = state-domain;
 +DEBUG(SSSDBG_TRACE_FUNC, Using domain %s\n, 
 domain-name);
 +} else {
 +continue;
 +}
 }
 
 I think this is a bit dangerous. I wonder if we should have some
 modification of find_subdomain_by_sid that would return the first
 configured domain if no subdomain provider was configured or if no
 domains had a SID. This could be a separate function.
 
 This sounds even more dangerous to me.
 
 
 Anyhow, find_subdomain_by_sid is misnamed, we routinely use the function
 to find the primary domain.
 
 I think find_subdomain_by_sid() does what the name says and of course it
 can return the primary domain as long as the SID of the domain is know
  ^^
 fwiw, this was my concern, the function is named find_subdomain yet it
 can find both main domain and subdomain. But I won't bikeshed any further.
 
 ah, sorry, now I see your point. I agree that the name misleading but I
 think this can be fixed after the release.
 
 Would 's/find_subdomain_by_sid/find_domain_by_sid/' be sufficient
 solution?
 
 bye,
 Sumit
 
 
 which is the case for the IPA and AD provider.
 
 
 What about adding an explicit check if the running id provider is the
 plain LDAP provider?
 Would that be acceptable with you Jakub?
 
 It's a bit hackish but I don't see any other way.
 
 
 As an alternative the LDAP provider can add a
 special value in the id member of the sss_dom_info struct and then
 find_subdomain_by_sid can handle this case specially?
 
 bye,
 Sumit
 
 ___
 sssd-devel mailing list
 sssd-devel@lists.fedorahosted.org
 https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
 ___
 sssd-devel mailing list
 sssd-devel@lists.fedorahosted.org
 https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
 
 
 ___
 sssd-devel mailing list
 sssd-devel@lists.fedorahosted.org
 https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
 ___
 sssd-devel mailing list
 sssd-devel@lists.fedorahosted.org
 https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
 
 0001-LDAP-tokengroups-do-not-work-with-id_provider-ldap.patch0002-SDAP-Continue-resolving-SID-even-if-some-fail.patch___
 sssd-devel mailing list
 sssd-devel@lists.fedorahosted.org
 https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

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