Bug 10773 - SECINFO_PROTECTED_DACL is not ignored
Summary: SECINFO_PROTECTED_DACL is not ignored
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: File services (show other bugs)
Version: 4.1.4
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-08-20 13:10 UTC by Stefan Metzmacher
Modified: 2014-09-03 07:15 UTC (History)
2 users (show)

See Also:


Attachments
Possible patches for master (6.94 KB, patch)
2014-08-20 13:14 UTC, Stefan Metzmacher
no flags Details
Possible patches for master (6.95 KB, patch)
2014-08-20 13:16 UTC, Stefan Metzmacher
no flags Details
git-am fix for master. (1.16 KB, patch)
2014-08-22 00:12 UTC, Jeremy Allison
no flags Details
git-am fix for 4.1.next and 4.0.next. (9.21 KB, patch)
2014-08-22 17:00 UTC, Jeremy Allison
metze: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Stefan Metzmacher 2014-08-20 13:10:58 UTC
Sometimes Windows clients doesn't filter SECINFO_[UN]PROTECTED_[D|S]ACL flags
before sending the security_information to the server.

security_information = SECINFO_PROTECTED_DACL| SECINFO_DACL
results in a NULL dacl being returned from an GetSecurityDecriptor
request. This happens because posix_get_nt_acl_common()
has the following logic:

if ((security_info & SECINFO_DACL) && !(security_info & SECINFO_PROTECTED_DACL)) {
    ... create DACL ...
}

I'm not sure if the logic is correct or wrong in this place (I guess it's wrong...).

But what I know is that the SMB server should filter the given security_information flags before passing to the filesystem.

[MS-SMB2] 3.3.5.20.3 Handling SMB2_0_INFO_SECURITY
...
The server MUST ignore any flag value in the AdditionalInformation field that is not specified in section 2.2.37.

Section 2.2.37 lists:
OWNER_SECURITY_INFORMATION
GROUP_SECURITY_INFORMATION
DACL_SECURITY_INFORMATION
SACL_SECURITY_INFORMATION
LABEL_SECURITY_INFORMATION
ATTRIBUTE_SECURITY_INFORMATION
SCOPE_SECURITY_INFORMATION
BACKUP_SECURITY_INFORMATION
Comment 1 Stefan Metzmacher 2014-08-20 13:14:18 UTC
Created attachment 10206 [details]
Possible patches for master
Comment 2 Stefan Metzmacher 2014-08-20 13:16:51 UTC
Created attachment 10207 [details]
Possible patches for master
Comment 3 Alexander Werth 2014-08-20 15:44:00 UTC
Actually the DACL protected bit is stored in the filesystem, along with these other flags:
SACL_PROTECTED
DACL_PROTECTED
SACL_AUTO_INHERITED
DACL_AUTO_INHERITED
DACL_DEFAULTED
SACL_DEFAULTED

They are the same that are stored in NFSv4.
And the vfs GPFS module already depends on them being passed through.

Filtering out SECINFO_PROTECTED_DACL in Samba would be a step backwards.
Comment 4 Stefan Metzmacher 2014-08-20 19:08:02 UTC
(In reply to comment #3)
> Actually the DACL protected bit is stored in the filesystem, along with these
> other flags:
> SACL_PROTECTED
> DACL_PROTECTED
> SACL_AUTO_INHERITED
> DACL_AUTO_INHERITED
> DACL_DEFAULTED
> SACL_DEFAULTED

These are the flags of security_descriptor_type,
which are not filtered.

> They are the same that are stored in NFSv4.
> And the vfs GPFS module already depends on them being passed through.
> 
> Filtering out SECINFO_PROTECTED_DACL in Samba would be a step backwards.

The SECINFO_* flags are something else.
Comment 5 Jeremy Allison 2014-08-22 00:12:04 UTC
Created attachment 10214 [details]
git-am fix for master.

Just FYI - that patch looks good. Might want to add this follow-up though ?
Comment 6 Jeremy Allison 2014-08-22 17:00:23 UTC
Created attachment 10218 [details]
git-am fix for 4.1.next and 4.0.next.

Patch that went into master. Applies cleanly to 4.1.next, 4.0.next.
Comment 7 Jeremy Allison 2014-08-25 18:29:17 UTC
Karolin please pick for 4.1.next, 4.0.next.

Thanks !

Jeremy.
Comment 8 Karolin Seeger 2014-09-01 18:54:52 UTC
Pushed to autobuild-v4-[0|1]-test.
Comment 9 Karolin Seeger 2014-09-03 07:15:56 UTC
Pushed to both branches.
Closing out bug report.

Thanks!