[SCM] Samba Shared Repository - branch master updated
Douglas Bagnall
dbagnall at samba.org
Wed Aug 28 05:40:01 UTC 2024
The branch, master has been updated
via b5e0e831500 dsdb:cracknames: free more on error (CID 240724)
via 3f83d029dea dsdb:util: dsdb_module_dn initialises on failure
via 24b0dad5b50 dsdb:mod:operational: initialise a pointer (CID 1499411)
via e2174dde746 ndr:dnsp: avoid theoretical int overflow (CID 1609418)
via e2a74963fb8 ldb:kv_index: help static analysers to not worry (CID 1615192)
via 7dac035896b s4:drs:test:getncchanges skips some tests with reserved_usn = 0
via 44a478038b6 s4:drs:test:getncchanges: remove timeout failure
via 7a623d8d562 s4:drsuapi:getncchanges: allow 0 reserved_usn reply
via 5ef27019033 s4:drsuapi:getncchanges: use DBG_ERR() macro
via 2e1ccb35239 s4:drsuapi:getncchanges: fix whitespace
via 67c7609ab75 s4:drs:tests: repeat getncchanges test with zero reserved_usn
via 796e92a5300 s4:drs:tests: add hook for changing highwatermark
via 4b4a7c3fd46 s4:drs:test:getncchanges: add a timeout failure
via 0425b0fcbee libcli:auth: Remove unreachable code (CID 1272968)
via 228dd73cae7 util:charset: Remove unreachable code (CID 1272948)
via 09655e13eab librpc: Speed up GUID_buf_string()
via 6b6413da932 lib/util: Speed up slow data-blob-to-hex functions
from 8ab5a032d72 bootstrap: Migrate to Rocky8
https://git.samba.org/?p=samba.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit b5e0e831500469fbf53f70494c3d02ddd494dae1
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Wed Jul 31 13:39:46 2024 +1200
dsdb:cracknames: free more on error (CID 240724)
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Jennifer Sutton <josutton at catalyst.net.nz>
Autobuild-User(master): Douglas Bagnall <dbagnall at samba.org>
Autobuild-Date(master): Wed Aug 28 05:39:36 UTC 2024 on atb-devel-224
commit 3f83d029dea02798612403c52042ec11003e3d92
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Wed Jul 31 13:31:02 2024 +1200
dsdb:util: dsdb_module_dn initialises on failure
I think this may be a root cause of some Coverity false positives.
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Jennifer Sutton <josutton at catalyst.net.nz>
commit 24b0dad5b5014d80b8c62014808e3659d6cad29c
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Wed Jul 31 13:27:40 2024 +1200
dsdb:mod:operational: initialise a pointer (CID 1499411)
A Coverity false positive (we check for error) but it is worth
doing per README.Coding
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Jennifer Sutton <josutton at catalyst.net.nz>
commit e2174dde7465f14026bb55cc000665dde1baa0eb
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Wed Jul 31 10:41:54 2024 +1200
ndr:dnsp: avoid theoretical int overflow (CID 1609418)
Coverity points out that if the string is longer than INT_MAX, the int
will overflow and the cast to uint8_t will discard bits.
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Jennifer Sutton <josutton at catalyst.net.nz>
commit e2a74963fb89f5409c236a0fbe4cd070e1a75a43
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Wed Jul 31 09:20:50 2024 +1200
ldb:kv_index: help static analysers to not worry (CID 1615192)
The point of this realloc is that we are not using this array, but
keeping it around to remain a node the talloc tree. We'd prefer to
reduce it to nothing.
Coverity rightly spotted that it was reallocing an array of `struct
ldb_val` to an array of `struct ldb_val *`, which has a different size
and all. But it doesn't matter in this case, because we will never use
it.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15590
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Jennifer Sutton <josutton at catalyst.net.nz>
commit 7dac035896b368bf3a86acf58260eef39d195d19
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Fri Aug 9 11:48:06 2024 +1200
s4:drs:test:getncchanges skips some tests with reserved_usn = 0
These tests are not affected by the reserved_usn change, so there is
no need to run them twice.
The test_repl_get_tgt_multivalued_links fails with or without
reserved_usn set to zero, but it fails differently in either case.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15701
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Jennifer Sutton <josutton at catalyst.net.nz>
commit 44a478038b6ec78aaec832d9dbde7fa6b2cdd639
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Fri Aug 9 11:29:11 2024 +1200
s4:drs:test:getncchanges: remove timeout failure
We don't need a timeout failure any more, since replication should
always work. Leaving the timeout in might sometimes cause a flapping
test if replication is being slow for some reason.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15701
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Jennifer Sutton <josutton at catalyst.net.nz>
commit 7a623d8d5626b4e6c88ffb85e36f0934d89ed830
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Wed Aug 7 17:25:30 2024 +1200
s4:drsuapi:getncchanges: allow 0 reserved_usn reply
Azure AD will set reserved_usn to zero when we expect it to be
the number we gave them.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15701
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Jennifer Sutton <josutton at catalyst.net.nz>
commit 5ef27019033fd73decd111f9426e7f8982cbb806
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Thu Jun 13 17:23:23 2024 +1200
s4:drsuapi:getncchanges: use DBG_ERR() macro
The next commit will indent this more, so it's a bit squished up.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15701
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Jennifer Sutton <josutton at catalyst.net.nz>
commit 2e1ccb35239fc6fe129c943bb7305bd4612d72d7
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Wed Aug 7 17:05:48 2024 +1200
s4:drsuapi:getncchanges: fix whitespace
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15701
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Jennifer Sutton <josutton at catalyst.net.nz>
commit 67c7609ab755291de27c620120a1c71b557452e4
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Wed Aug 14 13:26:37 2024 +1200
s4:drs:tests: repeat getncchanges test with zero reserved_usn
This emulates the behaviour of Azure AD.
As this is quite slow we will later reduce the test load in this case,
but for now we want to run all the getncchanges tests this way.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15701
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Jennifer Sutton <josutton at catalyst.net.nz>
commit 796e92a530004406dcb3fea33f54833c722480a0
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Fri Aug 9 10:16:29 2024 +1200
s4:drs:tests: add hook for changing highwatermark
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15701
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Jennifer Sutton <josutton at catalyst.net.nz>
commit 4b4a7c3fd465267c43d9586ab79ca8f84c0cad24
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Fri Aug 9 11:20:38 2024 +1200
s4:drs:test:getncchanges: add a timeout failure
In the next commit we are going to add tests in which the client
modifies the highwatermark in a way that resets replication (on Samba
only). After that we'll fix it.
If we leave the test in an eternal loop, the commit history will not
be bisectable, so we are temporarily going to turn long waits into
failures.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15701
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Jennifer Sutton <josutton at catalyst.net.nz>
commit 0425b0fcbee2ee3028ae40a10fa1471da52e2605
Author: Joseph Sutton <josephsutton at catalyst.net.nz>
Date: Fri Oct 6 12:36:13 2023 +1300
libcli:auth: Remove unreachable code (CID 1272968)
For us to reach the statement ‘if (0 < len1)’, ‘len1’ must be equal to
‘len2’, and they must not both be equal to zero. That cannot be the case
if ‘len1’ is equal to zero, and therefore the ‘else’ branch cannot be
reached.
Signed-off-by: Joseph Sutton <josephsutton at catalyst.net.nz>
Reviewed-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
commit 228dd73cae7ee6181c01f0aca87830afa8bf19d5
Author: Joseph Sutton <josephsutton at catalyst.net.nz>
Date: Fri Oct 6 10:54:57 2023 +1300
util:charset: Remove unreachable code (CID 1272948)
Suppose that ‘slen’ is equal to (size_t)-1. A few lines up, we had:
if (lastp != 0) goto slow_path;
Therefore, ‘lastp’ must evaluate to false.
Now suppose that ‘slen’ is not equal to (size_t)-1. In that case, we
would have executed:
if (slen != 0) goto slow_path;
Therefore, ‘slen’ must evaluate to false.
Consequently, this code can be seen to be unreachable.
Signed-off-by: Joseph Sutton <josephsutton at catalyst.net.nz>
Reviewed-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
commit 09655e13eab4ab2fe4c36bf899e5fe5e2c3622a0
Author: Jo Sutton <josutton at catalyst.net.nz>
Date: Wed Mar 1 14:54:14 2023 +1300
librpc: Speed up GUID_buf_string()
This is faster than calling snprintf().
Signed-off-by: Jo Sutton <josutton at catalyst.net.nz>
Reviewed-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
commit 6b6413da9326944478d0418a31d759a2962a67e8
Author: Jo Sutton <josutton at catalyst.net.nz>
Date: Wed Mar 1 14:50:45 2023 +1300
lib/util: Speed up slow data-blob-to-hex functions
This is much faster than calling sprintf() for every byte of data, and
improves the performance of functions outputting binary DNs.
Signed-off-by: Jo Sutton <josutton at catalyst.net.nz>
Reviewed-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
-----------------------------------------------------------------------
Summary of changes:
lib/ldb/ldb_key_value/ldb_kv_index.c | 2 +-
lib/util/charset/convert_string.c | 9 ---
lib/util/data_blob.c | 12 ++--
lib/util/samba_util.h | 52 +++++++++++++++++
libcli/auth/msrpc_parse.c | 21 +++----
librpc/ndr/ndr_dnsp.c | 5 +-
librpc/ndr/uuid.c | 61 +++++++++++++++++---
selftest/knownfail.d/getncchanges | 3 +
source4/dsdb/samdb/cracknames.c | 3 +
source4/dsdb/samdb/ldb_modules/operational.c | 4 +-
source4/dsdb/samdb/ldb_modules/util.c | 1 +
source4/rpc_server/drsuapi/getncchanges.c | 83 +++++++++++++++++++++++-----
source4/torture/drs/python/drs_base.py | 7 +++
source4/torture/drs/python/getncchanges.py | 49 ++++++++++++++++
14 files changed, 256 insertions(+), 56 deletions(-)
Changeset truncated at 500 lines:
diff --git a/lib/ldb/ldb_key_value/ldb_kv_index.c b/lib/ldb/ldb_key_value/ldb_kv_index.c
index ff8c04af015..9014ae7eec3 100644
--- a/lib/ldb/ldb_key_value/ldb_kv_index.c
+++ b/lib/ldb/ldb_key_value/ldb_kv_index.c
@@ -3916,7 +3916,7 @@ static int ldb_kv_sub_transaction_traverse(
* node.
*/
talloc_realloc(index_in_top_level,
- index_in_top_level->dn, struct ldb_val *, 1);
+ index_in_top_level->dn, struct ldb_val, 1);
index_in_top_level->dn
= talloc_steal(index_in_top_level,
index_in_subtransaction->dn);
diff --git a/lib/util/charset/convert_string.c b/lib/util/charset/convert_string.c
index 859b002ecbc..b15273ce2bb 100644
--- a/lib/util/charset/convert_string.c
+++ b/lib/util/charset/convert_string.c
@@ -210,15 +210,6 @@ bool convert_string_error_handle(struct smb_iconv_handle *ic,
}
*converted_size = retval;
-
- if (!dlen) {
- /* Even if we fast path we should note if we ran out of room. */
- if (((slen != (size_t)-1) && slen) ||
- ((slen == (size_t)-1) && lastp)) {
- errno = E2BIG;
- return false;
- }
- }
return true;
slow_path:
diff --git a/lib/util/data_blob.c b/lib/util/data_blob.c
index b5b78bc7a8a..470aa67cc61 100644
--- a/lib/util/data_blob.c
+++ b/lib/util/data_blob.c
@@ -171,8 +171,10 @@ _PUBLIC_ char *data_blob_hex_string_lower(TALLOC_CTX *mem_ctx, const DATA_BLOB *
/* this must be lowercase or w2k8 cannot join a samba domain,
as this routine is used to encode extended DNs and windows
only accepts lowercase hexadecimal numbers */
- for (i = 0; i < blob->length; i++)
- slprintf(&hex_string[i*2], 3, "%02x", blob->data[i]);
+ for (i = 0; i < blob->length; i++) {
+ hex_string[i * 2] = nybble_to_hex_lower(blob->data[i] >> 4);
+ hex_string[i * 2 + 1] = nybble_to_hex_lower(blob->data[i]);
+ }
hex_string[(blob->length*2)] = '\0';
return hex_string;
@@ -188,8 +190,10 @@ _PUBLIC_ char *data_blob_hex_string_upper(TALLOC_CTX *mem_ctx, const DATA_BLOB *
return NULL;
}
- for (i = 0; i < blob->length; i++)
- slprintf(&hex_string[i*2], 3, "%02X", blob->data[i]);
+ for (i = 0; i < blob->length; i++) {
+ hex_string[i * 2] = nybble_to_hex_upper(blob->data[i] >> 4);
+ hex_string[i * 2 + 1] = nybble_to_hex_upper(blob->data[i]);
+ }
hex_string[(blob->length*2)] = '\0';
return hex_string;
diff --git a/lib/util/samba_util.h b/lib/util/samba_util.h
index 81e6b6f9c39..a38cc8a92f2 100644
--- a/lib/util/samba_util.h
+++ b/lib/util/samba_util.h
@@ -650,4 +650,56 @@ struct tevent_context *samba_tevent_context_init(TALLOC_CTX *mem_ctx);
*/
void samba_tevent_set_debug(struct tevent_context *ev, const char *name);
+static inline char nybble_to_hex_lower(uint8_t val)
+{
+ uint8_t nybble = val & 0xf;
+
+ switch (nybble) {
+ case 0x0: case 0x1: case 0x2: case 0x3: case 0x4:
+ case 0x5: case 0x6: case 0x7: case 0x8: case 0x9:
+ return '0' + nybble;
+ case 0xa:
+ return 'a';
+ case 0xb:
+ return 'b';
+ case 0xc:
+ return 'c';
+ case 0xd:
+ return 'd';
+ case 0xe:
+ return 'e';
+ case 0xf:
+ return 'f';
+ }
+
+ /* unreachable */
+ return '\0';
+}
+
+static inline char nybble_to_hex_upper(uint8_t val)
+{
+ uint8_t nybble = val & 0xf;
+
+ switch (nybble) {
+ case 0x0: case 0x1: case 0x2: case 0x3: case 0x4:
+ case 0x5: case 0x6: case 0x7: case 0x8: case 0x9:
+ return '0' + nybble;
+ case 0xa:
+ return 'A';
+ case 0xb:
+ return 'B';
+ case 0xc:
+ return 'C';
+ case 0xd:
+ return 'D';
+ case 0xe:
+ return 'E';
+ case 0xf:
+ return 'F';
+ }
+
+ /* unreachable */
+ return '\0';
+}
+
#endif /* _SAMBA_UTIL_H_ */
diff --git a/libcli/auth/msrpc_parse.c b/libcli/auth/msrpc_parse.c
index 8326261e838..d5661983a31 100644
--- a/libcli/auth/msrpc_parse.c
+++ b/libcli/auth/msrpc_parse.c
@@ -262,6 +262,8 @@ bool msrpc_parse(TALLOC_CTX *mem_ctx,
goto cleanup;
}
} else {
+ size_t pull_len;
+
/* make sure its in the right format - be strict */
if ((len1 != len2) || (ptr + len1 < ptr) || (ptr + len1 < len1) || (ptr + len1 > blob->length)) {
ret = false;
@@ -278,20 +280,11 @@ bool msrpc_parse(TALLOC_CTX *mem_ctx,
goto cleanup;
}
- if (0 < len1) {
- size_t pull_len;
- if (!convert_string_talloc(mem_ctx, CH_UTF16, CH_UNIX,
- blob->data + ptr, len1,
- ps, &pull_len)) {
- ret = false;
- goto cleanup;
- }
- } else {
- *ps = talloc_strdup(mem_ctx, "");
- if (*ps == NULL) {
- ret = false;
- goto cleanup;
- }
+ if (!convert_string_talloc(mem_ctx, CH_UTF16, CH_UNIX,
+ blob->data + ptr, len1,
+ ps, &pull_len)) {
+ ret = false;
+ goto cleanup;
}
}
break;
diff --git a/librpc/ndr/ndr_dnsp.c b/librpc/ndr/ndr_dnsp.c
index b4cad86392d..2f218edade4 100644
--- a/librpc/ndr/ndr_dnsp.c
+++ b/librpc/ndr/ndr_dnsp.c
@@ -171,11 +171,12 @@ _PUBLIC_ enum ndr_err_code ndr_pull_dnsp_string(struct ndr_pull *ndr, ndr_flags_
enum ndr_err_code ndr_push_dnsp_string(struct ndr_push *ndr, ndr_flags_type ndr_flags, const char *string)
{
- int total_len;
+ size_t total_len;
total_len = strlen(string);
if (total_len > 255) {
return ndr_push_error(ndr, NDR_ERR_BUFSIZE,
- "dns_name of length %d larger than 255", total_len);
+ "dns_name of length %zu larger than 255",
+ total_len);
}
NDR_CHECK(ndr_push_uint8(ndr, ndr_flags, (uint8_t)total_len));
NDR_CHECK(ndr_push_bytes(ndr, (const uint8_t *)string, total_len));
diff --git a/librpc/ndr/uuid.c b/librpc/ndr/uuid.c
index 9fcf12bda90..ce094cc9a50 100644
--- a/librpc/ndr/uuid.c
+++ b/librpc/ndr/uuid.c
@@ -205,15 +205,58 @@ _PUBLIC_ char* GUID_buf_string(const struct GUID *guid,
if (!guid) {
return NULL;
}
- snprintf(dst->buf, sizeof(dst->buf),
- "%08"PRIx32"-%04"PRIx16"-%04"PRIx16"-%02"PRIx8"%02"PRIx8"-%02"PRIx8"%02"PRIx8"%02"PRIx8"%02"PRIx8"%02"PRIx8"%02"PRIx8,
- guid->time_low, guid->time_mid,
- guid->time_hi_and_version,
- guid->clock_seq[0],
- guid->clock_seq[1],
- guid->node[0], guid->node[1],
- guid->node[2], guid->node[3],
- guid->node[4], guid->node[5]);
+
+ if (sizeof(dst->buf) < 37) {
+ return NULL;
+ }
+
+ dst->buf[0] = nybble_to_hex_lower(guid->time_low >> 28);
+ dst->buf[1] = nybble_to_hex_lower(guid->time_low >> 24);
+ dst->buf[2] = nybble_to_hex_lower(guid->time_low >> 20);
+ dst->buf[3] = nybble_to_hex_lower(guid->time_low >> 16);
+ dst->buf[4] = nybble_to_hex_lower(guid->time_low >> 12);
+ dst->buf[5] = nybble_to_hex_lower(guid->time_low >> 8);
+ dst->buf[6] = nybble_to_hex_lower(guid->time_low >> 4);
+ dst->buf[7] = nybble_to_hex_lower(guid->time_low);
+
+ dst->buf[8] = '-';
+
+ dst->buf[9] = nybble_to_hex_lower(guid->time_mid >> 12);
+ dst->buf[10] = nybble_to_hex_lower(guid->time_mid >> 8);
+ dst->buf[11] = nybble_to_hex_lower(guid->time_mid >> 4);
+ dst->buf[12] = nybble_to_hex_lower(guid->time_mid);
+
+ dst->buf[13] = '-';
+
+ dst->buf[14] = nybble_to_hex_lower(guid->time_hi_and_version >> 12);
+ dst->buf[15] = nybble_to_hex_lower(guid->time_hi_and_version >> 8);
+ dst->buf[16] = nybble_to_hex_lower(guid->time_hi_and_version >> 4);
+ dst->buf[17] = nybble_to_hex_lower(guid->time_hi_and_version);
+
+ dst->buf[18] = '-';
+
+ dst->buf[19] = nybble_to_hex_lower(guid->clock_seq[0] >> 4);
+ dst->buf[20] = nybble_to_hex_lower(guid->clock_seq[0]);
+ dst->buf[21] = nybble_to_hex_lower(guid->clock_seq[1] >> 4);
+ dst->buf[22] = nybble_to_hex_lower(guid->clock_seq[1]);
+
+ dst->buf[23] = '-';
+
+ dst->buf[24] = nybble_to_hex_lower(guid->node[0] >> 4);
+ dst->buf[25] = nybble_to_hex_lower(guid->node[0]);
+ dst->buf[26] = nybble_to_hex_lower(guid->node[1] >> 4);
+ dst->buf[27] = nybble_to_hex_lower(guid->node[1]);
+ dst->buf[28] = nybble_to_hex_lower(guid->node[2] >> 4);
+ dst->buf[29] = nybble_to_hex_lower(guid->node[2]);
+ dst->buf[30] = nybble_to_hex_lower(guid->node[3] >> 4);
+ dst->buf[31] = nybble_to_hex_lower(guid->node[3]);
+ dst->buf[32] = nybble_to_hex_lower(guid->node[4] >> 4);
+ dst->buf[33] = nybble_to_hex_lower(guid->node[4]);
+ dst->buf[34] = nybble_to_hex_lower(guid->node[5] >> 4);
+ dst->buf[35] = nybble_to_hex_lower(guid->node[5]);
+
+ dst->buf[36] = '\0';
+
return dst->buf;
}
diff --git a/selftest/knownfail.d/getncchanges b/selftest/knownfail.d/getncchanges
index bda9b31a1b1..7288309c32a 100644
--- a/selftest/knownfail.d/getncchanges
+++ b/selftest/knownfail.d/getncchanges
@@ -6,3 +6,6 @@ samba4.drs.getncchanges.python\(promoted_dc\).getncchanges.DrsReplicaSyncIntegri
samba4.drs.getncchanges.python\(promoted_dc\).getncchanges.DrsReplicaSyncIntegrityTestCase.test_repl_get_tgt_multivalued_links\(promoted_dc\)
# Samba chooses to always increment the USN for the NC root at the point where it would otherwise show up.
samba4.drs.getncchanges.python\(.*\).getncchanges.DrsReplicaSyncIntegrityTestCase.test_repl_nc_is_first_nc_change_only\(
+
+# test_repl_get_tgt_multivalued_links also fails with DrsReplicaSyncFakeAzureAdTests on promoted_dc
+samba4.drs.getncchanges.python\(promoted_dc\).getncchanges.DrsReplicaSyncFakeAzureAdTests.test_repl_get_tgt_multivalued_links\(promoted_dc\)
diff --git a/source4/dsdb/samdb/cracknames.c b/source4/dsdb/samdb/cracknames.c
index 46b41fd2cee..91e02f9d94d 100644
--- a/source4/dsdb/samdb/cracknames.c
+++ b/source4/dsdb/samdb/cracknames.c
@@ -307,11 +307,14 @@ static WERROR DsCrackNameUPN(struct ldb_context *sam_ctx, TALLOC_CTX *mem_ctx,
realm = smb_krb5_principal_get_realm(
mem_ctx, smb_krb5_context->krb5_context, principal);
if (realm == NULL) {
+ krb5_free_principal(smb_krb5_context->krb5_context, principal);
return WERR_NOT_ENOUGH_MEMORY;
}
realm_encoded = ldb_binary_encode_string(mem_ctx, realm);
if (realm_encoded == NULL) {
+ TALLOC_FREE(realm);
+ krb5_free_principal(smb_krb5_context->krb5_context, principal);
return WERR_NOT_ENOUGH_MEMORY;
}
diff --git a/source4/dsdb/samdb/ldb_modules/operational.c b/source4/dsdb/samdb/ldb_modules/operational.c
index d9111184178..b86449b3b3b 100644
--- a/source4/dsdb/samdb/ldb_modules/operational.c
+++ b/source4/dsdb/samdb/ldb_modules/operational.c
@@ -557,7 +557,7 @@ static int construct_msds_isrodc_with_server_dn(struct ldb_module *module,
struct ldb_dn *dn,
struct ldb_request *parent)
{
- struct ldb_dn *server_dn;
+ struct ldb_dn *server_dn = NULL;
const char *attr_obj_cat[] = { "objectCategory", NULL };
struct ldb_result *res;
struct ldb_message_element *object_category;
@@ -594,7 +594,7 @@ static int construct_msds_isrodc_with_computer_dn(struct ldb_module *module,
struct ldb_request *parent)
{
int ret;
- struct ldb_dn *server_dn;
+ struct ldb_dn *server_dn = NULL;
ret = dsdb_module_reference_dn(module, msg, msg->dn, "serverReferenceBL",
&server_dn, parent);
diff --git a/source4/dsdb/samdb/ldb_modules/util.c b/source4/dsdb/samdb/ldb_modules/util.c
index b343828d508..df70c8ed886 100644
--- a/source4/dsdb/samdb/ldb_modules/util.c
+++ b/source4/dsdb/samdb/ldb_modules/util.c
@@ -887,6 +887,7 @@ int dsdb_module_reference_dn(struct ldb_module *module, TALLOC_CTX *mem_ctx, str
ret = dsdb_module_search_dn(module, mem_ctx, &res, base, attrs,
DSDB_FLAG_NEXT_MODULE | DSDB_SEARCH_SHOW_EXTENDED_DN, parent);
if (ret != LDB_SUCCESS) {
+ *dn = NULL;
return ret;
}
diff --git a/source4/rpc_server/drsuapi/getncchanges.c b/source4/rpc_server/drsuapi/getncchanges.c
index 0ae6aa510cf..a73efe93eb9 100644
--- a/source4/rpc_server/drsuapi/getncchanges.c
+++ b/source4/rpc_server/drsuapi/getncchanges.c
@@ -1,4 +1,4 @@
-/*
+/*
Unix SMB/CIFS implementation.
implement the DSGetNCChanges call
@@ -11,12 +11,12 @@
it under the terms of the GNU General Public License as published by
the Free Software Foundation; either version 3 of the License, or
(at your option) any later version.
-
+
This program is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU General Public License for more details.
-
+
You should have received a copy of the GNU General Public License
along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
@@ -443,7 +443,7 @@ static WERROR get_nc_changes_filter_attrs(struct drsuapi_DsReplicaObjectListItem
return WERR_OK;
}
-/*
+/*
drsuapi_DsGetNCChanges for one object
*/
static WERROR get_nc_changes_build_object(struct drsuapi_DsReplicaObjectListItemEx *obj,
@@ -698,7 +698,7 @@ static WERROR get_nc_changes_build_object(struct drsuapi_DsReplicaObjectListItem
}
/* some attributes needs to be encrypted
before being sent */
- werr = drsuapi_encrypt_attribute(obj, session_key, rid,
+ werr = drsuapi_encrypt_attribute(obj, session_key, rid,
&obj->object.attribute_ctr.attributes[i]);
if (!W_ERROR_IS_OK(werr)) {
DEBUG(0,("Unable to encrypt %s on %s in DRS object - %s\n",
@@ -2760,7 +2760,7 @@ WERROR dcesrv_drsuapi_DsGetNCChanges(struct dcesrv_call_state *dce_call, TALLOC_
r->out.ctr->ctr6.source_dsa_invocation_id = *(samdb_ntds_invocation_id(sam_ctx));
r->out.ctr->ctr6.first_object = NULL;
- /* Check request revision.
+ /* Check request revision.
*/
switch (r->in.level) {
case 8:
@@ -3029,13 +3029,67 @@ allowed:
ret = drsuapi_DsReplicaHighWaterMark_cmp(&getnc_state->last_hwm,
&req10->highwatermark);
if (ret != 0) {
- DEBUG(0,(__location__ ": DsGetNCChanges 2nd replication "
- "on DN %s %s highwatermark (last_dn %s)\n",
- ldb_dn_get_linearized(getnc_state->ncRoot_dn),
- (ret > 0) ? "older" : "newer",
- ldb_dn_get_linearized(getnc_state->last_dn)));
- TALLOC_FREE(getnc_state);
- b_state->getncchanges_full_repl_state = NULL;
+ if (req10->highwatermark.reserved_usn == 0) {
+ /*
+ * Entra ID Connect / Azure AD is known to set
+ * reserved_usn to zero in replies, when we
+ * were expecting it to be returned unchanged
+ * (it is supposed to be an opaque cookie).
+ *
+ * If the only difference is in the
+ * reserved_usn, and it is 0 on the return, we
+ * assume it is Azure AD and treat the
+ * highwatermarks as equal.
+ */
+ req10->highwatermark.reserved_usn =
+ getnc_state->last_hwm.reserved_usn;
+
+ ret = drsuapi_DsReplicaHighWaterMark_cmp(
+ &getnc_state->last_hwm,
+ &req10->highwatermark);
+
+ /* put things as they were */
+ req10->highwatermark.reserved_usn = 0;
+
+ if (ret != 0) {
+ /* we will start again */
+ DBG_ERR("DsGetNCChanges 2nd replication "
+ "on DN %s %s highwatermark "
+ "(last_dn %s) after Azure AD "
+ "reserved_usn adjustment\n",
+ ldb_dn_get_linearized(
+ getnc_state->ncRoot_dn),
+ (ret > 0) ? "older" : "newer",
+ ldb_dn_get_linearized(
+ getnc_state->last_dn));
+ TALLOC_FREE(getnc_state);
+ b_state->getncchanges_full_repl_state =
+ NULL;
+ } else {
+ /* log and continue as normal */
+ DBG_NOTICE(
+ "DsGetNCChanges 2nd replication "
+ "on DN %s: highwatermark "
+ "matches only after Azure AD "
+ "reserved_usn adjustment "
+ "(last_dn %s)\n",
+ ldb_dn_get_linearized(
+ getnc_state->ncRoot_dn),
+ ldb_dn_get_linearized(
+ getnc_state->last_dn));
+ }
+ } else {
+ DBG_ERR("DsGetNCChanges 2nd replication "
+ "on DN %s %s highwatermark "
+ "(last_dn %s)\n",
+ ldb_dn_get_linearized(
+ getnc_state->ncRoot_dn),
+ (ret > 0) ? "older" : "newer",
+ ldb_dn_get_linearized(
+ getnc_state->last_dn));
+ TALLOC_FREE(getnc_state);
+ b_state->getncchanges_full_repl_state = NULL;
+ }
}
}
@@ -3201,7 +3255,7 @@ allowed:
return WERR_DS_DRA_INTERNAL_ERROR;
}
- /*
+ /*
TODO: MS-DRSR section 4.1.10.1.1
Work out if this is the start of a new cycle */
@@ -3854,4 +3908,3 @@ allowed:
return WERR_OK;
}
-
diff --git a/source4/torture/drs/python/drs_base.py b/source4/torture/drs/python/drs_base.py
index bf98e599784..10aa17fb0ec 100644
--- a/source4/torture/drs/python/drs_base.py
+++ b/source4/torture/drs/python/drs_base.py
@@ -311,6 +311,13 @@ class DrsBaseTestCase(SambaToolCmdTest):
if drs_handle is None:
drs_handle = self.drs_handle
+ # Add a modify_highwatermark method to cause damage to the
+ # highwatermark. For example, it could set reserved_usn to
+ # zero as Entra ID does.
+ modify_hwm = getattr(self, "modify_highwatermark", None)
+ if modify_hwm is not None:
+ modify_hwm(highwatermark)
+
req10 = self._getnc_req10(dest_dsa=dest_dsa,
invocation_id=invocation_id,
nc_dn_str=nc_dn_str,
diff --git a/source4/torture/drs/python/getncchanges.py b/source4/torture/drs/python/getncchanges.py
index 6b5456a66a8..bef5fc9b27c 100644
--- a/source4/torture/drs/python/getncchanges.py
+++ b/source4/torture/drs/python/getncchanges.py
@@ -1416,6 +1416,55 @@ class DrsReplicaSyncIntegrityTestCase(drs_base.DrsBaseTestCase):
# The NC should not be present in this replication
self._test_repl_nc_is_first(start_at_zero=False, nc_change=False, ou_change=False)
+
+class DrsReplicaSyncFakeAzureAdTests(DrsReplicaSyncIntegrityTestCase):
+ """This repeats all of DrsReplicaSyncIntegrityTestCase, but the client
+ always sets highwatermark.reserved_usn = 0. This is what Azure AD
+ / Entra ID Connect does.
+ """
+ @staticmethod
+ def modify_highwatermark(hwm):
+ if hwm is not None:
+ hwm.reserved_usn = 0
+
+ SKIPPED_TESTS = {
+ "test_repl_get_tgt",
+ "test_repl_get_tgt_and_anc",
+ "test_repl_get_tgt_chain",
--
Samba Shared Repository
More information about the samba-cvs
mailing list