auditing patch for samba

[Andrew Bartlett]

Andy Bakun wrote:
> 
> FYI, anywhere I mention NT in this message, I'm talking about Windows NT
> 4.0 Service Pack 6.
> 
> >The 'restrict anonymous' paramater has been removed from HEAD, as I
> >broke it during the auth-rewrite and could not find the rationale behind
> >it.  In particular, its highly confusing name (conflicting with the NT
> >Registry key of the same name) didn't help.
> >
> Most discussion on it can be found by searching marc.theaimsgroup.com's
> samba-technical for 'restrict anonymous', discussion took place mostly
> around the end of 1998.  Note the conversations on the %U and %G
> expansions.  It's actually named 'restrict anonymous' (I notice in your
> notes on the removal of it that it is misnamed) as Luke Kenneth Casso
> suggested that name, and it was the only name for that parameter that
> really got anyone's attention on the problem I was having with %U and %G
> expansions.  History lesson time: I had shares defined in include files,
> named based on the user name and group name, and they would randomly
> disappear on the client.  Turned out that the NT client was browsing the
> server anonymously (even though it had an already authed connection) and
> smbd would reread the config files and would include other files because
> there is no user name associated with an anonymous connection.
> 
> I don't know which copy of the source you were reading :) , but if my
> memory serves, it did exactly what it should do based on its name -- if
> an anonymous connection was attempted over a connection that already had
> been authed, it was denied.  This has the (intended) effect of making
> the NT client try again, but this time with the username and password.

The problem was the it was confused with the NT registry setting that
denies anonymous access to the server for security reasons (prevent user
listing etc).  Even some team members were convinced that was what it
was intended to do....

> 2.2.3a's smbd/reply.c (I've removed my comments)
> 
>   if (done_sesssetup && lp_restrict_anonymous()) {
>     if (!*user && !*smb_apasswd && !*domain) {
>       DEBUG(0, ("restrict anonymous is True and anonymous connection attempted. Denying access.\n"));
>       END_PROFILE(SMBsesssetupX);
>       return(ERROR_DOS(ERRDOS,ERRnoaccess));
>     }
>   }
> 
> The meat of the functionality being the second line above (879 of reply.c)
> 
> Unfortunately, this also caused problems with NT clients reauthing their
> machine accounts after logout, because in MS's infinite braindeadness
> (my judgement call), they updated the profile over a NEW connection
> (authed as the user) after closing the logged on connection and sending
> the LOGOFF signal to the server (the sending of which wasn't even
> reliable in my experience).  So now a new connection had been
> established to update the profile, and the machine account auth attempt
> fails because an anonymous connection is attempted first in order to do
> that.
> 
> All this is academic though if you've already removed it and your new
> auth scheme is in there and %U and %G still work all the time.  

They probably don't - but I won't allow another patch like that one into
the tree if I can help it.  Its just too ugly.

> I
> suspose another solution to the macro expansion issue would be to
> iterate over the vuser (?) array's elements and try expanding all macros
> for each vuser connected.  I don't remember why this solution wasn't
> used at the time.
> 
> Forgive me if I'm getting the terms wrong (ie, LOGOFF signal, vuser
> struct), it's been over two years since I've dealt with the samba source
> on a daily basis!

I think I understand what it was for - and I still think its an ugly
hack.

> >>I really don't think auditing should be an add-on VFS module.  Bugs in
> >>other VFS modules (assuming they will eventually be nested) might keep
> >>it from working.  I'm of the opinion that it should be integrated.
> >>
> >I don't find this a convincing argument.  If your module is first in the
> >stack (which is where an audit module should be) then no other module
> >should be able to affect it.
> >
>  ...
> 
> >Auditing is an interesting area - and I don't object to the idea of
> >including this functionality in Samba - but I think it is best to use
> >the VFS interface.  (This should not preclude inclusion in the tree
> >however - but I'm not exactly sure how it would work).
> >
> True.  But the example VFS code, audit.c, doesn't audit logons and
> logoffs, or reasons for failed logons, nor can it, nor could it handle
> other events that don't fit within the file-access scheme (I can't think
> of any others right now -- maybe using NT User Manager to change account
> settings, etc).  I suspose more function pointers could be added to the
> tail of struct vfs_ops -- this would also require changes to the rest of
> the source to use the VFS interface/modules at those points.  But the
> more things like that that are added, the percentage of FS in VFS goes
> down, lest we just allow random function overloading at run time from
> the environment (precident: smbsh), or change the name from VFS, or keep
> it (precident: restrict anonymous being misnamed). :)

No, we should not add these things to the VFS.  Outside share level
secruity, the auth subsystem provides an ideal basis for this kind of
work - and I am more than willing to assist in providing the approprate
hooks.  Likewise, we now have 'session' code that allow sane
logon/logoff tracking.  PAM modules are already hooked into this.

A general 'audit' interface/module would be a good thing.  As I said,
the VFS (and similar things) allow functionality to be abstracted - but
its not to say that we shouldn't have this in Samba at all, just a way
of writing it.

Andrew Bartlett

-- 
Andrew Bartlett                                 abartlet at pcug.org.au
Manager, Authentication Subsystems, Samba Team  abartlet at samba.org
Student Network Administrator, Hawker College   abartlet at hawkerc.net
http://samba.org     http://build.samba.org     http://hawkerc.net