[PATCH][BUG 12725] winbindd: Fix password policy for pam authentication

Stefan Metzmacher metze at samba.org
Sun Apr 2 22:20:37 UTC 2017


Am 29.03.2017 um 19:01 schrieb Christof Schmitt via samba-technical:
> On Wed, Mar 29, 2017 at 10:11:21AM +0200, Andreas Schneider wrote:
>> On Tuesday, 28 March 2017 23:09:51 CEST Christof Schmitt via samba-technical 
>> wrote:
>>> From d993da727d8af96ba4717fbd18d261ce69db21d7 Mon Sep 17 00:00:00 2001
>>> From: Christof Schmitt <cs at samba.org>
>>> Date: Mon, 27 Mar 2017 15:11:08 -0700
>>> Subject: [PATCH] winbindd: Fix password policy for pam authentication
>>>
>>> Authenticating users from trusted domains would return the password
>>> policy of the joined domain. Fix the code so that the password policy of
>>> the joined domain is only returned for users from that domain.

I think it's wrong to look at the password policy at all.
There are so many situations where the global password policy
for the domain is not what applies to the user
(newer Windows domains support fine granted
password policies).

The force_password_change value from the netr_SamBaseInfo, is the
effective value from the
correct policy that applies to the user. We already look at
pass_must_change_time (force_password_change)
in pam_winbind before doing our own calculation (which can't be more
correct)
based on the domain password policy.

The following patch removes the only user of WBFLAG_PAM_GET_PWD_POLICY
(within samba).
I guess we should be able remove the WBFLAG_PAM_GET_PWD_POLICY handling
in winbindd too.

What is the use case you're trying to solve?
Would my patch also solve the problem?

Comments please, I'm not sure yet if my change belongs to
https://bugzilla.samba.org/show_bug.cgi?id=12725...

I didn't test this (beside making sure it compiles)...
I think we really need to use pam_wrapper and add testcases for
pam_winbind.

metze
-------------- next part --------------
From 2f8f642a87751c0a3d42e7b130711ad8f7ed2ecc Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Mon, 3 Apr 2017 00:19:25 +0200
Subject: [PATCH] BUG??? pam_winbind: no longer wbcUserPasswordPolicyInfo when
 authenticating

The expiry time for the specific user comes from
info->pass_must_change_time and nothing else.

The authenticating DC knows which password policy applies
to the user, that's nothing the client can do, as
domain trusts and fine-grained password policies makes
this a very complex task.

BUG??? https://bugzilla.samba.org/show_bug.cgi?id=12725

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 nsswitch/pam_winbind.c | 58 +++++++++++---------------------------------------
 1 file changed, 12 insertions(+), 46 deletions(-)

diff --git a/nsswitch/pam_winbind.c b/nsswitch/pam_winbind.c
index 746b157..4ae6464 100644
--- a/nsswitch/pam_winbind.c
+++ b/nsswitch/pam_winbind.c
@@ -1004,7 +1004,6 @@ static bool _pam_send_password_expiry_message(struct pwb_context *ctx,
 
 static void _pam_warn_password_expiry(struct pwb_context *ctx,
 				      const struct wbcAuthUserInfo *info,
-				      const struct wbcUserPasswordPolicyInfo *policy,
 				      int warn_pwd_expire,
 				      bool *already_expired,
 				      bool *change_pwd)
@@ -1012,7 +1011,7 @@ static void _pam_warn_password_expiry(struct pwb_context *ctx,
 	time_t now = time(NULL);
 	time_t next_change = 0;
 
-	if (!info || !policy) {
+	if (info == NULL) {
 		return;
 	}
 
@@ -1044,23 +1043,6 @@ static void _pam_warn_password_expiry(struct pwb_context *ctx,
 		return;
 	}
 
-	/* now check for the global password policy */
-	/* good catch from Ralf Haferkamp: an expiry of "never" is translated
-	 * to -1 */
-	if ((policy->expire == (int64_t)-1) ||
-	    (policy->expire == 0)) {
-		return;
-	}
-
-	next_change = info->pass_last_set_time + policy->expire;
-
-	if (_pam_send_password_expiry_message(ctx, next_change, now,
-					      warn_pwd_expire,
-					      already_expired,
-					      change_pwd)) {
-		return;
-	}
-
 	/* no warning sent */
 }
 
@@ -1696,23 +1678,17 @@ static int winbind_auth_request(struct pwb_context *ctx,
 				const int warn_pwd_expire,
 				struct wbcAuthErrorInfo **p_error,
 				struct wbcLogonUserInfo **p_info,
-				struct wbcUserPasswordPolicyInfo **p_policy,
 				time_t *pwd_last_set,
 				char **user_ret)
 {
 	wbcErr wbc_status;
-
 	struct wbcLogonUserParams logon;
 	char membership_of[1024];
 	uid_t user_uid = -1;
-	uint32_t flags = WBFLAG_PAM_INFO3_TEXT |
-			 WBFLAG_PAM_GET_PWD_POLICY;
-
+	uint32_t flags = WBFLAG_PAM_INFO3_TEXT;
 	struct wbcLogonUserInfo *info = NULL;
 	struct wbcAuthUserInfo *user_info = NULL;
 	struct wbcAuthErrorInfo *error = NULL;
-	struct wbcUserPasswordPolicyInfo *policy = NULL;
-
 	int ret = PAM_AUTH_ERR;
 	int i;
 	const char *codes[] = {
@@ -1845,7 +1821,7 @@ static int winbind_auth_request(struct pwb_context *ctx,
 				     &logon,
 				     &info,
 				     &error,
-				     &policy);
+				     NULL);
 	ret = wbc_auth_error_to_pam_error(ctx, error, wbc_status,
 					  user, "wbcLogonUser");
 	wbcFreeMemory(logon.blobs);
@@ -1863,10 +1839,6 @@ static int winbind_auth_request(struct pwb_context *ctx,
 		*p_info = info;
 	}
 
-	if (p_policy && policy) {
-		*p_policy = policy;
-	}
-
 	if (p_error && error) {
 		/* We want to process the error in the caller. */
 		*p_error = error;
@@ -1881,13 +1853,13 @@ static int winbind_auth_request(struct pwb_context *ctx,
 		}
 	}
 
-	if ((ret == PAM_SUCCESS) && user_info && policy && info) {
+	if ((ret == PAM_SUCCESS) && user_info && info) {
 
 		bool already_expired = false;
 		bool change_pwd = false;
 
 		/* warn a user if the password is about to expire soon */
-		_pam_warn_password_expiry(ctx, user_info, policy,
+		_pam_warn_password_expiry(ctx, user_info,
 					  warn_pwd_expire,
 					  &already_expired,
 					  &change_pwd);
@@ -1895,15 +1867,15 @@ static int winbind_auth_request(struct pwb_context *ctx,
 		if (already_expired == true) {
 
 			SMB_TIME_T last_set = user_info->pass_last_set_time;
+			SMB_TIME_T must_set = user_info->pass_must_change_time;
 
 			_pam_log_debug(ctx, LOG_DEBUG,
 				       "Password has expired "
 				       "(Password was last set: %lld, "
-				       "the policy says it should expire here "
+				       "it must be changed here "
 				       "%lld (now it's: %ld))\n",
 				       (long long int)last_set,
-				       (long long int)last_set +
-				       policy->expire,
+				       (long long int)must_set,
 				       (long)time(NULL));
 
 			return PAM_AUTHTOK_EXPIRED;
@@ -1942,9 +1914,6 @@ static int winbind_auth_request(struct pwb_context *ctx,
 	if (info && !p_info) {
 		wbcFreeMemory(info);
 	}
-	if (policy && !p_policy) {
-		wbcFreeMemory(policy);
-	}
 
 	return ret;
 }
@@ -2741,8 +2710,7 @@ int pam_sm_authenticate(pam_handle_t *pamh, int flags,
 	/* Now use the username to look up password */
 	retval = winbind_auth_request(ctx, real_username, password,
 				      member, cctype, warn_pwd_expire,
-				      NULL, NULL, NULL,
-				      NULL, &username_ret);
+				      NULL, NULL, NULL, &username_ret);
 
 	if (retval == PAM_NEW_AUTHTOK_REQD ||
 	    retval == PAM_AUTHTOK_EXPIRED) {
@@ -3152,7 +3120,7 @@ int pam_sm_chauthtok(pam_handle_t * pamh, int flags,
 
 		ret = winbind_auth_request(ctx, user, pass_old,
 					   NULL, NULL, 0,
-					   &error, NULL, NULL,
+					   &error, NULL,
 					   &pwdlastset_prelim, NULL);
 
 		if (ret != PAM_ACCT_EXPIRED &&
@@ -3260,7 +3228,6 @@ int pam_sm_chauthtok(pam_handle_t * pamh, int flags,
 			const char *cctype = NULL;
 			int warn_pwd_expire;
 			struct wbcLogonUserInfo *info = NULL;
-			struct wbcUserPasswordPolicyInfo *policy = NULL;
 
 			member = get_member_from_config(ctx);
 			cctype = get_krb5_cc_type_from_config(ctx);
@@ -3276,7 +3243,7 @@ int pam_sm_chauthtok(pam_handle_t * pamh, int flags,
 
 			ret = winbind_auth_request(ctx, user, pass_new,
 						   member, cctype, 0,
-						   &error, &info, &policy,
+						   &error, &info,
 						   NULL, &username_ret);
 			pass_old = pass_new = NULL;
 
@@ -3290,7 +3257,7 @@ int pam_sm_chauthtok(pam_handle_t * pamh, int flags,
 
 				/* warn a user if the password is about to
 				 * expire soon */
-				_pam_warn_password_expiry(ctx, user_info, policy,
+				_pam_warn_password_expiry(ctx, user_info,
 							  warn_pwd_expire,
 							  NULL, NULL);
 
@@ -3316,7 +3283,6 @@ int pam_sm_chauthtok(pam_handle_t * pamh, int flags,
 				wbcFreeMemory(info->blobs);
 			}
 			wbcFreeMemory(info);
-			wbcFreeMemory(policy);
 
 			goto out;
 		}
-- 
1.9.1

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20170403/d2cde62e/signature.sig>


More information about the samba-technical mailing list