Bug 10440 - Lookup the user gecos field if samlogon doesn't provide it.
Summary: Lookup the user gecos field if samlogon doesn't provide it.
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: Winbind (show other bugs)
Version: 4.1.4
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Andreas Schneider
QA Contact: Samba QA Contact
URL:
Keywords:
: 8459 (view as bug list)
Depends on:
Blocks: 10891
  Show dependency treegraph
 
Reported: 2014-02-11 17:06 UTC by Andreas Schneider
Modified: 2023-07-27 09:01 UTC (History)
7 users (show)

See Also:


Attachments
v4-1-test patch (29.13 KB, patch)
2014-09-16 14:17 UTC, Andreas Schneider
metze: review-
Details
allow giving nss_template_get_info a hint about the full_name (like for gid) (2.30 KB, patch)
2014-10-31 09:21 UTC, Michael Saxl
no flags Details
v4-1-test patch (975.69 KB, patch)
2014-11-19 10:23 UTC, Andreas Schneider
asn: review? (metze)
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Andreas Schneider 2014-02-11 17:06:38 UTC
We should lookup and fill out he gecos field in case of a samlogon.
Comment 1 Andreas Schneider 2014-02-12 09:09:18 UTC
Our customer observed an issue with the winbind cache that resulted in an empty full_name/gecos field for an AD user that is _not_ using any POSIX sfu attributes. The problem occurs when using the default "template" nss info winbind method and there is a currently existing netsamlogon cache entry for this user (ie. the user has logged in). After the winbind cache time expires, the next user query will return with a NULL full_name field.

Patch currently under testing:
https://git.samba.org/?p=asn/samba.git;a=shortlog;h=refs/heads/gecos
Comment 2 Andreas Schneider 2014-03-20 14:50:19 UTC
Marc, I worked on a dcerpc bug with Metze today. And as chance would have it, it is also the issue in 4.1 you discovered.

You need b5f30205931a4b9d0b3b257d5855869e606f8b63g and 
https://git.samba.org/?p=asn/samba.git;a=commitdiff;h=59b1ea120690034572faec2c342048048fb1b1ef
Comment 3 Stefan Metzmacher 2014-03-20 16:09:57 UTC
(In reply to comment #2)
> Marc, I worked on a dcerpc bug with Metze today. And as chance would have it,
> it is also the issue in 4.1 you discovered.
> 
> You need b5f30205931a4b9d0b3b257d5855869e606f8b63g and 
> https://git.samba.org/?p=asn/samba.git;a=commitdiff;h=59b1ea120690034572faec2c342048048fb1b1ef

This belongs to https://bugzilla.samba.org/show_bug.cgi?id=10481
Comment 4 Andreas Schneider 2014-03-20 16:18:26 UTC
Ups :)
Comment 5 Andreas Schneider 2014-09-16 14:17:29 UTC
Created attachment 10289 [details]
v4-1-test patch

I've checked what we have to do to fully support this. It would mean porting gd's ndr talbe changes, the aes changes to the client then the schannel stuff from metze and finally the patches for the samlogon ex call in winbind for interactive logon.

I think it would be to big. So I ported the patches we have in our 3.6 tree to 4.1.

It needs a hack to reset the logon pipe to have the right session key. I'm not sure if this is acceptable but here is the patchset.
Comment 6 Michael Saxl 2014-10-30 21:20:07 UTC
To workaround this case I have set the option "winbind rpc only = yes"

I've checked the source and think the problem where the NULL comes from is in
nss_info_template.c nss_template_get_info. Here gecos is set to NULL. This function in turn is called by winbindd_cache.c nss_get_info_cached (only if nothing is cached).

I also discovered that winbindd_ads.c uses nss_get_info_cached, but winbindd_msrpc.c/winbindd_rpc.c does not.
Comment 7 Michael Saxl 2014-10-31 09:21:43 UTC
Created attachment 10392 [details]
allow giving nss_template_get_info a hint about the full_name (like for gid)

allow the gecos parameter of nss_*_get_info to be a hint.
make the callers aware of this fact.

this is a different approach solving same problem until the whole thing is refactored like proposed by Andreas Schneider.
Comment 8 Stefan Metzmacher 2014-11-18 08:33:13 UTC
Comment on attachment 10289 [details]
v4-1-test patch

We can't use dcerpc_netr_LogonSamLogonEx and if (NT_STATUS_EQUAL(result, NT_STATUS_WRONG_PASSWORD)). We need to use dcerpc_netr_LogonSamLogon
with the credential step protection instead.
Comment 9 Andreas Schneider 2014-11-19 10:23:27 UTC
Created attachment 10438 [details]
v4-1-test patch

Here is the complete patchset with all backports from master.
Comment 10 Stefan Metzmacher 2014-11-22 03:00:27 UTC
(In reply to Andreas Schneider from comment #9)

Do we really need all of this in v4-1-* ?

At least we need to change the defaults of some new options,
as we should not break existing 4.1 setups.
Comment 11 Michael Adam 2015-03-12 11:13:21 UTC
*** Bug 8459 has been marked as a duplicate of this bug. ***
Comment 12 Björn Jacke 2023-07-27 09:01:25 UTC
looks like this has been fixed in recent Samba versions. Feel free to reopen with more detailled information otherwise.