Patch to handle sid compression for consideration ...

Jeremy Allison jra at samba.org
Tue Jun 17 00:37:48 MDT 2014


On Mon, Jun 16, 2014 at 10:36:12PM -0700, Jeremy Allison wrote:
> 
> Yep - in that case IMHO the first patch in the series
> should be to change the make_session_info_krb5() interface
> to use a passed in struct netr_SamInfo3 *, as you noticed
> it doesn't use the full struct PAC_LOGON_INFO * at all.
> 
> In fact - I think:
> 
> Patch #1 should be to change:
> 
> make_server_info_info3() to use a const struct netr_SamInfo3 *info3,
> as it only reads from info3, never modifies.
> 
> Patch #2 should be to change make_session_info_krb5() to
> to take a const struct netr_SamInfo3 *info3, not the
> full struct PAC_LOGON_INFO *.
> 
> Patch #3 changes auth3_generate_session_info_pac() to
> make a info3 copy from logon_info->info3, and then
> does the merge resource sids, caches the info3 copy
> using netsamlogon_cache_store(), and then simply passes
> that copied info3 as a const pointer to make_session_info_krb5()
> in place of the existing struct PAC_LOGON_INFO *logon_info pointer.
> 
> Patch #4 changes winbindd_pam_auth_pac_send() to
> make a info3 copy from logon_info->info3, and
> calls the netsamlogon_cache_store() with the copy.
> 
> Does that work as a patchset for you ?
> 
> I can make those changes for you to test
> later on this week if I get the chance
> whilst I'm at the plugfest, or I'm happy
> to review these changes if you want to
> code 'em up !!

Ok, couldn't get to sleep :-). Here is the patchset.
Ended up being 5 patches.

Compiles but isn't tested. If you can test it
that would help greatly !

Simo & Andrew, does this address your concerns ?
Rchard, does it do what you need ?

Cheers,

	Jeremy.
-------------- next part --------------
>From 348e640bccf2c7c435dfcd27e1335d9172e10f99 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Mon, 16 Jun 2014 22:49:29 -0700
Subject: [PATCH 1/5] s3: auth: Add some const to the struct netr_SamInfo3 *
 arguments of copy_netr_SamInfo3() and make_server_info_info3()

Both functions only read from the struct netr_SamInfo3 * argument.

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/auth/auth_util.c   | 2 +-
 source3/auth/proto.h       | 4 ++--
 source3/auth/server_info.c | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/source3/auth/auth_util.c b/source3/auth/auth_util.c
index cab77b4..2986fb4 100644
--- a/source3/auth/auth_util.c
+++ b/source3/auth/auth_util.c
@@ -1335,7 +1335,7 @@ NTSTATUS make_server_info_info3(TALLOC_CTX *mem_ctx,
 				const char *sent_nt_username,
 				const char *domain,
 				struct auth_serversupplied_info **server_info,
-				struct netr_SamInfo3 *info3)
+				const struct netr_SamInfo3 *info3)
 {
 	static const char zeros[16] = {0, };
 
diff --git a/source3/auth/proto.h b/source3/auth/proto.h
index 9e11a0c..686582a 100644
--- a/source3/auth/proto.h
+++ b/source3/auth/proto.h
@@ -242,7 +242,7 @@ NTSTATUS make_server_info_info3(TALLOC_CTX *mem_ctx,
 				const char *sent_nt_username,
 				const char *domain,
 				struct auth_serversupplied_info **server_info,
-				struct netr_SamInfo3 *info3);
+				const struct netr_SamInfo3 *info3);
 struct wbcAuthUserInfo;
 NTSTATUS make_server_info_wbcAuthUserInfo(TALLOC_CTX *mem_ctx,
 					  const char *sent_nt_username,
@@ -304,7 +304,7 @@ NTSTATUS passwd_to_SamInfo3(TALLOC_CTX *mem_ctx,
 			    const struct passwd *pwd,
 			    struct netr_SamInfo3 **pinfo3);
 struct netr_SamInfo3 *copy_netr_SamInfo3(TALLOC_CTX *mem_ctx,
-					 struct netr_SamInfo3 *orig);
+					 const struct netr_SamInfo3 *orig);
 
 /* The following definitions come from auth/auth_wbc.c  */
 
diff --git a/source3/auth/server_info.c b/source3/auth/server_info.c
index df0be54..4ce57c1 100644
--- a/source3/auth/server_info.c
+++ b/source3/auth/server_info.c
@@ -580,7 +580,7 @@ done:
 	} } while(0)
 
 struct netr_SamInfo3 *copy_netr_SamInfo3(TALLOC_CTX *mem_ctx,
-					 struct netr_SamInfo3 *orig)
+					 const struct netr_SamInfo3 *orig)
 {
 	struct netr_SamInfo3 *info3;
 	unsigned int i;
-- 
1.9.1


>From 6477b73d2c17436a5898757808cc51990152dd5f Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Mon, 16 Jun 2014 22:54:45 -0700
Subject: [PATCH 2/5] s3: auth: Change make_server_info_info3() to take a const
 struct netr_SamInfo3 pointer instead of a struct PAC_LOGON_INFO.

make_server_info_info3() only reads from the info3 pointer.

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/auth/auth_generic.c | 2 +-
 source3/auth/proto.h        | 2 +-
 source3/auth/user_krb5.c    | 8 ++++----
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/source3/auth/auth_generic.c b/source3/auth/auth_generic.c
index 05c4ddc..6b146a0 100644
--- a/source3/auth/auth_generic.c
+++ b/source3/auth/auth_generic.c
@@ -113,7 +113,7 @@ static NTSTATUS auth3_generate_session_info_pac(struct auth4_context *auth_ctx,
 
 	status = make_session_info_krb5(mem_ctx,
 					ntuser, ntdomain, username, pw,
-					logon_info, is_guest, is_mapped, NULL /* No session key for now, caller will sort it out */,
+					&logon_info->info3, is_guest, is_mapped, NULL /* No session key for now, caller will sort it out */,
 					session_info);
 	if (!NT_STATUS_IS_OK(status)) {
 		DEBUG(1, ("Failed to map kerberos pac to server info (%s)\n",
diff --git a/source3/auth/proto.h b/source3/auth/proto.h
index 686582a..ab8fb89 100644
--- a/source3/auth/proto.h
+++ b/source3/auth/proto.h
@@ -372,7 +372,7 @@ NTSTATUS make_session_info_krb5(TALLOC_CTX *mem_ctx,
 				char *ntdomain,
 				char *username,
 				struct passwd *pw,
-				struct PAC_LOGON_INFO *logon_info,
+				const struct netr_SamInfo3 *info3,
 				bool mapped_to_guest, bool username_was_mapped,
 				DATA_BLOB *session_key,
 				struct auth_session_info **session_info);
diff --git a/source3/auth/user_krb5.c b/source3/auth/user_krb5.c
index 6b8fad2..7442ea4 100644
--- a/source3/auth/user_krb5.c
+++ b/source3/auth/user_krb5.c
@@ -186,7 +186,7 @@ NTSTATUS make_session_info_krb5(TALLOC_CTX *mem_ctx,
 				char *ntdomain,
 				char *username,
 				struct passwd *pw,
-				struct PAC_LOGON_INFO *logon_info,
+				const struct netr_SamInfo3 *info3,
 				bool mapped_to_guest, bool username_was_mapped,
 				DATA_BLOB *session_key,
 				struct auth_session_info **session_info)
@@ -202,14 +202,14 @@ NTSTATUS make_session_info_krb5(TALLOC_CTX *mem_ctx,
 			return status;
 		}
 
-	} else if (logon_info) {
+	} else if (info3) {
 		/* pass the unmapped username here since map_username()
 		   will be called again in make_server_info_info3() */
 
 		status = make_server_info_info3(mem_ctx,
 						ntuser, ntdomain,
 						&server_info,
-						&logon_info->info3);
+						info3);
 		if (!NT_STATUS_IS_OK(status)) {
 			DEBUG(1, ("make_server_info_info3 failed: %s!\n",
 				  nt_errstr(status)));
@@ -298,7 +298,7 @@ NTSTATUS make_session_info_krb5(TALLOC_CTX *mem_ctx,
 				char *ntdomain,
 				char *username,
 				struct passwd *pw,
-				struct PAC_LOGON_INFO *logon_info,
+				const struct netr_SamInfo3 *info3,
 				bool mapped_to_guest, bool username_was_mapped,
 				DATA_BLOB *session_key,
 				struct auth_session_info **session_info)
-- 
1.9.1


>From f3b61999b2886cff5077a318cfe19c95c52a9990 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Mon, 16 Jun 2014 23:11:58 -0700
Subject: [PATCH 3/5] s3: auth: Add merge_resource_sids() to merge resource
 group SIDs into an info3.

Originally written by Richard Sharpe Richard Sharpe <realrichardsharpe at gmail.com>.

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/auth/proto.h       |  2 ++
 source3/auth/server_info.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+)

diff --git a/source3/auth/proto.h b/source3/auth/proto.h
index ab8fb89..b302b0e 100644
--- a/source3/auth/proto.h
+++ b/source3/auth/proto.h
@@ -294,6 +294,8 @@ NTSTATUS serverinfo_to_SamInfo3(const struct auth_serversupplied_info *server_in
 				struct netr_SamInfo3 *sam3);
 NTSTATUS serverinfo_to_SamInfo6(struct auth_serversupplied_info *server_info,
 				struct netr_SamInfo6 *sam6);
+NTSTATUS merge_resource_sids(struct PAC_LOGON_INFO *logon_info,
+				struct netr_SamInfo3 *info3);
 NTSTATUS samu_to_SamInfo3(TALLOC_CTX *mem_ctx,
 			  struct samu *samu,
 			  const char *login_server,
diff --git a/source3/auth/server_info.c b/source3/auth/server_info.c
index 4ce57c1..a078fa7 100644
--- a/source3/auth/server_info.c
+++ b/source3/auth/server_info.c
@@ -253,6 +253,58 @@ static NTSTATUS group_sids_to_info3(struct netr_SamInfo3 *info3,
 	return NT_STATUS_OK;
 }
 
+/*
+ * Merge resource SIDs, if any, into the passed in info3 structure.
+ */
+
+NTSTATUS merge_resource_sids(struct PAC_LOGON_INFO *logon_info,
+				struct netr_SamInfo3 *info3)
+{
+	uint32_t i = 0;
+
+	if (!(logon_info->info3.base.user_flags & NETLOGON_RESOURCE_GROUPS)) {
+		return NT_STATUS_OK;
+	}
+
+	/*
+	 * If there are any resource groups (SID Compression) add
+	 * them to the extra sids portion of the info3 in the PAC.
+	 *
+	 * This makes the info3 look like it would if we got the info
+	 * from the DC rather than the PAC.
+	 */
+
+	/*
+	 * Construct a SID for each RID in the list and then append it
+	 * to the info3.
+	 */
+	for (i = 0; i < logon_info->res_groups.count  ; i++) {
+		NTSTATUS status;
+		struct dom_sid new_sid;
+		uint32_t attributes = logon_info->res_groups.rids[i].attributes;
+
+		sid_compose(&new_sid,
+			logon_info->res_group_dom_sid,
+			logon_info->res_groups.rids[i].rid);
+
+		DEBUG(10, ("Adding SID %s to extra SIDS\n",
+			sid_string_dbg(&new_sid)));
+
+		status = append_netr_SidAttr(info3, &info3->sids,
+					&info3->sidcount,
+					&new_sid,
+					attributes);
+		if (!NT_STATUS_IS_OK(status)) {
+			DEBUG(1, ("failed to append SID %s to extra SIDS: %s\n",
+				sid_string_dbg(&new_sid),
+				nt_errstr(status)));
+			return status;
+		}
+	}
+
+	return NT_STATUS_OK;
+}
+
 #define RET_NOMEM(ptr) do { \
 	if (!ptr) { \
 		TALLOC_FREE(info3); \
-- 
1.9.1


>From eec0cfc9c7569a67021982112ebec451be41ff62 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Mon, 16 Jun 2014 23:15:21 -0700
Subject: [PATCH 4/5] s3: auth: Change auth3_generate_session_info_pac() to use
 a copy of the info3 struct from the struct PAC_LOGON_INFO.

Call merge_resource_sids() to add in any resource SIDs
from the struct PAC_LOGON_INFO to the info3.

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/auth/auth_generic.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/source3/auth/auth_generic.c b/source3/auth/auth_generic.c
index 6b146a0..01b748c 100644
--- a/source3/auth/auth_generic.c
+++ b/source3/auth/auth_generic.c
@@ -45,6 +45,7 @@ static NTSTATUS auth3_generate_session_info_pac(struct auth4_context *auth_ctx,
 {
 	TALLOC_CTX *tmp_ctx;
 	struct PAC_LOGON_INFO *logon_info = NULL;
+	struct netr_SamInfo3 *info3_copy = NULL;
 	bool is_mapped;
 	bool is_guest;
 	char *ntuser;
@@ -102,7 +103,16 @@ static NTSTATUS auth3_generate_session_info_pac(struct auth4_context *auth_ctx,
 
 	/* save the PAC data if we have it */
 	if (logon_info) {
-		netsamlogon_cache_store(ntuser, &logon_info->info3);
+		info3_copy = copy_netr_SamInfo3(tmp_ctx, &logon_info->info3);
+		if (info3_copy == NULL) {
+			status = NT_STATUS_NO_MEMORY;
+			goto done;
+		}
+		status = merge_resource_sids(logon_info, info3_copy);
+		if (!NT_STATUS_IS_OK(status)) {
+			goto done;
+		}
+		netsamlogon_cache_store(ntuser, info3_copy);
 	}
 
 	/* setup the string used by %U */
@@ -113,7 +123,7 @@ static NTSTATUS auth3_generate_session_info_pac(struct auth4_context *auth_ctx,
 
 	status = make_session_info_krb5(mem_ctx,
 					ntuser, ntdomain, username, pw,
-					&logon_info->info3, is_guest, is_mapped, NULL /* No session key for now, caller will sort it out */,
+					info3_copy, is_guest, is_mapped, NULL /* No session key for now, caller will sort it out */,
 					session_info);
 	if (!NT_STATUS_IS_OK(status)) {
 		DEBUG(1, ("Failed to map kerberos pac to server info (%s)\n",
-- 
1.9.1


>From 26fc8885ef60e2589c759c5f687699b600aadc52 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Mon, 16 Jun 2014 23:27:35 -0700
Subject: [PATCH 5/5] s3: auth: Fix winbindd_pam_auth_pac_send() to merge in
 resource groups from a trusted PAC.

Based on a patch from Richard Sharpe <realrichardsharpe at gmail.com>.

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/winbindd/winbindd_pam.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/source3/winbindd/winbindd_pam.c b/source3/winbindd/winbindd_pam.c
index 1fb4360..1ad5fec 100644
--- a/source3/winbindd/winbindd_pam.c
+++ b/source3/winbindd/winbindd_pam.c
@@ -2450,6 +2450,7 @@ NTSTATUS winbindd_pam_auth_pac_send(struct winbindd_cli_state *state,
 	struct winbindd_request *req = state->request;
 	DATA_BLOB pac_blob;
 	struct PAC_LOGON_INFO *logon_info = NULL;
+	struct netr_SamInfo3 *info3_copy = NULL;
 	NTSTATUS result;
 
 	pac_blob = data_blob_const(req->extra_data.data, req->extra_len);
@@ -2463,7 +2464,16 @@ NTSTATUS winbindd_pam_auth_pac_send(struct winbindd_cli_state *state,
 
 	if (logon_info) {
 		/* Signature verification succeeded, trust the PAC */
-		netsamlogon_cache_store(NULL, &logon_info->info3);
+		info3_copy = copy_netr_SamInfo3(state->mem_ctx,
+					&logon_info->info3);
+		if (info3_copy == NULL) {
+			return NT_STATUS_NO_MEMORY;
+		}
+		result = merge_resource_sids(logon_info, info3_copy);
+		if (!NT_STATUS_IS_OK(result)) {
+			return result;
+		}
+		netsamlogon_cache_store(NULL, info3_copy);
 
 	} else {
 		/* Try without signature verification */
@@ -2475,9 +2485,16 @@ NTSTATUS winbindd_pam_auth_pac_send(struct winbindd_cli_state *state,
 				   nt_errstr(result)));
 			return result;
 		}
+		if (logon_info) {
+			info3_copy = copy_netr_SamInfo3(state->mem_ctx,
+					&logon_info->info3);
+			if (info3_copy == NULL) {
+				return NT_STATUS_NO_MEMORY;
+			}
+		}
 	}
 
-	*info3 = &logon_info->info3;
+	*info3 = info3_copy;
 
 	return NT_STATUS_OK;
 }
-- 
1.9.1



More information about the samba-technical mailing list