From 9fda812d01d899603f7a6f7470e57de53f5c0593 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Mon, 30 Jun 2003 18:53:48 +0000 Subject: Finally ! Fixed the ACL ordering bug reported by jcmd. I realised we were not sorting returned ACE's correctly w.r.t. W2K - implemented the correct algorithm. Jeremy. (This used to be commit fa23a4158ec23c0b8dbdc6c53f29958243107dee) --- source3/rpc_parse/parse_sec.c | 122 +++++++++++++++++++++++++++++++++++++++--- source3/smbd/posix_acls.c | 30 ++++------- 2 files changed, 125 insertions(+), 27 deletions(-) (limited to 'source3') diff --git a/source3/rpc_parse/parse_sec.c b/source3/rpc_parse/parse_sec.c index 870402db5d..3848bd7051 100644 --- a/source3/rpc_parse/parse_sec.c +++ b/source3/rpc_parse/parse_sec.c @@ -3,7 +3,7 @@ * Version 1.9. * RPC Pipe client / server routines * Copyright (C) Andrew Tridgell 1992-1998, - * Copyright (C) Jeremy R. Allison 1995-1998 + * Copyright (C) Jeremy R. Allison 1995-2003. * Copyright (C) Luke Kenneth Casson Leighton 1996-1998, * Copyright (C) Paul Ashton 1997-1998. * @@ -901,7 +901,7 @@ BOOL sec_io_desc_buf(const char *desc, SEC_DESC_BUF **ppsdb, prs_struct *ps, int } /******************************************************************* - adds new SID with its permissions to SEC_DESC + Add a new SID with its permissions to SEC_DESC. ********************************************************************/ NTSTATUS sec_desc_add_sid(TALLOC_CTX *ctx, SEC_DESC **psd, DOM_SID *sid, uint32 mask, size_t *sd_size) @@ -913,7 +913,8 @@ NTSTATUS sec_desc_add_sid(TALLOC_CTX *ctx, SEC_DESC **psd, DOM_SID *sid, uint32 *sd_size = 0; - if (!ctx || !psd || !sid || !sd_size) return NT_STATUS_INVALID_PARAMETER; + if (!ctx || !psd || !sid || !sd_size) + return NT_STATUS_INVALID_PARAMETER; status = sec_ace_add_sid(ctx, &ace, psd[0]->dacl->ace, &psd[0]->dacl->num_aces, sid, mask); @@ -933,14 +934,15 @@ NTSTATUS sec_desc_add_sid(TALLOC_CTX *ctx, SEC_DESC **psd, DOM_SID *sid, uint32 } /******************************************************************* - modify SID's permissions at SEC_DESC + Modify a SID's permissions in a SEC_DESC. ********************************************************************/ NTSTATUS sec_desc_mod_sid(SEC_DESC *sd, DOM_SID *sid, uint32 mask) { NTSTATUS status; - if (!sd || !sid) return NT_STATUS_INVALID_PARAMETER; + if (!sd || !sid) + return NT_STATUS_INVALID_PARAMETER; status = sec_ace_mod_sid(sd->dacl->ace, sd->dacl->num_aces, sid, mask); @@ -951,7 +953,7 @@ NTSTATUS sec_desc_mod_sid(SEC_DESC *sd, DOM_SID *sid, uint32 mask) } /******************************************************************* - delete SID from SEC_DESC + Delete a SID from a SEC_DESC. ********************************************************************/ NTSTATUS sec_desc_del_sid(TALLOC_CTX *ctx, SEC_DESC **psd, DOM_SID *sid, size_t *sd_size) @@ -963,7 +965,8 @@ NTSTATUS sec_desc_del_sid(TALLOC_CTX *ctx, SEC_DESC **psd, DOM_SID *sid, size_t *sd_size = 0; - if (!ctx || !psd[0] || !sid || !sd_size) return NT_STATUS_INVALID_PARAMETER; + if (!ctx || !psd[0] || !sid || !sd_size) + return NT_STATUS_INVALID_PARAMETER; status = sec_ace_del_sid(ctx, &ace, psd[0]->dacl->ace, &psd[0]->dacl->num_aces, sid); @@ -981,3 +984,108 @@ NTSTATUS sec_desc_del_sid(TALLOC_CTX *ctx, SEC_DESC **psd, DOM_SID *sid, size_t sd = 0; return NT_STATUS_OK; } + +/******************************************************************* + Comparison function to sort non-inherited first. +*******************************************************************/ + +static int nt_ace_inherit_comp( SEC_ACE *a1, SEC_ACE *a2) +{ + int a1_inh = a1->flags & SEC_ACE_FLAG_INHERITED_ACE; + int a2_inh = a2->flags & SEC_ACE_FLAG_INHERITED_ACE; + + if (a1_inh == a2_inh) + return 0; + + if (!a1_inh && a2_inh) + return -1; + return 1; +} + +/******************************************************************* + Comparison function to apply the order explained below in a group. +*******************************************************************/ + +static int nt_ace_canon_comp( SEC_ACE *a1, SEC_ACE *a2) +{ + if ((a1->type == SEC_ACE_TYPE_ACCESS_DENIED) && + (a2->type != SEC_ACE_TYPE_ACCESS_DENIED)) + return -1; + + if ((a2->type == SEC_ACE_TYPE_ACCESS_DENIED) && + (a1->type != SEC_ACE_TYPE_ACCESS_DENIED)) + return 1; + + /* Both access denied or access allowed. */ + + /* 1. ACEs that apply to the object itself */ + + if (!(a1->flags & SEC_ACE_FLAG_INHERIT_ONLY) && + (a2->flags & SEC_ACE_FLAG_INHERIT_ONLY)) + return -1; + else if (!(a2->flags & SEC_ACE_FLAG_INHERIT_ONLY) && + (a1->flags & SEC_ACE_FLAG_INHERIT_ONLY)) + return 1; + + /* 2. ACEs that apply to a subobject of the object, such as + * a property set or property. */ + + if (a1->flags & (SEC_ACE_FLAG_CONTAINER_INHERIT|SEC_ACE_FLAG_OBJECT_INHERIT) && + !(a2->flags & (SEC_ACE_FLAG_CONTAINER_INHERIT|SEC_ACE_FLAG_OBJECT_INHERIT))) + return -1; + else if (a2->flags & (SEC_ACE_FLAG_CONTAINER_INHERIT|SEC_ACE_FLAG_OBJECT_INHERIT) && + !(a1->flags & (SEC_ACE_FLAG_CONTAINER_INHERIT|SEC_ACE_FLAG_OBJECT_INHERIT))) + return 1; + + return 0; +} + +/******************************************************************* + Functions to convert a SEC_DESC ACE DACL list into canonical order. + JRA. + +--- from http://msdn.microsoft.com/library/default.asp?url=/library/en-us/security/security/order_of_aces_in_a_dacl.asp + +The following describes the preferred order: + + To ensure that noninherited ACEs have precedence over inherited ACEs, + place all noninherited ACEs in a group before any inherited ACEs. + This ordering ensures, for example, that a noninherited access-denied ACE + is enforced regardless of any inherited ACE that allows access. + + Within the groups of noninherited ACEs and inherited ACEs, order ACEs according to ACE type, as the following shows: + 1. Access-denied ACEs that apply to the object itself + 2. Access-denied ACEs that apply to a subobject of the object, such as a property set or property + 3. Access-allowed ACEs that apply to the object itself + 4. Access-allowed ACEs that apply to a subobject of the object" + +********************************************************************/ + +void dacl_sort_into_canonical_order(SEC_ACE *srclist, unsigned int num_aces) +{ + unsigned int i; + + if (!srclist || num_aces == 0) + return; + + /* Sort so that non-inherited ACE's come first. */ + qsort( srclist, num_aces, sizeof(srclist[0]), QSORT_CAST nt_ace_inherit_comp); + + /* Find the boundary between non-inherited ACEs. */ + for (i = 0; i < num_aces; i++ ) { + SEC_ACE *curr_ace = &srclist[i]; + + if (curr_ace->flags & SEC_ACE_FLAG_INHERITED_ACE) + break; + } + + /* i now points at entry number of the first inherited ACE. */ + + /* Sort the non-inherited ACEs. */ + if (i) + qsort( srclist, i, sizeof(srclist[0]), QSORT_CAST nt_ace_canon_comp); + + /* Now sort the inherited ACEs. */ + if (num_aces - i) + qsort( &srclist[i], num_aces - i, sizeof(srclist[0]), QSORT_CAST nt_ace_canon_comp); +} diff --git a/source3/smbd/posix_acls.c b/source3/smbd/posix_acls.c index 209387f422..f429fcb852 100644 --- a/source3/smbd/posix_acls.c +++ b/source3/smbd/posix_acls.c @@ -2548,16 +2548,6 @@ posix perms.\n", fsp->fsp_name )); return True; } -static int nt_ace_comp( SEC_ACE *a1, SEC_ACE *a2) -{ - if (a1->type == a2->type) - return 0; - - if (a1->type == SEC_ACE_TYPE_ACCESS_DENIED && a2->type == SEC_ACE_TYPE_ACCESS_ALLOWED) - return -1; - return 1; -} - /**************************************************************************** Incoming NT ACLs on a directory can be split into a default POSIX acl (CI|OI|IO) and a normal POSIX acl. Win2k needs these split acls re-merging into one ACL @@ -2653,7 +2643,8 @@ size_t get_nt_acl(files_struct *fsp, uint32 security_info, SEC_DESC **ppdesc) canon_ace *dir_ace = NULL; size_t num_profile_acls = 0; struct pai_val *pal = NULL; - + SEC_DESC *psd = NULL; + *ppdesc = NULL; DEBUG(10,("get_nt_acl: called for file %s\n", fsp->fsp_name )); @@ -2858,12 +2849,6 @@ size_t get_nt_acl(files_struct *fsp, uint32 security_info, SEC_DESC **ppdesc) num_aces = merge_default_aces(nt_ace_list, num_aces); - /* - * Sort to force deny entries to the front. - */ - - if (num_aces) - qsort( nt_ace_list, num_aces, sizeof(nt_ace_list[0]), QSORT_CAST nt_ace_comp); } if (num_aces) { @@ -2874,13 +2859,13 @@ size_t get_nt_acl(files_struct *fsp, uint32 security_info, SEC_DESC **ppdesc) } } /* security_info & DACL_SECURITY_INFORMATION */ - *ppdesc = make_standard_sec_desc( main_loop_talloc_get(), + psd = make_standard_sec_desc( main_loop_talloc_get(), (security_info & OWNER_SECURITY_INFORMATION) ? &owner_sid : NULL, (security_info & GROUP_SECURITY_INFORMATION) ? &group_sid : NULL, psa, &sd_size); - if(!*ppdesc) { + if(!psd) { DEBUG(0,("get_nt_acl: Unable to malloc space for security descriptor.\n")); sd_size = 0; } else { @@ -2894,9 +2879,14 @@ size_t get_nt_acl(files_struct *fsp, uint32 security_info, SEC_DESC **ppdesc) * flag doesn't seem to bother Windows NT. */ if (get_protected_flag(pal)) - (*ppdesc)->type |= SE_DESC_DACL_PROTECTED; + psd->type |= SE_DESC_DACL_PROTECTED; } + if (psd->dacl) + dacl_sort_into_canonical_order(psd->dacl->ace, (unsigned int)psd->dacl->num_aces); + + *ppdesc = psd; + done: if (posix_acl) -- cgit