access_mask needed for rename

Kenny Dinh kdinh at peaxy.net
Wed Nov 2 21:31:52 UTC 2016


Thank again for the tips, Jeremy!

I was going to make that change locally since SMB2 is more prevalent than
SMB1 but one never knows.

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

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


More information about the samba-technical mailing list