Bug 4678 - valid (NIS) users are no longer accepted
Summary: valid (NIS) users are no longer accepted
Alias: None
Product: Samba 3.0
Classification: Unclassified
Component: File Services (show other bugs)
Version: 3.0.24
Hardware: Other Linux
: P3 normal
Target Milestone: none
Assignee: Jeremy Allison
QA Contact: Samba QA Contact
URL: http://bugs.debian.org/387076
Depends on:
Blocks: 4697
  Show dependency treegraph
Reported: 2007-06-03 12:33 UTC by Christian Perrier (dead mail address)
Modified: 2007-06-29 07:00 UTC (History)
2 users (show)

See Also:

Level 10 debug log of a failed session (34.32 KB, application/octet-stream)
2007-06-03 12:36 UTC, Christian Perrier (dead mail address)
no flags Details
Simple workaround(?) for samba (470 bytes, patch)
2007-06-15 18:36 UTC, Tomasz Fortuna
no flags Details
Another workaround. Maybe better than previous. ;p (487 bytes, patch)
2007-06-16 12:05 UTC, Tomasz Fortuna
no flags Details
Another solution (432 bytes, patch)
2007-06-16 15:31 UTC, Tomasz Fortuna
no flags Details
Replacement patch (1.64 KB, patch)
2007-06-17 00:41 UTC, Jeremy Allison
no flags Details
Patch (2.05 KB, patch)
2007-06-17 14:19 UTC, Jeremy Allison
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Christian Perrier (dead mail address) 2007-06-03 12:33:24 UTC
Details of this bugs are in http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=387076.

As a summary, our user reported that 'valid users' is no longer working for him since 3.0.23d (3.0.22 is OK). He reproduces this with 3.0.24.

I couldn't reproduce this but he mentioned that his 'valid users' are indeed NIS users, which may explain why I can't reproduce the bug.


       workgroup            = TARGIT01.DE
       server string        = targit share server
       wins support         = yes
       dns proxy            = no
       interfaces           = eth0
       bind interfaces only = true
       hosts allow          = 192.168.6. 192.168.2. 127. 
       log file             = /var/log/samba/log.%m
       log level            = 2
       max log size         = 5000

       local master         = yes
       os level             = 100
       domain master        = no
       domain logons        = no
       preferred master     = yes

       security             = user
       map to guest         = Never
       guest ok             = false
       invalid users        = root

       load printers        = yes
       printing             = cups
       printcap name        = cups

       socket options       = TCP_NODELAY

       browseable           = true
       encrypt passwords    = false

       unix charset         = ISO8859-
       display charset      = ISO8859-1

       comment             = targit Project Share
       path                = /ProjectShare
       browseable          = yes
       writeable           = true
       printable           = false
       create mask         = 0666
       directory mask      = 0777
       guest ok            = true

       comment            = targit Office Share (restricted)
       path               = /OfficeShare
       guest ok           = false
       valid users        = peter, martinh
       admin users        =
       force group        = gleitung
       writeable          = true
       printable          = false
       create mask        = 0760
       force create mode  = 0760
       directory mask     = 0770
       browseable         = false

He recorded a level 10 log which I'll attach later
Comment 1 Christian Perrier (dead mail address) 2007-06-03 12:36:10 UTC
Created attachment 2732 [details]
Level 10 debug log of a failed session

Comment from our user:

attached a level 10 log with the current stable (etch) samba
packages 3.0.24-6.

The logs contain:

- connect from (ntmhvm) to TargitShare on targit1
  with the user martinh
  this connect works
  the share has no "valid users" setup
- connect from (ntmhvm) to OfficeShare on targit1
  with the user martinh
  this connect works NOT
  the share uses a "valid users" setup
- probably there are some connects from (ntckvm)
  and (ltpb) in the log which can be ignored
  for this test
Comment 2 Tomasz Fortuna 2007-06-15 18:36:24 UTC
Created attachment 2760 [details]
Simple workaround(?) for samba

We've experienced similar problem. User was not allowed to access a share (NT_STATUS_ACCESS_DENIED) if he was listed in "valid users" but not added into a passdb backend. If "valid users" had an group entry (@group) users from this group were allowed.

I'm not a samba developer, but I've written simple patch/workaround which solved this issue (I've been testing it for whole 5 minutes). I hope somebody will accept this, or invent a better resolution for a problem.
Comment 3 Volker Lendecke 2007-06-16 03:49:56 UTC
Sorry, but I think this is the wrong approach. We need to figure out why either the token is wrong or lookup_name can't find your user. We've gone through bit pains to get rid of the name-based access checks, and I'm a bit reluctant to go back.

Comment 4 Tomasz Fortuna 2007-06-16 04:40:53 UTC
lookup_name returns through this if:

        if (!(flags & LOOKUP_NAME_EXPLICIT) && strequal(domain, unix_users_domain_name())) {
                if (lookup_unix_user_name(name, &sid)) {
                        type = SID_NAME_USER;
                        goto ok; <-- here.
                return False;

Without copying SID first like in previous if:
            sid_copy(&sid, get_global_sam_sid());
            sid_append_rid(&sid, rid);

So that's probably why nt_token_check_sid() fails I guess.

I was sure that my approach is not good. But it was everything i could done in this subject.
Comment 5 Tomasz Fortuna 2007-06-16 04:46:47 UTC
> Without copying SID first like in previous if:
Uh, well - wrong.
SID of course is copied by lookup_unix_user_name().

Comment 6 Volker Lendecke 2007-06-16 04:49:05 UTC
So you need to look in the token creation routines in auth_util.c why the correct user sid is not in the token
Comment 7 Tomasz Fortuna 2007-06-16 07:01:49 UTC
(In reply to comment #6)
> So you need to look in the token creation routines in auth_util.c why the
> correct user sid is not in the token
Just before nt_token_check_sid:

User SID:
(gdb) print sid
$14 = {sid_rev_num = 1 '\001', num_auths = 2 '\002', id_auth = "\000\000\000\000\000\026", sub_auths = {1, 1000, 
    0 <repeats 13 times>}}

In 13 SIDs read from token id_auth is _mostly_ the same, but sub_auths differ.

(gdb) print sid2[0]
$30 = {sid_rev_num = 1 '\001', num_auths = 2 '\002', id_auth = "\000\000\000\000\000\026", sub_auths = {2, 7, 0 <repeats 13 times>}}

$32 = {sid_rev_num = 1 '\001', num_auths = 2 '\002', id_auth = "\000\000\000\000\000\026", sub_auths = {2, 10, 0 <repeats 13 times>}}

$36 = {sid_rev_num = 1 '\001', num_auths = 2 '\002', id_auth = "\000\000\000\000\000\026", sub_auths = {2, 18, 0 <repeats 13 times>}}


I guess these are this tokens:
se_access_check: user sid is S-1-5-21-2176992431-4081783690-2698828043-3000
se_access_check: also S-1-22-2-4
se_access_check: also S-1-1-0
se_access_check: also S-1-5-2
se_access_check: also S-1-5-11
se_access_check: also S-1-22-2-7
se_access_check: also S-1-22-2-10
se_access_check: also S-1-22-2-18
se_access_check: also S-1-22-2-19
se_access_check: also S-1-22-2-102
se_access_check: also S-1-22-2-250
se_access_check: also S-1-22-2-16
se_access_check: also S-1-22-2-35

I think that the function which creates sid for unix user creates it in a different manner that it should:
sid_copy(sid, &global_sid_Unix_Users);
sid_append_rid(sid, pwd->pw_uid); /* For 64-bit uid's we have enough

Hmhm. ;-)
Comment 8 Tomasz Fortuna 2007-06-16 12:05:02 UTC
Created attachment 2761 [details]
Another workaround. Maybe better than previous. ;p

This patch makes samba call create_token_from_username instead of create_local_nt_token in auth_util.c for users which are found in PAM (I guess). If that function is called token gets SIDs created from UID, which are then accepted by functions in share_access.c

There might be side effects. ;p I hope that somebody with bigger experience will be able to fix this issue.
Comment 9 Volker Lendecke 2007-06-16 14:04:04 UTC
Can you try to put this in auth_unix.c:119 (branches/SAMBA_3_0..) and see what happens? Then it would only apply to plain text passwords, in that case I'd be less reluctant to buy this.

Comment 10 Tomasz Fortuna 2007-06-16 15:31:37 UTC
Created attachment 2762 [details]
Another solution

It worked generally:
It accepted user from "valid users" while refusing others. I'm testing it with "3.0.27pre1-SVN-build-". It would need a few more tests, maybe someone will find a better resolution?

We just need to force user creation with "create_token_from_username" for users without passdb entry, which are authenticated with PAM. I guess.

If you like I can submit mine testparm -v output and connection log.
Comment 11 Tomasz Fortuna 2007-06-16 16:02:18 UTC
22:54 < vl> vorago: I'm here
22:56 < vl> vorago: With a good comment I think I could accept that solution if it works for you. To be honest, I don't really care about non-encrypted passwords anymore...
22:56 < vorago> Heh.
22:56 < vorago> I guess that patch needs testing. Lots of
22:56 < vorago> I can add comment explaining 'why'.
22:56 < vl> No, this is quite obvious what it does.
22:57 < vorago> You think? I'm not quite clear about 'mapped users'
22:57 < vl> That's the beauty of the new code. It's all quite centralized when you find the right spots.
22:57 < vorago> It's not so hard to debug samba.
22:58 < vl> The way you did it is a bit of a hack. But a mapped user just needs to get in via the "token" it got from /etc/passwd and /etc/group, because it has changed its identity from what Windows or passdb told us.
22:58 < vl> Same applies to users without encrypted passwords authenticated via PAM.
22:58 < vorago> So the comment just might look like:
22:58 < vl> So setting the is_mapped flag at this spot triggers exactly the right action.
22:59 < vorago> Force Samba to use information from PAM while creating user SIDS
22:59 < vl> It's not exactly using the PAM info, it's using NSS info.
22:59 < vorago> Hm, right.
22:59 < vl> PAM is out of the game at this point.
23:00 < vl> vorago: Feel free to add my comments here on irc for reference. I'd like Jerry/Jeremy to sign this off also, but you've got my go.

Comment 12 Jeremy Allison 2007-06-16 21:30:02 UTC
This fix is a good band-aid, but I think the real bug is in 


where we say :

        /* For now we throw away the gids and convert via sid_to_gid
         * later. This needs fixing, but I'd like to get the code straight and
         * simple first. */

I think we should be using gid_to_sid here in the same way as in create_token_from_username().


Comment 13 Jeremy Allison 2007-06-16 21:36:09 UTC
Ah - no. In make_server_info_pw() we're already calling 
pdb_enum_group_memberships() which should bounce down to
pdb_default_enum_group_memberships() which does exactly the
getgroups_unix_user() call followed by the :

        for (i=0; i<*p_num_groups; i++) {
                gid_to_sid(&(*pp_sids)[i], (*pp_gids)[i]);

translation. Why isn't this working and returning a correct
token with the right SIDs ?

Comment 14 Jeremy Allison 2007-06-16 22:12:16 UTC
Interesting. Looking at the logs the problem seems to be that the logged on user gets the SID prefix of the local machine, but when being searched for in "valid users = martinh" he gets the SID prefix of "Unix User". I think on Logon the SID prefix should be being set to S-1-1-22-<rid>.
This is the real bug I think.
Comment 15 Jeremy Allison 2007-06-17 00:41:41 UTC
Created attachment 2763 [details]
Replacement patch

Ok, can you check this patch please ? I think this is more correct, even though much longer. The real problem is that the user SID in the token is wrong. This really should be a UNIX-only SID.
Comment 16 Tomasz Fortuna 2007-06-17 01:30:15 UTC
(In reply to comment #15)
> Ok, can you check this patch please ? I think this is more correct, even though
> much longer. The real problem is that the user SID in the token is wrong. This
> really should be a UNIX-only SID.
> Jeremy.
Checked; it worked in NIS environment in mine configuration.

It generally does the same thing (adds uid based SID) but in much straight forward way and doesn't look like a hack. ;)
Comment 17 Jeremy Allison 2007-06-17 14:19:38 UTC
Created attachment 2765 [details]

Ok, sorry - I think this is a better patch - it should also fix bug #4678 and not cause any backwards compatibility problems in environments with plaintext passwords and no backend SAM. Let me know - in testing this I discovered some minor memory leaks in our auth subsystem but I'll fix these for 3.0.26, too dangerous for 3.0.25b. I think this is safe though.
Comment 18 Jeremy Allison 2007-06-17 14:32:56 UTC
Never mind - I was mistaken about the auth memory leaks (just ^D'ed smbclient instead of logging off correctly). This should fix the bug generically for 3.0.25b and beyond though.
Comment 19 Gerald (Jerry) Carter (dead mail address) 2007-06-29 07:00:40 UTC
fixed in 3.0.25b