Attachment 10218 Details for Bug 10773 – git-am fix for 4.1.next and 4.0.next.

[patch] git-am fix for 4.1.next and 4.0.next.

look (text/plain), 9.21 KB, created by Jeremy Allison on 2014-08-22 17:00:23 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jeremy Allison

Created: 2014-08-22 17:00:23 UTC

Size: 9.21 KB

patch

obsolete

>From f56bfffa51d86f96f0e71cf0c3fe23f1008ddd88 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/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 <metze@samba.org>
>Reviewed-by: Andreas Schneider <asn@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,
>-- 
>2.1.0.rc2.206.gedb03e5
>
>
>From 1b3ee5e5a336064f324715d46f80661305d93c28 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/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 <metze@samba.org>
>Reviewed-by: Andreas Schneider <asn@samba.org>
>---
> 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 <metze@samba.org>
>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 <metze@samba.org>
>Reviewed-by: Andreas Schneider <asn@samba.org>
>
>Autobuild-User(master): Stefan Metzmacher <metze@samba.org>
>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 <jra@samba.org>
>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 <jra@samba.org>
>Reviewed-by: Stefan Metzmacher <metze@samba.org>
>
>Autobuild-User(master): Stefan Metzmacher <metze@samba.org>
>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
>

Flags:

metze: review+

Actions: View

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