1
0
mirror of https://github.com/samba-team/samba.git synced 2024-12-22 13:34:15 +03:00
Commit Graph

137460 Commits

Author SHA1 Message Date
Douglas Bagnall
623adcf4aa s4:dsdb:mod:operational: use NUMERIC_CMP in pso_compare
prec_{1,2} are uint32_t, and if one is not set we are defaulting to
0xffffffff (a.k.a UINT32_MAX), so an overflow when cast to int seems
extremely likely.

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

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2024-04-10 22:56:33 +00:00
Douglas Bagnall
a6d76d6ee9 s4:ntvfs: use NUMERIC_CMP in stream_name_cmp
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2024-04-10 22:56:33 +00:00
Douglas Bagnall
75e51bd99b ldb:ldb_dn: use safe NUMERIC_CMP in ldb_dn_compare()
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2024-04-10 22:56:33 +00:00
Douglas Bagnall
5150b318f4 ldb:ldb_dn: use safe NUMERIC_CMP in ldb_dn_compare_base()
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2024-04-10 22:56:33 +00:00
Douglas Bagnall
de1b94f79e ldb: add NUMERIC_CMP macro to ldb.h
In other places we tend to include tsort.h, which also has TYPESAFE_QSORT.

ldb.h already has TYPESAFE_QSORT, so it might as well have NUMERIC_CMP.

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

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2024-04-10 22:56:33 +00:00
Douglas Bagnall
5ab93f48c5 util:tsort.h: add a macro for safely comparing numbers
In many places we use `return a - b;` in a comparison function. This can
be problematic if the comparison is used in a sort, as `a - b` is not
guaranteed to do what we expect. For example:

* if a and b are 2s-complement ints, a is INT_MIN and b is INT_MAX, then
  a - b = 1, which is wrong.

* if a and b are 64 bit pointers, a - b could wrap around many times in
  a cmp function returning 32 bit ints. (We do this often).

The issue is not just that a sort could go haywire.
Due to a bug in glibc, this could result in out-of-bounds access:

https://www.openwall.com/lists/oss-security/2024/01/30/7

(We have replicated this bug in ldb_qsort).

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

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2024-04-10 22:56:33 +00:00
Douglas Bagnall
8b6a584170 lib/fuzzing/decode_ndr_X_crash: guess the pipe from filename
Usually we are dealing with a filename that tells you what the pipe is,
and there is no reason for this debug helper not to be convenient

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

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2024-04-10 22:56:33 +00:00
Douglas Bagnall
73e4f6026a ldb: avoid out of bounds read and write in ldb_qsort()
If a compare function is non-transitive (for example, if it evaluates
A > B and B > C, but A < C), this implementation of qsort could access
out-of-bounds memory. This was found in glibc's qsort by Qualys, and
their write-up for OSS-Security explains it very well:

 https://www.openwall.com/lists/oss-security/2024/01/30/7

An example of a non-transitive compare is one in which does this

 int cmp(const void *_a, const void *_b)
 {
        int a = *(int *)_a;
        int b = *(int *)_b;
        return a - b;
 }

which does the right thing when the magnitude of the numbers is small,
but which will go wrong if a is INT_MIN and b is INT_MAX. Likewise, if
a and b are e.g. uint32_t, the value can wrap when cast to int.

We have functions that are non-transitive regardless of subtraction.
For example, here (which is not used with ldb_qsort):

 int codepoint_cmpi(codepoint_t c1, codepoint_t c2)
        if (c1 == c2 ||
            toupper_m(c1) == toupper_m(c2)) {
                return 0;
        }
        return c1 - c2;
 }

The toupper_m() is only called on equality case. Consider {'a', 'A', 'B'}.
     'a' == 'A'
     'a' >  'B'  (lowercase letters come after upper)
     'A' <  'B'

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

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2024-04-10 22:56:33 +00:00
Douglas Bagnall
60df2a09a4 selftest: move some more expected failures to expectedfail.d
Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

Autobuild-User(master): Andrew Bartlett <abartlet@samba.org>
Autobuild-Date(master): Wed Apr 10 06:15:46 UTC 2024 on atb-devel-224
2024-04-10 06:15:46 +00:00
Andrew Bartlett
bda4e1233a ldb: Add more segfault tests DN handling
- from_dict DN use-after-free
- check for the same directly creating the ldb.Message

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
2024-04-10 05:13:32 +00:00
Douglas Bagnall
8ac18495ba pyldb: don't allow py_ldb_dn_copy() with the wrong pyldb
Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2024-04-10 05:13:32 +00:00
Douglas Bagnall
d38a9e93cf python:upgrade/upgradeprovision: use dn.copy to align ldbs
We need to do this when the dn is on a message from another ldb.

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2024-04-10 05:13:32 +00:00
Douglas Bagnall
345eb854c3 pyldb: add dn.copy() python method.
Sometimes you want to use a Dn object from one LDB with another LDB,
but this no longer works.

One way to do it is:

  new_dn = ldb.Dn(samdb, str(old_dn))

but with this, you can just:

  new_dn = old_dn.copy(samdb)

or, if you are putting it on a message which has a DN:

  msg.dn = old_dn.copy(msg.ldb)

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2024-04-10 05:13:32 +00:00
Douglas Bagnall
fa9a32139f s4:samba_upgradeprovision: align DN ownership
Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2024-04-10 05:13:32 +00:00
Douglas Bagnall
ed6d151c1b pyldb: add Message.ldb accessor
See the last commit for comments about how this is useful for
debugging.

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2024-04-10 05:13:32 +00:00
Douglas Bagnall
8b6df2d0bc pyldb: add Dn.ldb accessor
This, and the next commit, might help in debugging when you see a
traceback that ends like this:

  File "/data/samba/samba/bin/samba_upgradeprovision", line 664, in add_missing_object
      delta.dn = dn
  RuntimeError: DN is from the wrong LDB

in this case you could force a solution with something like:

 delta.dn = ldb.dn(delta.ldb, str(dn))

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2024-04-10 05:13:32 +00:00
Douglas Bagnall
a95e6aa566 pyldb: add PyErr_internal_LDB_DN_OR_RAISE
This might be faster than the circuitous route.

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2024-04-10 05:13:32 +00:00
Douglas Bagnall
d712c8d2ed pyldb: normalise name of pyldb_Message_Check
c.f. pyldb_MessageElement_Check, pyldb_Dn_Check.

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2024-04-10 05:13:32 +00:00
Douglas Bagnall
72ad126ab7 ldb:pyldb: reorder structs for possible type-punning
Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2024-04-10 05:13:32 +00:00
Douglas Bagnall
c39021a494 pyldb: py_ldb_msg_set_dn checks dn ldb equality
Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2024-04-10 05:13:32 +00:00
Douglas Bagnall
61ba0cc17d pyldb: py_ldb_msg_elements uses PyErr_LDB_MESSAGE_OR_RAISE
Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2024-04-10 05:13:32 +00:00
Douglas Bagnall
9cadc61cd4 pyldb: py_ldb_msg_items checks for more errors
Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2024-04-10 05:13:32 +00:00
Douglas Bagnall
6a2e6139ad pldb: py_ldb_msg_items uses PyErr_LDB_MESSAGE_OR_RAISE
Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2024-04-10 05:13:32 +00:00
Douglas Bagnall
b5fcc55b5e pyldb: py_ldb_msg_contains() checks ldb equality
We can't use PyErr_LDB_MESSAGE_OR_RAISE() here, because the return type
is int, not PyObject*.

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2024-04-10 05:13:32 +00:00
Douglas Bagnall
acba42b126 pyldb: py_ldb_msg_keys() uses PyErr_LDB_MESSAGE_OR_RAISE
We change the [unused, because it always cast] signature of
py_ldb_msg_iter() in the same commit, because that is just a wrapper
around _keys() and this maintains bisectability with the least fuss.

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2024-04-10 05:13:32 +00:00
Douglas Bagnall
d05ae6872b pyldb: py_ldb_msg_richcmp() uses PyErr_LDB_MESSAGE_OR_RAISE()
Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2024-04-10 05:13:32 +00:00
Douglas Bagnall
bc45a258d2 pyldb: use PyErr_LDB_MESSAGE_OR_RAISE() in various functions
In these simple cases, we are:

1. replacing the first argument `PyObject *` with `PyLdbMessageObject *`.
2. adding a `struct ldb_message *msg = NULL;` variable.
3. `PyErr_LDB_MESSAGE_OR_RAISE(self, msg);`.
4. changing the `self->msg` to `msg`.
5. adding { } to the `if (!PyArg_ParseTuple() return NULL;`.
6. replacing `self->pyldb` with `pyldb_Message_get_pyldb(self)`

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2024-04-10 05:13:32 +00:00
Douglas Bagnall
f0e665f4a9 pyldb: add PyErr_LDB_MESSAGE_OR_RAISE() macro
The Python level message has a reference to an LDB, which should be NULL,
or the same as the dn's LDB, lest one of them is freed early.

The message LDB will be NULL until a DN is set, and if the DN is replaced,
the LDB is also be replaced (see py_ldb_msg_set_dn), so it is *unlikely*
for these to get out of sync. In addition, fetching msg.dn via python
compares the LDBs at that point (py_ldb_msg_get_dn).

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2024-04-10 05:13:32 +00:00
Douglas Bagnall
b81b2578ad pyldb: catch up with README.Coding for some PyArg_ParseTuples
Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2024-04-10 05:13:32 +00:00
Douglas Bagnall
13545ed139 pyldb: py_ldb_dn_concat() uses PyErr_LDB_DN_OR_RAISE
Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2024-04-10 05:13:32 +00:00
Douglas Bagnall
1bbca1e3b4 pyldb: py_ldb_dn_len checks dn and ldb validity
Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2024-04-10 05:13:32 +00:00
Douglas Bagnall
f8b92e5281 pyldb: make py_ldb_dn_add_base() a bit less leaky
Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2024-04-10 05:13:32 +00:00
Douglas Bagnall
b83ea997e7 pyldb: py_ldb_dn_add_base() uses PyErr_LDB_DN_OR_RAISE
Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2024-04-10 05:13:32 +00:00
Douglas Bagnall
67a9e573b0 pyldb: make py_ldb_dn_add_child() a bit less leaky
Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2024-04-10 05:13:32 +00:00
Douglas Bagnall
310624ead5 pyldb: py_ldb_dn_add_child() uses PyErr_LDB_DN_OR_RAISE
for self->dn only. The other dn is a different story, next commit.

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2024-04-10 05:13:32 +00:00
Douglas Bagnall
1eeb0e3651 pyldb: py_ldb_dn_get_parent() uses PyErr_LDB_DN_OR_RAISE
Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2024-04-10 05:13:32 +00:00
Douglas Bagnall
8830149ef9 pyldb: py_ldb_dn_richcmp() uses PyErr_LDB_DN_OR_RAISE
The `if (!pyldb_Dn_Check(pydn2))` might seem redundant, but we
need it to return Py_NotImplemented before the _OR_RAISE macro
raises TypeError.

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2024-04-10 05:13:32 +00:00
Douglas Bagnall
982a87cedf pyldb: py_ldb_dn_get_extended_component() uses PyErr_LDB_DN_OR_RAISE
Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2024-04-10 05:13:32 +00:00
Douglas Bagnall
5154c8c996 pyldb: py_ldb_dn_extended_str() uses PyErr_LDB_DN_OR_RAISE()
Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2024-04-10 05:13:32 +00:00
Douglas Bagnall
0ce3f35502 pyldb: py_ldb_dn_get_casefold() uses PyErr_LDB_DN_OR_RAISE()
Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

# Conflicts:
#	selftest/knownfail.d/ldb-use-after-free-segfault
2024-04-10 05:13:32 +00:00
Douglas Bagnall
85ba5d2c8f pyldb: py_ldb_dn_get_extended_component uses PyErr_LDB_DN_OR_RAISE()
Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2024-04-10 05:13:32 +00:00
Douglas Bagnall
087d43ac61 pyldb: adapt some simple dn methods to use LDB_DN_OR_RAISE()
We treat self as PyObject, and only trust its DN once it has been
laundered by PyErr_LDB_DN_OR_RAISE().

There are more of these to come in the next few commits, but these are
the simplest ones (on a textual level -- the others are simple too, but
look different).


Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2024-04-10 05:13:32 +00:00
Douglas Bagnall
f98035a2a3 ldb:pyldb: PyErr_LDB_DN_OR_RAISE makes more rigourous checks
This changes what happens all over the place
(lib/ldb/pyldb.c, source4/dns_server/pydns.c, source4/dsdb/pydsdb.c),
but causes no problems because it just checks what we always assumed.

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2024-04-10 05:13:32 +00:00
Douglas Bagnall
8bb6287c3b pytest:segfault: some more ldb crashes
Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2024-04-10 05:13:32 +00:00
Andrew Bartlett
0bf80c10ca samba-tool domain backup: Use new ldb.disconnect() method to force-close files during backup
Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
2024-04-10 05:13:32 +00:00
Douglas Bagnall
8612b3e38b ldb:pytests: test ldb.connect() works after .disconnect()
Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2024-04-10 05:13:32 +00:00
Andrew Bartlett
fdc3212275 pyldb: Add ldb.disconnect() method to ensure DB handles are closed
This is vital in our backup code, which needs to actually close the
LMDB at the correct point.

The Python ldb object itself is left in more or less the same state as
one that has not connected to a server or database (it is a very
simple wrapper in itself), and can be reconnected using the .connect()
method.

Pair-programmed-with: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
2024-04-10 05:13:32 +00:00
Andrew Bartlett
784ee21616 pyldb: Include a reference to the Ldb in objects that use
This will help avoid use-after-free of the internally cached ldb within
struct ldb_dn by ensuring that it lives as long.

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
2024-04-10 05:13:32 +00:00
Andrew Bartlett
ffbe623963 selftest: Add tests that demonstrate the issues with ldb use after free
Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
2024-04-10 05:13:32 +00:00
Douglas Bagnall
3ffc6c139b pytest:krb5/lockout: associate user DN with the ldb it is used with
LDB is soon going to object strongly to Python DNs that don't come from
the ldb that they are being used with, for memory safety reasons.

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2024-04-10 05:13:32 +00:00