Generate ACB_PW_EXPIRED correctly

More correctly handle expired passwords, and do not expire machine accounts.

Test that the behaviour is consistant with windows, using the RPC-SAMR test.

Change NETLOGON to directly query the userAccountControl, just because
we don't want to do the extra expiry processing here.

Andrew Bartlett
(This used to be commit acda1f69bc9b9c43e157e254d0bae54d11363661)

6 files changed, 115 insertions, 54 deletions

diff --git a/source4/auth/auth_sam.c b/source4/auth/auth_sam.c
index 9189640150..4cb8d2b304 100644
--- a/source4/auth/auth_sam.c
+++ b/source4/auth/auth_sam.c
@@ -226,7 +226,9 @@ static NTSTATUS authsam_authenticate(struct auth_context *auth_context,
 {
 	struct samr_Password *lm_pwd, *nt_pwd;
 	NTSTATUS nt_status;
-	uint16_t acct_flags = samdb_result_acct_flags(msgs[0], "userAccountControl");
+	struct ldb_dn *domain_dn = samdb_result_dn(sam_ctx, mem_ctx, msgs_domain_ref[0], "nCName", NULL);
+
+	uint16_t acct_flags = samdb_result_acct_flags(sam_ctx, mem_ctx, msgs[0], domain_dn);
 	
 	/* Quit if the account was locked out. */
 	if (acct_flags & ACB_AUTOLOCK) {
diff --git a/source4/auth/sam.c b/source4/auth/sam.c
index fdd7de7c71..abcb72f292 100644
--- a/source4/auth/sam.c
+++ b/source4/auth/sam.c
@@ -156,7 +156,7 @@ _PUBLIC_ NTSTATUS authsam_account_ok(TALLOC_CTX *mem_ctx,
 	NTTIME now;
 	DEBUG(4,("authsam_account_ok: Checking SMB password for user %s\n", name_for_logs));
 
-	acct_flags = samdb_result_acct_flags(msg, "userAccountControl");
+	acct_flags = samdb_result_acct_flags(sam_ctx, mem_ctx, msg, domain_dn);
 	
 	acct_expiry = samdb_result_nttime(msg, "accountExpires", 0);
 	must_change_time = samdb_result_force_password_change(sam_ctx, mem_ctx, 
@@ -186,22 +186,20 @@ _PUBLIC_ NTSTATUS authsam_account_ok(TALLOC_CTX *mem_ctx,
 		return NT_STATUS_ACCOUNT_EXPIRED;
 	}
 
-	if (!(acct_flags & ACB_PWNOEXP)) {
-		/* check for immediate expiry "must change at next logon" */
-		if (must_change_time == 0 && last_set_time != 0) {
-			DEBUG(1,("sam_account_ok: Account for user '%s' password must change!.\n", 
-				 name_for_logs));
-			return NT_STATUS_PASSWORD_MUST_CHANGE;
-		}
+	/* check for immediate expiry "must change at next logon" */
+	if (!(acct_flags & ACB_PWNOEXP) && (must_change_time == 0 && last_set_time != 0)) {
+		DEBUG(1,("sam_account_ok: Account for user '%s' password must change!.\n", 
+			 name_for_logs));
+		return NT_STATUS_PASSWORD_MUST_CHANGE;
+	}
 
-		/* check for expired password */
-		if ((must_change_time != 0) && (must_change_time < now)) {
-			DEBUG(1,("sam_account_ok: Account for user '%s' password expired!.\n", 
-				 name_for_logs));
-			DEBUG(1,("sam_account_ok: Password expired at '%s' unix time.\n", 
-				 nt_time_string(mem_ctx, must_change_time)));
-			return NT_STATUS_PASSWORD_EXPIRED;
-		}
+	/* check for expired password (dynamicly gnerated in samdb_result_acct_flags) */
+	if (acct_flags & ACB_PW_EXPIRED) {
+		DEBUG(1,("sam_account_ok: Account for user '%s' password expired!.\n", 
+			 name_for_logs));
+		DEBUG(1,("sam_account_ok: Password expired at '%s' unix time.\n", 
+			 nt_time_string(mem_ctx, must_change_time)));
+		return NT_STATUS_PASSWORD_EXPIRED;
 	}
 
 	/* Test workstation. Workstation list is comma separated. */
@@ -267,6 +265,7 @@ _PUBLIC_ NTSTATUS authsam_make_server_info(TALLOC_CTX *mem_ctx, struct ldb_conte
 	struct dom_sid **groupSIDs = NULL;
 	struct dom_sid *account_sid;
 	struct dom_sid *primary_group_sid;
+	struct ldb_dn *domain_dn;
 	const char *str;
 	struct ldb_dn *ncname;
 	int i;
@@ -368,7 +367,10 @@ _PUBLIC_ NTSTATUS authsam_make_server_info(TALLOC_CTX *mem_ctx, struct ldb_conte
 	server_info->logon_count = samdb_result_uint(msg, "logonCount", 0);
 	server_info->bad_password_count = samdb_result_uint(msg, "badPwdCount", 0);
 
-	server_info->acct_flags = samdb_result_acct_flags(msg, "userAccountControl");
+	domain_dn = samdb_result_dn(sam_ctx, mem_ctx, msg_domain_ref, "nCName", NULL);
+
+	server_info->acct_flags = samdb_result_acct_flags(sam_ctx, mem_ctx, 
+							  msg, domain_dn);
 
 	server_info->user_session_key = user_sess_key;
 	server_info->lm_session_key = lm_sess_key;
diff --git a/source4/dsdb/common/util.c b/source4/dsdb/common/util.c
index bee1eac480..c9c0285604 100644
--- a/source4/dsdb/common/util.c
+++ b/source4/dsdb/common/util.c
@@ -596,11 +596,37 @@ struct samr_LogonHours samdb_result_logon_hours(TALLOC_CTX *mem_ctx, struct ldb_
 
 /*
   pull a set of account_flags from a result set. 
+
+  This requires that the attributes: 
+   pwdLastSet
+   userAccountControl
+  be included in 'msg'
 */
-uint16_t samdb_result_acct_flags(struct ldb_message *msg, const char *attr)
-{
-	uint_t userAccountControl = ldb_msg_find_attr_as_uint(msg, attr, 0);
-	return samdb_uf2acb(userAccountControl);
+uint32_t samdb_result_acct_flags(struct ldb_context *sam_ctx, TALLOC_CTX *mem_ctx, 
+				 struct ldb_message *msg, struct ldb_dn *domain_dn)
+{
+	uint32_t userAccountControl = ldb_msg_find_attr_as_uint(msg, "userAccountControl", 0);
+	uint32_t acct_flags = samdb_uf2acb(userAccountControl); 
+	if ((userAccountControl & UF_NORMAL_ACCOUNT) && !(userAccountControl & UF_DONT_EXPIRE_PASSWD)) {
+		NTTIME must_change_time;
+		NTTIME pwdLastSet = samdb_result_nttime(msg, "pwdLastSet", 0);
+		if (pwdLastSet == 0) {
+			acct_flags |= ACB_PW_EXPIRED;
+		} else {
+			NTTIME now;
+			
+			must_change_time = samdb_result_force_password_change(sam_ctx, mem_ctx, 
+									      domain_dn, msg);
+			
+			/* Test account expire time */
+			unix_to_nt_time(&now, time(NULL));
+			/* check for expired password */
+			if ((must_change_time != 0) && (must_change_time < now)) {
+				acct_flags |= ACB_PW_EXPIRED;
+			}
+		}
+	}
+	return acct_flags;
 }
 
 
diff --git a/source4/rpc_server/netlogon/dcerpc_netlogon.c b/source4/rpc_server/netlogon/dcerpc_netlogon.c
index 4d38dc069e..37e6351864 100644
--- a/source4/rpc_server/netlogon/dcerpc_netlogon.c
+++ b/source4/rpc_server/netlogon/dcerpc_netlogon.c
@@ -27,6 +27,7 @@
 #include "auth/auth.h"
 #include "auth/auth_sam_reply.h"
 #include "dsdb/samdb/samdb.h"
+#include "dsdb/common/flags.h"
 #include "rpc_server/samr/proto.h"
 #include "util/util_ldb.h"
 #include "libcli/auth/libcli_auth.h"
@@ -76,7 +77,7 @@ static NTSTATUS dcesrv_netr_ServerAuthenticate3(struct dcesrv_call_state *dce_ca
 	struct creds_CredentialState *creds;
 	void *sam_ctx;
 	struct samr_Password *mach_pwd;
-	uint16_t acct_flags;
+	uint32_t user_account_control;
 	int num_records;
 	struct ldb_message **msgs;
 	NTSTATUS nt_status;
@@ -113,27 +114,28 @@ static NTSTATUS dcesrv_netr_ServerAuthenticate3(struct dcesrv_call_state *dce_ca
 		return NT_STATUS_INTERNAL_DB_CORRUPTION;
 	}
 
-	acct_flags = samdb_result_acct_flags(msgs[0], 
-					     "userAccountControl");
+	
+	user_account_control = ldb_msg_find_attr_as_uint(msgs[0], "userAccountControl", 0);
 
-	if (acct_flags & ACB_DISABLED) {
+	if (user_account_control & UF_ACCOUNTDISABLE) {
 		DEBUG(1, ("Account [%s] is disabled\n", r->in.account_name));
 		return NT_STATUS_ACCESS_DENIED;
 	}
 
 	if (r->in.secure_channel_type == SEC_CHAN_WKSTA) {
-		if (!(acct_flags & ACB_WSTRUST)) {
-			DEBUG(1, ("Client asked for a workstation secure channel, but is not a workstation (member server) acb flags: 0x%x\n", acct_flags));
+		if (!(user_account_control & UF_WORKSTATION_TRUST_ACCOUNT)) {
+			DEBUG(1, ("Client asked for a workstation secure channel, but is not a workstation (member server) acb flags: 0x%x\n", user_account_control));
 			return NT_STATUS_ACCESS_DENIED;
 		}
 	} else if (r->in.secure_channel_type == SEC_CHAN_DOMAIN) {
-		if (!(acct_flags & ACB_DOMTRUST)) {
-			DEBUG(1, ("Client asked for a trusted domain secure channel, but is not a trusted domain: acb flags: 0x%x\n", acct_flags));
+		if (!(user_account_control & UF_INTERDOMAIN_TRUST_ACCOUNT)) {
+			DEBUG(1, ("Client asked for a trusted domain secure channel, but is not a trusted domain: acb flags: 0x%x\n", user_account_control));
+			
 			return NT_STATUS_ACCESS_DENIED;
 		}
 	} else if (r->in.secure_channel_type == SEC_CHAN_BDC) {
-		if (!(acct_flags & ACB_SVRTRUST)) {
-			DEBUG(1, ("Client asked for a server secure channel, but is not a server (domain controller): acb flags: 0x%x\n", acct_flags));
+		if (!(user_account_control & UF_SERVER_TRUST_ACCOUNT)) {
+			DEBUG(1, ("Client asked for a server secure channel, but is not a server (domain controller): acb flags: 0x%x\n", user_account_control));
 			return NT_STATUS_ACCESS_DENIED;
 		}
 	} else {
diff --git a/source4/rpc_server/samr/dcesrv_samr.c b/source4/rpc_server/samr/dcesrv_samr.c
index 760d774f2e..2ad35e0eb3 100644
--- a/source4/rpc_server/samr/dcesrv_samr.c
+++ b/source4/rpc_server/samr/dcesrv_samr.c
@@ -56,7 +56,7 @@
 #define QUERY_LHOURS(msg, field, attr) \
 	r->out.info->field = samdb_result_logon_hours(mem_ctx, msg, attr);
 #define QUERY_AFLAGS(msg, field, attr) \
-	r->out.info->field = samdb_result_acct_flags(msg, attr);
+	r->out.info->field = samdb_result_acct_flags(sam_ctx, mem_ctx, msg, a_state->domain_state->domain_dn);
 
 
 /* these are used to make the Set[User|Group]Info code easier to follow */
@@ -102,10 +102,11 @@
         set_el = ldb_msg_find_element(msg, attr);			\
  	set_el->flags = LDB_FLAG_MOD_REPLACE;				\
 } while (0)								
-									
+	
+/* Set account flags, discarding flags that cannot be set with SAMR */								
 #define SET_AFLAGS(msg, field, attr) do {				\
 	struct ldb_message_element *set_el;				\
-	if (samdb_msg_add_acct_flags(sam_ctx, mem_ctx, msg, attr, r->in.info->field) != 0) { \
+	if (samdb_msg_add_acct_flags(sam_ctx, mem_ctx, msg, attr, (r->in.info->field & ~(ACB_AUTOLOCK|ACB_PW_EXPIRED))) != 0) { \
 		return NT_STATUS_NO_MEMORY;				\
 	}								\
         set_el = ldb_msg_find_element(msg, attr);			\
@@ -1484,8 +1485,8 @@ static NTSTATUS dcesrv_samr_EnumDomainUsers(struct dcesrv_call_state *dce_call,
 	for (i=0;i<count;i++) {
 		/* Check if a mask has been requested */
 		if (r->in.acct_flags
-		    && ((samdb_result_acct_flags(res[i], 
-						 "userAccountControl") & r->in.acct_flags) == 0)) {
+		    && ((samdb_result_acct_flags(d_state->sam_ctx, mem_ctx, res[i], 
+						 d_state->domain_dn) & r->in.acct_flags) == 0)) {
 			continue;
 		}
 		entries[num_filtered_entries].idx = samdb_result_rid_from_sid(mem_ctx, res[i], "objectSid", 0);
@@ -3066,7 +3067,7 @@ static NTSTATUS dcesrv_samr_QueryUserInfo(struct dcesrv_call_state *dce_call, TA
 	}
 	case 16:
 	{
-		static const char * const attrs2[] = {"userAccountControl", NULL};
+		static const char * const attrs2[] = {"userAccountControl", "pwdLastSet", NULL};
 		attrs = attrs2;
 		break;
 	}
@@ -3613,7 +3614,7 @@ static NTSTATUS dcesrv_samr_QueryDisplayInfo(struct dcesrv_call_state *dce_call,
 	struct ldb_message **res;
 	int ldb_cnt, count, i;
 	const char * const attrs[] = { "objectSid", "sAMAccountName", "displayName",
-					"description", "userAccountControl", NULL };
+				       "description", "userAccountControl", "pwdLastSet", NULL };
 	struct samr_DispEntryFull *entriesFull = NULL;
 	struct samr_DispEntryFullGroup *entriesFullGroup = NULL;
 	struct samr_DispEntryAscii *entriesAscii = NULL;
@@ -3702,8 +3703,9 @@ static NTSTATUS dcesrv_samr_QueryDisplayInfo(struct dcesrv_call_state *dce_call,
 			entriesGeneral[count].rid = 
 				objectsid->sub_auths[objectsid->num_auths-1];
 			entriesGeneral[count].acct_flags =
-				samdb_result_acct_flags(res[i], 
-							"userAccountControl");
+				samdb_result_acct_flags(d_state->sam_ctx, mem_ctx,
+							res[i], 
+							d_state->domain_dn);
 			entriesGeneral[count].account_name.string =
 				samdb_result_string(res[i],
 						    "sAMAccountName", "");
@@ -3719,8 +3721,9 @@ static NTSTATUS dcesrv_samr_QueryDisplayInfo(struct dcesrv_call_state *dce_call,
 
 			/* No idea why we need to or in ACB_NORMAL here, but this is what Win2k3 seems to do... */
 			entriesFull[count].acct_flags =
-				samdb_result_acct_flags(res[i], 
-							"userAccountControl") | ACB_NORMAL;
+				samdb_result_acct_flags(d_state->sam_ctx, mem_ctx,
+							res[i], 
+							d_state->domain_dn) | ACB_NORMAL;
 			entriesFull[count].account_name.string =
 				samdb_result_string(res[i], "sAMAccountName",
 						    "");
@@ -3731,9 +3734,6 @@ static NTSTATUS dcesrv_samr_QueryDisplayInfo(struct dcesrv_call_state *dce_call,
 			entriesFullGroup[count].idx = count + 1;
 			entriesFullGroup[count].rid =
 				objectsid->sub_auths[objectsid->num_auths-1];
-			entriesFullGroup[count].acct_flags =
-				samdb_result_acct_flags(res[i], 
-							"userAccountControl");
 			/* We get a "7" here for groups */
 			entriesFullGroup[count].acct_flags
 				= SE_GROUP_MANDATORY | SE_GROUP_ENABLED_BY_DEFAULT | SE_GROUP_ENABLED;
diff --git a/source4/torture/rpc/samr.c b/source4/torture/rpc/samr.c
index 9d6c73891b..1d6ec43399 100644
--- a/source4/torture/rpc/samr.c
+++ b/source4/torture/rpc/samr.c
@@ -416,11 +416,6 @@ static bool test_SetUserInfo(struct dcerpc_pipe *p, struct torture_context *tctx
 	TEST_USERINFO_INT(21, logon_hours.bits[3], 21, logon_hours.bits[3], 4, 
 			  SAMR_FIELD_LOGON_HOURS);
 
-	if (torture_setting_bool(tctx, "samba4", false)) {
-		printf("skipping Set Account Flag tests against Samba4\n");
-		return ret;
-	}
-
 	TEST_USERINFO_INT_EXP(16, acct_flags, 5, acct_flags, 
 			      (base_acct_flags  | ACB_DISABLED | ACB_HOMDIRREQ), 
 			      (base_acct_flags  | ACB_DISABLED | ACB_HOMDIRREQ | user_extra_flags), 
@@ -1989,9 +1984,12 @@ static bool test_user_ops(struct dcerpc_pipe *p,
 			  const char *base_acct_name, enum torture_samr_choice which_ops)
 {
 	char *password = NULL;
+	struct samr_QueryUserInfo q;
+	NTSTATUS status;
 
 	bool ret = true;
 	int i;
+	uint32_t rid;
 	const uint32_t password_fields[] = {
 		SAMR_FIELD_PASSWORD,
 		SAMR_FIELD_PASSWORD2,
@@ -1999,6 +1997,11 @@ static bool test_user_ops(struct dcerpc_pipe *p,
 		0
 	};
 	
+	status = test_LookupName(p, tctx, domain_handle, base_acct_name, &rid);
+	if (!NT_STATUS_IS_OK(status)) {
+		ret = false;
+	}
+
 	switch (which_ops) {
 	case TORTURE_SAMR_USER_ATTRIBUTES:
 		if (!test_QuerySecurity(p, tctx, user_handle)) {
@@ -2091,6 +2094,29 @@ static bool test_user_ops(struct dcerpc_pipe *p,
 			ret = false;
 		}	
 
+		q.in.user_handle = user_handle;
+		q.in.level = 5;
+		
+		status = dcerpc_samr_QueryUserInfo(p, tctx, &q);
+		if (!NT_STATUS_IS_OK(status)) {
+			printf("QueryUserInfo level %u failed - %s\n", 
+			       q.in.level, nt_errstr(status));
+			ret = false;
+		} else {
+			uint32_t expected_flags = (base_acct_flags | ACB_PWNOTREQ | ACB_DISABLED);
+			if ((q.out.info->info5.acct_flags) != expected_flags) {
+				printf("QuerUserInfo level 5 failed, it returned 0x%08x when we expected flags of 0x%08x\n",
+				       q.out.info->info5.acct_flags, 
+				       expected_flags);
+				ret = false;
+			}
+			if (q.out.info->info5.rid != rid) {
+				printf("QuerUserInfo level 5 failed, it returned %u when we expected rid of %u\n",
+				       q.out.info->info5.rid, rid);
+
+			}
+		}
+
 		break;
 	case TORTURE_SAMR_OTHER:
 		/* We just need the account to exist */
@@ -2667,10 +2693,14 @@ static bool test_CreateUser2(struct dcerpc_pipe *p, struct torture_context *tctx
 				       q.in.level, nt_errstr(status));
 				ret = false;
 			} else {
-				if ((q.out.info->info5.acct_flags & acct_flags) != acct_flags) {
+				uint32_t expected_flags = (acct_flags | ACB_PWNOTREQ | ACB_DISABLED);
+				if (acct_flags == ACB_NORMAL) {
+					expected_flags |= ACB_PW_EXPIRED;
+				}
+				if ((q.out.info->info5.acct_flags) != expected_flags) {
 					printf("QuerUserInfo level 5 failed, it returned 0x%08x when we expected flags of 0x%08x\n",
 					       q.out.info->info5.acct_flags, 
-					       acct_flags);
+					       expected_flags);
 					ret = false;
 				} 
 				switch (acct_flags) {
@@ -3887,7 +3917,6 @@ static bool test_GroupList(struct dcerpc_pipe *p, TALLOC_CTX *mem_ctx,
 			for (j=0; j<num_names; j++) {
 				if (names[j] == NULL)
 					continue;
-				/* Hmm. No strequal in samba4 */
 				if (strequal(names[j], name)) {
 					names[j] = NULL;
 					found = true;