question about make_connection_snum function

[Jeremy Allison]

On Thu, Feb 22, 2007 at 09:32:51PM -0800, Herb Lewis wrote:
> We have a local patch to this function that I'm trying
> to figure out if it is valid or not.
> 
> It is about the middle of the function
> 
>         /*
>          * New code to check if there's a share security descripter
>          * added from NT server manager. This is done after the
>          * smb.conf checks are done as we need a uid and token. JRA.
>          *
>          */
> 
>         {
>                 NT_USER_TOKEN *token = conn->nt_user_token ?
>                         conn->nt_user_token : vuser->nt_user_token;
> 
>                 BOOL can_write = share_access_check(token,
>                                                   lp_servicename(snum),
>                                                   FILE_WRITE_DATA);
> 
>                 if (!can_write) {
>                         if (!share_access_check(token,
>                                                 lp_servicename(snum),
>                                                 FILE_READ_DATA)) {
>                                 /* No access, read or write. */
>                                 DEBUG(0,("make_connection: connection 
> to %s "
>                                          "denied due to security "
>                                          "descriptor.\n",
>                                           lp_servicename(snum)));
>                                 conn_free(conn);
>                                 *status = NT_STATUS_ACCESS_DENIED;
>                                 return NULL;
>                         } else {
>                                 conn->read_only = True;
>                         }
>                 }
>         }
> 
> 
> We changed the open brace to be
> 
>         if (!IS_IPC(conn)) {
> 
> Can anyone see a reason this change would be incorrect?

Yes I can see a reason it isn't correct. It's valid to add
a security descriptor to the IPC$ share. Why exempt IPC$
from security checks ?

Jeremy.