Borken vuid_cache in connection_struct?

boyang boyang at samba.org
Tue Dec 1 00:04:54 MST 2009


Hi, *:
     I think the vuid_cache is broken when dynamic share permission is
introduced. Please correct me if I am wrong. :-)
1). security = share.
    user A on share A, user B on share B, on the the same tcp connect.
    user A performs some user activity on share A(eg, open a file, write
to it and keep it open), then user B performs some user activity on
share B. Therefore, current user changed from user A to user B. Share
A's permission changes here to deny write access for user A. user A
tries to write to the opened file. have a look at what change_to_user()
does, current user is not user A and  will not find vuid_cache
entry(because share = security, vuid = -1 is not even cached), perform
access check from start, deny user A from write access.(bad, the second
write is expected to succeed).
2). security != share.
    user 1-33 performs user activity sequentialy on share A.
    user 1 performs something like this(open a file, write to it and
keep it open), then user 2-33 performs some user activity. current user
changes from 1 to 33, and vuid_cache is full when user 32 performs user
activity. Then user 33 performs user activity, vuid_cache for user 1
will be replace with user 33. user 1 tries to write to the file again,
have a look at what change_to_user() does. current_user is not user 1,
look up in the vuid_cache failed, do a access check from start, deny
user A from write access. (Bad, the second write is expected to succeed.)

Recommendations:
   We use two linked lists to watch on share's permission. the entry
looks like vuid_cache. One list is used to record information about
share(read_only, admin_users, server_info) when the first call to
change_to_user(), ie, when user performs first user acitivty, record
share status here. Another list is used to store most-updated share
info. When smbd receives MSG_SMB_REEVALUATE_SHARE, it modifies this list
to reflect changes.
   When handling smb commands which must be sensitive to share
permission change(like open, unlink, etc), we update the
conn->{read_only, admin_user, server_info} from the second list. In
change_to_user(), we use the first list to update info in
connection_struct, if we cannot find in the cache, we know this is our
first user activity, do access check from start and update the two list.
Even vuid = -1 is cached here.
   Am I right about this? comment please. :-)

-- 
Bo Yang, Software Engineer, Suse Labs
GPG-key-ID   538C4C1A
Samba Team   boyang at samba.org    http://www.samba.org/
SUSE Linux   boyang at suse.de      http://www.novell.com/



More information about the samba-technical mailing list