RFC also store resource group ids available from pac logon from successful pam authentication

Jeremy Allison jra at samba.org
Wed Jun 10 17:33:31 MDT 2015


On Wed, Jun 10, 2015 at 02:11:36PM +0100, Noel Power wrote:
> Hi
> 
> came across a bug where sometimes groups returned (e.g. from id command)
> were missing some group sids, turns out these group ids are resource
> groups. If we successfully authenticate via pam then  the netsamlogon
> cache is updated but is missing any of those resource group ids, this
> patch attempts to address that.

Looks good to me - Reviewed-by: Jeremy Allison.

So it's at least as good as what we already have.
Having said that, I noticed in the:

        for (i=0; i < pac_data->num_buffers; i++) {

code just before it, it's theoretically possible
to exit that look with logon_info == NULL (if
there was no pac_data->buffers[i].type == PAC_TYPE_LOGON_INFO
sent in the PAC).

So I think the there should be an additional fix on top
of your patch to tidy that possibility up.

Can I get a second Team reviewer for the 2 patches please ?

Cheers,

	Jeremy.
-------------- next part --------------
From c7f0fe8e87e7da303ddd5d1f8730c20ea20f057a Mon Sep 17 00:00:00 2001
From: Noel Power <noel.power at suse.com>
Date: Wed, 10 Jun 2015 13:13:25 +0100
Subject: [PATCH 1/2] kerberos auth info3 should contain resource group ids
 available from pac_logon

successful pam auth (e.g. from ssh) will cache group sids (but not any
resource group sids)) The subsequent cached entry used for groups lookups
can be missing those resource groups

Signed-off-by: Noel Power <noel.power at suse.com>
Reviewed-by: Jeremy Allison <jra at samba.org>
---
 source3/winbindd/winbindd_pam.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/source3/winbindd/winbindd_pam.c b/source3/winbindd/winbindd_pam.c
index 864382e..018f70f 100644
--- a/source3/winbindd/winbindd_pam.c
+++ b/source3/winbindd/winbindd_pam.c
@@ -594,6 +594,7 @@ static NTSTATUS winbindd_raw_kerberos_login(TALLOC_CTX *mem_ctx,
 	struct PAC_DATA_CTR *pac_data_ctr = NULL;
 	const char *local_service;
 	int i;
+	struct netr_SamInfo3 *info3_copy = NULL;
 
 	*info3 = NULL;
 
@@ -713,11 +714,15 @@ static NTSTATUS winbindd_raw_kerberos_login(TALLOC_CTX *mem_ctx,
 		break;
 	}
 
-	*info3 = &logon_info->info3;
 
 	DEBUG(10,("winbindd_raw_kerberos_login: winbindd validated ticket of %s\n",
 		principal_s));
 
+	result = create_info3_from_pac_logon_info(mem_ctx, logon_info, &info3_copy);
+	if (!NT_STATUS_IS_OK(result)) {
+		goto failed;
+	}
+
 	/* if we had a user's ccache then return that string for the pam
 	 * environment */
 
@@ -753,7 +758,7 @@ static NTSTATUS winbindd_raw_kerberos_login(TALLOC_CTX *mem_ctx,
 		}
 
 	}
-
+	*info3 = info3_copy;
 	return NT_STATUS_OK;
 
 failed:
-- 
2.2.0.rc0.207.ga3a616c


From 0e47b1ecb9161f8759176727653d31563fcf2c44 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Wed, 10 Jun 2015 16:31:21 -0700
Subject: [PATCH 2/2] winbindd: winbindd_raw_kerberos_login - ensure logon_info
 exists in PAC.

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

diff --git a/source3/winbindd/winbindd_pam.c b/source3/winbindd/winbindd_pam.c
index 018f70f..a274d4f 100644
--- a/source3/winbindd/winbindd_pam.c
+++ b/source3/winbindd/winbindd_pam.c
@@ -714,6 +714,11 @@ static NTSTATUS winbindd_raw_kerberos_login(TALLOC_CTX *mem_ctx,
 		break;
 	}
 
+	if (logon_info == NULL) {
+		DEBUG(10,("Missing logon_info in ticket of %s\n",
+			principal_s));
+		return NT_STATUS_INVALID_PARAMETER;
+	}
 
 	DEBUG(10,("winbindd_raw_kerberos_login: winbindd validated ticket of %s\n",
 		principal_s));
-- 
2.2.0.rc0.207.ga3a616c



More information about the samba-technical mailing list