access_mask needed for rename

[Jeremy Allison]

On Wed, Nov 02, 2016 at 01:48:05PM -0700, Kenny Dinh wrote:
> That explains a lot.  Thank you!

Well I'm looking at it again. Also, we could easly change
the can_rename() to do a:

	if (smb1 && (access_mask & FILE_WRITE_ATTRIBUTES)) {
		return NT_STATUS_OK;
	}
	if (access_mask & DELETE_ACCESS) {
		return NT_STATUS_OK;
	}
	return NT_STATUS_ACCESS_DENIED;

to make it more correct for SMB2-only.

> > 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.
> >