From 5043215f219f90a899a8dc75518540a04b93301f Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Thu, 28 Feb 2008 08:50:00 +1100 Subject: 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) --- source4/auth/auth_sam.c | 4 ++- source4/auth/sam.c | 36 +++++++++++---------- source4/dsdb/common/util.c | 34 +++++++++++++++++--- source4/rpc_server/netlogon/dcerpc_netlogon.c | 22 +++++++------ source4/rpc_server/samr/dcesrv_samr.c | 28 ++++++++--------- source4/torture/rpc/samr.c | 45 ++++++++++++++++++++++----- 6 files changed, 115 insertions(+), 54 deletions(-) diff --git a/source4/auth/auth_sam.c b/source4/auth/auth_sam.c index 91896401503..4cb8d2b304f 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 fdd7de7c710..abcb72f2929 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 bee1eac480b..c9c0285604d 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 4d38dc069e9..37e63518640 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 760d774f2ea..2ad35e0eb3c 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;iin.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 9d6c73891b8..1d6ec433998 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