Bug 9395 - Samba fails the simple_nodelete test of smb2.rename tests from master
Summary: Samba fails the simple_nodelete test of smb2.rename tests from master
Status: ASSIGNED
Alias: None
Product: Samba 4.0
Classification: Unclassified
Component: File services (show other bugs)
Version: unspecified
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Jeremy Allison
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on: 9396
Blocks:
  Show dependency treegraph
 
Reported: 2012-11-15 04:50 UTC by Richard Sharpe
Modified: 2015-10-26 19:42 UTC (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Richard Sharpe 2012-11-15 04:50:45 UTC
The smb2.rename test in the Master branch fails the simple_nodelete test.

It succeeds against Win2K08 in that we get ACCESS_DENIED while it
fails against Samba 3.6.6 and master:

time: 2012-11-14 16:46:10.618975
failure: simple_nodelete) [
(../source4/torture/smb2/rename.c:184) Incorrect status NT_STATUS_OK -
should be NT_STATUS_ACCESS_DENIED
]

The fix is as we discussed on email.
Comment 1 Jeremy Allison 2012-11-16 19:49:49 UTC
Fix for this bug has just been uploaded as a patchset to bug #9396.

Jeremy.
Comment 2 Hemanth 2015-10-26 19:06:46 UTC
Recently(I think it is revisited) we have across this torture failure against our samba 3.6.12 stack. 

After going through the history, I found that issue has been discussed already and a fix is proposed for the same here..
https://lists.samba.org/archive/samba-technical/2012-November/088848.html

Suggested change was:

>         if (fsp->access_mask & (DELETE_ACCESS|FILE_WRITE_ATTRIBUTES)) {
>                 return NT_STATUS_OK;
>         }
>
>         return NT_STATUS_ACCESS_DENIED;
>
> Aha! That probably should be:
>
>         if ((fsp->access_mask & (DELETE_ACCESS|FILE_WRITE_ATTRIBUTES)) ==
>                         (DELETE_ACCESS|FILE_WRITE_ATTRIBUTES)) {
>                 return NT_STATUS_OK;
>         }

This will actually fix the smb torture test but it might break other rename scenarios due to extra permission check.

I was wondering why we do we need to check for FILE_WRITE_ATTRIBUTES permission(along DELETE) during rename operation.

Actually [MS-SMB2] spec talks about checking DELETE access during the rename but not any other specific permissions.

Section 3.3.5.21.1
...
If the object store supports security and the information class is FileRenameInformation, FileDispositionInformation, or FileShortNameInformation, and Open.GrantedAccess does not include DELETE, the server MUST fail the request with STATUS_ACCESS_DENIED.
...

WRITE_ATTRIBUTES is mentioned for file basic information.
...
If the object store supports security and the information class is FileBasicInformation or FilePipeInformation, and Open.GrantedAccess does not include FILE_WRITE_ATTRIBUTES, the server MUST fail the request with STATUS_ACCESS_DENIED.
...

Moreover we have seen a real application using a handle without write attribute permission to perform the rename operation.

11854	2015-10-21 09:54:11.684374	192.168.101.79	192.168.101.21	SMB2	558	Create Request File: Permission\AI\house.ai;SetInfo Request FILE_INFO/SMB2_FILE_RENAME_INFO NewName:Permission\AI\Acr62114212880665616649.tmp;Close Request
Access Mask: 0x00110080
.... .... .... .... .... ...0 .... .... = Write Attributes: NO write attributes access
.... .... .... ...1 .... .... .... .... = Delete: DELETE access
11855	2015-10-21 09:54:11.722049	192.168.101.21	192.168.101.79	SMB2	450	Create Response File: Permission\AI\house.ai;SetInfo Response;Close Response

If we ahead with above fix, it will cause rename failure for above case. 

Would like to know if there is any reason why we are checking the FILE_WRITE_ATTRIBUTES permissions along DELETE, when it is not mentioned in spec.

I think the logic is same in samba 4.2 code as well.
Comment 3 Jeremy Allison 2015-10-26 19:42:28 UTC
OK the problem with the previous fix was that it broke raw oplocks. Let me look at that some more.