access_mask needed for rename

[Kenny Dinh]

That explains a lot.  Thank you!

On Wed, Nov 2, 2016 at 1:30 PM, Jeremy Allison <jra at samba.org> wrote:

> On Wed, Nov 02, 2016 at 01:07:23PM -0700, Jeremy Allison wrote:
> > On Wed, Nov 02, 2016 at 11:29:33AM -0700, Kenny Dinh wrote:
> > > Hi all,
> > >
> > > In the *can_rename* function in *source3/smbd/reply.c*, we check for
> of an
> > > existing open handle's access_mask to have either DELETE_ACCESS or
> > > FILE_WRITE_ATTRIBUTES to decide if the rename operation is allowed.
> This
> > > means that if the file was opened with only FILE_WRITE_ATTRIBUTES
> without
> > > DELETE_ACCESS, the rename would be allowed to go through.  This failed
> the
> > > smbtorture *smb2.rename.simple_no_delete *test.
> > > The strict rule (which is what MS does) is to allow rename request to
> go
> > > through only if DELETE_ACCESS is specified.
> > >
> > > Was there any reason to relax to rule, (and deviate from the behavior
> on MS
> > > server), to allow the rename to go through even when the file handle
> was
> > > opened with only the FILE_WRITE_ATTRIBUTES access_mask?
> >
> > There's a long history to this I recall.
> >
> > Can you try all possible rename tests with your change (remove
> > the FILE_WRITE_ATTRIBUTES). Might be some issue with SMB1-only
> > tests.
> >
> > This also may correspond to older code that didn't have a
> > file handle.
> >
> > I do remember trying to remove this several years ago
> > and not being able to.
>
> Ah - now I remember. The problem is in the pathname-based
> call to smb_file_rename_information(), via smbd_do_setfilepathinfo().
>
> As I recall there is a test around this that checks
> oplock breaks that SMB_FILE_RENAME_INFORMATION under
> SMB1 fails with.
>
> The one case where this is used is in:
>
> source3/smbd/trans2.c:smb_file_rename_information()
>
> 7032         if (fsp) {
> 7033                 DEBUG(10,("smb_file_rename_information: "
> 7034                           "SMB_FILE_RENAME_INFORMATION (%s) %s ->
> %s\n",
> 7035                           fsp_fnum_dbg(fsp), fsp_str_dbg(fsp),
> 7036                           smb_fname_str_dbg(smb_fname_dst)));
> 7037                 status = rename_internals_fsp(conn, fsp,
> smb_fname_dst, 0,
> 7038                                               overwrite);
> 7039         } else {
> 7040                 DEBUG(10,("smb_file_rename_information: "
> 7041                           "SMB_FILE_RENAME_INFORMATION %s -> %s\n",
> 7042                           smb_fname_str_dbg(smb_fname_src),
> 7043                           smb_fname_str_dbg(smb_fname_dst)));
> 7044                 status = rename_internals(ctx, conn, req,
> smb_fname_src,
> 7045                                           smb_fname_dst, 0,
> overwrite, false,
> 7046                                           dest_has_wcard,
> 7047                                           FILE_WRITE_ATTRIBUTES);
> 7048         }
>
> Note the 'FILE_WRITE_ATTRIBUTES' in line 7047.
>
> If you change that to DELETE_ACCESS, something in the tests breaks :-).
>
> Jeremy.
>