1
0
mirror of https://github.com/samba-team/samba.git synced 2025-01-25 06:04:04 +03:00

58 Commits

Author SHA1 Message Date
Joseph Sutton
0a202264d3 s4:dsdb: Access correct member of union
Accessing the wrong member of a union invokes undefined behaviour.

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2023-08-14 04:57:34 +00:00
Joseph Sutton
12d990ac9f s4-acl: Make parameter const
Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andreas Schneider <asn@samba.org>
2023-04-12 13:52:31 +00:00
Joseph Sutton
197633cc2a CVE-2023-0614 ldb: Use binary search to check whether attribute is secret
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15270

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2023-04-05 02:10:35 +00:00
Joseph Sutton
3a70c6464d CVE-2023-0614 s4-acl: Avoid calling dsdb_module_am_system() if we can help it
If the AS_SYSTEM control is present, we know we have system privileges,
and have no need to call dsdb_module_am_system().

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

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2023-04-05 02:10:35 +00:00
Joseph Sutton
d5d0e71279 CVE-2023-0614 ldb: Prevent disclosure of confidential attributes
Add a hook, acl_redact_msg_for_filter(), in the aclread module, that
marks inaccessible any message elements used by an LDAP search filter
that the user has no right to access. Make the various ldb_match_*()
functions check whether message elements are accessible, and refuse to
match any that are not. Remaining message elements, not mentioned in the
search filter, are checked in aclread_callback(), and any inaccessible
elements are removed at this point.

Certain attributes, namely objectClass, distinguishedName, name, and
objectGUID, are always present, and hence the presence of said
attributes is always allowed to be checked in a search filter. This
corresponds with the behaviour of Windows.

Further, we unconditionally allow the attributes isDeleted and
isRecycled in a check for presence or equality. Windows is not known to
make this special exception, but it seems mostly harmless, and should
mitigate the performance impact on searches made by the show_deleted
module.

As a result of all these changes, our behaviour regarding confidential
attributes happens to match Windows more closely. For the test in
confidential_attr.py, we can now model our attribute handling with
DC_MODE_RETURN_ALL, which corresponds to the behaviour exhibited by
Windows.

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

Pair-Programmed-With: Andrew Bartlett <abartlet@samba.org>

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2023-04-05 02:10:35 +00:00
Joseph Sutton
748bbbe70d CVE-2023-0614 s4-acl: Split out function to set up access checking variables
These variables are often used together, and it is useful to have the
setup code in one place.

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

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2023-04-05 02:10:35 +00:00
Joseph Sutton
5c334918a2 CVE-2023-0614 s4-acl: Split out logic to remove access checking attributes
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15270

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2023-04-05 02:10:35 +00:00
Joseph Sutton
a43977499c CVE-2023-0614 s4-acl: Use ldb functions for handling inaccessible message elements
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15270

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2023-04-05 02:10:35 +00:00
Joseph Sutton
a7222faade CVE-2023-0614 s4:dsdb: Use talloc_get_type_abort() more consistently
It is better to explicitly abort than to dereference a NULL pointer or
try to read data cast to the wrong type.

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

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2023-04-05 02:10:35 +00:00
Nadezhda Ivanova
5073d5997c CVE-2020-25720: s4-acl: Owner no longer has implicit Write DACL
The implicit right of an object's owner to modify its security
descriptor no longer exists, according to the new access rules. However,
we continue to grant this implicit right for fileserver access checks.

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

Signed-off-by: Nadezhda Ivanova <nivanova@symas.com>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2022-09-16 02:32:36 +00:00
Stefan Metzmacher
7223f6453b s4:dsdb:acl_read: Implement "List Object" mode feature
See [MS-ADTS] 5.1.3.3.6 Checking Object Visibility

I tried to avoid any possible overhead for the common cases:

- SEC_ADS_LIST (List Children) is already granted by default
- fDoListObject is off by default

Overhead is only added if the administrator turned on
the fDoListObject feature and removed SEC_ADS_LIST (List Children)
from a parent object.

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>

Autobuild-User(master): Stefan Metzmacher <metze@samba.org>
Autobuild-Date(master): Wed Oct 21 08:48:02 UTC 2020 on sn-devel-184
2020-10-21 08:48:01 +00:00
Stefan Metzmacher
e1529bedb2 s4:dsdb:acl_read: defer LDB_ERR_NO_SUCH_OBJECT
We may need to return child objects even if the base dn
is invisible.

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
2020-10-21 07:25:37 +00:00
Stefan Metzmacher
faff8e6c89 s4:dsdb:acl_read: make use of aclread_check_object_visible() for the search base
We should only have one place to do access checks.

Use 'git show -w' to see the minimal diff.

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
2020-10-21 07:25:37 +00:00
Stefan Metzmacher
c4a3028de7 s4:dsdb:acl_read: fully set up 'struct aclread_context' before the search base acl check
This makes further change much easier.

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
2020-10-21 07:25:37 +00:00
Stefan Metzmacher
d2dd7c2a5c s4:dsdb:acl_read: introduce aclread_check_object_visible() helper
In future this will do more than aclread_check_parent(),
if we implement fDoListObject and SEC_ADS_LIST_OBJECT handling.

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
2020-10-21 07:25:37 +00:00
Alexander Bokovoy
234957a2e4 Fix build after removal of an extra safe_string.h
Move of strcasecmp redefine to lib/util/safe_string.h in
https://gitlab.com/samba-team/samba/-/merge_requests/1507 broke build on
Fedora 33 with GCC 10.2.1 for those compilation units that use
ldb_att_cmp().

The reason for that is that ldb_attr_cmp() defined as

   #define ldb_attr_cmp(a, b) strcasecmp(a, b)

because attribute names restricted to be ASCII by RFC2251 (LDAPv3 spec).

A solution is to add

   #undef strcasecmp

to all source code files which use ldb_attr_cmp().

Signed-off-by: Alexander Bokovoy <ab@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>

Autobuild-User(master): Jeremy Allison <jra@samba.org>
Autobuild-Date(master): Thu Oct  1 22:45:29 UTC 2020 on sn-devel-184
2020-10-01 22:45:29 +00:00
Garming Sam
a2b1970a37 acl_read: Fix regression caused by db15fcfa899e1fe4d6994f68ceb299921b8aa6f1 for empty lists
The original code never dereferenced attrs and only added "*" if attrs
was NULL (not if attrs[0] was NULL).

This causes significant performance issues with the new paged_results
module introduced for 4.10 as the initial GUID search requests no
attributes. This GUID search turns into a search for "*" and ends up
allocating memory for the entire database.

This never appears to cause changes in the final result set, only
intermediate processing.

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

Signed-off-by: Garming Sam <garming@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

Autobuild-User(master): Andrew Bartlett <abartlet@samba.org>
Autobuild-Date(master): Fri Mar 29 18:37:29 UTC 2019 on sn-devel-144
2019-03-29 18:37:29 +00:00
Andreas Schneider
c195134e35 s4:dsdb: Fix size type for num_of_attrs in acl_read
This fixes a compile error on sn-devel184.

Signed-off-by: Andreas Schneider <asn@samba.org>
2019-01-19 12:24:18 +01:00
Tim Beale
6c1ff59099 acl_read: Rework Samba code to reflect Windows logic
This patch should not alter functionality. It is just updating the Samba
code to better match the Windows specification docs.

When fixing Samba BUG #13434, the Microsoft behaviour wasn't clearly
documented, so we made a best guess based on observed behaviour.
The problem was an exception was made to allow "objectClass=*" searches
to return objects, even if you didn't have Read Property rights for the
object's objectClass attribute. However, the logic behind what
attributes were and weren't covered by this exception wasn't clear.

I made a guess that it was attributes belonging to the Public Info
property-set that also have the systemOnly flag set.

Microsoft have confirmed the object visibility behaviour. It turns out
that an optimization is made for the 4 attributes that are always
present for every object (i.e. objectClass, distinguishedName,
name, objectGUID). They're updating their Docs to reflect this.

Now that we know the Windows logic, we can update the Samba code.
This simplifies the code somewhat.

Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2018-09-21 20:04:23 +02:00
Tim Beale
fc45da529d CVE-2018-10919 acl_read: Fix unauthorized attribute access via searches
A user that doesn't have access to view an attribute can still guess the
attribute's value via repeated LDAP searches. This affects confidential
attributes, as well as ACLs applied to an object/attribute to deny
access.

Currently the code will hide objects if the attribute filter contains an
attribute they are not authorized to see. However, the code still
returns objects as results if confidential attribute is in the search
expression itself, but not in the attribute filter.

To fix this problem we have to check the access rights on the attributes
in the search-tree, as well as the attributes returned in the message.

Points of note:
- I've preserved the existing dirsync logic (the dirsync module code
  suppresses the result as long as the replPropertyMetaData attribute is
  removed). However, there doesn't appear to be any test that highlights
  that this functionality is required for dirsync.
- To avoid this fix breaking the acl.py tests, we need to still permit
  searches like 'objectClass=*', even though we don't have Read Property
  access rights for the objectClass attribute. The logic that Windows
  uses does not appear to be clearly documented, so I've made a best
  guess that seems to mirror Windows behaviour.

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

Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Gary Lockyer <gary@catalyst.net.nz>
2018-08-14 13:57:16 +02:00
Tim Beale
98c2e6a14f CVE-2018-10919 acl_read: Flip the logic in the dirsync check
This better reflects the special case we're making for dirsync, and gets
rid of a 'if-else' clause.

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

Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Gary Lockyer <gary@catalyst.net.nz>
2018-08-14 13:57:16 +02:00
Tim Beale
4234579a5d CVE-2018-10919 acl_read: Small refactor to aclread_callback()
Flip the dirsync check (to avoid a double negative), and use a helper
boolean variable.

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

Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Gary Lockyer <gary@catalyst.net.nz>
2018-08-14 13:57:16 +02:00
Tim Beale
80c4e17f0f CVE-2018-10919 acl_read: Split access_mask logic out into helper function
So we can re-use the same logic laster for checking the search-ops.

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

Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Gary Lockyer <gary@catalyst.net.nz>
2018-08-14 13:57:16 +02:00
Andrew Bartlett
aafc1c2828 dsdb: Remember the last ACL we read during a search and what it expanded to
It may well be the same as the next one we need to check, so we can
avoid parsing it again.

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>

Autobuild-User(master): Andrew Bartlett <abartlet@samba.org>
Autobuild-Date(master): Fri Jun 16 07:39:24 CEST 2017 on sn-devel-144
2017-06-16 07:39:24 +02:00
Andrew Bartlett
f7bcf7b972 dsdb: Cache the result of checking the parent ACL
This should help a lot for large one-level searches and for subtree searches that are of
flat tree structures

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
2017-06-16 03:21:29 +02:00
Andrew Bartlett
9b24f6523e dsdb: Expand on what the error finding the ntSecurityDescriptor was in acl_read
Reviewed-by: Stefan Metzmacher <metze@samba.org>
2013-05-16 19:02:04 +02:00
Andrew Bartlett
e8cc59eb78 dsdb-acl: Pass the structural objectClass into acl_check_access_on_attribute
This will, when the GUID is entered into the object tree (not in this
commit) ensure that access rights assigned to the structural
objectClass are also available, as well as rights assigned to the
attribute property groups.

Andrew Bartlett

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>

Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2013-01-21 16:12:45 +01:00
Andrew Bartlett
a1b421e8cc dsdb-acl: ask for the objectClass attribute if it's not in the scope of the clients search
This will be used later.

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>

Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2013-01-21 16:12:45 +01:00
Stefan Metzmacher
22bb2fd868 s4:dsdb/acl_read: return the nTSecurityDescriptor attr if the sd_flags control is given (bug #9470)
Not returning the nTSecurityDescriptor causes a lot of problems.

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Michael Adam <obnox@samba.org>
2012-12-10 13:53:47 +01:00
Stefan Metzmacher
4f8558ffaf s4:dsdb/acl_read: give some variables a better name
Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Michael Adam <obnox@samba.org>
2012-12-10 13:53:47 +01:00
Stefan Metzmacher
db15fcfa89 s4:dsdb/acl_read: fix the calculation of the attribute array for the sub search
Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Michael Adam <obnox@samba.org>
2012-12-10 13:53:47 +01:00
Stefan Metzmacher
e2181617a0 s4:dsdb/acl_read: check the ldb_attr_list_copy_add() result
Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Michael Adam <obnox@samba.org>
2012-12-10 13:53:46 +01:00
Stefan Metzmacher
8021247895 s4:dsdb/acl_read: improve debugging for fatal error
Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Michael Adam <obnox@samba.org>
2012-12-07 16:04:26 +01:00
Stefan Metzmacher
14b5b72904 s4:dsdb/acl_read: keep the ldb_message of the sub search (bug #9470)
Some modules might not allocate values on the correct memory context.

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Michael Adam <obnox@samba.org>
2012-12-07 14:28:25 +01:00
Stefan Metzmacher
990448b499 s4:dsdb/acl_read: enable acl checking on search by default (bug #8620)
Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Michael Adam <obnox@samba.org>
2012-11-30 17:17:20 +01:00
Stefan Metzmacher
fa676769e0 s4:dsdb/acl_read: specify the correct access_mask for nTSecurityDescriptor
We need to base the access mask on the given SD Flags.
Originally, we always checked for SEC_FLAG_SYSTEM_SECURITY,
which could lead to INSUFFICIENT_RIGHTS when we should
have been allowed to read.

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Michael Adam <obnox@samba.org>
2012-11-30 17:17:20 +01:00
Stefan Metzmacher
ca3c0e28ef s4:dsdb/acl_read: do search for instanceType AS_SYSTEM and with SHOW_RECYCLED
Note that SHOW_RECYCLED implies SHOW_DELETED.

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Michael Adam <obnox@samba.org>
2012-11-30 17:17:20 +01:00
Stefan Metzmacher
e0ab14f52a s4:dsdb/acl_read: make sure confidential attributes require CONTROL_ACCESS (bug #8620)
Signed-off-by: Stefan Metzmacher <metze@samba.org>
Signed-off-by: Andrew Bartlett <abartlet@samba.org>

Reviewed-by: Andrew Bartlett <abartlet@samba.org>

Autobuild-User(master): Andrew Bartlett <abartlet@samba.org>
Autobuild-Date(master): Mon Nov 12 01:25:21 CET 2012 on sn-devel-104
2012-11-12 01:25:19 +01:00
Stefan Metzmacher
21dfaefda0 s4:dsdb/acl_read: fix whitespace formatting errors
Signed-off-by: Stefan Metzmacher <metze@samba.org>
Signed-off-by: Andrew Bartlett <abartlet@samba.org>

Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2012-11-12 09:42:36 +11:00
Matthieu Patou
37b1662a38 s4-dsdb: relax a bit the checks on read acl when dirsync control is specified
Signed-off-by: Andrew Tridgell <tridge@samba.org>
2011-05-21 14:39:12 +04:00
Matthieu Patou
85e8c86302 s4-dsdb: Add more information on why we don't check the SD control
Signed-off-by: Nadezhda Ivanova <nivanova@samba.org>

Autobuild-User: Nadezhda Ivanova <nivanova@samba.org>
Autobuild-Date: Fri Apr 15 16:16:27 CEST 2011 on sn-devel-104
2011-04-15 16:16:27 +02:00
Matthieu Patou
cf4a3081cb s4-dsdb: If current attribute list is empty use the one from the request
This will avoid overwritting attribute list made by upper modules.

Signed-off-by: Nadezhda Ivanova <nivanova@samba.org>
2011-04-15 16:28:08 +03:00
Matthieu Patou
4a15c7e750 dsdb: read acl, sd can be null and ret == LDB_SUCCESS 2011-03-20 11:27:26 +01:00
Matthieu Patou
cbb0f881ac dsdb: acl_read fix a missed talloc_steal 2011-03-20 11:27:26 +01:00
Andrew Tridgell
87f3151047 s4-dsdb: pass parent request to dsdb_module_*() functions
this preserves the request hierarchy for dsdb_module_*() calls inside
dsdb ldb modules

Pair-Programmed-With: Andrew Bartlett <abartlet@samba.org>
2011-01-17 05:23:04 +01:00
Nadezhda Ivanova
bf7b026a9a s4-acl: Replaced talloc_reference with talloc_steal, as aclread is the only one using this result message.
No need to reference as no one further up the stack uses the result, it is the result of a secondary request sent by aclread.

As a result from code review by Kamen Mazdrashki and Anatoliy Atanasov

Autobuild-User: Nadezhda Ivanova <nivanova@samba.org>
Autobuild-Date: Wed Dec  8 15:01:51 CET 2010 on sn-devel-104
2010-12-08 15:01:51 +01:00
Nadezhda Ivanova
2079a6d110 s4-acl: Changed the mechanism of attribute removal to speed it up.
Instead of using ldb_msg_remove_attr, now we are flagging the attributes to be removed,
and allocating the new elements array to be returned at once. This seems to decrease the
overhead by 50 percent.

Autobuild-User: Nadezhda Ivanova <nivanova@samba.org>
Autobuild-Date: Wed Dec  8 12:00:27 CET 2010 on sn-devel-104
2010-12-08 12:00:27 +01:00
Nadezhda Ivanova
ec97c9f7c7 s4-acl: Remove unused variables from aclread module.
Autobuild-User: Nadezhda Ivanova <nivanova@samba.org>
Autobuild-Date: Mon Dec  6 16:48:35 CET 2010 on sn-devel-104
2010-12-06 16:48:34 +01:00
Matthias Dieter Wallnöfer
15a2eff516 s4:acl_read LDB module - fix attributes list
Autobuild-User: Matthias Dieter Wallnöfer <mdw@samba.org>
Autobuild-Date: Mon Dec  6 15:11:44 CET 2010 on sn-devel-104
2010-12-06 15:11:44 +01:00
Nadezhda Ivanova
91bf9133a6 s4-acl: Some optimisation of the aclread module
Modified the aclread module to now insert the attributes needed to perform access checks in the same request,
instead of doind a separate search per entry. Also, instanceType is now used to determine id the object has a parent
instead of parentGUID, which saves one additional search in operational.

Autobuild-User: Nadezhda Ivanova <nivanova@samba.org>
Autobuild-Date: Mon Dec  6 13:50:19 CET 2010 on sn-devel-104
2010-12-06 13:50:19 +01:00