sssd-devel November 2017

sssd-devel@lists.fedorahosted.org

10 participants
210 discussions

[sssd PR#426][comment] nss-idmap: add nss like calls with timeout and flags
by jhrozek 05 Nov '17

05 Nov '17

URL: https://github.com/SSSD/sssd/pull/426 Title: #426: nss-idmap: add nss like calls with timeout and flags jhrozek commented: """ Finally, I re-run the downstream tests that were failing several times and no other test showed the failure in the netgroup midpoint refresh test, so I think it was a fluke. """ See the full comment at https://github.com/SSSD/sssd/pull/426#issuecomment-341999997

1 0

[sssd PR#426][comment] nss-idmap: add nss like calls with timeout and flags
by jhrozek 05 Nov '17

05 Nov '17

URL: https://github.com/SSSD/sssd/pull/426 Title: #426: nss-idmap: add nss like calls with timeout and flags jhrozek commented: """ CI currently needs a bit love, some platforms fail due to the new ldb and additionally on rawhide I noticed a dlopen failure, but I'm also able to reproduce it without your patches, so I filed https://pagure.io/SSSD/sssd/issue/3565 """ See the full comment at https://github.com/SSSD/sssd/pull/426#issuecomment-341999946

1 0

[sssd PR#434][comment] sudo: require sudoHost attribute and do not merge rules
by jhrozek 05 Nov '17

05 Nov '17

URL: https://github.com/SSSD/sssd/pull/434 Title: #434: sudo: require sudoHost attribute and do not merge rules jhrozek commented: """ Well, I don't think this PR is urgent, so I would prefer if you could check the codepaths and not add another bool parameter unless it's needed. """ See the full comment at https://github.com/SSSD/sssd/pull/434#issuecomment-341961349

1 0

Re: sssd crash on RHEL 7.3
by Steve French 03 Nov '17

03 Nov '17

>> one has Samba 4.5 (built the same way). sssd config looks >> identical, "net ads info" shows the same output, klist -ke output >> matches - but sssd crashes on one (see below) when starting the >> service. Ideas? > You need to rebuild sssd against libldb required by the newer samba (4.7). This was discussed >extensively on the list right before 4.7 RC1 release. Do you know if more recent RHEL7 (1.15 in RHEL7.4 e.g.) addressed the libldb incompatibility (and resultant sssd crash) that Alexander noted? The version of sssd in RHEL7.5 beta didn't appear any newer (but is there a RHEL7.x build of 1.16 we could try)? -- Thanks, Steve

4 4

[sssd PR#432][comment] CACHE_REQ: Better debugging for email conflicts
by mzidek-rh 03 Nov '17

03 Nov '17

URL: https://github.com/SSSD/sssd/pull/432 Title: #432: CACHE_REQ: Better debugging for email conflicts mzidek-rh commented: """ On 11/03/2017 05:41 PM, lslebodn wrote: > On (03/11/17 16:19), mzidek-rh wrote: > >No, I used good example. Your example is the wrong one and would not > trigger the error. > > > > There are more ways how you can store users with email to cache. > The most important is that email is lowercased in backed and stored to > nameAlias and not in responder. In theory, you can use even getpwuid. > (I didn't try) > > But `sysdb_getpwnam` will find them because it uses filter > `SYSDB_PWNAM_FILTER` > I do not see, how your above sentences are relevant to anything. Why do you mention getpwuid? > > > >What do you think will happen if in my example scenario I search for > 'getent passwd email2' ? It will return correct results. Do you see why? > > It is not about returning correct results. The purpose of ticket is to > inform > about conflicts (or even prevent such conflicts if possible) The reason why I said that it will return correct results was to demonstrate that your example was invalid. It would not trigger the code, so you showing message with username that has no connection to the email was invalid. You tried to demonstrate how useless this patch is using that invalid example. > > > >I already answered it in one of my previous comments, but here I tell > it again. Because in my example email2 (fqdn version > email2(a)ipadomain.test) is just a username. > >Same as your benutzer123 and user123. It will never trigger the error > because they do not conflict with any email address of another user. > > > > They have the same email. Just case is different (unless I did a typo) > Therefore lowercased email in "nameAlias" will be the same and will return > two users. If you search 'by the email', yes. That is what the patch is about. Again, I fail to see what is your point with that sentence. > > > >In your example, in order to trigger the error you would have to > search for getent passwd lorem.ipsum(a)jmail.com. Then you would see the > error message telling you that lorem.ipsum(a)jmail.com is is probably > conflicting with another users email or fqdn. > > > > As I mentioned about there are more ways how you can store user with email > to sssd cache. The problem is that `sysdb_getpwnam` can return more users > (e.g. `benutzer123` and `user123`) at the same time because lowercased email > stored in "nameAlias" is the same. Again, if you search by the email address. Yes. Which is why this patch adds the message. Your point would be valid if the example you showed was real and the message would print the username that is not the same as someone else's email. If you now see that it is not the case and still refer to this, then I really fail to see your point. > > The other related problem is that `sysdb_getpwnam` should not indirectly > search by email address. Search by email address should be done only > in `sysdb_search_user_by_upn_res` which explicitly search either by UPN > or by email address. When you are saying that sysdb_getpwnam should not indirectly search for email address, do you mean that sysdb_getpwnam should not look for email address at all? Why? Because I do not think that is the case, and I checked some code paths, and it looks like we do expect sysdb_getpwnam to also find users by emails. Or do you mean that sysdb_getpwnam should not use nameAlias to search for the email and instead use a new attribute called for example emailAlias? In that case, how is such change related to this message and this PR/BZ? We would still hit the issue no matter what attribute is used in the filter, so it makes sense to print the message. You mentioned that this PR should be potentially about preventing the issue with the collisions and not just informing about it. At this point I am still not sure if you understand what the actual problem is, but let me inform you that it is not possible to avoid the collisions unless we want to drop the support for lookup by emails. There is a workaround that will disable the lookup by email and I document it in the man page with this patch. IMO the man page change and the debug message do improve the situation. This PR has 'changes requested' label. Can you please tell me what are the changes that are requested? Because you said a lot of sentences, but the only thing that I can see is the vague request/non request for adding the syslog call (would be good if you confirmed that you want/do not want the syscall). """ See the full comment at https://github.com/SSSD/sssd/pull/432#issuecomment-341809232

1 0

[sssd PR#433][-Changes requested] PAM: Multiple certificates on a Smartcard
by sumit-bose 03 Nov '17

03 Nov '17

URL: https://github.com/SSSD/sssd/pull/433 Title: #433: PAM: Multiple certificates on a Smartcard Label: -Changes requested

1 0

[sssd PR#433][comment] PAM: Multiple certificates on a Smartcard
by sumit-bose 03 Nov '17

03 Nov '17

URL: https://github.com/SSSD/sssd/pull/433 Title: #433: PAM: Multiple certificates on a Smartcard sumit-bose commented: """ Thank you for the review, I include all your comments in the latest version. """ See the full comment at https://github.com/SSSD/sssd/pull/433#issuecomment-341763543

1 0

[sssd PR#433][synchronized] PAM: Multiple certificates on a Smartcard
by sumit-bose 03 Nov '17

03 Nov '17

URL: https://github.com/SSSD/sssd/pull/433 Author: sumit-bose Title: #433: PAM: Multiple certificates on a Smartcard Action: synchronized To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/433/head:pr433 git checkout pr433

1 0

[sssd PR#432][comment] CACHE_REQ: Better debugging for email conflicts
by lslebodn 03 Nov '17

03 Nov '17

URL: https://github.com/SSSD/sssd/pull/432 Title: #432: CACHE_REQ: Better debugging for email conflicts lslebodn commented: """ On (03/11/17 16:36), mzidek-rh wrote: >Sorry, but this: >``` >(Fri Nov 3 15:52:42 2017) [sssd[nss]] [sysdb_getpwnam] (0x0020): Search for [email1(a)ipadomain.test] returned multiple results. It can be an email address shared among multiple users or an email address of a user that conflicts with another user's fully qualified name. SSSD will not be able to handle those users properly. >``` > >Does not look to me like this: >``` >(Fri Nov 3 15:52:42 2017) [sssd[nss]] [sysdb_getpwnam] (0x0020): Please contact sssd-users(a)lists.fedorahosted.org for help with finding problematic entries in sssd cache." >``` > I never wrote that text is equal but value of message is the same. Because users will not be able to resolve problem themselves without additional help. LS """ See the full comment at https://github.com/SSSD/sssd/pull/432#issuecomment-341760491

1 0

[sssd PR#433][comment] PAM: Multiple certificates on a Smartcard
by fidencio 03 Nov '17

03 Nov '17

URL: https://github.com/SSSD/sssd/pull/433 Title: #433: PAM: Multiple certificates on a Smartcard fidencio commented: """ So, coverity found some issues: - **Error: USE_AFTER_FREE (CWE-825): [#def1]** ``` sssd-1.16.1/src/sss_client/pam_sss.c:154: alias: Assigning: "cai" = "cai->next". Now both point to the same storage. sssd-1.16.1/src/sss_client/pam_sss.c:155: freed_arg: "free_cai" frees "cai". sssd-1.16.1/src/sss_client/pam_sss.c:145:9: freed_arg: "free" frees parameter "cai". sssd-1.16.1/src/sss_client/pam_sss.c:154: deref_after_free: Dereferencing freed pointer "cai". # 152| # 153| if (list != NULL) { # 154|-> DLIST_FOR_EACH(cai, list) { # 155| free_cai(cai); # 156| } ``` Here it seems that a DLIST_FOR_EACH_SAFE() should be used. Something like: ``` diff --git a/src/sss_client/pam_sss.c b/src/sss_client/pam_sss.c index b2968634e..e6d559b44 100644 --- a/src/sss_client/pam_sss.c +++ b/src/sss_client/pam_sss.c @@ -148,10 +148,11 @@ static void free_cai(struct cert_auth_info *cai) static void free_cert_list(struct cert_auth_info *list) { - struct cert_auth_info *cai; + struct cert_auth_info *cai, *cai_next; if (list != NULL) { - DLIST_FOR_EACH(cai, list) { + DLIST_FOR_EACH_SAFE(cai, cai_next, list) { + DLIST_REMOVE(list, cai); free_cai(cai); } } ``` - **Error: FORWARD_NULL (CWE-476): [#def2]** ``` sssd-1.16.1/src/responder/pam/pamsrv_p11.c:433: var_compare_op: Comparing "pd->authtok" to null implies that "pd->authtok" might be null. sssd-1.16.1/src/responder/pam/pamsrv_p11.c:461: var_deref_model: Passing null pointer "pd->authtok" to "sss_authtok_get_type", which dereferences it. sssd-1.16.1/src/util/authtok.c:30:5: deref_parm: Directly dereferencing parameter "tok". # 28| enum sss_authtok_type sss_authtok_get_type(struct sss_auth_token *tok) # 29| { # 30|-> return tok->type; # 31| } # 32| ``` This one could either be solved by a simple check on the called to ensure that tok is not NULL or by the suggested patch below: ``` diff --git a/src/util/authtok.c b/src/util/authtok.c index c2f78be32..2c5a26ce3 100644 --- a/src/util/authtok.c +++ b/src/util/authtok.c @@ -27,6 +27,10 @@ struct sss_auth_token { enum sss_authtok_type sss_authtok_get_type(struct sss_auth_token *tok) { + if (tok == NULL) { + return SSS_AUTHTOK_TYPE_EMPTY; + } + return tok->type; } ``` The patch above would also avoid a check done as part of the one patches to ensure that `pd->authtok != NULL` """ See the full comment at https://github.com/SSSD/sssd/pull/433#issuecomment-341727235

1 0

← Newer
1
...
14
15
16
17
18
19
20
21
Older →

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

sssd-devel November 2017