[SSSD] [sssd PR#59][comment] ipa_netgroups: Lowercase key to htable
URL: https://github.com/SSSD/sssd/pull/59 Title: #59: ipa_netgroups: Lowercase key to htable mzidek-rh commented: """ Hi, I am reopening this ticket, because FreeIPA UQE have a test. Btw. I can change the patch to not lowercase the key in the first place (while creating the entry in the table). The point is the cases need to be constant. I am just not sure if that can brake something. We do not have extensive upstream tests for FreeIPA and netgroups and the lowercasing seem to be intentional, even though I do believe it is just result of unnecessary normalization. """ See the full comment at https://github.com/SSSD/sssd/pull/59#issuecomment-259368496 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#76][+Rejected] AD: Fix crash in AD subdomain reinit
URL: https://github.com/SSSD/sssd/pull/76 Title: #76: AD: Fix crash in AD subdomain reinit Label: +Rejected ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#76][closed] AD: Fix crash in AD subdomain reinit
URL: https://github.com/SSSD/sssd/pull/76 Author: justin-stephenson Title: #76: AD: Fix crash in AD subdomain reinit Action: closed To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/76/head:pr76 git checkout pr76 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#76][comment] AD: Fix crash in AD subdomain reinit
URL: https://github.com/SSSD/sssd/pull/76 Title: #76: AD: Fix crash in AD subdomain reinit lslebodn commented: """ Closing as duplicate of PR #74 """ See the full comment at https://github.com/SSSD/sssd/pull/76#issuecomment-259370381 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#59][closed] ipa_netgroups: Lowercase key to htable
URL: https://github.com/SSSD/sssd/pull/59 Author: mzidek-rh Title: #59: ipa_netgroups: Lowercase key to htable Action: closed To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/59/head:pr59 git checkout pr59 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#59][comment] ipa_netgroups: Lowercase key to htable
URL: https://github.com/SSSD/sssd/pull/59 Title: #59: ipa_netgroups: Lowercase key to htable lslebodn commented: """ Please open new PR with correct ticket attached. You can inspire in https://fedorahosted.org/sssd/ticket/2275 """ See the full comment at https://github.com/SSSD/sssd/pull/59#issuecomment-259370918 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#59][comment] ipa_netgroups: Lowercase key to htable
URL: https://github.com/SSSD/sssd/pull/59 Title: #59: ipa_netgroups: Lowercase key to htable lslebodn commented: """ It looks like you forgot how to read. this PR is completely wrong. Subject and fix are wrong. Reopening does not help. """ See the full comment at https://github.com/SSSD/sssd/pull/59#issuecomment-259370731 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#59][reopened] ipa_netgroups: Lowercase key to htable
URL: https://github.com/SSSD/sssd/pull/59 Author: mzidek-rh Title: #59: ipa_netgroups: Lowercase key to htable Action: reopened To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/59/head:pr59 git checkout pr59 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#59][comment] ipa_netgroups: Lowercase key to htable
URL: https://github.com/SSSD/sssd/pull/59 Title: #59: ipa_netgroups: Lowercase key to htable mzidek-rh commented: """ Well, changing the ticket is a detail, we can change the ticket wording if necessary. I would like to get some comments on the lowercasing and if we want to go with the solution or if we want to skip the lowercasing when the entry is created (or some other solution). I do not think that the discussion need to be moved from this PR. """ See the full comment at https://github.com/SSSD/sssd/pull/59#issuecomment-259372380 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#77][opened] Qualify ghost users with RFC2307bis and ldap_nesting_level=0
URL: https://github.com/SSSD/sssd/pull/77
Author: jhrozek
Title: #77: Qualify ghost users with RFC2307bis and ldap_nesting_level=0
Action: opened
PR body:
"""
None
"""
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/77/head:pr77
git checkout pr77
From 33efb0fdc7e6efce325b7a6fbcaa943f15a91a98 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek
Date: Wed, 9 Nov 2016 11:59:10 +0100
Subject: [PATCH 1/2] Qualify ghost user attribute in case
ldap_group_nesting_level is set to 0
When the sssd is set to not resolve nested groups with RFC2307bis, then
the LDAP provider takes a different path. We didn't qualify the ghost
users in this case.
Resolves:
https://fedorahosted.org/sssd/ticket/3236
---
src/providers/ldap/sdap_async_groups.c | 15 +++
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/src/providers/ldap/sdap_async_groups.c b/src/providers/ldap/sdap_async_groups.c
index 08dfa01..8150379 100644
--- a/src/providers/ldap/sdap_async_groups.c
+++ b/src/providers/ldap/sdap_async_groups.c
@@ -1668,7 +1668,7 @@ static void sdap_process_group_members(struct tevent_req *subreq)
struct sdap_process_group_state *state =
tevent_req_data(req, struct sdap_process_group_state);
struct ldb_message_element *el;
-uint8_t* name_string;
+char *name_string;
state->check_count--;
DEBUG(SSSDBG_TRACE_ALL, "Members remaining: %zu\n", state->check_count);
@@ -1694,11 +1694,18 @@ static void sdap_process_group_members(struct tevent_req *subreq)
goto next;
}
-name_string = el[0].values[0].data;
+name_string = sss_create_internal_fqname(state,
+(const char *) el[0].values[0].data,
+state->dom->name);
+if (name_string == NULL) {
+ret = ENOMEM;
+goto next;
+}
+
state->ghost_dns->values[state->ghost_dns->num_values].data =
-talloc_steal(state->ghost_dns->values, name_string);
+talloc_steal(state->ghost_dns->values, (uint8_t *) name_string);
state->ghost_dns->values[state->ghost_dns->num_values].length =
-strlen((char *)name_string);
+strlen(name_string);
state->ghost_dns->num_values++;
next:
From 0ff80facde7856c062fa4ace49f89301d75446bb Mon Sep 17 00:00:00 2001
From: Jakub Hrozek
Date: Wed, 9 Nov 2016 11:59:34 +0100
Subject: [PATCH 2/2] tests: Add a test for group resolution with
ldap_group_nesting_level=0
---
src/tests/intg/test_ldap.py | 26 ++
1 file changed, 26 insertions(+)
diff --git a/src/tests/intg/test_ldap.py b/src/tests/intg/test_ldap.py
index 7f0b8ff..667ae27 100644
--- a/src/tests/intg/test_ldap.py
+++ b/src/tests/intg/test_ldap.py
@@ -951,3 +951,29 @@ def test_remove_user_from_nested_group(ldap_conn,
dict(mem=ent.contains_only("user2")))
ent.assert_group_by_name("group3",
dict(mem=ent.contains_only()))
+
+def zero_nesting_sssd_conf(ldap_conn, schema):
+"""Format an SSSD configuration with group nesting disabled"""
+return \
+format_basic_conf(ldap_conn, schema) + \
+unindent("""
+[domain/LDAP]
+ldap_group_nesting_level= 0
+""").format(INTERACTIVE_TIMEOUT)
+
+@pytest.fixture
+def rfc2307bis_no_nesting(request, ldap_conn):
+ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
+ent_list.add_user("user1", 1001, 2001)
+ent_list.add_group_bis("group1", 20001, member_uids=["user1"])
+create_ldap_fixture(request, ldap_conn, ent_list)
+create_conf_fixture(request,
+zero_nesting_sssd_conf(
+ldap_conn,
+SCHEMA_RFC2307_BIS))
+create_sssd_fixture(request)
+return None
+
+def test_zero_nesting_level(ldap_conn, rfc2307bis_no_nesting):
+ent.assert_group_by_name("group1",
+ dict(mem=ent.contains_only("user1")))
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] Re: about letting the responder choose the sysdb optimization level
On Tue, Nov 08, 2016 at 10:28:20AM +0100, Jakub Hrozek wrote:
> Hi,
>
> I would like to ask for opinions about:
> https://fedorahosted.org/sssd/ticket/3126
>
> The basic idea is that the responder would choose what kind of optimization
> would the back end perform when saving the sysdb entries. Requests that
> just return information might choose to optimize very aggressively (using
> modifyTimestamp) and requests that actually authenticate or authorize the
> user might choose to not optimize at all to avoid issues like the ones we saw
> with virtual attributes that don't bump the modifyTimestamp attribute at all.
>
> On the responder side, this is quite easy, just send an additional flag
> during the responder request. It's the provider part I'm not so sure
> about, because there the optimizations are performed at the sysdb level.
>
> So far I can only think about extending sysdb_transaction_start() (or
> providing sysdb_opt_transaction_start and letting the old
> sysdb_transaction_start default to no optimization) which would
> internally keep track of the active transaction and the optimization we
> want to perform. Since only sssd_be is the cache writer and there is
> only one cache per domain.
>
> Additionally, we would have to keep the transaction optimization level around
> in some context until the request bubbles from the data provider handler to
> actually saving the transaction. I don't I hope this won't be too messy, but
> since the requests are asynchronous, so far I don't see any way around
> it. The only thing that might be less messy in the long term is to provide
> a bit more generic structure ("request status") that would so far only
> include the optimization level and later might be extended to include
> e.g. intermediate data. But on the other hand, I'm not sure I have
> thought about passing the data between requests hard enough to design
> this properly. Should I?
>
> Any other opinions? Thoughts?
I'm not sure if this wouldn't confuse users. If I understood the
proposal correctly the typical nss calls like getpwnam(), getgrnam() etc
will use the optimized requests with modifyTimestamp checks. If there is
a server which does not change modifyTimestamp on updates changes in
e.g. the user's shell or in the list of group-members will not be
visible even after calling 'sss_cache -E' because modifyTimestamp didn't
change on the server. But typically a user does not log in again to
check for such changes but just calls e.g. 'getent group changed_group'
again and again waiting for changes.
I think if there are really such servers out there which do not update
modifyTimestamp it should be just possible to switch of the time-stamp
cache generally in this case (I wonder if this is already possible by
setting ldap_{user|group|netgroup}_modify_timestamp to a non-existing
attribute?). In this case the number of cache updates can be reduced by
increasing the cache timeout because during authenticating and
authorization fresh data is requested from the backend anyways.
I think the time would be better spend e.g. on
https://fedorahosted.org/sssd/ticket/3211 "Refactor the
sdap_async_groups.c module" and make sure all needed data is read from
LDAP (multiple LDAP connections might be involved here) first and
written to the cache in a single transaction.
bye,
Sumit
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#59][closed] ipa_netgroups: Lowercase key to htable
URL: https://github.com/SSSD/sssd/pull/59 Author: mzidek-rh Title: #59: ipa_netgroups: Lowercase key to htable Action: closed To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/59/head:pr59 git checkout pr59 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#59][comment] ipa_netgroups: Lowercase key to htable
URL: https://github.com/SSSD/sssd/pull/59 Title: #59: ipa_netgroups: Lowercase key to htable lslebodn commented: """ Please do not reopen this PR anymore. Mails are much more suitable for such discussion. """ See the full comment at https://github.com/SSSD/sssd/pull/59#issuecomment-259401554 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#65][comment] Fixing of nitpicks
URL: https://github.com/SSSD/sssd/pull/65
Title: #65: Fixing of nitpicks
celestian commented:
"""
@pbrezina please, could you join discussion?
How we can see we call function ```rdp_process_pending_call()``` on two places.
And we are not consistent on return value checking. Is there reason for this?
Or should we use one form for it?
```
$ git grep -n -a5 'rdp_process_pending_call' -- '*.c'
src/responder/common/data_provider/rdp_message.c-81-}
src/responder/common/data_provider/rdp_message.c-82-
src/responder/common/data_provider/rdp_message.c-83-return ret;
src/responder/common/data_provider/rdp_message.c-84-}
src/responder/common/data_provider/rdp_message.c-85-
src/responder/common/data_provider/rdp_message.c:86:static errno_t
rdp_process_pending_call(TALLOC_CTX *mem_ctx,
src/responder/common/data_provider/rdp_message.c-87-
DBusPendingCall *pending,
src/responder/common/data_provider/rdp_message.c-88-
DBusMessage **_reply)
src/responder/common/data_provider/rdp_message.c-89-{
src/responder/common/data_provider/rdp_message.c-90-DBusMessage *reply;
src/responder/common/data_provider/rdp_message.c-91-dbus_bool_t bret;
--
src/responder/common/data_provider/rdp_message.c-198-errno_t ret;
src/responder/common/data_provider/rdp_message.c-199-
src/responder/common/data_provider/rdp_message.c-200-req =
talloc_get_type(ptr, struct tevent_req);
src/responder/common/data_provider/rdp_message.c-201-state =
tevent_req_data(req, struct rdp_message_state);
src/responder/common/data_provider/rdp_message.c-202-
src/responder/common/data_provider/rdp_message.c:203:ret =
rdp_process_pending_call(state, pending, &state->reply);
src/responder/common/data_provider/rdp_message.c-204-if (ret != EOK) {
src/responder/common/data_provider/rdp_message.c-205-
tevent_req_error(req, ret);
src/responder/common/data_provider/rdp_message.c-206-return;
src/responder/common/data_provider/rdp_message.c-207-}
src/responder/common/data_provider/rdp_message.c-208-
--
src/responder/common/data_provider/rdp_message.c-266-dbus_bool_t dbret;
src/responder/common/data_provider/rdp_message.c-267-errno_t ret;
src/responder/common/data_provider/rdp_message.c-268-
src/responder/common/data_provider/rdp_message.c-269-sbus_req =
talloc_get_type(ptr, struct sbus_request);
src/responder/common/data_provider/rdp_message.c-270-
src/responder/common/data_provider/rdp_message.c:271:ret =
rdp_process_pending_call(sbus_req, pending, &reply);
src/responder/common/data_provider/rdp_message.c-272-if (reply == NULL) {
src/responder/common/data_provider/rdp_message.c-273-/* Something bad
happened. Just kill the request. */
src/responder/common/data_provider/rdp_message.c-274-ret = EIO;
src/responder/common/data_provider/rdp_message.c-275-goto done;
src/responder/common/data_provider/rdp_message.c-276-}
```
"""
See the full comment at
https://github.com/SSSD/sssd/pull/65#issuecomment-259402267
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] Re: about letting the responder choose the sysdb optimization level
On Wed, 2016-11-09 at 13:08 +0100, Sumit Bose wrote:
> On Tue, Nov 08, 2016 at 10:28:20AM +0100, Jakub Hrozek wrote:
> > Hi,
> >
> > I would like to ask for opinions about:
> > https://fedorahosted.org/sssd/ticket/3126
> >
> > The basic idea is that the responder would choose what kind of optimization
> > would the back end perform when saving the sysdb entries. Requests that
> > just return information might choose to optimize very aggressively (using
> > modifyTimestamp) and requests that actually authenticate or authorize the
> > user might choose to not optimize at all to avoid issues like the ones we
> > saw
> > with virtual attributes that don't bump the modifyTimestamp attribute at
> > all.
> >
> > On the responder side, this is quite easy, just send an additional flag
> > during the responder request. It's the provider part I'm not so sure
> > about, because there the optimizations are performed at the sysdb level.
> >
> > So far I can only think about extending sysdb_transaction_start() (or
> > providing sysdb_opt_transaction_start and letting the old
> > sysdb_transaction_start default to no optimization) which would
> > internally keep track of the active transaction and the optimization we
> > want to perform. Since only sssd_be is the cache writer and there is
> > only one cache per domain.
> >
> > Additionally, we would have to keep the transaction optimization level
> > around
> > in some context until the request bubbles from the data provider handler to
> > actually saving the transaction. I don't I hope this won't be too messy, but
> > since the requests are asynchronous, so far I don't see any way around
> > it. The only thing that might be less messy in the long term is to provide
> > a bit more generic structure ("request status") that would so far only
> > include the optimization level and later might be extended to include
> > e.g. intermediate data. But on the other hand, I'm not sure I have
> > thought about passing the data between requests hard enough to design
> > this properly. Should I?
> >
> > Any other opinions? Thoughts?
>
> I'm not sure if this wouldn't confuse users. If I understood the
> proposal correctly the typical nss calls like getpwnam(), getgrnam() etc
> will use the optimized requests with modifyTimestamp checks. If there is
> a server which does not change modifyTimestamp on updates changes in
> e.g. the user's shell or in the list of group-members will not be
> visible even after calling 'sss_cache -E' because modifyTimestamp didn't
> change on the server. But typically a user does not log in again to
> check for such changes but just calls e.g. 'getent group changed_group'
> again and again waiting for changes.
>
> I think if there are really such servers out there which do not update
> modifyTimestamp it should be just possible to switch of the time-stamp
> cache generally in this case (I wonder if this is already possible by
> setting ldap_{user|group|netgroup}_modify_timestamp to a non-existing
> attribute?). In this case the number of cache updates can be reduced by
> increasing the cache timeout because during authenticating and
> authorization fresh data is requested from the backend anyways.
There are cases where things like CoS are used that can change an
attribute value (even dynamically) but do not alter the modifyTimestamp.
In general operational attributes can behave that way.
> I think the time would be better spend e.g. on
> https://fedorahosted.org/sssd/ticket/3211 "Refactor the
> sdap_async_groups.c module" and make sure all needed data is read from
> LDAP (multiple LDAP connections might be involved here) first and
> written to the cache in a single transaction.
The problem is deciding when to forcibly reload data we already have in
the cache, regardless of cache status.
Simo.
--
Simo Sorce * Red Hat, Inc * New York
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#72][comment] Remove two unused methods from monitor's sbus interface
URL: https://github.com/SSSD/sssd/pull/72 Title: #72: Remove two unused methods from monitor's sbus interface lslebodn commented: """ Obvious ACK LS """ See the full comment at https://github.com/SSSD/sssd/pull/72#issuecomment-259404349 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#72][comment] Remove two unused methods from monitor's sbus interface
URL: https://github.com/SSSD/sssd/pull/72 Title: #72: Remove two unused methods from monitor's sbus interface lslebodn commented: """ master: * fd25e68446ae86135489edb0823607b394f4ec40 * ab792150c97bd6eba1f8cd46653f41a0c64fd765 LS """ See the full comment at https://github.com/SSSD/sssd/pull/72#issuecomment-259404664 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#72][+Pushed] Remove two unused methods from monitor's sbus interface
URL: https://github.com/SSSD/sssd/pull/72 Title: #72: Remove two unused methods from monitor's sbus interface Label: +Pushed ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#72][closed] Remove two unused methods from monitor's sbus interface
URL: https://github.com/SSSD/sssd/pull/72 Author: jhrozek Title: #72: Remove two unused methods from monitor's sbus interface Action: closed To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/72/head:pr72 git checkout pr72 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#46][synchronized] sss_client: Defer thread cancellation until completion of nss/pam operations
URL: https://github.com/SSSD/sssd/pull/46
Author: HouzuoGuo
Title: #46: sss_client: Defer thread cancellation until completion of nss/pam
operations
Action: synchronized
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/46/head:pr46
git checkout pr46
From 176a84c1add79398f342157a12945838bbb1ef0e Mon Sep 17 00:00:00 2001
From: Howard Guo
Date: Tue, 11 Oct 2016 10:35:13 +0200
Subject: [PATCH 1/2] sss_client: Defer thread cancellation until completion of
nss/pam operations
https://fedorahosted.org/sssd/ticket/3156
The client code is not cancellation-safe, an application which has cancelled an NSS operation
will experience subtle bugs, hence thread cancellation is deferred until completion of client
operations.
---
Makefile.am | 4 ---
src/sss_client/common.c | 74 +
2 files changed, 7 insertions(+), 71 deletions(-)
diff --git a/Makefile.am b/Makefile.am
index 1ea8f50..ca561de 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -771,10 +771,6 @@ endif
CLIENT_LIBS = $(LTLIBINTL)
-if HAVE_PTHREAD
-CLIENT_LIBS += -lpthread
-endif
-
if WITH_JOURNALD
SYSLOG_LIBS = $(JOURNALD_LIBS)
endif
diff --git a/src/sss_client/common.c b/src/sss_client/common.c
index 20106b1..61ec55e 100644
--- a/src/sss_client/common.c
+++ b/src/sss_client/common.c
@@ -1071,85 +1071,33 @@ struct sss_mutex {
pthread_mutex_t mtx;
pthread_once_t once;
-sss_mutex_init init;
+int old_cancel_state;
};
-static void sss_nss_mt_init(void);
-static void sss_pam_mt_init(void);
static void sss_nss_mc_mt_init(void);
static struct sss_mutex sss_nss_mtx = { .mtx = PTHREAD_MUTEX_INITIALIZER,
-.once = PTHREAD_ONCE_INIT,
-.init = sss_nss_mt_init };
+.once = PTHREAD_ONCE_INIT };
static struct sss_mutex sss_pam_mtx = { .mtx = PTHREAD_MUTEX_INITIALIZER,
-.once = PTHREAD_ONCE_INIT,
-.init = sss_pam_mt_init };
+.once = PTHREAD_ONCE_INIT };
static struct sss_mutex sss_nss_mc_mtx = { .mtx = PTHREAD_MUTEX_INITIALIZER,
- .once = PTHREAD_ONCE_INIT,
- .init = sss_nss_mc_mt_init };
-
-/* Wrappers for robust mutex support */
-static int sss_mutexattr_setrobust (pthread_mutexattr_t *attr)
-{
-#ifdef HAVE_PTHREAD_MUTEXATTR_SETROBUST
-return pthread_mutexattr_setrobust(attr, PTHREAD_MUTEX_ROBUST);
-#elif defined(HAVE_PTHREAD_MUTEXATTR_SETROBUST_NP)
-return pthread_mutexattr_setrobust_np(attr, PTHREAD_MUTEX_ROBUST_NP);
-#else
-#warning Robust mutexes are not supported on this platform.
-return 0;
-#endif
-}
-
-static int sss_mutex_consistent(pthread_mutex_t *mtx)
-{
-#ifdef HAVE_PTHREAD_MUTEX_CONSISTENT
-return pthread_mutex_consistent(mtx);
-#elif defined(HAVE_PTHREAD_MUTEX_CONSISTENT_NP)
-return pthread_mutex_consistent_np(mtx);
-#else
-#warning Robust mutexes are not supported on this platform.
-return 0;
-#endif
-}
-
-/* Generic mutex init, lock, unlock functions */
-static void sss_mt_init(struct sss_mutex *m)
-{
-pthread_mutexattr_t attr;
-
-if (pthread_mutexattr_init(&attr) != 0) {
-return;
-}
-if (sss_mutexattr_setrobust(&attr) != 0) {
-return;
-}
-
-pthread_mutex_init(&m->mtx, &attr);
-pthread_mutexattr_destroy(&attr);
-}
+ .once = PTHREAD_ONCE_INIT };
static void sss_mt_lock(struct sss_mutex *m)
{
-pthread_once(&m->once, m->init);
-if (pthread_mutex_lock(&m->mtx) == EOWNERDEAD) {
-sss_cli_close_socket();
-sss_mutex_consistent(&m->mtx);
-}
+pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &m->old_cancel_state);
+pthread_mutex_lock(&m->mtx);
}
static void sss_mt_unlock(struct sss_mutex *m)
{
pthread_mutex_unlock(&m->mtx);
+pthread_setcancelstate(m->old_cancel_state, NULL);
}
/* NSS mutex wrappers */
-static void sss_nss_mt_init(void)
-{
-sss_mt_init(&sss_nss_mtx);
-}
void sss_nss_lock(void)
{
sss_mt_lock(&sss_nss_mtx);
@@ -1160,10 +1108,6 @@ void sss_nss_unlock(void)
}
/* NSS mutex wrappers */
-static void sss_pam_mt_init(void)
-{
-sss_mt_init(&sss_pam_mtx);
-}
void sss_pam_lock(void)
{
sss_mt_lock(&sss_pam_mtx);
@@ -1174,10 +1118,6 @@ void sss_pam_unlock(void)
}
/* NSS mutex wrappers */
-static void sss_nss_mc_mt_init(void)
-{
-sss_mt_init(&sss_nss_mc_mtx);
-}
void sss_nss_mc_lock(void)
{
sss_mt_lock(&sss_nss_mc_mtx);
From 22762ebecdead93764197b0fc245a1d6909b68d7 Mon Sep 17 00:00:00 2001
From: HouzuoGuo
Date: Wed, 9 Nov 2016 12:10:25 +0100
Subject: [PATCH 2/2] Remove unused sss_mutex.once; correct the order of
locking mutex and setting thread cancel state.
---
src/sss_client/common.
[SSSD] Re: [PATCH] Unit tests for pam_sss using pam_wrapper (need help with CI..)
On (11/10/16 19:56), Jakub Hrozek wrote: >On Tue, Sep 13, 2016 at 09:27:32AM +0200, Lukas Slebodnik wrote: >> On (09/08/16 10:18), Jakub Hrozek wrote: >> >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. >> >> http://sssd-ci.duckdns.org/logs-test/job/4/48/summary.html >> http://sssd-ci.duckdns.org/logs-test/job/4/49/summary.html >> >> All test passed only on rhel6 :-( >> Maybe because new test was skipped. >> >> cwrap tests were not executed on debian_testing due to following failures >> /var/lib/jenkins/workspace/ci-test/label/debian_testing/src/tests/cwrap/become_user-tests.sh: >> 3: .: Can't open /cwrap_test_setup.sh >> FAIL become_user-tests.sh (exit status: 2) >> >> @see >> http://sssd-ci.duckdns.org/logs-test/job/4/48/debian_testing/ci-build-debug/src/tests/cwrap/become_user-tests.sh.log >> http://sssd-ci.duckdns.org/logs-test/job/4/48/debian_testing/ci-build-debug/src/tests/cwrap/responder_common-tests.sh.log >> http://sssd-ci.duckdns.org/logs-test/job/4/48/debian_testing/ci-build-debug/src/tests/cwrap/server-tests.sh.log >> http://sssd-ci.duckdns.org/logs-test/job/4/48/debian_testing/ci-build-debug/src/tests/cwrap/usertools-tests.sh.log >> >> mock build failed on epel7 (maybe missing pam_sss-tests.sh in tarball) >> make[3]: Entering directory >> `/builddir/build/BUILD/sssd-1.14.2/src/tests/cwrap' >> make[4]: Entering directory >> `/builddir/build/BUILD/sssd-1.14.2/src/tests/cwrap' >> make[4]: *** No rule to make target `pam_sss-tests.sh', needed by >> `pam_sss-tests.sh.log'. Stop. >> make[4]: *** Waiting for unfinished jobs... >> >> >> The pam_sss-tests test failed on fedora24. There are valgrind errors. >> ==28387== Invalid read of size 1 >> ==28387==at 0x9CC2600: __GI_strchr (in /usr/lib64/libc-2.23.so) >> ==28387==by 0x402234: service_arg (test_wrapper_pam_sss.c:93) >> ==28387==by 0x402D73: test_pam_authenticate_root_ignore >> (test_wrapper_pam_sss.c:571) >> ==28387==by 0x565B542: ??? (in /u
[SSSD] [sssd PR#46][comment] sss_client: Defer thread cancellation until completion of nss/pam operations
URL: https://github.com/SSSD/sssd/pull/46 Title: #46: sss_client: Defer thread cancellation until completion of nss/pam operations HouzuoGuo commented: """ All right, `once` is now gone and mutex is now guarding cancellation state. """ See the full comment at https://github.com/SSSD/sssd/pull/46#issuecomment-259411671 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#50][synchronized] [RFC] Use GNULIB's compiler warning code
URL: https://github.com/SSSD/sssd/pull/50
Author: fidencio
Title: #50: [RFC] Use GNULIB's compiler warning code
Action: synchronized
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/50/head:pr50
git checkout pr50
From 185ff593a0e1010ad67914e5573de4e37cde45a5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?=
Date: Wed, 12 Oct 2016 15:00:04 +0200
Subject: [PATCH 01/19] RESOLV: Fix "-Werror=null-dereference" caught by GCC
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Signed-off-by: Fabiano Fidêncio
---
src/resolv/async_resolv.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/resolv/async_resolv.c b/src/resolv/async_resolv.c
index e859556..e7a4217 100644
--- a/src/resolv/async_resolv.c
+++ b/src/resolv/async_resolv.c
@@ -2247,7 +2247,9 @@ static int reply_weight_rearrange(int len,
new_end = r;
}
}
-new_end->next = NULL;
+if (new_end) {
+new_end->next = NULL;
+}
/* return the rearranged list */
*start = new_start;
From 689f54fd41eb4ec256284d29074fa01d245b4bf5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?=
Date: Wed, 12 Oct 2016 16:02:55 +0200
Subject: [PATCH 02/19] SIFP: Fix "-Wjump-misses-init" caught by GCC
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Signed-off-by: Fabiano Fidêncio
---
src/lib/sifp/sss_sifp_parser.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/lib/sifp/sss_sifp_parser.c b/src/lib/sifp/sss_sifp_parser.c
index 65babb5..43eab4d 100644
--- a/src/lib/sifp/sss_sifp_parser.c
+++ b/src/lib/sifp/sss_sifp_parser.c
@@ -283,7 +283,8 @@ sss_sifp_parse_basic(sss_sifp_ctx *ctx,
uint64_t, uint64_t, uint64, done);
break;
case DBUS_TYPE_STRING:
-case DBUS_TYPE_OBJECT_PATH: ;
+case DBUS_TYPE_OBJECT_PATH:
+{
const char *val = NULL;
dbus_message_iter_get_basic(iter, &val);
@@ -306,6 +307,7 @@ sss_sifp_parse_basic(sss_sifp_ctx *ctx,
ret = SSS_SIFP_OK;
break;
+}
default:
ret = SSS_SIFP_INVALID_ARGUMENT;
break;
From 9c3f6ecf4702e7c3f9a999417fe8553215cacd65 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?=
Date: Wed, 12 Oct 2016 17:08:16 +0200
Subject: [PATCH 03/19] NSS: Fix "-Wold-style-definition" caught by GCC
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Signed-off-by: Fabiano Fidêncio
---
src/responder/nss/nss_iface.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/responder/nss/nss_iface.c b/src/responder/nss/nss_iface.c
index b01732e..6c55f91 100644
--- a/src/responder/nss/nss_iface.c
+++ b/src/responder/nss/nss_iface.c
@@ -32,7 +32,7 @@ static struct sbus_iface_map iface_map[] = {
{ NULL, NULL }
};
-struct sbus_iface_map *nss_get_sbus_interface()
+struct sbus_iface_map *nss_get_sbus_interface(void)
{
return iface_map;
}
From 02a0954853a0c6b0061134b622c964d25ad0de3c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?=
Date: Wed, 19 Oct 2016 09:05:30 +0200
Subject: [PATCH 04/19] TESTS: Fix "-Werror=null-dereference" caught by GCC
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Signed-off-by: Fabiano Fidêncio
---
src/tests/krb5_child-test.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/src/tests/krb5_child-test.c b/src/tests/krb5_child-test.c
index d570d52..ec81826 100644
--- a/src/tests/krb5_child-test.c
+++ b/src/tests/krb5_child-test.c
@@ -210,8 +210,18 @@ create_dummy_req(TALLOC_CTX *mem_ctx, const char *user,
/* The Kerberos context */
kr->krb5_ctx = create_dummy_krb5_ctx(kr, realm);
+if (!kr->krb5_ctx) {
+DEBUG(SSSDBG_FATAL_FAILURE,
+ "Failed to create dummy krb5_ctx\n");
+goto fail;
+}
/* PAM Data structure */
kr->pd = create_dummy_pam_data(kr, user, password);
+if (!kr->pd) {
+DEBUG(SSSDBG_FATAL_FAILURE,
+ "Failed to create dummy pam_data");
+goto fail;
+}
ret = krb5_get_simple_upn(kr, kr->krb5_ctx, NULL, kr->pd->user, NULL,
&kr->upn);
From 6483eb57123e3331ad952e8a1d7b060016beb1c4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?=
Date: Tue, 8 Nov 2016 20:51:42 +0100
Subject: [PATCH 05/19] TOOLS: Fix "-Wstack-protector" caught by GCC
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
This warning only happens when building SSSD on RHEL6.
Signed-off-by: Fabiano Fidêncio
---
src/tools/tools_util.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/src/tools/tools_util.c b/src/tools/tools_util.c
index 5e51a40..f32d85b 100644
--- a/src/tools/tools_util.c
+++ b/src/tools/tools_util.c
@@ -73,18 +73
[SSSD] [sssd PR#50][-Changes requested] [RFC] Use GNULIB's compiler warning code
URL: https://github.com/SSSD/sssd/pull/50 Title: #50: [RFC] Use GNULIB's compiler warning code Label: -Changes requested ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#50][comment] [RFC] Use GNULIB's compiler warning code
URL: https://github.com/SSSD/sssd/pull/50 Title: #50: [RFC] Use GNULIB's compiler warning code fidencio commented: """ I've updated this series and I'm still waiting for CI to finish. A lot of warnings were just caught when building on RHEL6 machine (which passes gracefully now). I'll post the CI results as soon as I have them. """ See the full comment at https://github.com/SSSD/sssd/pull/50#issuecomment-259435498 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [Q] t3222 sssd still showing ipa user after removed from last group
Hi all,
I came back to ticket #3222 "sssd still showing ipa user after removed
from last group" [1]. And I have new knowledge. But I still do not see
the light at the end of the tunnel.
[1] https://fedorahosted.org/sssd/ticket/3222
I attached patch which enables some basic debug on using of memcache.
And two reproducers (with and without memcache) which are based on
reproducer written in ticket.
If we use memcache, the issue occurs only sometimes.
The difference between both cases is mixed state of switch after
sss_nss_mc_getgrnam() call in _nss_sss_getgrnam_r() function.
Note: code says (for default case):
/* if using the mmaped cache failed,
* fall back to socket based comms */
Could anyone help, please?
The report is:
#--- WRONG
[root@mirach sssd]# date && getent group testgroup
Wed Nov 9 16:01:05 CET 2016
>>> [A] record not found (time[1478703665])
>>> [B] record not found (time[1478703665])
testgroup:*:1703800674:
Number of members added 1
[root@mirach sssd]# sss_cache -UG && date && getent group testgroup
Wed Nov 9 16:01:07 CET 2016
>>> [A] record not found (time[1478703667])
>>> [B] default (time[1478703667])
testgroup:*:1703800674:testuser
Number of members removed 1
[root@mirach sssd]# sss_cache -UG && date && getent group testgroup
Wed Nov 9 16:01:09 CET 2016
>>> mc record expires at [1478703967] | now [1478703669]
>>> [A] MC used (time[1478703669])
testgroup:*:1703800674:testuser
[root@mirach sssd]# grep '>>>' *.log
sssd_nss.log:(Wed Nov 9 16:01:06 2016) [sssd[nss]]
[sss_mmap_set_rec_header] (0x0010): >>> MC STORE expiration [1478703966]
| now [1478703666] | delta [300]
sssd_nss.log:(Wed Nov 9 16:01:06 2016) [sssd[nss]]
[sss_mmap_cache_gr_store] (0x0010): >>> MC STORE [testgroup] [300]
members [0]
sssd_nss.log:(Wed Nov 9 16:01:07 2016) [sssd[nss]]
[sss_mmap_set_rec_header] (0x0010): >>> MC STORE expiration [1478703967]
| now [1478703667] | delta [300]
sssd_nss.log:(Wed Nov 9 16:01:07 2016) [sssd[nss]]
[sss_mmap_cache_gr_store] (0x0010): >>> MC STORE [testgroup] [300]
members [1]
#--- RIGHT
[root@mirach sssd]# date && getent group testgroup
Wed Nov 9 15:56:54 CET 2016
>>> [A] record not found (time[1478703414])
>>> [B] record not found (time[1478703414])
testgroup:*:1703800674:
Number of members added 1
[root@mirach sssd]# sss_cache -UG && date && getent group testgroup
Wed Nov 9 15:56:56 CET 2016
>>> [A] default (time[1478703416])
>>> [B] default (time[1478703416])
testgroup:*:1703800674:testuser
Number of members removed 1
[root@mirach sssd]# sss_cache -UG && date && getent group testgroup
Wed Nov 9 15:56:58 CET 2016
>>> [A] record not found (time[1478703418])
>>> [B] record not found (time[1478703418])
testgroup:*:1703800674:
[root@mirach sssd]# grep '>>>' *.log
sssd_nss.log:(Wed Nov 9 15:56:54 2016) [sssd[nss]]
[sss_mmap_set_rec_header] (0x0010): >>> MC STORE expiration [1478703714]
| now [1478703414] | delta [300]
sssd_nss.log:(Wed Nov 9 15:56:54 2016) [sssd[nss]]
[sss_mmap_cache_gr_store] (0x0010): >>> MC STORE [testgroup] [300]
members [0]
sssd_nss.log:(Wed Nov 9 15:56:56 2016) [sssd[nss]]
[sss_mmap_set_rec_header] (0x0010): >>> MC STORE expiration [1478703716]
| now [1478703416] | delta [300]
sssd_nss.log:(Wed Nov 9 15:56:56 2016) [sssd[nss]]
[sss_mmap_cache_gr_store] (0x0010): >>> MC STORE [testgroup] [300]
members [1]
sssd_nss.log:(Wed Nov 9 15:56:58 2016) [sssd[nss]]
[sss_mmap_set_rec_header] (0x0010): >>> MC STORE expiration [1478703718]
| now [1478703418] | delta [300]
sssd_nss.log:(Wed Nov 9 15:56:58 2016) [sssd[nss]]
[sss_mmap_cache_gr_store] (0x0010): >>> MC STORE [testgroup] [300]
members [0]
Regards
--
Petr^4 Čech
>From 08ec8bbaaab760396747420e46f8190c3a2dfead Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Petr=20=C4=8Cech?=
Date: Mon, 24 Oct 2016 15:16:34 +0200
Subject: [PATCH] WIP: debug for t3222
This patch enables debug messages needed for investigation of memory
cache.
---
src/responder/nss/nsssrv_mmap_cache.c | 8
src/sss_client/nss_group.c| 10 ++
src/sss_client/nss_mc_group.c | 3 +++
3 files changed, 21 insertions(+)
diff --git a/src/responder/nss/nsssrv_mmap_cache.c b/src/responder/nss/nsssrv_mmap_cache.c
index f7f62733941cd3ae3b071d6d54c801f9be1ce800..f25357712bf06da49e3a96f0ff7a4812c4f63dca 100644
--- a/src/responder/nss/nsssrv_mmap_cache.c
+++ b/src/responder/nss/nsssrv_mmap_cache.c
@@ -643,6 +643,8 @@ static inline void sss_mmap_set_rec_header(struct sss_mc_ctx *mcc,
rec->expire = time(NULL) + ttl;
rec->hash1 = sss_mc_hash(mcc, key1, key1_len);
rec->hash2 = sss_mc_hash(mcc, key2, key2_len);
+
+DEBUG(SSSDBG_FATAL_FAILURE, ">>> MC STORE expiration [%lu] | now [%lu] | delta [%li]\n", rec->expire, time(NULL), rec->expire - time(NULL));
}
static inline void sss_mmap_chain_in_rec(struct sss_mc_ctx *mcc,
@@ -846,11 +848,13 @@ int sss_mmap_cache_gr_store(struct sss_mc_ctx **_mcc,
if (mcc == NULL) {
/* cache not initialized
[SSSD] Re: about letting the responder choose the sysdb optimization level
On Wed, Nov 09, 2016 at 07:26:14AM -0500, Simo Sorce wrote:
> On Wed, 2016-11-09 at 13:08 +0100, Sumit Bose wrote:
> > On Tue, Nov 08, 2016 at 10:28:20AM +0100, Jakub Hrozek wrote:
> > > Hi,
> > >
> > > I would like to ask for opinions about:
> > > https://fedorahosted.org/sssd/ticket/3126
> > >
> > > The basic idea is that the responder would choose what kind of
> > > optimization
> > > would the back end perform when saving the sysdb entries. Requests that
> > > just return information might choose to optimize very aggressively (using
> > > modifyTimestamp) and requests that actually authenticate or authorize the
> > > user might choose to not optimize at all to avoid issues like the ones we
> > > saw
> > > with virtual attributes that don't bump the modifyTimestamp attribute at
> > > all.
> > >
> > > On the responder side, this is quite easy, just send an additional flag
> > > during the responder request. It's the provider part I'm not so sure
> > > about, because there the optimizations are performed at the sysdb level.
> > >
> > > So far I can only think about extending sysdb_transaction_start() (or
> > > providing sysdb_opt_transaction_start and letting the old
> > > sysdb_transaction_start default to no optimization) which would
> > > internally keep track of the active transaction and the optimization we
> > > want to perform. Since only sssd_be is the cache writer and there is
> > > only one cache per domain.
> > >
> > > Additionally, we would have to keep the transaction optimization level
> > > around
> > > in some context until the request bubbles from the data provider handler
> > > to
> > > actually saving the transaction. I don't I hope this won't be too messy,
> > > but
> > > since the requests are asynchronous, so far I don't see any way around
> > > it. The only thing that might be less messy in the long term is to provide
> > > a bit more generic structure ("request status") that would so far only
> > > include the optimization level and later might be extended to include
> > > e.g. intermediate data. But on the other hand, I'm not sure I have
> > > thought about passing the data between requests hard enough to design
> > > this properly. Should I?
> > >
> > > Any other opinions? Thoughts?
> >
> > I'm not sure if this wouldn't confuse users. If I understood the
> > proposal correctly the typical nss calls like getpwnam(), getgrnam() etc
> > will use the optimized requests with modifyTimestamp checks. If there is
> > a server which does not change modifyTimestamp on updates changes in
> > e.g. the user's shell or in the list of group-members will not be
> > visible even after calling 'sss_cache -E' because modifyTimestamp didn't
> > change on the server. But typically a user does not log in again to
> > check for such changes but just calls e.g. 'getent group changed_group'
> > again and again waiting for changes.
> >
> > I think if there are really such servers out there which do not update
> > modifyTimestamp it should be just possible to switch of the time-stamp
> > cache generally in this case (I wonder if this is already possible by
> > setting ldap_{user|group|netgroup}_modify_timestamp to a non-existing
> > attribute?).
Yes, this alrady works (I haven't tested this now, but I did when I was
working on the sysdb speedups feature)
>> In this case the number of cache updates can be reduced by
> > increasing the cache timeout because during authenticating and
> > authorization fresh data is requested from the backend anyways.
>
> There are cases where things like CoS are used that can change an
> attribute value (even dynamically) but do not alter the modifyTimestamp.
> In general operational attributes can behave that way.
Right, nsAccountLock behaves this way for example. What we did actually
to mitigate this was never use modifyTimestamp to compare user entries,
but only for groups -- it's the user entries that typically contain
critical attributes for access control. For users we always fall back to
"deep" attribute value comparison. This "should" also be OK since the
number of attributes of users is typically low.
>
> > I think the time would be better spend e.g. on
> > https://fedorahosted.org/sssd/ticket/3211 "Refactor the
> > sdap_async_groups.c module" and make sure all needed data is read from
> > LDAP (multiple LDAP connections might be involved here) first and
> > written to the cache in a single transaction.
Yes, this is a good enhancement, but it's not so much about writing in a
single transaction -- for plain LDAP groups we already use a single
transaction. The problem in the sdap_async_groups.c module is that we
often iterate over the attribute list several times to e.g. look at what
members are already cached. Witht the refactoring, the aim would be to
only iterate over the potentially large list once and store the
intermediate data (like a list of cached members, list of uncached
members) in memory.
The problem with multiple transactions happen
[SSSD] [sssd PR#50][comment] [RFC] Use GNULIB's compiler warning code
URL: https://github.com/SSSD/sssd/pull/50 Title: #50: [RFC] Use GNULIB's compiler warning code fidencio commented: """ http://sssd-ci.duckdns.org/logs/job/56/78/summary.html """ See the full comment at https://github.com/SSSD/sssd/pull/50#issuecomment-259444704 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#50][comment] [RFC] Use GNULIB's compiler warning code
URL: https://github.com/SSSD/sssd/pull/50 Title: #50: [RFC] Use GNULIB's compiler warning code lslebodn commented: """ I do not think that initialisation to NULL is the best solution for Wunitialized on RHLE6. My experience is that such bugs occurs when there is some suspicious code in functions which are inlined. """ See the full comment at https://github.com/SSSD/sssd/pull/50#issuecomment-259445650 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [Q] t3222 sssd still showing ipa user after removed from last group
Hi all,
I came back to ticket #3222 "sssd still showing ipa user after removed
from last group" [1]. And I have new knowledge. But I still do not see
the light at the end of the tunnel.
[1] https://fedorahosted.org/sssd/ticket/3222
I attached patch which enables some basic debug on using of memcache.
And two reproducers (with and without memcache) which are based on
reproducer written in ticket.
If we use memcache, the issue occurs only sometimes.
The difference between both cases is mixed state of switch after
sss_nss_mc_getgrnam() call in _nss_sss_getgrnam_r() function.
Note: code says (for default case):
/* if using the mmaped cache failed,
* fall back to socket based comms */
Could anyone help, please?
The report is:
#--- WRONG
[root@mirach sssd]# date && getent group testgroup
Wed Nov 9 16:01:05 CET 2016
>>> [A] record not found (time[1478703665])
>>> [B] record not found (time[1478703665])
testgroup:*:1703800674:
Number of members added 1
[root@mirach sssd]# sss_cache -UG && date && getent group testgroup
Wed Nov 9 16:01:07 CET 2016
>>> [A] record not found (time[1478703667])
>>> [B] default (time[1478703667])
testgroup:*:1703800674:testuser
Number of members removed 1
[root@mirach sssd]# sss_cache -UG && date && getent group testgroup
Wed Nov 9 16:01:09 CET 2016
>>> mc record expires at [1478703967] | now [1478703669]
>>> [A] MC used (time[1478703669])
testgroup:*:1703800674:testuser
[root@mirach sssd]# grep '>>>' *.log
sssd_nss.log:(Wed Nov 9 16:01:06 2016) [sssd[nss]]
[sss_mmap_set_rec_header] (0x0010): >>> MC STORE expiration [1478703966]
| now [1478703666] | delta [300]
sssd_nss.log:(Wed Nov 9 16:01:06 2016) [sssd[nss]]
[sss_mmap_cache_gr_store] (0x0010): >>> MC STORE [testgroup] [300]
members [0]
sssd_nss.log:(Wed Nov 9 16:01:07 2016) [sssd[nss]]
[sss_mmap_set_rec_header] (0x0010): >>> MC STORE expiration [1478703967]
| now [1478703667] | delta [300]
sssd_nss.log:(Wed Nov 9 16:01:07 2016) [sssd[nss]]
[sss_mmap_cache_gr_store] (0x0010): >>> MC STORE [testgroup] [300]
members [1]
#--- RIGHT
[root@mirach sssd]# date && getent group testgroup
Wed Nov 9 15:56:54 CET 2016
>>> [A] record not found (time[1478703414])
>>> [B] record not found (time[1478703414])
testgroup:*:1703800674:
Number of members added 1
[root@mirach sssd]# sss_cache -UG && date && getent group testgroup
Wed Nov 9 15:56:56 CET 2016
>>> [A] default (time[1478703416])
>>> [B] default (time[1478703416])
testgroup:*:1703800674:testuser
Number of members removed 1
[root@mirach sssd]# sss_cache -UG && date && getent group testgroup
Wed Nov 9 15:56:58 CET 2016
>>> [A] record not found (time[1478703418])
>>> [B] record not found (time[1478703418])
testgroup:*:1703800674:
[root@mirach sssd]# grep '>>>' *.log
sssd_nss.log:(Wed Nov 9 15:56:54 2016) [sssd[nss]]
[sss_mmap_set_rec_header] (0x0010): >>> MC STORE expiration [1478703714]
| now [1478703414] | delta [300]
sssd_nss.log:(Wed Nov 9 15:56:54 2016) [sssd[nss]]
[sss_mmap_cache_gr_store] (0x0010): >>> MC STORE [testgroup] [300]
members [0]
sssd_nss.log:(Wed Nov 9 15:56:56 2016) [sssd[nss]]
[sss_mmap_set_rec_header] (0x0010): >>> MC STORE expiration [1478703716]
| now [1478703416] | delta [300]
sssd_nss.log:(Wed Nov 9 15:56:56 2016) [sssd[nss]]
[sss_mmap_cache_gr_store] (0x0010): >>> MC STORE [testgroup] [300]
members [1]
sssd_nss.log:(Wed Nov 9 15:56:58 2016) [sssd[nss]]
[sss_mmap_set_rec_header] (0x0010): >>> MC STORE expiration [1478703718]
| now [1478703418] | delta [300]
sssd_nss.log:(Wed Nov 9 15:56:58 2016) [sssd[nss]]
[sss_mmap_cache_gr_store] (0x0010): >>> MC STORE [testgroup] [300]
members [0]
Regards
--
Petr^4 Čech
>From 08ec8bbaaab760396747420e46f8190c3a2dfead Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Petr=20=C4=8Cech?=
Date: Mon, 24 Oct 2016 15:16:34 +0200
Subject: [PATCH] WIP: debug for t3222
This patch enables debug messages needed for investigation of memory
cache.
---
src/responder/nss/nsssrv_mmap_cache.c | 8
src/sss_client/nss_group.c| 10 ++
src/sss_client/nss_mc_group.c | 3 +++
3 files changed, 21 insertions(+)
diff --git a/src/responder/nss/nsssrv_mmap_cache.c b/src/responder/nss/nsssrv_mmap_cache.c
index f7f62733941cd3ae3b071d6d54c801f9be1ce800..f25357712bf06da49e3a96f0ff7a4812c4f63dca 100644
--- a/src/responder/nss/nsssrv_mmap_cache.c
+++ b/src/responder/nss/nsssrv_mmap_cache.c
@@ -643,6 +643,8 @@ static inline void sss_mmap_set_rec_header(struct sss_mc_ctx *mcc,
rec->expire = time(NULL) + ttl;
rec->hash1 = sss_mc_hash(mcc, key1, key1_len);
rec->hash2 = sss_mc_hash(mcc, key2, key2_len);
+
+DEBUG(SSSDBG_FATAL_FAILURE, ">>> MC STORE expiration [%lu] | now [%lu] | delta [%li]\n", rec->expire, time(NULL), rec->expire - time(NULL));
}
static inline void sss_mmap_chain_in_rec(struct sss_mc_ctx *mcc,
@@ -846,11 +848,13 @@ int sss_mmap_cache_gr_store(struct sss_mc_ctx **_mcc,
if (mcc == NULL) {
/* cache not initialized
[SSSD] [sssd PR#50][comment] [RFC] Use GNULIB's compiler warning code
URL: https://github.com/SSSD/sssd/pull/50 Title: #50: [RFC] Use GNULIB's compiler warning code fidencio commented: """ @lslebodn, a way easier solution would be to disable -Werror when running CI on RHEL6 than fixing those issues only caught there. Is there a simple way to do that? """ See the full comment at https://github.com/SSSD/sssd/pull/50#issuecomment-259448662 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#50][comment] [RFC] Use GNULIB's compiler warning code
URL: https://github.com/SSSD/sssd/pull/50 Title: #50: [RFC] Use GNULIB's compiler warning code lslebodn commented: """ On (09/11/16 07:54), fidencio wrote: >@lslebodn, a way easier solution would be to disable -Werror when running CI >on RHEL6 than fixing those issues only caught there. Is there a simple way to >do that? > I agree that it would be better to disable Werror on rhel6. But your patch "BUILD: Make use of GNULIB's compiler warning code " introduced Werror. And I am still not satisfied with the patch because of increased time in CI (10%) caused by detection in manywarnings. LS """ See the full comment at https://github.com/SSSD/sssd/pull/50#issuecomment-259450717 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#50][comment] [RFC] Use GNULIB's compiler warning code
URL: https://github.com/SSSD/sssd/pull/50 Title: #50: [RFC] Use GNULIB's compiler warning code fidencio commented: """ Disabling Werror on RHEL6 will be just a matter of passing "--enable-werror=no" to configure. It should be **really** simple as long as we agree on doing that. About the extra extra time, it was 5% last time you reviewed. 100 seconds for a process that we already have to wait for an hour or so. IMO should not block, at all, introducing more warnings to our code. """ See the full comment at https://github.com/SSSD/sssd/pull/50#issuecomment-259455058 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#50][comment] [RFC] Use GNULIB's compiler warning code
URL: https://github.com/SSSD/sssd/pull/50 Title: #50: [RFC] Use GNULIB's compiler warning code fidencio commented: """ Lukáš, I'm afraid you didn't understand how the usage of the GNULIB's compiler warnings is supposed to be. There are basically 3 essential files: - warnings.m4 - manywarnings.m4 - sssd-compile-warnings.m4 The first two you can just get from GNULIB's git repo and update in our repo whenever there's a update there. So, tweaking those files is something I could do, but it's wrong per si. There's the third file then, sssd-compile-warnings.m4, where we disable the warnings we don't need, want or can't cope with. And in this file I've added all the warnings we don't need, want or can't cope with. In this very same file, we check whether a warning is or is not supported. That's how it's supposed to work. If you think patching Makefile.am is better, okay, just reject this patch series, open a bug and we will mark as "patches welcome". Again, I really don't see a valid point for rejecting this series because it introduces 100s more in a process that takes an hour or so. So, what's your suggestion? Can we accept the series? Can you, at least, finish reviewing the patches that are not RHEL6 specific? """ See the full comment at https://github.com/SSSD/sssd/pull/50#issuecomment-259478258 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#50][synchronized] [RFC] Use GNULIB's compiler warning code
URL: https://github.com/SSSD/sssd/pull/50
Author: fidencio
Title: #50: [RFC] Use GNULIB's compiler warning code
Action: synchronized
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/50/head:pr50
git checkout pr50
From 185ff593a0e1010ad67914e5573de4e37cde45a5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?=
Date: Wed, 12 Oct 2016 15:00:04 +0200
Subject: [PATCH 1/9] RESOLV: Fix "-Werror=null-dereference" caught by GCC
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Signed-off-by: Fabiano Fidêncio
---
src/resolv/async_resolv.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/resolv/async_resolv.c b/src/resolv/async_resolv.c
index e859556..e7a4217 100644
--- a/src/resolv/async_resolv.c
+++ b/src/resolv/async_resolv.c
@@ -2247,7 +2247,9 @@ static int reply_weight_rearrange(int len,
new_end = r;
}
}
-new_end->next = NULL;
+if (new_end) {
+new_end->next = NULL;
+}
/* return the rearranged list */
*start = new_start;
From 689f54fd41eb4ec256284d29074fa01d245b4bf5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?=
Date: Wed, 12 Oct 2016 16:02:55 +0200
Subject: [PATCH 2/9] SIFP: Fix "-Wjump-misses-init" caught by GCC
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Signed-off-by: Fabiano Fidêncio
---
src/lib/sifp/sss_sifp_parser.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/lib/sifp/sss_sifp_parser.c b/src/lib/sifp/sss_sifp_parser.c
index 65babb5..43eab4d 100644
--- a/src/lib/sifp/sss_sifp_parser.c
+++ b/src/lib/sifp/sss_sifp_parser.c
@@ -283,7 +283,8 @@ sss_sifp_parse_basic(sss_sifp_ctx *ctx,
uint64_t, uint64_t, uint64, done);
break;
case DBUS_TYPE_STRING:
-case DBUS_TYPE_OBJECT_PATH: ;
+case DBUS_TYPE_OBJECT_PATH:
+{
const char *val = NULL;
dbus_message_iter_get_basic(iter, &val);
@@ -306,6 +307,7 @@ sss_sifp_parse_basic(sss_sifp_ctx *ctx,
ret = SSS_SIFP_OK;
break;
+}
default:
ret = SSS_SIFP_INVALID_ARGUMENT;
break;
From 9c3f6ecf4702e7c3f9a999417fe8553215cacd65 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?=
Date: Wed, 12 Oct 2016 17:08:16 +0200
Subject: [PATCH 3/9] NSS: Fix "-Wold-style-definition" caught by GCC
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Signed-off-by: Fabiano Fidêncio
---
src/responder/nss/nss_iface.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/responder/nss/nss_iface.c b/src/responder/nss/nss_iface.c
index b01732e..6c55f91 100644
--- a/src/responder/nss/nss_iface.c
+++ b/src/responder/nss/nss_iface.c
@@ -32,7 +32,7 @@ static struct sbus_iface_map iface_map[] = {
{ NULL, NULL }
};
-struct sbus_iface_map *nss_get_sbus_interface()
+struct sbus_iface_map *nss_get_sbus_interface(void)
{
return iface_map;
}
From 1baeeabf895686aaf0fe09117be4b717d33e3f18 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?=
Date: Wed, 19 Oct 2016 09:05:30 +0200
Subject: [PATCH 4/9] TESTS: Fix "-Werror=null-dereference" caught by GCC
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Signed-off-by: Fabiano Fidêncio
Reviewed-by: Lukáš Slebodník
---
src/tests/krb5_child-test.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/src/tests/krb5_child-test.c b/src/tests/krb5_child-test.c
index d570d52..ec81826 100644
--- a/src/tests/krb5_child-test.c
+++ b/src/tests/krb5_child-test.c
@@ -210,8 +210,18 @@ create_dummy_req(TALLOC_CTX *mem_ctx, const char *user,
/* The Kerberos context */
kr->krb5_ctx = create_dummy_krb5_ctx(kr, realm);
+if (!kr->krb5_ctx) {
+DEBUG(SSSDBG_FATAL_FAILURE,
+ "Failed to create dummy krb5_ctx\n");
+goto fail;
+}
/* PAM Data structure */
kr->pd = create_dummy_pam_data(kr, user, password);
+if (!kr->pd) {
+DEBUG(SSSDBG_FATAL_FAILURE,
+ "Failed to create dummy pam_data");
+goto fail;
+}
ret = krb5_get_simple_upn(kr, kr->krb5_ctx, NULL, kr->pd->user, NULL,
&kr->upn);
From e5ad8cf02cba0a67900c18123e1826d7c3aa45c3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?=
Date: Tue, 8 Nov 2016 20:51:42 +0100
Subject: [PATCH 5/9] TOOLS: Fix "-Wstack-protector" caught by GCC
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
This warning only happens when building SSSD on RHEL6.
Signed-off-by: Fabiano Fidêncio
Reviewed-by: Lukáš Slebodník
---
src/tools/tools_util.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/src/tools/tools_util.c b/src/tools/tools_util.c
index 5e51a40..f32d85b 100644
--- a/src/tools/tools
[SSSD] [sssd PR#50][comment] [RFC] Use GNULIB's compiler warning code
URL: https://github.com/SSSD/sssd/pull/50 Title: #50: [RFC] Use GNULIB's compiler warning code fidencio commented: """ @lslebodn: After some proper checking, seems that all "-Wunitialized" reports are false positives, so I've removed the patches from this series. I've added "Reviewed-by: " to the patches you've already ACKed and dropped the ones you've already NACKed. For the ones you gave some suggestions, I followed them. The build is failing in CentOS CI and I'll take a look on it later on. I've updated my branch based only on my local tests (and they're okay on F25). I haven't done any test on our CI because it seems to be broken due to lack of space. Feel free to wait till I solve those issues before starting reviewing. Or feel free to NACK the series as soon as possible, so I don't spend time on it anymore. """ See the full comment at https://github.com/SSSD/sssd/pull/50#issuecomment-259482349 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
