From f56bfffa51d86f96f0e71cf0c3fe23f1008ddd88 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 20 Aug 2014 13:43:13 +0200 Subject: [PATCH 1/6] 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 Reviewed-by: Andreas Schneider --- librpc/idl/security.idl | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) 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, -- 2.1.0.rc2.206.gedb03e5 From 1b3ee5e5a336064f324715d46f80661305d93c28 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 20 Aug 2014 13:58:38 +0200 Subject: [PATCH 2/6] 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 Reviewed-by: Andreas Schneider --- source3/smbd/nttrans.c | 7 ++++--- source3/smbd/posix_acls.c | 4 ++++ source3/smbd/smb2_getinfo.c | 3 ++- source3/smbd/smb2_setinfo.c | 3 ++- 4 files changed, 12 insertions(+), 5 deletions(-) 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); -- 2.1.0.rc2.206.gedb03e5 From 5cbda7e24873ffb5946c7578576ad1af1579ae60 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 20 Aug 2014 15:00:59 +0200 Subject: [PATCH 3/6] 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 Reviewed-by: Andreas Schneider Autobuild-User(master): Stefan Metzmacher Autobuild-Date(master): Fri Aug 22 02:52:50 CEST 2014 on sn-devel-104 --- libcli/security/secdesc.c | 36 ++++++++++++++++++++---------------- 1 file changed, 20 insertions(+), 16 deletions(-) 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; -- 2.1.0.rc2.206.gedb03e5 From 815bde28eeb698904d38c351f67ad1b58aa5a2cc Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 21 Aug 2014 16:28:42 -0700 Subject: [PATCH 6/6] s3: smbd: POSIX ACLs. Remove incorrect check for SECINFO_PROTECTED_DACL in incoming security_information flags in posix_get_nt_acl_common(). Tidy-up of code obsoleted by fixes for bug #10773 (SECINFO_PROTECTED_DACL is not ignored). We now never pass SECINFO_PROTECTED_DACL in security_information flags to this layer. Bug: https://bugzilla.samba.org/show_bug.cgi?id=10773 Signed-off-by: Jeremy Allison Reviewed-by: Stefan Metzmacher Autobuild-User(master): Stefan Metzmacher Autobuild-Date(master): Fri Aug 22 11:26:57 CEST 2014 on sn-devel-104 --- source3/smbd/posix_acls.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/source3/smbd/posix_acls.c b/source3/smbd/posix_acls.c index b71fd89..126b822 100644 --- a/source3/smbd/posix_acls.c +++ b/source3/smbd/posix_acls.c @@ -3280,11 +3280,7 @@ 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)) { + if (security_info & SECINFO_DACL) { /* * In the optimum case Creator Owner and Creator Group would be used for -- 2.1.0.rc2.206.gedb03e5