samba.git/libcli/security/access_check.c, branch talloc-2.3.4

libcli/security: fix handling of deny type ACEs in access_check_max_allowed()

2019-03-04T18:11:16+00:00

Deny ACEs must always be evaluated against explicitly granted rights
from previous ACEs.

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

Signed-off-by: Ralph Boehme 
Reviewed-by: Jeremy Allison

libcli/security: correct access check and maximum access calculation for Owner Rights ACEs

2019-03-04T18:11:16+00:00

We basically must process the Owner Rights ACEs as any other ACE wrt to the
order of adding granted permissions and checking denied permissions. According
to MS-DTYP 2.5.3.2 Owner Rights ACEs must be evaluated in the main loop over
the ACEs in an ACL and the corresponding access_mask must be directly applied
to bits_remaining. We currently defer this to after the loop over the ACEs in
ACL, this is wrong.

We just have to do some initial magic to determine if an ACL contains and
Owner Rights ACEs, and in case it doesn't we grant SEC_STD_WRITE_DAC |
SEC_STD_READ_CONTROL at the *beginning*. MS-DTYP:

-- the owner of an object is always granted READ_CONTROL and WRITE_DAC.
CALL SidInToken(Token, SecurityDescriptor.Owner, PrincipalSelfSubst)
IF SidInToken returns True THEN
   IF DACL does not contain ACEs from object owner THEN
       Remove READ_CONTROL and WRITE_DAC from RemainingAccess
       Set GrantedAccess to GrantedAccess or READ_CONTROL or WRITE_OWNER
   END IF
END IF

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

Signed-off-by: Ralph Boehme 
Reviewed-by: Jeremy Allison

libcli/security: add "Owner Rights" calculation to access_check_max_allowed()

2019-02-28T19:18:16+00:00

This was missing in 44590c1b70c0a24f853c02d5fcdb3c609401e2ca.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13812

Signed-off-by: Ralph Boehme 
Reviewed-by: David Disseldorp 

Autobuild-User(master): David Disseldorp 
Autobuild-Date(master): Thu Feb 28 19:18:16 UTC 2019 on sn-devel-144

CVE-2018-10919 security: Fix checking of object-specific CONTROL_ACCESS rights

2018-08-14T11:57:16+00:00

An 'Object Access Allowed' ACE that assigned 'Control Access' (CR)
rights to a specific attribute would not actually grant access.

What was happening was the remaining_access mask for the object_tree
nodes would be Read Property (RP) + Control Access (CR). The ACE mapped
to the schemaIDGUID for a given attribute, which would end up being a
child node in the tree. So the CR bit was cleared for a child node, but
not the rest of the tree. We would then check the user had the RP access
right, which it did. However, the RP right was cleared for another node
in the tree, which still had the CR bit set in its remaining_access
bitmap, so Samba would not grant access.

Generally, the remaining_access only ever has one bit set, which means
this isn't a problem normally. However, in the Control Access case there
are 2 separate bits being checked, i.e. RP + CR.

One option to fix this problem would be to clear the remaining_access
for the tree instead of just the node. However, the Windows spec is
actually pretty clear on this: if the ACE has a CR right present, then
you can stop any further access checks.

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

Signed-off-by: Tim Beale 
Reviewed-by: Andrew Bartlett 
Reviewed-by: Gary Lockyer

CVE-2018-10919 security: Add more comments to the object-specific access checks

2018-08-14T11:57:15+00:00

Reading the spec and then reading the code makes sense, but we could
comment the code more so it makes sense on its own.

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

Signed-off-by: Tim Beale 
Reviewed-by: Andrew Bartlett 
Reviewed-by: Gary Lockyer

CVE-2018-10919 security: Move object-specific access checks into separate function

2018-08-14T11:57:15+00:00

Object-specific access checks refer to a specific section of the
MS-ADTS, and the code closely matches the spec. We need to extend this
logic to properly handle the Control-Access Right (CR), so it makes
sense to split the logic out into its own function.

This patch just moves the code, and should not alter the logic (apart
from ading in the boolean grant_access return variable.

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

Signed-off-by: Tim Beale 
Reviewed-by: Andrew Bartlett 
Reviewed-by: Gary Lockyer

lib: security: se_access_check() incorrectly processes owner rights (S-1-3-4) DENY ace entries

2016-12-10T05:24:11+00:00

Reported and proposed fix by Shilpa K .

When processing DENY ACE entries for owner rights SIDs (S-1-3-4) the
code OR's in the deny access mask bits without taking into account if
they were being requested in the requested access mask.

E.g. The current logic has:

An ACL containining:

[0] SID: S-1-3-4
    TYPE: DENY
    MASK: WRITE_DATA
[1] SID: S-1-3-4
    TYPE: ALLOW
    MASK: ALLOW_ALL

prohibits an open request by the owner for READ_DATA - even though this
is explicitly allowed.

Furthermore a non-canonical ACL containing:

[0] SID: User SID 1-5-21-something
    TYPE: ALLOW
    MASK: READ_DATA

[1] SID: S-1-3-4
    TYPE: DENY
    MASK: READ_DATA

[2] SID: User SID 1-5-21-something
    TYPE: ALLOW
    MASK: WRITE_DATA

prohibits an open request by the owner for READ_DATA|WRITE_DATA - even
though READ_DATA is explicitly allowed in ACE no 0 and is thus already
filtered out of the "access-still-needed" mask when the deny ACE no 1 is
evaluated.

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

Signed-off-by: Jeremy Allison 
Signed-off-by: Ralph Boehme 
Reviewed-by: Ralph Boehme

s4-dsacl: Fixed incorrect handling of privileges in sec_access_check_ds

2013-10-24T20:45:57+00:00

Restore and backup privileges are not relevant to ldap
access checks, and the TakeOwnership privilege should
grant write_owner right

Signed-off-by: Nadezhda Ivanova 

Reviewed-by: Andrew Bartlett

Fix bug #9674 - Samba denies owner Read Control when there is a DENY entry while W2K08 does not.

2013-02-23T18:28:15+00:00

Signed-off-by: Richard Sharpe 
Reviewed-by: Jeremy Allison 

Autobuild-User(master): Jeremy Allison 
Autobuild-Date(master): Sat Feb 23 19:28:15 CET 2013 on sn-devel-104

libcli-acl: add documentation

2013-01-22T10:14:05+00:00

Reviewed-by: Andrew Bartlett