`rmdir` implementation within smbclient

[Jeremy Allison]

On Wed, Aug 08, 2018 at 11:42:52AM -0700, Jeremy Allison via samba-technical wrote:
> On Thu, Aug 09, 2018 at 12:05:58AM +0530, Anoop C S wrote:
> > 
> > Also `rmdir` is handled differently for client connections with protocol dialects >=PROTOCOL_SMB2_02
> > when compared to those below PROTOCOL_SMB2_02 inside cli_rmdir(). NT_STATUS_DIRECTORY_NOT_EMPTY is
> > returned correctly on non-empty directory for client connection below PROTOCOL_SMB2_02 using
> > smb_command SMBrmdir(with send, recv approach).
> > 
> > Whereas for higher protocol clients we go via the procedure of creating a fnum with
> > FILE_DELETE_ON_CLOSE and assumes to get it deleted while closing the handle(fnum). But inside
> > open_directory() we purposefully skip NT_STATUS_DIRECTORY_NOT_EMPTY and fnum creation is successful
> > without setting fsp->initial_delete_on_close. Thus a non-empty directory is not deleted during a
> > subsequent close unless we explicitly set delete_on_close token i.e, `rmdir` fakes directory
> > deletion.
> > 
> > Therefore I can think of the following ways to fix this:
> > [1] Do not skip NT_STATUS_DIRECTORY_NOT_EMPTY for can_set_delete_on_close() inside open_directory()
> > to return the error.
> 
> I'm guessing (although I'll need to check Windows to be sure)
> that setting FILE_DELETE_ON_CLOSE on a non-empty directory
> handle open over SMB2 to Windows doesn't error out. We need
> a test for this if we don't already have one.
> 
> If Windows errors out in this case, then that's the correct fix.
> 
> The slightly tricky case here is if "delete veto files = yes"
> is set. I need to think about that some more.
> 
> If not...

Yep, just tested it. As expected, Windows does not error out
error out when a non-empty directory is opened with DELETE_ON_CLOSE
set, it just allows the open and doesn't set delete on close
(the close doesn't delete it, obviously).

> > [2] Make sure that we do either of the following to return NT_STATUS_DIRECTORY_NOT_EMPTY during a
> > subsequent close:
> >     (a) set fsp->initial_delete_on_close by handling NT_STATUS_DIRECTORY_NOT_EMPTY separately inside
> >         open_directory().
> >         OR
> >     (b) explicitly set delete_on_close token using cli_smb2_delete_on_close() after creating fnum
> >         inside cli_smb2_rmdir(). Note that cli_smb2_delete_on_close() is not yet consumed in code
> >         base.
> 
> Then (b) is the correct fix.
> 
> > Or do let me know if you have an entirely different approach in fixing the issue.
> 
> Nope, let's do the tests and work it out.

So we need to use cli_smb2_delete_on_close() into cli_smb2_rmdir().

Jeremy.