[SSSD] More upstream CI tests

2015-08-19 Thread Jakub Hrozek 
Hi,

as we're stabilizing the 1.13 branch and before we plan what we want to
work on during the 1.14 development, we should use that time to write
some more tests!

Here are some areas where we could add tests. Please discuss or add your
ideas, I would like to turn this list into tickets we can start
implementing:

* Extend the LDAP provider tests with more dynamic test cases.
- add a user to a group, run sss_cache, assert id user displays the
  new group and getent group displays the new member
- conversely with removing users from groups
* Background refresh
- could be built atop the LDAP NSS tests as well. I think we have
  all the infrastructure in place.
* Local overrides integration test:
- this could be relatively easy, just call the overrides tool and
  request the entry. Could be built atop the existing LDAP tests
  or even use the local provider.
* Add a KDC
- until we have a pam_wrapper, this would only be useful to test
  ldap_child, but adding the KDC instantiation might be worth it
  nonetheless
- there is a protorype of KDC instantiation on the list for some
  time now, since we enabled rootless SSSD
* IFP - could we reuse the existing sbus tests to spawn a custom bus?
* SUDO - can we trick sudo into connecting to our test sssd instance?

I think the order of priority is roughly as above. I think the LDAP
provider is critical enough to be well tested. The refresh and local
override tests might be nice to have because we would be refactoring the
NSS responder in 1.14, so we should have it tested.

I'll be happy to hear other opionions, though!
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCH] NSS: Fix use after free

2015-08-19 Thread Jakub Hrozek 
On Thu, Aug 13, 2015 at 07:41:02AM +0200, Lukas Slebodnik wrote:
> On (12/08/15 14:17), Jakub Hrozek wrote:
> >On Mon, Aug 10, 2015 at 06:38:29AM +0200, Lukas Slebodnik wrote:
> >> ehlo,
> >> 
> >> Use after free can happed if there are two domains and user is not found
> >> in the first one.
> >> 
> >> LS
> >
> >Would it be possible to write a testcase in the NSS responder test?
> It requires multi domain setup.
> So I created different test.
> My intention was to cover most test cases and not just initgroups,
> But attached ins a POC patch which prove there is a use after free.
> make check passes; you need to test with valgrind.
> libtool --mode=execute valgrind -v ./nss-srv-multi-tests
> 
> Would you prefer to use current version of patch and add othter test cases
> later? (it will take some time) or current version is enought for fix?

Ideally I think we should have only one NSS responder test, otherwise we
would end up adding some testcases to one test and not the other...but I
haven't tried, so I don't know how easy or hard that is.

ACK to your crash patch, I'll push it and apply to downstream.
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCH] sss_override: support domains that requires fqname

2015-08-19 Thread Jakub Hrozek 
On Wed, Aug 19, 2015 at 01:52:10PM +0200, Pavel Březina wrote:
> https://fedorahosted.org/sssd/ticket/2757
> 
> I wrote it on top of my previous override patches, but I think it will apply
> on master.

ACK 

(but please fix the ordering issue with the other patches)
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCH] sss_override: support import and export

2015-08-19 Thread Jakub Hrozek 
On Wed, Aug 19, 2015 at 12:46:23PM +0200, Pavel Březina wrote:
> On 08/18/2015 05:24 PM, Pavel Březina wrote:
> >On 08/16/2015 05:59 PM, Pavel Březina wrote:
> >>https://fedorahosted.org/sssd/ticket/2737
> >>
> >>Hi, it should work... :-) However, I wanted to make import as
> >>transaction so no changes are made if some error occurs, but I had some
> >>troubles with it so I gave up eventually.
> >>
> >>Of course, it would be quite difficult to make it safe across domains
> >>thus my intention was only to ensure that either all changes are done in
> >>a domain or none. But still leaving the possibility that changes are
> >>commited in one domain but cancelled in another.
> >>
> >>I tried to start sysdb transaction on all used domains and then
> >>commit/cancel it, writing some neat mechanism for it. However, I
> >>occasionally run into a problem when data provider hangs when trying to
> >>safe a user. It looked like some sort of race condition.
> >>
> >>Unfortunately I managed to delete the code so I can't show it to you, I
> >>think it would be a nice feature so if anyone familiar with ldb want to
> >>step in, he's welcome.
> >
> >New patches attached.
> 
> I split it into more patches.
> 

There is a strange ordering between this patch and the FQDN -- these
patches can't be compiled without the FQDN one, but the FQDN doesn't
apply on master.

There are still some Coverity errors:
Error: COMPILER_WARNING:
sssd-1.13.1/src/tools/common/sss_colondb.c: scope_hint: In function 
'sss_colondb_open'
sssd-1.13.1/src/tools/common/sss_colondb.c:273:12: warning: 'fp' may be used 
uninitialized in this function [-Wmaybe-uninitialized]
# if (fp == NULL && filename != NULL) {
#^
sssd-1.13.1/src/tools/common/sss_colondb.c:259:11: note: 'fp' was declared here
# FILE *fp;
#   ^
#  271|   }
#  272|   
#  273|-> if (fp == NULL && filename != NULL) {
#  274|   ret = errno;
#  275|   DEBUG(SSSDBG_CRIT_FAILURE, "Unable to open file %s [%d]: 
%s\n",

Error: NEGATIVE_RETURNS (CWE-394):
sssd-1.13.1/src/tools/sss_override.c:340: negative_return_fn: Function 
"sss_fqname(NULL, 0UL, domain->names, domain, name)" returns a negative number.
sssd-1.13.1/src/util/usertools.c:629:41: return_negative_constant: Explicitly 
returning negative value "-22".
sssd-1.13.1/src/tools/sss_override.c:340: var_assign: Assigning: unsigned 
variable "fqlen" = "sss_fqname".
sssd-1.13.1/src/tools/sss_override.c:342: var_tested_neg: Unsigned variable 
"fqlen" is incremented, which might cause an integer overflow.
#  340|   fqlen = sss_fqname(NULL, 0, domain->names, domain, name);
#  341|   if (fqlen > 0) {
#  342|-> fqlen++; /* \0 */
#  343|   } else {
#  344|   return NULL;

> From a8063d92d84c13b55f880f09993b5b9769dec8a2 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Pavel=20B=C5=99ezina?= 
> Date: Sat, 15 Aug 2015 13:53:25 +0200
> Subject: [PATCH 1/5] sss_override: print input name if unable to parse it

ACK


> From fcf865ec29f6ef1717a7ca24af6f1bf67ba363a8 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Pavel=20B=C5=99ezina?= 
> Date: Wed, 19 Aug 2015 12:34:08 +0200
> Subject: [PATCH 2/5] TOOLS: add sss_colondb API
> 
> To simplify import/export users and groups.

More or less ack :-) see some questions inline

> +errno_t sss_colondb_readline(TALLOC_CTX *mem_ctx,
> + struct sss_colondb *db,
> + struct sss_colondb_read_field *table)
> +{
> +int readchars;
> +size_t linelen = 0;
> +char *line = NULL;
> +char *tcline;
> +char *rest;
> +errno_t ret;
> +int i;
> +
> +if (db->mode != SSS_COLONDB_READ) {
> +return ERR_INTERNAL;
> +}
> +
> +readchars = getline(&line, &linelen, db->file);
> +if (readchars == -1) {
> +/* Nothing was read. */
> +if (errno != 0) {
> +ret = errno;
> +DEBUG(SSSDBG_CRIT_FAILURE, "Unable to read line [%d]: %s",
> +  ret, sss_strerror(ret));
> +return ret;
> +}
> +
> +return EOF;
> +}
> +
> +/* Copy line to mem_ctx. */
> +tcline = talloc_strdup(mem_ctx, line);

Do we need mem_ctx in this function? What about using db (or a special memctx
instead of db) instead, then the caller could just talloc_free_children
(or we can talloc_free_children even in this function..)

> +
> +free(line);
> +line = NULL;

> From 0877b501f01dff26a312944a47a98cb88eb47d1e Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Pavel=20B=C5=99ezina?= 
> Date: Wed, 19 Aug 2015 12:43:15 +0200
> Subject: [PATCH 3/5] sss_override: decompose code better

ACK

thanks for splitting the patch, makes the review much easier.

> From 1556d7f744302e8e7e3aa4e3459c5a4c7467bbd9 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Pavel=20B=C5=99ezina?= 
> Date: Wed, 19 Aug 2015 12:35:12 +0200
> Subject: [PATCH 4/5] sss_override: support import and export
> 
> Resolves:
> https://fedorahosted.org/sssd/ticket/2737

Please a

[SSSD] [PATCH] TESTS: ldap_id_cleanup timeouts

2015-08-19 Thread Michal Židek 

Hi!

This is another patch to avoid failing tests
in the CI (make-check-valgrind). This time
the ldap_id_cleanup tests.

Looks like the one second cache timeout was too short
and the tests sometimes failed because they expected the
entries to be still valid for a short while
after they were added to sysdb.

I saw the failures only on Fedora 20 CI machine.

See the attached patch.

Michal
>From 35ee376cc0e9674b5fa9ef1c1c3cc3e67152560e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michal=20=C5=BDidek?= 
Date: Wed, 19 Aug 2015 20:10:28 +0200
Subject: [PATCH] TESTS: ldap_id_cleanup timeouts

The one second timeout interval was sometimes
too short when the tests where running under
Valgrind in the CI and the entries expired
too soon.
---
 src/tests/cmocka/test_ldap_id_cleanup.c | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/src/tests/cmocka/test_ldap_id_cleanup.c b/src/tests/cmocka/test_ldap_id_cleanup.c
index 941427e..bdc2568 100644
--- a/src/tests/cmocka/test_ldap_id_cleanup.c
+++ b/src/tests/cmocka/test_ldap_id_cleanup.c
@@ -186,23 +186,26 @@ static void test_id_cleanup_exp_group(void **state)
 const char *empty_special_grp = "empty_gr*o/u\\p(2016)";
 const char *empty_grp = "empty_grp";
 const char *grp = "grp";
+/* This timeout can be bigger because we will call invalidate_group
+ * to expire entries without waiting. */
+uint64_t cache_timeout = 30;
 struct sysdb_test_ctx *test_ctx = talloc_get_type_abort(*state,
 struct sysdb_test_ctx);
 
 ret = sysdb_store_group(test_ctx->domain, special_grp,
-10002, NULL, 1, 0);
+10002, NULL, cache_timeout, 0);
 assert_int_equal(ret, EOK);
 
 ret = sysdb_store_group(test_ctx->domain, empty_special_grp,
-10003, NULL, 1, 0);
+10003, NULL, cache_timeout, 0);
 assert_int_equal(ret, EOK);
 
 ret = sysdb_store_group(test_ctx->domain, grp,
-10004, NULL, 1, 0);
+10004, NULL, cache_timeout, 0);
 assert_int_equal(ret, EOK);
 
 ret = sysdb_store_group(test_ctx->domain, empty_grp,
-10005, NULL, 1, 0);
+10005, NULL, cache_timeout, 0);
 assert_int_equal(ret, EOK);
 
 ret = sysdb_store_user(test_ctx->domain, "test_user", NULL,
-- 
2.1.0

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

Re: [SSSD] [PATCH] SYSDB: Index the objectSIDString attribute

2015-08-19 Thread Jakub Hrozek 
On Tue, Aug 18, 2015 at 05:31:43PM +0200, Michal Židek wrote:
> On 08/17/2015 10:35 PM, Jakub Hrozek wrote:
> >Hi,
> >
> >the attached patch was confirmed to work, so the code review should be
> >easy. But because it adds an index to objectSID, which all AD objects
> >have, there are two catches:
> > 1) How log the upgrade takes
> 
> My main concern was the heartbeat interval. But I just tested it by
> adding sleep(50) into your newly added upgrade function and it
> work fine with default heartbeat (10 seconds). So I guess monitor
> starts pinging the domain backend after it is fully initialized
> and not sooner.
> 
> > 2) How much the database grows
> >
> >To test, I created an AD instance with 10.000 users and 10.000 groups
> >(adcli is great for this type of testing btw). Fetched all groups and
> >users with the old db, then ran the upgrade.
> >
> >On a VM running on my local laptop (granted, it has an SSD drive), the
> >upgrade takes 10 seconds. The default systemd startup timeout
> >(DefaultTimeoutStartSec) seems to be 90 seconds, which sounds OK to me.
> >
> >The size, though, has nearly doubled. This is backup before upgrade:
> > [root@adclient ~]# du -sh /root/cache_win.trust.test.ldb
> > 47M /root/cache_win.trust.test.ldb
> >And this is the cache after I upgraded:
> > [root@adclient ~]# du -sh /var/lib/sss/db/cache_win.trust.test.ldb
> > 97M /var/lib/sss/db/cache_win.trust.test.ldb
> >
> >Unfortunately, I don't see us having another option than doing the
> >upgrade. For attributes that are indexed, an index miss means that ldb
> >is not going to search sequentially, but just return not found -- so
> >adding indexes for newly added entries is not possible.
> >
> >I would like to document that for huge databases (tens of thousands of
> >cached entries), the timeout might need to be raised during the upgrade
> >and than for deployments whose cache resides in tmpfs, the cache size
> >might grow.
> >
> >Is everyone OK with this?
> >
> 
> It should be in realease notes, but I am not sure if that
> is enough visible place, so I would suggest putting it to Wiki
> as well (https://fedorahosted.org/sssd/wiki/Troubleshooting)
> for the case when someone hits the systemd timeout.
> 
> ACK to the patch.

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

Re: [SSSD] [PATCH] sdap_async: Use specific errmsg when available

2015-08-19 Thread Jakub Hrozek 
On Wed, Aug 19, 2015 at 03:40:48PM +0200, Pavel Reichl wrote:
> 
> 
> On 08/19/2015 01:02 PM, Michal Židek wrote:
> >On 08/19/2015 10:12 AM, Jakub Hrozek wrote:
> >>On Tue, Aug 18, 2015 at 09:34:09PM +0200, Michal Židek wrote:
> >>>Hi!
> >>>
> >>>The attached change was requested by:
> >>>https://bugzilla.redhat.com/show_bug.cgi?id=1254723
> >>>
> >>>Thanks,
> >>>
> >>>Michal
> >>
> >>Thanks for the patch. Can you please resubmit wit the ticket URL in the
> >>commit message and build a test RPM for the original reporter in the
> >>RHBZ ?
> >
> >Sure. Patch with amended commit message is attached.
> >Going to build the RPM now.
> >
> >Michal
> >
> >
> >
> >___
> >sssd-devel mailing list
> >sssd-devel@lists.fedorahosted.org
> >https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
> Code LGTM. I tested patch against openldap with enabled ppolicy with set
> password quality and password history.
> 
> ci passed
> http://sssd-ci.duckdns.org/logs/job/23/46/summary.html
> 
> ACK

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

Re: [SSSD] [PATCH] UTIL: Alternative way for debug message initialisation

2015-08-19 Thread Michal Židek 

On 08/12/2015 01:11 PM, Lukas Slebodnik wrote:

ehlo,

attached patch shoul avoid situation as in commit 
7c69221077c780e62f6c536e78675f2dc1c131bc
The comments does not guarantee anything.

Author: Michal Zidek 
Date:   Tue Mar 10 17:30:48 2015 +0100

 DEBUG: Add missing strings for error messages

 We had more error codes than corresponding
 messages. Also order of two messages was wrong.

Unfortunatelly, the index of enum do not start from 0
and need to be converted using SSSD_ERR_IDX. I used shorter version of
macro "idx". Feel free to propose nicer name.


I would use the SSSD_ERR_IDX and not create an alias for it.
I think it is always better to have properly namespaced macros
even if the are a little longer.



LS




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

Re: [SSSD] [PATCH] sdap_async: Use specific errmsg when available

2015-08-19 Thread Pavel Reichl 



On 08/19/2015 01:02 PM, Michal Židek wrote:

On 08/19/2015 10:12 AM, Jakub Hrozek wrote:

On Tue, Aug 18, 2015 at 09:34:09PM +0200, Michal Židek wrote:

Hi!

The attached change was requested by:
https://bugzilla.redhat.com/show_bug.cgi?id=1254723

Thanks,

Michal


Thanks for the patch. Can you please resubmit wit the ticket URL in the
commit message and build a test RPM for the original reporter in the 
RHBZ ?


Sure. Patch with amended commit message is attached.
Going to build the RPM now.

Michal



___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Code LGTM. I tested patch against openldap with enabled ppolicy with set 
password quality and password history.


ci passed
http://sssd-ci.duckdns.org/logs/job/23/46/summary.html

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

[SSSD] [PATCH] sss_override: support domains that requires fqname

2015-08-19 Thread Pavel Březina 

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

I wrote it on top of my previous override patches, but I think it will 
apply on master.
>From 8b44829f117e8078a65f0d167f2ad44ecc467497 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= 
Date: Wed, 19 Aug 2015 12:28:21 +0200
Subject: [PATCH] sss_override: support domains that requires fqname

Resolves:
https://fedorahosted.org/sssd/ticket/2757
---
 src/tools/sss_override.c | 59 ++--
 1 file changed, 57 insertions(+), 2 deletions(-)

diff --git a/src/tools/sss_override.c b/src/tools/sss_override.c
index ff32951856816fcab2e57bc74a7f8a0711d3d6c1..af09f13950677c7f152f997681b3a0a18ee3420c 100644
--- a/src/tools/sss_override.c
+++ b/src/tools/sss_override.c
@@ -324,6 +324,54 @@ static struct sysdb_attrs *build_group_attrs(TALLOC_CTX *mem_ctx,
 return build_attrs(mem_ctx, group->name, 0, group->gid, 0, NULL, NULL);
 }
 
+static char *get_fqname(TALLOC_CTX *mem_ctx,
+struct sss_domain_info *domain,
+const char *name)
+{
+char *fqname;
+size_t fqlen;
+size_t check;
+
+if (domain == NULL) {
+return NULL;
+}
+
+/* Get length. */
+fqlen = sss_fqname(NULL, 0, domain->names, domain, name);
+if (fqlen > 0) {
+fqlen++; /* \0 */
+} else {
+return NULL;
+}
+
+fqname = talloc_zero_array(mem_ctx, char, fqlen);
+if (fqname == NULL) {
+DEBUG(SSSDBG_CRIT_FAILURE, "talloc_zero_array() failed\n");
+return NULL;
+}
+
+check = sss_fqname(fqname, fqlen, domain->names, domain, name);
+if (check != fqlen - 1) {
+DEBUG(SSSDBG_CRIT_FAILURE, "Failed to generate a fully qualified name "
+  "for user [%s] in [%s]! Skipping user.\n", name, domain->name);
+talloc_free(fqname);
+return NULL;
+}
+
+return fqname;
+}
+
+static char *get_sysname(TALLOC_CTX *mem_ctx,
+ struct sss_domain_info *domain,
+ const char *name)
+{
+if (domain == NULL || !domain->fqnames) {
+return talloc_strdup(mem_ctx, name);
+}
+
+return get_fqname(mem_ctx, domain, name);
+}
+
 static struct sss_domain_info *
 get_object_domain(enum sysdb_member_type type,
   const char *name,
@@ -334,6 +382,7 @@ get_object_domain(enum sysdb_member_type type,
 struct sss_domain_info *dom;
 struct ldb_result *res;
 const char *strtype;
+char *sysname;
 bool check_next;
 errno_t ret;
 
@@ -342,16 +391,22 @@ get_object_domain(enum sysdb_member_type type,
 return NULL;
 }
 
+sysname = get_sysname(tmp_ctx, domain, name);
+if (sysname == NULL) {
+ret = ENOMEM;
+goto done;
+}
+
 /* Ensure that the object is in cache. */
 switch (type) {
 case SYSDB_MEMBER_USER:
-if (getpwnam(name) == NULL) {
+if (getpwnam(sysname) == NULL) {
 ret = ENOENT;
 goto done;
 }
 break;
 case SYSDB_MEMBER_GROUP:
-if (getgrnam(name) == NULL) {
+if (getgrnam(sysname) == NULL) {
 ret = ENOENT;
 goto done;
 }
-- 
1.9.3

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

Re: [SSSD] [PATCH] sdap_async: Use specific errmsg when available

2015-08-19 Thread Michal Židek 

On 08/19/2015 10:12 AM, Jakub Hrozek wrote:

On Tue, Aug 18, 2015 at 09:34:09PM +0200, Michal Židek wrote:

Hi!

The attached change was requested by:
https://bugzilla.redhat.com/show_bug.cgi?id=1254723

Thanks,

Michal


Thanks for the patch. Can you please resubmit wit the ticket URL in the
commit message and build a test RPM for the original reporter in the RHBZ ?


Sure. Patch with amended commit message is attached.
Going to build the RPM now.

Michal

>From 0c58ab37742cfaafd9bd245f33f0f126edb36cda Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michal=20=C5=BDidek?= 
Date: Tue, 18 Aug 2015 20:39:03 +0200
Subject: [PATCH] sdap_async: Use specific errmsg when available

Ticket:
https://fedorahosted.org/sssd/ticket/2762

Use specific errmsg when ldap returns
LDAP_CONSTRAINT_VIOLATION code if that specific
message is available.
---
 src/providers/ldap/sdap_async.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/src/providers/ldap/sdap_async.c b/src/providers/ldap/sdap_async.c
index c5be856..97c9ea5 100644
--- a/src/providers/ldap/sdap_async.c
+++ b/src/providers/ldap/sdap_async.c
@@ -648,8 +648,14 @@ static void sdap_exop_modify_passwd_done(struct sdap_op *op,
 ret = EOK;
 break;
 case LDAP_CONSTRAINT_VIOLATION:
-state->user_error_message = talloc_strdup(state,
-"Please make sure the password meets the complexity constraints.");
+if (errmsg && strlen(errmsg) != 0) {
+state->user_error_message = talloc_strdup(state, errmsg);
+} else {
+state->user_error_message = talloc_strdup(state,
+"Please make sure the password meets the "
+"complexity constraints.");
+}
+
 if (state->user_error_message == NULL) {
 DEBUG(SSSDBG_CRIT_FAILURE, "talloc_strdup failed\n");
 ret = ENOMEM;
-- 
2.1.0

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

Re: [SSSD] [PATCH] sss_override: support import and export

2015-08-19 Thread Pavel Březina 

On 08/18/2015 05:24 PM, Pavel Březina wrote:

On 08/16/2015 05:59 PM, Pavel Březina wrote:

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

Hi, it should work... :-) However, I wanted to make import as
transaction so no changes are made if some error occurs, but I had some
troubles with it so I gave up eventually.

Of course, it would be quite difficult to make it safe across domains
thus my intention was only to ensure that either all changes are done in
a domain or none. But still leaving the possibility that changes are
commited in one domain but cancelled in another.

I tried to start sysdb transaction on all used domains and then
commit/cancel it, writing some neat mechanism for it. However, I
occasionally run into a problem when data provider hangs when trying to
safe a user. It looked like some sort of race condition.

Unfortunately I managed to delete the code so I can't show it to you, I
think it would be a nice feature so if anyone familiar with ldb want to
step in, he's welcome.


New patches attached.


I split it into more patches.

>From a8063d92d84c13b55f880f09993b5b9769dec8a2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= 
Date: Sat, 15 Aug 2015 13:53:25 +0200
Subject: [PATCH 1/5] sss_override: print input name if unable to parse it

---
 src/tools/sss_override.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/tools/sss_override.c b/src/tools/sss_override.c
index 5e901e2e31de64dacb171337defc03d428f8ed57..e84a7b922dfcf179f8010dc4cced0eafd89a2c76 100644
--- a/src/tools/sss_override.c
+++ b/src/tools/sss_override.c
@@ -74,7 +74,7 @@ static int parse_cmdline(struct sss_cmdline *cmdline,
 ret = sss_tool_parse_name(tool_ctx, tool_ctx, input_name,
   &orig_name, &domain);
 if (ret != EOK) {
-fprintf(stderr, _("Unable to parse name.\n"));
+fprintf(stderr, _("Unable to parse name %s.\n"), input_name);
 return ret;
 }
 
-- 
1.9.3

>From fcf865ec29f6ef1717a7ca24af6f1bf67ba363a8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= 
Date: Wed, 19 Aug 2015 12:34:08 +0200
Subject: [PATCH 2/5] TOOLS: add sss_colondb API

To simplify import/export users and groups.
---
 src/tools/common/sss_colondb.c | 305 +
 src/tools/common/sss_colondb.h |  73 ++
 2 files changed, 378 insertions(+)
 create mode 100644 src/tools/common/sss_colondb.c
 create mode 100644 src/tools/common/sss_colondb.h

diff --git a/src/tools/common/sss_colondb.c b/src/tools/common/sss_colondb.c
new file mode 100644
index ..a49776a69e6b701ec867cb0db3dcb41405b5b75b
--- /dev/null
+++ b/src/tools/common/sss_colondb.c
@@ -0,0 +1,305 @@
+/*
+Authors:
+Pavel Březina 
+
+Copyright (C) 2015 Red Hat
+
+This program 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; either version 3 of the License, or
+(at your option) any later version.
+
+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 not, see .
+*/
+
+#include 
+
+#include "util/util.h"
+#include "util/strtonum.h"
+#include "tools/common/sss_colondb.h"
+
+#define IS_STD_FILE(db) ((db)->file == stdin || (db)->file == stdout)
+
+static char *read_field_as_string(char *line,
+  const char **_value)
+{
+char *rest;
+char *value;
+
+if (line == NULL || *line == '\n' || *line == '\0') {
+/* There is nothing else to read. */
+rest = NULL;
+value = NULL;
+goto done;
+}
+
+if (*line == ':') {
+/* Special case for empty value. */
+*line = '\0';
+rest = line + 1;
+value = NULL;
+goto done;
+}
+
+/* Value starts at current position. */
+value = line;
+
+/* Find next field delimiter. */
+rest = strchr(line, ':');
+if (rest == NULL) {
+/* There is no more field. Remove \n from the end. */
+rest = strchr(line, '\n');
+if (rest != NULL) {
+*rest = '\0';
+rest = NULL;
+}
+goto done;
+}
+
+/* Remove it and step one character further. */
+*rest = '\0';
+rest++;
+
+done:
+*_value = value;
+
+return rest;
+}
+
+static char *read_field_as_uint32(char *line,
+  uint32_t *_value)
+{
+const char *str;
+char *rest;
+errno_t ret;
+
+rest = read_field_as_string(line, &str);
+if (rest == NULL || str == NULL) {
+*_value = 0;
+return rest;
+}
+
+*_value =

Re: [SSSD] [PATCH] sdap_async: Use specific errmsg when available

2015-08-19 Thread Jakub Hrozek 
On Tue, Aug 18, 2015 at 09:34:09PM +0200, Michal Židek wrote:
> Hi!
> 
> The attached change was requested by:
> https://bugzilla.redhat.com/show_bug.cgi?id=1254723
> 
> Thanks,
> 
> Michal

Thanks for the patch. Can you please resubmit wit the ticket URL in the
commit message and build a test RPM for the original reporter in the RHBZ ?
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel