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

34321 Commits

Author SHA1 Message Date
Christof Schmitt
508aebf40a torture: Add testcase for compound CREATE-WRITE-CLOSE request
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13047

Signed-off-by: Christof Schmitt <cs@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
2017-09-22 05:45:21 +02:00
Richard Sharpe
2d97c8a4a5 Make sure smbtorture tests can run if someone has set their min protocol above NT1.
This code is SMB1 only, and already modifies
maxprotocol, so this change is appropriate.

Signed-off-by: Richard Sharpe <richard.sharpe@primarydata.com>
Reviewed-by: Jeremy Allison <jra@samba.org>
2017-09-20 22:48:15 +02:00
Andrew Bartlett
ee4418e73f dsdb: Only trigger a re-index once per @INDEXLIST modification
A modify of both @INDEXLIST and @ATTRIBUTES will still trigger two re-index passes
but that is a task for later.

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

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

Autobuild-User(master): Andrew Bartlett <abartlet@samba.org>
Autobuild-Date(master): Wed Sep 20 12:29:49 CEST 2017 on sn-devel-144
2017-09-20 12:29:49 +02:00
Andrew Bartlett
9e9a8d8f88 s4-dnsserver: Check for too many DNS results
If we had this check in when the wildcard DNS tests were written, we would have
noticed that the name needed to be escaped (see previous commit).

BUG: https://bugzilla.samba.org/show_bug.cgi?id=12994
Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Garming Sam <garming@catalyst.net.nz>
2017-09-20 08:15:21 +02:00
Andrew Bartlett
c174702107 s4-dnsserver: Always encode user-supplied names when looking up DNS records
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12994

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Garming Sam <garming@catalyst.net.nz>
2017-09-20 08:15:21 +02:00
Garming Sam
3e1870c26c kcc: Remove unused, untested KCC code
This code tries to implement the full KCC algorithm, but never
actually worked correctly.

Removing this doesn't affect the full-mesh KCC. This code only
attempted to calculate a graph using the "proper" algorithm, though it
neglected to write its results back into the database. The full-mesh
calculation occurs elsewhere.

Signed-off-by: Garming Sam <garming@catalyst.net.nz>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>

Autobuild-User(master): Douglas Bagnall <dbagnall@samba.org>
Autobuild-Date(master): Wed Sep 20 06:28:07 CEST 2017 on sn-devel-144
2017-09-20 06:28:07 +02:00
Andrew Bartlett
dd53be2756 ldap_server: Plumb ldb error string from a failed connect to ldapsrv_terminate_connection()
However, do not plumb it to the client-seen error string, as it could contain server paths.

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
2017-09-20 02:25:30 +02:00
Andrew Bartlett
c1e41d489d samdb: Rework samdb_connect_url() to return LDB error code and an error string
This allows debugging of why the LDB failed to start up.

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
2017-09-20 02:25:30 +02:00
Günther Deschner
2c745cfac5 s4-torture: move lease break handler outside the lease testsuite.
Guenther

Signed-off-by: Guenther Deschner <gd@samba.org>
Pair-Programmed-With: Jose A. Rivera <jarrpa@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

Autobuild-User(master): Ralph Böhme <slow@samba.org>
Autobuild-Date(master): Tue Sep 19 09:36:40 CEST 2017 on sn-devel-144
2017-09-19 09:36:40 +02:00
Tim Beale
1541c50b37 selftest: Add some tests for linked attribute conflicts
Currently we have tests that check we can resolve object conflicts, but
these don't test anything related to conflicting linked attributes.
This patch adds some basic tests that checks that Samba can resolve
conflicting linked attributes.

This highlights some problems with Samba, as the following tests
currently fail:
- test_conflict_single_valued_link: Samba currently can't resolve a
  conflicting targets for a single-valued linked attribute - the
  replication exits with an error.
- test_link_deletion_conflict: If 2 DCs add the same linked attribute,
  currently when they resolve this conflict the RMD_VERSION for the
  linked attribute incorrectly gets incremented. This means the version
  numbers get out of step and subsequent changes to the linked attribute
  can be dropped/ignored.
- test_full_sync_link_conflict: fails for the same reason as above.

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

Autobuild-User(master): Garming Sam <garming@samba.org>
Autobuild-Date(master): Mon Sep 18 09:56:41 CEST 2017 on sn-devel-144
2017-09-18 09:56:41 +02:00
Tim Beale
46c1f7bdee getncchanges.c: max_links calculation didn't work well in some cases
The max_links calculation didn't work particularly well if max_links was
set to a value lower than max_objects.

As soon as repl_chunk->object_count exceeded repl_chunk->max_links, the
chunk would be deemed full, even if there was only one link to send (or
even worse, no links to send). For example, if max_objects=100 and
max_links=10, then it would send back chunks of 10 objects (or 9 objects
and 1 link).

I believe the historic reason this logic exists is to avoid overfilling
the response message. It's hard to tell what the appropriate limit would
be because the total message size would depend on how many attributes
each object has.

I couldn't think of logic that would be suitable for all cases. I toyed
with the idea of working out a percentage of how full the message is.
However, adjusting the max_links doesn't really make sense when the
settings are small enough, e.g. max_objects=100 and max_links=100 is
never going to overfill the message, so there's no reason to alter the
values.

In the end I went with:
- If the user is using non-default values, just use those.
- In the default value case, just use the historic calculation

Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12972
Reviewed-by: Garming Sam <garming@catalyst.net.nz>
2017-09-18 05:51:25 +02:00
Tim Beale
3c0d80d0c3 replmd: Avoid duplicated debug/warnings
We display warnings if a target object is missing but it's still OK to
continue the replication. Currently we need to check the target twice -
once to verify it when we first receive it, and once when we actually
commit it (we can't skip the 2nd check altogether because in the join
case, they could occur quite far apart).

One annoying side-effect is we get the same warning message coming out
twice in these special cases.

In the cases where we're checking the dsdb_repl_flags, we can actually
just bypass the verification checks for the target object (if it doesn't
exist we still continue anyway). This may save us a tiny bit of
unnecessary work.

For cross-partition links, we can limit logging these warnings to when
the objects are actually being committed. This avoids spurious warnings
in the join case (i.e. we receive the link before we receive the target
object's partition, but we have received all partitions by the time we
actually commit the objects).

Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12972
Reviewed-by: Garming Sam <garming@catalyst.net.nz>
2017-09-18 05:51:25 +02:00
Tim Beale
44ca84166e replmd: Allow missing targets if GET_TGT has already been set
While running the selftests, I noticed a case where DC replication
unexpectedly sends a linked attribute for a deleted object (created in
the drs.ridalloc_exop tests). The problem is due to the
msDS-NC-Replica-Locations attribute, which is a (known) one-way link.
Because it is a one-way link, when the test demotes the DC and deletes
the link target, there is no backlink to delete the link from the source
object.

After much debate and head-scratching, we decided that there wasn't an
ideal way to resolve this problem. Any automated intervention could
potentially do the wrong thing, especially if the link spans partitions.
Running dbcheck will find this problem and is able to fix it (providing
the deleted object is still a tombstone). So the recommendation is to
run dbcheck on your DCs every 6 months (or more frequently if using a
lower tombstone lifetime setting).

However, it does highlight a problem with the current GET_TGT
implementation. If the tombstone object had been expunged and you
upgraded to 4.8, then you would be stuck - replication would fail
because the target object can't be resolved, even with GET_TGT, and
dbcheck would not be able to fix the hanging link. The solution is to
not fail the replication for an unknown target if GET_TGT has already
been set (i.e. the dsdb_repl_flags contains
DSDB_REPL_FLAG_TARGETS_UPTODATE).

It's debatable whether we should add a hanging link in this case or
ignore/drop the link. Some cases to consider:
- If you're talking to a DC that still sends all the links last, you
  could still get object deletion between processing the source object's
  links and sending the target (GET_TGT just restarts the replication
  cycle from scratch). Adding a hanging link in this case would be
  incorrect and would add spurious information to the DB.
- Suppose there's a bug in Samba that incorrectly results in an object
  disappearing. If other DCs then remove any links that pointed to that
  object, it makes recovering from the problem harder. However, simply
  ignoring the link shouldn't result in data loss, i.e. replication won't
  remove the existing link information from other DCs. Data loss in this
  case would only occur if a new DC were brought online, or if it were a
  new link that was affected.
Based on this, I think ignoring the link does the least harm.

This problem also highlights that we should really be using the same
logic in both the unknown target and the deleted target cases.
Combining the logic and moving it into a common
replmd_allow_missing_target() function fixes the problem. (This also has
the side-effect of fixing another logic flaw - in the deleted object
case we would unnecessarily retry with GET_TGT if the target object was
in another partition. This is pointless work, because GET_TGT won't
resolve the target).

Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12972
Reviewed-by: Garming Sam <garming@catalyst.net.nz>
2017-09-18 05:51:25 +02:00
Tim Beale
278039ff78 getncchanges.c: Support GET_TGT better with large numbers of links
A source object can potentially link to thousands of target objects.
We have to be careful not to overfill the GetNCChanges response message
with more data than it's possible to send. We also don't want the client
to timeout while we're busy checking the linked attributes. The GET_TGT
support added so far is fairly dumb - this patch extends it to better
handle larger numbers of links.

To do so, this extends the repl_chunk usage so that it also works out if
the current chunk is full of links. Now as soon as the chunk is full of
either links or objects, we stop and send it back.

These changes now mean that we need to also check:
- that all the links for the last source object in the previous chunk
  have been sent, before we move on and send the next object. This only
  takes effect when immediate_link_sync is configured. It also means
  that a chunk in the middle of the replication cycle can now contain
  only links, and no objects.
- when GET_TGT is used, we only send back the links that we've verified
  the target object for. i.e. if we stop checking links because we timed
  out, we only send back the links whose targets were checked.

Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Garming Sam <garming@catalyst.net.nz>
2017-09-18 05:51:25 +02:00
Garming Sam
7ba10844d1 getncchanges.c: Refactor to track more state using repl_chunk
To prepare GET_TGT to deal with a large number of links better, there
is now a 'repl_chunk' struct to help keep track of all the factors
relating to the current chunk of replication data (i.e. how many
objects/links we can send and how many we've already processed). This
means we can have a consistent way of working out whether the current
chunk is full (whether that be due to objects, links, or just too much
time taken).

This patch should not alter functionality. This is just a refactor to
add the basic framework, which will be used in the next patch.

Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Signed-off-by: Garming Sam <garming@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2017-09-18 05:51:25 +02:00
Tim Beale
10df9f6bfd getncchanges.py: Add a multi-valued linked attribute test
Add a test where a source object links to multiple different targets.
First we do the replication without GET_TGT and check that the server
can handle sending a chunk containing only links (in the middle of the
replication). Then we repeat the replication forcing GET_TGT to be used.

To avoid having to create 1500 objects/links, I've lowered the 'max
link sync' setting on the vampire_dc testenv to 250.

Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Garming Sam <garming@catalyst.net.nz>
2017-09-18 05:51:25 +02:00
Tim Beale
693e3adc1a getncchanges.py: Add a test for dropped cross-partition links
Samba would drop linked attributes that span partitions if it didn't
know about the target object. This patch adds a test that exposes the
problem.

I've re-used the code from the previous re-animation test to do this.
I've also added a very basic DcConnection helper class that basically
stores the connection state information the drs_base.py uses for
replication. This allows us to switch the DC we want to replicate from
easily. This approach could potentially be retro-fitted to some of the
existing test cases, as it allows us to test both the DRS client code
and server code at the same time.

Note this test case relates to the code change for commit
fae5df891c11f642cb.

Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12972
Reviewed-by: Garming Sam <garming@catalyst.net.nz>
2017-09-18 05:51:24 +02:00
Tim Beale
9697190600 getncchanges.py: Add test for replicating reanimated objects
Reading between the lines, this scenario seems to be the main reason
that Microsoft added the GET_TGT flag. MS AD can handle getting links
for unknown targets OK, but if it receives links for a deleted/recycled
target then it would tend to drop the received links. Samba client also
used to drop the links if talking to a Microsoft DC (or a Samba server
with GET_TGT support).

The specific scenario is the client side already knows about a deleted
object. That object is then re-animated and used as the target for a
linked attribute. *Then* the target object gets updated again so it gets
sent in a later replication chunk to the linked attribute, i.e. the
client receives the link before it learns that the target object has
been re-animated.

In this test we're interested in particular at how the client behaves
when it receives a linked attribute for a deleted object. (It *should*
retry with GET_TGT to make sure the target is up-to-date. However, it
was just dropping the linked attribute).

To exercise the client-side, we disable replication, setup the
links/objects on one DC the way we want them, then force a replication
to the second DC. We then check that when we query each DC, they both
tell us about the links/objects we're expecting (i.e. no links got
lost).

Note that this wasn't a problem with older versions of Samba-to-Samba
because sending the links last guaranteed that the target objects were
always up-to-date.

Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Garming Sam <garming@catalyst.net.nz>
2017-09-18 05:51:24 +02:00
Tim Beale
ed2fc52243 drs: Add basic GET_TGT support
This adds basic DRS_GET_TGT support. If the GET_TGT flag is specified
then the server will use the object cache to store the objects it sends
back. If the target object for a linked attribute is not in the cache
(i.e. it has not been sent already), then it is added to the response
message.

Note that large numbers of linked attributes will not be handled well
yet - the server could potentially try to send more than will fit in a
single repsonse message.

Also note that the client can sometimes set the GET_TGT flag even if the
server is still sending the links last. In this case, we know the client
supports GET_TGT so it's safe to send the links interleaved with the
source objects (the alternative of fetching the target objects but not
sending the links until last doesn't really make any sense).

Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Garming Sam <garming@catalyst.net.nz>
2017-09-18 05:51:24 +02:00
Tim Beale
821094d50b getncchanges.py: Add tests for object deletion during replication
Add tests that delete the source and target objects for linked
attributes in the middle of a replication cycle.

Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Garming Sam <garming@catalyst.net.nz>
2017-09-18 05:51:24 +02:00
Tim Beale
469aed088f getnc_exop.py: Extend EXOP_REPL_OBJ test case to use GET_TGT
We already check that when we use GET_ANC that we still only receive a
single object when EXOP_REPL_OBJ is used. This extends the test to also
check that only a single object is returned when GET_TGT is used.

Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Garming Sam <garming@catalyst.net.nz>
2017-09-18 05:51:24 +02:00
Tim Beale
00b20c825c getncchanges.py: Add test for GET_ANC and GET_TGT combined
The code has to handle needing GET_ANC and GET_TGT in combination, i.e.
where we fetch the target object for the linked attribute and the target
object's parent is unknown as well. This patch adds a test case to
exercise this code path.

The second part of this test exercises GET_ANC/GET_TGT for an
incremental replication, where the objects are getting filtered by an
uptodateness-vector/HWM.

Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Garming Sam <garming@catalyst.net.nz>
2017-09-18 05:51:24 +02:00
Tim Beale
6ec9ef2beb getncchanges.py: Add test for adding links during replication
We have identified a case where the Samba server can send linked
attributes but not the target object. In this case, the Samba DRS client
would hit the "Failed to re-resolve GUID" case in replmd and silently
discard the linked attribute.

However, Samba will resend the linked attribute in the next cycle
(because its USN is still higher than the committed HWM), so it should
recover OK. On older releases, this may have caused problems if the
first error resulting in a hanging link (which might mean the second
time it's processed it still fails to be added).

Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Garming Sam <garming@catalyst.net.nz>
2017-09-18 05:51:24 +02:00
Tim Beale
af82bdefcc getncchanges.py: Add some GET_TGT test cases
test_repl_get_tgt:
- Adds 2 sets of objects
- Links one set to the other
- Changes the order so the target object comes last in the
  replication (which means the client has to use GET_TGT)
- Checks that when GET_TGT is used that we have received all target
  objects we need to resolve the linked attibutes
- Checks that we expect to receive the linked attributes *before*
  the last chunk is sent (by default, Samba sends all the links at
  the end, so this fails)
- Checks that we eventually receive all expected objects, and all
  links we receive match what is expected

test_repl_get_tgt_chain:
  This adds the linked attributes in a more complicated chain. We add
  300 objects, but the links for 100 objects will point to a linked
  chain of 200 objects.
  This was mainly to determine whether or not Windows follows the
  target object (i.e. whether it sends all the links for the target
  object as well). It turns out Windows maintains its own linked
  attribute DB, so it sends the links based on USN.

Note that the 2 testenvs fail for different reasons. promoted_dc fails
because it is sending all the linked attributes last. vampire_dc fails
because it doesn't support GET_TGT yet, so it sends the link before the
peer knows about the target object.

Note that to test against vampire_dc (rather than the ad_dc_ntvfs DC),
we need to send the GetNCChanges requests to DC2 instead of DC1.
I've left the DC numbering scheme as is, but I've addeed a test_ldb_dc
handle to drs_base.py - it defaults to DC1, but tests can override it
easily and still have everything work.

While running the new tests through autobuild, I noticed an intermittent
LDAP_ENTRY_ALREADY_EXISTS failure in the test setup(). This appears to
be due to a timing issue in the background replication between the
multiple testenvs. Adding some randomness so that the test base OU is
unique seems to avoid the problem.

Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Garming Sam <garming@catalyst.net.nz>
2017-09-18 05:51:24 +02:00
Tim Beale
172eedc076 getnc_exop.py: Fix GET_TGT behaviour in DRS tests
The existing code never passed the more_flags parameter into the
actual getNCChanges request, i.e. _getnc_req10(). This meant the
existing GET_TGT tests effectively did nothing.

Passing the flag through properly means we have to now change the tests
as the DNs returned by Windows now include any target objects in the
linked attributes. These tests now fail against Samba (because it
doesn't support GET_TGT yet).

Also added comments to the tests to help explain what they are actually
doing.

Note that Samba and Windows can return the objects in different orders,
due to significant differences in their underlying DB implementations
(Windows stores links in a separate DB, so sends links ordered strictly
by USN, whereas Samba sends links based on the USN of the source
object). To make the test a fair comparison between Windows and Samba,
we need to use dn_ordered=False.

Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Garming Sam <garming@catalyst.net.nz>
2017-09-18 05:51:24 +02:00
Gary Lockyer
af38d73b06 s4/smbd: set the process group.
Set the process group in the samba daemon, the --no-process-group option
allows this to be disabled.  The no-process-group option needs to be
disabled in self test.

Signed-off-by: Gary Lockyer <gary@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

Autobuild-User(master): Ralph Böhme <slow@samba.org>
Autobuild-Date(master): Mon Sep 18 04:39:50 CEST 2017 on sn-devel-144
2017-09-18 04:39:50 +02:00
Andreas Schneider
05169a6047 samba_upgradedns: When we setup the internal dns cleanup bind-dns dir
Make sure to remove everything from the bind-dns directory to avoid
possible security issues with the named group having write access to all
AD partions

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

Signed-off-by: Andreas Schneider <asn@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
2017-09-16 00:57:25 +02:00
Andreas Schneider
8cf5c5f0fa samba_upgradedns: Print better hints after we migrated the config
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12957

Signed-off-by: Andreas Schneider <asn@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
2017-09-16 00:57:25 +02:00
Andreas Schneider
aef2b915a2 samba_upgradedns: Change the group of the 'binddns dir' too
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12957

Signed-off-by: Andreas Schneider <asn@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
2017-09-16 00:57:24 +02:00
Andreas Schneider
bf64939d22 s4:bind_dlz: Try the 'binddns dir' first
The directory is normally empty if you did not provision or call
samba_upgradedns for the bind_dlz module.

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

Signed-off-by: Andreas Schneider <asn@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
2017-09-16 00:57:24 +02:00
Tim Beale
e115a42318 getncchanges.c: Send linked attributes in each chunk
Instead of sending all the linked attributes at the end, add a
configurable option to send the links in each replication chunk.

The benefits of this approach are:
- it can reduce memory overhead, as we don't have to keep all the links
in memory over the entire replication cycle.
- the client should never end up knowing about objects but not their
links. (Although we're not sure that this has actually resulted in
replication problems, i.e. missing links).

Note that until we support GET_TGT, this approach can mean we now send
a link where the client doesn't know about the target object, causing
the client to siliently drop that linked attribute. Hence, this option
is switched off by default.

Implementation-wise, this code works fairly the same as before. Instead
of sorting the entire getnc_state->la_sorted array at the end and then
splitting it up over chunks, we now split the links up over chunks and
then sort them when we copy them into the message. This should be OK, as
I believe the MS-DRSR Doc says the links in the message should be sorted
(rather than sorting *all* the links overall). Windows behaviour seems
to chunk the links based on USN and then sort them.

getnc_state->la_idx now tracks which links in getnc_state->la_list[]
have already been sent (instead of tracking getnc_state->la_sorted).
This means the la_sorted array no longer needs to be stored in
getnc_state and we can free the array's memory once we've copied the
links into the message. Unfortunately, the link_given/link_total debug
no longer reports the correct information, so I've moved these into
getncchanges_state struct (and now free the struct a bit later so it's
safe to reference in the debug).

The vampire_dc testenv has been updated to use this new behaviour.

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

Autobuild-User(master): Douglas Bagnall <dbagnall@samba.org>
Autobuild-Date(master): Fri Sep 15 10:07:33 CEST 2017 on sn-devel-144
2017-09-15 10:07:33 +02:00
Tim Beale
c15c53817c getnchanges.c: Avoid unnecessary continue
There's not really much after the continue that we're skipping now. We
can just flip the logic and avoid the continue.

Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
2017-09-15 06:18:13 +02:00
Tim Beale
3a8dfcc6a1 getncchanges.c: Split out code to get an object for a response
Basically, everytime we try to add an object to the response, we want
to:
- Build it (i.e. pack it into an RPC message format)
- Add it to our object-cache if we're keeping one
- Add any ancestors needed for the client to resolve it (if GET_ANC)

GET_TGT is going to use the exact same code, so split this out into a
separate function, rather than duplicating it.

The GET_ANC case also uses almost identical code, but it differs in a
couple of minor aspects. I've left this as is for now, as I'm not sure
if this is by accident or by design.

Because all the memory was talloc'd off the 'obj' variable, we now need
to replace it with a tmp TALLOC_CTX.

Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
2017-09-15 06:18:13 +02:00
Tim Beale
ec8a7fa22e getncchanges.c: Reduce the parameters to get_nc_changes_build_object()
Fifteen parameters seems a bit excessive. Instead, pass it the structs
containing the information it cares about.

Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
2017-09-15 06:18:13 +02:00
Tim Beale
1d70375a10 getncchanges.c: Remove unused ncRoot_dn parameter
Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
2017-09-15 06:18:13 +02:00
Tim Beale
85898a6e2f getncchanges.c: Remove a really old TODO
This TODO was added in 2009 (before Samba supported linked_attributes
in getNCChanges())

Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
2017-09-15 06:18:12 +02:00
Tim Beale
b4b843b710 getncchanges.c: Replace hard-coded numbers with a define
Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
2017-09-15 06:18:12 +02:00
Tim Beale
63818f022f getncchanges.c: Refactor how objects get added to the response
Adding GET_TGT support is going to make things more complicated, and I
think we are going to struggle to do this without refactoring things a
bit.

This patch adds a helper struct to store state related to a single
GetNCChanges chunk. I plan to add to this with things like max_links,
max_objects, etc, which will cutdown on the number of variables/
parameters we pass around.

I found the double-pointer logic where we add objects to the response
confusing - hopefully this refactor simplifies things slightly, and it
allows us to reuse the code for the GET_TGT case.

Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
2017-09-15 06:18:12 +02:00
Tim Beale
f77d55f1be getncchanges.c: Refactor how we add ancestor links
If the current object had already been sent as an ancestor, we were
duplicating the code that added its links and updated the HWM mark.
We want these to occur when we reach the place where the object's USN
naturally occurs.

Instead of duplicating this code, we can just skip the call to
get_nc_changes_build_object() if the object has already been sent.
There is already an existing 'nothing to send'/continue case after we've
updated the highwater mark.

Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
2017-09-15 06:18:12 +02:00
Tim Beale
4057438e3b getncchanges.c: Add ancestor links when the object normally gets sent
Currently we add links each time we send an object, but we don't
actually send these links until the end of the replication cycle.

In subsequent patches we want the links to be sent in the same chunk as
their source object, ideally in as close to USN order as possible.
Processing ancestors complicates this a bit, as the ancestor will have a
higher USN than what we're currently up to, and so potentially will the
ancestor's links.

This patch moves where the ancestor's links get added to the
getnc_state->la_list. The ancestor's links now get added when the object
would normally get sent based purely on its USN (we update the highwater
mark at this point too).

This should not affect functionality, i.e. because we send all the links
at the end, it should make no difference at what point they get added to
the list.

This duplicates a tiny bit of code, but this will be cleaned up in the
next patch.

Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
2017-09-15 06:18:12 +02:00
Tim Beale
2abdd09aa2 getncchanges.c: Split GET_ANC block out into its own function
When we add GET_TGT support, it's going to need to reuse all this code
(i.e. to add any ancestors of the link target). This also trims down
the rather large dcesrv_drsuapi_DsGetNCChanges() function a bit.

Note also fixed a compiler warning in the WERR_DS_DRA_INCONSISTENT_DIT
error block which may have caused issues previously (statement was
terminated by a ',' rather than a ';').

Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
2017-09-15 06:18:12 +02:00
Tim Beale
46b3aab514 getncchanges.c: Split sorting linked attributes into separate function
Longer-term we want to split up the links so that they're sent over
multiple GetNCChanges response messages. So it makes sense to split this
code out into its own function. In the short-term, this removes some of
the complexity from dcesrv_drsuapi_DsGetNCChanges() so that the function
is not quite so big.

Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
2017-09-15 06:18:12 +02:00
Tim Beale
240e433611 getncchanges.c: Rename anc_cache to obj_cache
When we add GET_TGT support we will reuse the ancestor cache and it
should work the same way - if we've already sent an object because it
was needed for resolving a child object or a link target, then there's
no point sending it again.

This just renames anc_cache --> obj_cache.

An extra is_get_anc flag has been added to getnc_state - once GET_TGT
support is added, we can't assume GET_ANC based solely on the existence
of the obj_cache.

Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
2017-09-15 06:18:12 +02:00
Alexander Bokovoy
3f0938b938 Install dcerpc/__init__.py for all Python environments
Also fix whitespace. We use tabs, not spaces in Python/waf code.

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

Signed-off-by: Alexander Bokovoy <ab@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

Autobuild-User(master): Stefan Metzmacher <metze@samba.org>
Autobuild-Date(master): Thu Sep 14 22:29:39 CEST 2017 on sn-devel-144
2017-09-14 22:29:39 +02:00
Andrew Bartlett
24600e8e91 repl_meta_data: Show failing replicated entry in error code
This re-work of our LDIF printing avoids some of the privacy issue from
printing the full LDIF at level 4, while showing the entry that actually fails.

Instead, with e3988f8f74f4a11e8f26a548e0a33d20f4e863f7 we now print the DN
only at level 4, then the full message at 8.

With this patch on failure, we print the redacted failing message at 5.

While all of the DRS replication data is potentially sensitive
the passwords are most sensitive, and are now not printed unencrypted.

This discourages users from sending the full failing trace, as the
last entry is much more likely the issue.

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Garming Sam <garming@catalyst.net.nz>
2017-09-14 18:43:16 +02:00
Andrew Bartlett
5d9bb80a02 schema: Rework dsdb_schema_set_indices_and_attributes() db operations
Commit ec9b1e881c3eef503d6b4b311594113acf7d47d8 did not fully fix this.

There is no value in using dsdb_replace(), we are under the read lock
and replace just confuses things further.

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

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Garming Sam <garming@catalyst.net.nz>
2017-09-14 18:43:16 +02:00
Andrew Bartlett
8d8d31eb2b dsdb: Add missing \n to debug
Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
2017-09-07 06:56:27 +02:00
Andrew Bartlett
51289a6f9b debug: Add new debug class "drs_repl" for DRS replication processing
This is used in the client and in the server

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
2017-09-07 06:56:27 +02:00
Andrew Bartlett
e3988f8f74 repl_meta_data: Re-work printing of replicated entries
This re-work of our LDIF printing avoids some of the privacy issue from
printing the full LDIF at level 4, while showing the entry that actually fails.

Instead, we print the DN only at level 4, then the full message at 8.

While all of the DRS replication data is potentially sensitive
the passwords are most sensitive, and are now not printed unencrypted.

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
2017-09-07 06:56:27 +02:00
Andrew Bartlett
7cfaf70694 linked_attributes: Use ldb_ldif_message_redacted_string() for consistency
This avoids printing un-encrypted secret values in logs, and while links are not likely
secret, this avoids a future copy and paste using ldb_ldif_message_string() again.

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
2017-09-07 06:56:27 +02:00