fail authentication if user isn't member of *any* require_membership_of specified groups

[Noel Power]

On 20/11/13 16:16, Noel Power wrote:
> Hi Andreas,
>
> Thanks for the review(s) :-)
> On 20/11/13 12:13, Andreas Schneider wrote:
>> On Thursday 07 November 2013 10:34:14 Noel Power wrote:
>>> While playing with pam I came across some strange ( or at least strange
>>> to me ) behaviour. If for example you set
>>>
>>>     require_membership_of specified=bogus
>>>
>>> where bogus ( like it hints is a non existent name or group sid ) then
>>> you will be happily authenticated. This imho wrong and dangerous as you
>>> easily might not notice a typo when entering that field, it would be
>>> better to fail in this case ( and force the administrator to investigate
>>> ). The attached patch should fix that. Please review
>> I as strlen() return an integer I prefer strlen(sid_list_buffer) == 0 for 
>> readablity.
> will fix and repost later
sorry for the delay attached now,

Noel
-------------- next part --------------
From 0f64f76e6861a805748c9469c1a2311b89f0a780 Mon Sep 17 00:00:00 2001
From: Noel Power <noel.power at suse.com>
Date: Wed, 16 Oct 2013 16:30:55 +0100
Subject: [PATCH] fail authentication for single group name which cannot be
 converted to sid

furthermore if more than one name is supplied and no sid is converted
then also fail.

Signed-off-by: Noel Power <noel.power at suse.com>
---
 nsswitch/pam_winbind.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/nsswitch/pam_winbind.c b/nsswitch/pam_winbind.c
index d126494..8f5ad50 100644
--- a/nsswitch/pam_winbind.c
+++ b/nsswitch/pam_winbind.c
@@ -1184,6 +1184,12 @@ static bool winbind_name_list_to_sid_string_list(struct pwb_context *ctx,
 		_make_remark_format(ctx, PAM_TEXT_INFO, _("Cannot convert group %s "
 				"to sid, please contact your administrator to see "
 				"if group %s is valid."), search_location, search_location);
+
+		/* If no valid groups were converted we should fail outright */
+		if (name_list != NULL && strlen(sid_list_buffer) == 0) {
+			result = false;
+			goto out;
+		}
 		/*
 		 * The lookup of the last name failed..
 		 * It results in require_member_of_sid ends with ','
-- 
1.8.1.4