[PATCHES][BUG 13673] smbd: Fix DELETE_ON_CLOSE behaviour on files with READ_ONLY attribute

[Christof Schmitt]

On Fri, Nov 02, 2018 at 02:07:35PM -0700, Jeremy Allison wrote:
> On Fri, Nov 02, 2018 at 02:01:11PM -0700, Christof Schmitt wrote:
> > On Fri, Nov 02, 2018 at 01:53:55PM -0700, Jeremy Allison via samba-technical wrote:
> > > On Fri, Nov 02, 2018 at 01:29:55PM -0700, Christof Schmitt via samba-technical wrote:
> > > > smbd allowed CREATE on a file with the READONLY attribute set and
> > > > DELETE_ON_CLOSE requested, even though MS-FSA states that this should be
> > > > denied. On the other hand, SETINFO for DELETE_ON_CLOSE is correctly
> > > > rejected.
> > > > 
> > > > See the attached patches for a fix. This also adds a smbtorture test for
> > > > this case and the Samba override 'delete readonly=yes'
> > > > 
> > > > gitlab pipeline
> > > > https://gitlab.com/samba-team/devel/samba/pipelines/35263504
> > > 
> > > Oh this is a great catch ! How did you find it ?
> > 
> > Long story. For a special case we had some testing done of trying to
> > delete files with READONLY set. This showed differences between trying
> > this from Powershell or cmd.exe. It turns out that this triggers two
> > different patterns (CREATE then SETINFO-DELETE_ON_CLOSE vs.
> > CREATE-DELET_ON_CLOSE). Samba allowing one while denying the other
> > seemed inconsistent.
> > 
> > > 
> > > Reviewing now...
> > 
> > Please hold on. This fails the base.delete.deltest12 test for Samba, but
> > the same succeeds against a Windows server:
> > 
> > $ bin/smbtorture ... base.delete.deltest12 smb2.delete-on-close-perms.READONLY
> > smbtorture 4.10.0pre1-DEVELOPERBUILD
> > Using seed 1541192134
> > time: 2018-11-02 20:55:34.630579
> > test: deltest12
> > time: 2018-11-02 20:55:34.630684
> > time: 2018-11-02 20:55:37.730715
> > success: deltest12
> > time: 2018-11-02 20:55:37.739500
> > test: READONLY
> > time: 2018-11-02 20:55:37.741927
> > Creating file with READ_ONLY attribute.
> > Testing CREATE with DELETE_ON_CLOSE on READ_ONLY attribute file.
> > Testing setting DELETE_ON_CLOSE disposition on  file with READONLY
> > attribute.
> > time: 2018-11-02 20:55:42.679877
> > success: READONLY
> > 
> > I will look into this, maybe this is different in SMB1 vs. SMB2/3.
> 
> Thanks. Yes this very well could be different between SMB1 & SMB2
> on Windows. Can you ensure the torture tests pass against recent
> Windows then we'll implement that behavior in Samba.

The above smbtorture is against a Windows 2012R2, so these work against
Windows. I think the issue is that base.delete.deltest12 is testing a
CREATE on a new file while smb2.delete-on-close-perms.READONLY is
testing against an existing file. My assumption now is that we need two
checks to handle both cases
can_set_delete_on_close(fsp, new_dos_attributes);
and
can_set_delete_on_close(fsp, existing_dos_attributes);

Working on updated patches.

Christof