Attachment 10206 Details for Bug 10773 – Possible patches for master

[patch] Possible patches for master

tmp.diff (text/plain), 6.94 KB, created by Stefan Metzmacher on 2014-08-20 13:14:18 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Stefan Metzmacher

Created: 2014-08-20 13:14:18 UTC

Size: 6.94 KB

patch

obsolete

>From f1cc692d68177778bbf801a677afc0a77f54bf31 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Wed, 20 Aug 2014 13:43:13 +0200
>Subject: [PATCH 1/3] 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 <metze@samba.org>
>---
> 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,
>-- 
>1.9.1
>
>
>From 5662a169e2027a29f80cb4ab51ed425375320289 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Wed, 20 Aug 2014 13:58:38 +0200
>Subject: [PATCH 2/3] 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 <metze@samba.org>
>---
> source3/smbd/nttrans.c      | 7 ++++---
> source3/smbd/smb2_getinfo.c | 3 ++-
> source3/smbd/smb2_setinfo.c | 3 ++-
> 3 files changed, 8 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/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);
>-- 
>1.9.1
>
>
>From c2fc6d54925f0443946216b43541e140c9496678 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Wed, 20 Aug 2014 15:00:59 +0200
>Subject: [PATCH 3/3] 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 <metze@samba.org>
>---
> 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..4b9a2e1 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;
>-- 
>1.9.1
>

Actions: View

Attachments on bug 10773: 10206 | 10207 | 10214 | 10218