Re: sssd crash on RHEL 7.3
by Steve French
>> 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
6 years, 6 months
[sssd PR#432][comment] CACHE_REQ: Better debugging for email conflicts
by mzidek-rh
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
6 years, 6 months
[sssd PR#432][comment] CACHE_REQ: Better debugging for email conflicts
by lslebodn
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
6 years, 6 months
[sssd PR#433][comment] PAM: Multiple certificates on a Smartcard
by fidencio
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
6 years, 6 months