The branch, master has been updated
via 5cbda7e libcli/security: add better detection of
SECINFO_[UN]PROTECTED_[D|S]ACL in get_sec_info()
via 1b3ee5e s3:smbd: mask security_information input values with
SMB_SUPPORTED_SECINFO_FLAGS
via f56bfff security.idl: add SMB_SUPPORTED_SECINFO_FLAGS
from 52063e9 s3:libsmb: Set a max charge for SMB2 connections
http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit 5cbda7e24873ffb5946c7578576ad1af1579ae60
Author: Stefan Metzmacher <[email protected]>
Date: Wed Aug 20 15:00:59 2014 +0200
libcli/security: add better detection of SECINFO_[UN]PROTECTED_[D|S]ACL in
get_sec_info()
Bug: https://bugzilla.samba.org/show_bug.cgi?id=10773
Signed-off-by: Stefan Metzmacher <[email protected]>
Reviewed-by: Andreas Schneider <[email protected]>
Autobuild-User(master): Stefan Metzmacher <[email protected]>
Autobuild-Date(master): Fri Aug 22 02:52:50 CEST 2014 on sn-devel-104
commit 1b3ee5e5a336064f324715d46f80661305d93c28
Author: Stefan Metzmacher <[email protected]>
Date: Wed Aug 20 13:58:38 2014 +0200
s3:smbd: mask security_information input values with
SMB_SUPPORTED_SECINFO_FLAGS
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
Bug: https://bugzilla.samba.org/show_bug.cgi?id=10773
Signed-off-by: Stefan Metzmacher <[email protected]>
Reviewed-by: Andreas Schneider <[email protected]>
commit f56bfffa51d86f96f0e71cf0c3fe23f1008ddd88
Author: Stefan Metzmacher <[email protected]>
Date: Wed Aug 20 13:43:13 2014 +0200
security.idl: add SMB_SUPPORTED_SECINFO_FLAGS
A SMB server should only care about specific SECINFO flags
and ignore others e.g. SECINFO_PROTECTED_DACL.
Bug: https://bugzilla.samba.org/show_bug.cgi?id=10773
Signed-off-by: Stefan Metzmacher <[email protected]>
Reviewed-by: Andreas Schneider <[email protected]>
-----------------------------------------------------------------------
Summary of changes:
libcli/security/secdesc.c | 36 ++++++++++++++++++++----------------
librpc/idl/security.idl | 18 ++++++++++++++++++
source3/smbd/nttrans.c | 7 ++++---
source3/smbd/posix_acls.c | 4 ++++
source3/smbd/smb2_getinfo.c | 3 ++-
source3/smbd/smb2_setinfo.c | 3 ++-
6 files changed, 50 insertions(+), 21 deletions(-)
Changeset truncated at 500 lines:
diff --git a/libcli/security/secdesc.c b/libcli/security/secdesc.c
index 052bafb..46b820e 100644
--- a/libcli/security/secdesc.c
+++ b/libcli/security/secdesc.c
@@ -24,13 +24,6 @@
#include "librpc/gen_ndr/ndr_security.h"
#include "libcli/security/security.h"
-#define ALL_SECURITY_INFORMATION (SECINFO_OWNER|SECINFO_GROUP|\
- SECINFO_DACL|SECINFO_SACL|\
- SECINFO_UNPROTECTED_SACL|\
- SECINFO_UNPROTECTED_DACL|\
- SECINFO_PROTECTED_SACL|\
- SECINFO_PROTECTED_DACL)
-
/* Map generic permissions to file object specific permissions */
const struct generic_mapping file_generic_mapping = {
@@ -46,21 +39,32 @@ const struct generic_mapping file_generic_mapping = {
uint32_t get_sec_info(const struct security_descriptor *sd)
{
- uint32_t sec_info = ALL_SECURITY_INFORMATION;
+ uint32_t sec_info = 0;
SMB_ASSERT(sd);
- if (sd->owner_sid == NULL) {
- sec_info &= ~SECINFO_OWNER;
+ if (sd->owner_sid != NULL) {
+ sec_info |= SECINFO_OWNER;
+ }
+ if (sd->group_sid != NULL) {
+ sec_info |= SECINFO_GROUP;
}
- if (sd->group_sid == NULL) {
- sec_info &= ~SECINFO_GROUP;
+ if (sd->sacl != NULL) {
+ sec_info |= SECINFO_SACL;
}
- if (sd->sacl == NULL) {
- sec_info &= ~SECINFO_SACL;
+ if (sd->dacl != NULL) {
+ sec_info |= SECINFO_DACL;
+ }
+
+ if (sd->type & SEC_DESC_SACL_PROTECTED) {
+ sec_info |= SECINFO_PROTECTED_SACL;
+ } else if (sd->type & SEC_DESC_SACL_AUTO_INHERITED) {
+ sec_info |= SECINFO_UNPROTECTED_SACL;
}
- if (sd->dacl == NULL) {
- sec_info &= ~SECINFO_DACL;
+ if (sd->type & SEC_DESC_DACL_PROTECTED) {
+ sec_info |= SECINFO_PROTECTED_DACL;
+ } else if (sd->type & SEC_DESC_DACL_AUTO_INHERITED) {
+ sec_info |= SECINFO_UNPROTECTED_DACL;
}
return sec_info;
diff --git a/librpc/idl/security.idl b/librpc/idl/security.idl
index 381d6e5..eb80a86 100644
--- a/librpc/idl/security.idl
+++ b/librpc/idl/security.idl
@@ -630,6 +630,24 @@ interface security
SECINFO_PROTECTED_DACL = 0x80000000
} security_secinfo;
+ /*
+ * a SMB server should only support the following flags
+ * and ignore all others.
+ *
+ * See AdditionalInformation in [MS-SMB2] 2.2.37 SMB2 QUERY_INFO Request
+ * and 2.2.39 SMB2 SET_INFO Request.
+ */
+ const int SMB_SUPPORTED_SECINFO_FLAGS = (
+ SECINFO_OWNER |
+ SECINFO_GROUP |
+ SECINFO_DACL |
+ SECINFO_SACL |
+ SECINFO_LABEL |
+ SECINFO_ATTRIBUTE |
+ SECINFO_SCOPE |
+ SECINFO_BACKUP |
+ 0);
+
typedef [public,bitmap32bit] bitmap {
KERB_ENCTYPE_DES_CBC_CRC = 0x00000001,
KERB_ENCTYPE_DES_CBC_MD5 = 0x00000002,
diff --git a/source3/smbd/nttrans.c b/source3/smbd/nttrans.c
index 0cf1ea3..d7705e3 100644
--- a/source3/smbd/nttrans.c
+++ b/source3/smbd/nttrans.c
@@ -2036,7 +2036,8 @@ static void
call_nt_transact_query_security_desc(connection_struct *conn,
status = smbd_do_query_security_desc(conn,
talloc_tos(),
fsp,
- security_info_wanted,
+ security_info_wanted &
+ SMB_SUPPORTED_SECINFO_FLAGS,
max_data_count,
&marshalled_sd,
&sd_size);
@@ -2129,8 +2130,8 @@ static void
call_nt_transact_set_security_desc(connection_struct *conn,
return;
}
- status = set_sd_blob(fsp, (uint8 *)data, data_count,
security_info_sent);
-
+ status = set_sd_blob(fsp, (uint8 *)data, data_count,
+ security_info_sent & SMB_SUPPORTED_SECINFO_FLAGS);
if (!NT_STATUS_IS_OK(status)) {
reply_nterror(req, status);
return;
diff --git a/source3/smbd/posix_acls.c b/source3/smbd/posix_acls.c
index b9edf12..b71fd89 100644
--- a/source3/smbd/posix_acls.c
+++ b/source3/smbd/posix_acls.c
@@ -3280,6 +3280,10 @@ static NTSTATUS posix_get_nt_acl_common(struct
connection_struct *conn,
num_profile_acls = 3;
}
+ /*
+ * TODO: is this logic with SECINFO_PROTECTED_DACL, correct?
+ * See bug #10773.
+ */
if ((security_info & SECINFO_DACL) && !(security_info &
SECINFO_PROTECTED_DACL)) {
/*
diff --git a/source3/smbd/smb2_getinfo.c b/source3/smbd/smb2_getinfo.c
index 3139a32..7f44868 100644
--- a/source3/smbd/smb2_getinfo.c
+++ b/source3/smbd/smb2_getinfo.c
@@ -479,7 +479,8 @@ static struct tevent_req *smbd_smb2_getinfo_send(TALLOC_CTX
*mem_ctx,
state,
fsp,
/* Security info wanted. */
- in_additional_information,
+ in_additional_information &
+ SMB_SUPPORTED_SECINFO_FLAGS,
in_output_buffer_length,
&p_marshalled_sd,
&sd_size);
diff --git a/source3/smbd/smb2_setinfo.c b/source3/smbd/smb2_setinfo.c
index 3722697..d95bd3d 100644
--- a/source3/smbd/smb2_setinfo.c
+++ b/source3/smbd/smb2_setinfo.c
@@ -312,7 +312,8 @@ static struct tevent_req *smbd_smb2_setinfo_send(TALLOC_CTX
*mem_ctx,
status = set_sd_blob(fsp,
in_input_buffer.data,
in_input_buffer.length,
- in_additional_information);
+ in_additional_information &
+ SMB_SUPPORTED_SECINFO_FLAGS);
if (!NT_STATUS_IS_OK(status)) {
tevent_req_nterror(req, status);
return tevent_req_post(req, ev);
--
Samba Shared Repository
