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>

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>

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>

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>

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>

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>

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>

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>

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

winbindd: Remove an obsolete comment

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
Autobuild-User(master): Volker Lendecke <vl@samba.org>
Autobuild-Date(master): Sun Sep 17 23:35:51 CEST 2017 on sn-devel-144

util_runcmd: Free the fde in event handler.

Free the fde in the event handler to prevent the event triggering again
While not strictly necessary in this case, this code serves as an
example of the usage of tfork.

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

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): Sat Sep 16 23:50:27 CEST 2017 on sn-devel-144

lib/util: only close the event_fd in tfork if the caller didn't call tfork_event_fd()

Make closing of the event_fd the global responsibility of the
parent process if it called tfork_event_fd().

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

Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Gary Lockyer <gary@catalyst.net.nz>
Reviewed-by: Stefan Metzmacher <metze@samba.org>

util/tfork: Write to the status pipe

The previous design relied on only calling close() of the status pipe.

We now write a single 0 byte to the status FD as well as closing it in the
parent process.  Both of these operations typically trigger a read
event on the other end of the FD, held in the waiter process (the child).

The child process blocks on the status FD, until it becomes readable.

However if there is a sibling process that was launched after the waiter
process they also will hold the status FD open and the status FD would,
until this change, never become readable to the waiter process (the child).

This caused the waiter process (child) not to exit and the parent process
to hang in tfork_status() while expecting the waitpid() to return.

That is, file descriptors are essentially global variables copied
to children in the process tree.  The last child that (unwittingly) holds
the file descriptor open is the one that needs to trigger the close() this
code previously depended on.

Without this change, there is no notification of process death until
all these unrelated children exit for their own reasons.

We can write up to 4K (PIPE_BUF) into this pipe before blocking,
but we only write one byte.  Additionally sys_write() refuses to block.

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

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

tests util/tfork: Tests for status and event fd

Add tests to ensure that:
- The event_fd becomes readable once the worker process has terminated
- That the event_fd is not closed by the tfork code.
  - If this is done in tevent code and the event fde has not been
    freed, "Bad talloc magic value - " errors can result.
- That the status call does not block if the parent process launches
  more than one child process.
  - The status file descriptor for a child is passed to the
    subsequent children.  These processes hold the FD open, so that
    closing the fd does not make the read end go readable, and the
    process calling status blocks.

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

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

cli_credentials: Apply some const

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Autobuild-User(master): Andrew Bartlett <abartlet@samba.org>
Autobuild-Date(master): Sat Sep 16 12:28:17 CEST 2017 on sn-devel-144

libcli: Apply some const

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

notifyd: Clarify a comment

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

netlogon_creds_cli: Use data_blob_cmp in netlogon_creds_cli_validate

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

netlogon_creds_cli: Simplify netlogon_creds_cli_context_global

(require_sign_or_seal == false) looks odd :-)

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

netlogon_creds_cli: Simplify netlogon_creds_cli_context_common

IMHO a full talloc_stackframe is overkill for the one allocation that is left
here.

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

netlogon_creds_cli: Simplify netlogon_creds_cli_context_common

printf knows to only print part of a string. No need to talloc_strdup.

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

rpc_client3: Fix a debug message

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

netlogon_creds_cli: A netlogon_creds_cli_context needs a msg_ctx

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

netlogon_creds_cli: Remove an obsolete comment

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

netlogon_creds_cli: Avoid a static const struct

Same number of .text bytes, but simpler code.

Yes, this is {{0}} instead of {0}, which I always promote. I've just read a
comment on stackoverflow (which I've unfortunately just closed the tab for :-()
that {{0}} might actually be the correct way to init a struct to zero if the
first struct element is again a struct. I'm lost. 25 years of C coding and I
have no clue of the language :-(

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

cli_netlogon: Eliminate rpccli_setup_netlogon_creds_with_creds

Inlining the code from rpccli_setup_netlogon_creds

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

cli_netlogon: Rename rpccli_create_netlogon_creds_with_creds

This creates a context with access to a credentials, not credentials

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

cli_netlogon: Make rpccli_setup_netlogon_creds static

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

libnet: Use rpccli_setup_netlogon_creds_with_creds in join_unsecure

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

cli_netlogon: Make rpccli_create_netlogon_creds static

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

libnet: Use rpccli_create_netlogon_creds_with_creds in join_unsecure

rpccli_create_netlogon_creds_with_creds just extracts the values we set here
from cli_credentials, and the lower-level interface is supposed to go away.

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

cli_netlogon: Pass server_dns_domain through rpccli_create_netlogon_creds

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

netlogon_creds_cli: Pass "server_dns_domain" through netlogon_creds_cli_context_global

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

netlogon_creds_cli: Add "dns_domain" to netlogon_creds_cli_context

Used later for creating schannel cli_credentials

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

s3: Avoid netlogon_creds_cli.h in includes.h

There's no point recompiling all of source3 if netlogon_creds_cli.h is changed

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

lib: util_tdb.h needs tdb.h

It uses TDB_DATA

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

lib: tevent: Remove select backend.

select() is no longer useful on modern systems.

Signed-off-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Volker Lendecke <vl@samba.org>
Autobuild-User(master): Volker Lendecke <vl@samba.org>
Autobuild-Date(master): Sat Sep 16 08:35:39 CEST 2017 on sn-devel-144

wafsamba: We need to honor DESTDIR in INSTALL_DIR

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>
Autobuild-User(master): Jeremy Allison <jra@samba.org>
Autobuild-Date(master): Sat Sep 16 04:47:29 CEST 2017 on sn-devel-144

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>

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>

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>

python:provision: Do not change the owner of the sam.ldb.d dir

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>

python:provision: 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>

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>

dynconfig: Fix location of the default 'binddns dir'

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>

samba:provision: Give a hint to copy the krb5.conf and not symlink it

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>

wafsamba: Do not chmod already existing dirs on install

This might break backward compatibility.

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>

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

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>

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>

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>

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>

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>

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>

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>

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>

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>

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>

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>

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>

charset: fix str[n]casecmp_m() by comparing lower case values

The commits c615ebed6e3d273a682806b952d543e834e5630d^..f19ab5d334e3fb15761fb009e5de876dfc6ea785
replaced Str[n]CaseCmp() by str[n]casecmp_m().

The logic we had in str[n]casecmp_w() used to compare
the upper cased as well as the lower cased versions of the
characters and returned the difference between the lower cased versions.

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>
Autobuild-User(master): Ralph Böhme <slow@samba.org>
Autobuild-Date(master): Fri Sep 15 02:23:29 CEST 2017 on sn-devel-144

charset/tests: also tests the system str[n]casecmp()

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

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

charset/tests: add more str[n]casecmp_m() tests to demonstrate the bug

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

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

charset/tests: assert the exact values of str[n]casecmp_m()

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

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

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

s4-provision: Ensure the dummy main-domain DB used for DLZ has an @INDEXLIST

The other databases are created from copies of the main provision, but this one
is not, so did not previously get a valid @INDEXLIST.

This is important as otherwise we will not correctly notice support for
the GUID index or new DSDB features in @SAMBA_DSDB as this is gated
on seeing @SAMBA_FEATURES_SUPPORTED in @INDEXLIST.

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

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>

selftest: reindex in dbcheck-oldrelease after modifying the backend DB

Modifying the backend DB is not a supported operation, but helps us create test
situations.

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

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>

selftest: Check re-opening sam.ldb corrects the @ATTRIBUTES and @INDEXLIST

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

ctdb-protocol: Drop marshalling for monitor controls

Signed-off-by: Martin Schwenke <martin@meltin.net>
Reviewed-by: Amitay Isaacs <amitay@gmail.com>
Autobuild-User(master): Amitay Isaacs <amitay@samba.org>
Autobuild-Date(master): Thu Sep 14 18:42:28 CEST 2017 on sn-devel-144

ctdb-client: Drop client code for monitor controls

Signed-off-by: Martin Schwenke <martin@meltin.net>
Reviewed-by: Amitay Isaacs <amitay@gmail.com>

ctdb-client: Drop old client code for monitor controls

Signed-off-by: Martin Schwenke <martin@meltin.net>
Reviewed-by: Amitay Isaacs <amitay@gmail.com>

ctdb-daemon: Remove unused function ctdb_stop_monitoring()

Signed-off-by: Martin Schwenke <martin@meltin.net>
Reviewed-by: Amitay Isaacs <amitay@gmail.com>

ctdb-daemon: Drop monitoring mode

Signed-off-by: Martin Schwenke <martin@meltin.net>
Reviewed-by: Amitay Isaacs <amitay@gmail.com>

ctdb-tests: Drop implementation of monitor controls

Signed-off-by: Martin Schwenke <martin@meltin.net>
Reviewed-by: Amitay Isaacs <amitay@gmail.com>

ctdb-daemon: Drop implementation of monitor controls

Signed-off-by: Martin Schwenke <martin@meltin.net>
Reviewed-by: Amitay Isaacs <amitay@gmail.com>

ctdb-daemon: Mark monitoring controls obsolete

Signed-off-by: Martin Schwenke <martin@meltin.net>
Reviewed-by: Amitay Isaacs <amitay@gmail.com>

ctdb-docs: Drop mention of unimplemented commands

Some of these are only in a comment but git grep finds them.

Signed-off-by: Martin Schwenke <martin@meltin.net>
Reviewed-by: Amitay Isaacs <amitay@gmail.com>

ctdb-tools: Drop monitoring-related ctdb commands

Signed-off-by: Martin Schwenke <martin@meltin.net>
Reviewed-by: Amitay Isaacs <amitay@gmail.com>

ctdb-daemon: Don't explicitly stop monitoring during shutdown

Monitoring is skipped when not in run state RUNNING, so remove the
dependency on the monitoring code.

Signed-off-by: Martin Schwenke <martin@meltin.net>
Reviewed-by: Amitay Isaacs <amitay@gmail.com>

ctdb-daemon: Don't explicitly disable monitoring around recovery

Monitoring can fail during recovery due to databases (e.g. registry)
being unavailable.  This has been avoided by explicitly disabling
monitoring around recovery via the START_RECOVERY and END_RECOVERY
controls.  With this approach only there is still a window between
enabling recovery mode and START_RECOVERY when monitoring could be
attempted.  However, explicitly disabling monitoring is unnecessary
because monitoring is not done when a node is in recovery.

So remove the explicit disable/enable of monitoring and rely on
monitoring being skipped when recovery mode is active.

The only possible change of behaviour with this change is that there
is now a window between setting recovery mode to normal and the
END_RECOVERY control where monitoring is enabled.  However, at this
point databases would be available and the "recovered" event will
cancel any in-progress monitoring.

Signed-off-by: Martin Schwenke <martin@meltin.net>
Reviewed-by: Amitay Isaacs <amitay@gmail.com>

ctdb-daemon: Don't explicitly disable monitoring when stopping a node

Monitoring is now avoided for inactive nodes anyway.

Signed-off-by: Martin Schwenke <martin@meltin.net>
Reviewed-by: Amitay Isaacs <amitay@gmail.com>

ctdb-daemon: Skip monitoring when not in RUNNING runstate

Monitoring does not need to be done in other states.

Signed-off-by: Martin Schwenke <martin@meltin.net>
Reviewed-by: Amitay Isaacs <amitay@gmail.com>

ctdb-daemon: Skip monitoring when node is inactive

This is currently handled by explicitly disabling monitoring in
various places.  However, those places shouldn't need to know about
monitoring but it is OK for monitoring to know about global node
states.

Signed-off-by: Martin Schwenke <martin@meltin.net>
Reviewed-by: Amitay Isaacs <amitay@gmail.com>

ctdb-tests: Drop unused monitoring status support

Signed-off-by: Martin Schwenke <martin@meltin.net>
Reviewed-by: Amitay Isaacs <amitay@gmail.com>

ctdb-client: Initialize ctdb_ltdb_header completely for empty record

ctdb_ltdb_fetch() only fills in relevant portion of ctdb_ltdb_header
if the record does not exist.  This can result in uninitialized writes
to ctdb_rec_buffer.

Signed-off-by: Amitay Isaacs <amitay@gmail.com>
Reviewed-by: Martin Schwenke <martin@meltin.net>

ctdb-daemon: Free up record data if a call request is deferred

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

If a call request for a key (migration request) is in flight, then all
the subsequent call requests for the same key are deferred.  In that case,
the data corresponding to key read from the local tdb is useless and there
is no need to keep it around.  Once the deferred call is reprocessed,
the data corresponding to that key will be fetched again.

Signed-off-by: Amitay Isaacs <amitay@gmail.com>
Reviewed-by: Martin Schwenke <martin@meltin.net>

libcli: SMB2: NetApps negotiate SMB3_11 but also set the SMB2_CAP_ENCRYPTION flag.

This is a SHOULD not, not a MUST not.

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

Signed-off-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Steve French <sfrench@samba.org>
Autobuild-User(master): Jeremy Allison <jra@samba.org>
Autobuild-Date(master): Thu Sep 14 14:48:20 CEST 2017 on sn-devel-144

vfs_streams_xattr: Fix segfault when running with log level 10

This happens when vfs_streams_xattr is loaded, log level is set to 10
and the default stream of a file or directory is accessed. In that case
streams_xattr_open does not allocate the stream_io fsp extension. The
DBG_DEBUG message in streams_xattr_fstat tries to access the stream_io
before checking for a NULL value, resulting in the crash. Fix this by
moving the debug message after the check for a NULL pointer.

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

Signed-off-by: Christof Schmitt <cs@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>
Autobuild-User(master): Ralph Böhme <slow@samba.org>
Autobuild-Date(master): Thu Sep 14 10:58:12 CEST 2017 on sn-devel-144

ctdb-tests: Add 31.clamd eventscript unit tests

These test that ctdb_check_unix_socket() is working.

Signed-off-by: Martin Schwenke <martin@meltin.net>
Reviewed-by: Amitay Isaacs <amitay@gmail.com>
Autobuild-User(master): Martin Schwenke <martins@samba.org>
Autobuild-Date(master): Tue Sep 12 16:14:12 CEST 2017 on sn-devel-144

ctdb-tests: Enhance ss stub to check for listening Unix domain sockets

Generalise command-line parsing, taking hints from old netstat stub,
and use FAKE_NETSTAT_UNIX_LISTEN to specify listening Unix domain
sockets.

Signed-off-by: Martin Schwenke <martin@meltin.net>
Reviewed-by: Amitay Isaacs <amitay@gmail.com>

ctdb-scripts: Switch ctdb_check_unix_socket() to use ss

Signed-off-by: Martin Schwenke <martin@meltin.net>
Reviewed-by: Amitay Isaacs <amitay@gmail.com>

ctdb-scripts: Clean up ctdb_check_unix_socket()

Signed-off-by: Martin Schwenke <martin@meltin.net>
Reviewed-by: Amitay Isaacs <amitay@gmail.com>

ctdb-daemon: Don't release all IPs before "startup" event

This doesn't belong in the monitoring/startup code and it is already
done in the 10.interface "init" event.

Signed-off-by: Martin Schwenke <martin@meltin.net>
Reviewed-by: Amitay Isaacs <amitay@gmail.com>

ctdb-recoverd: Abort recovery/takeover if recmaster changes

Recovery and takeover are run via helper from recovery daemon.  While the
helpers are running, it's possible for the current node to lose election.
If that happens, abort the currently running recovery/takeover helper.

Signed-off-by: Amitay Isaacs <amitay@gmail.com>
Reviewed-by: Martin Schwenke <martin@meltin.net>

ctdb-daemon: GET_DB_SEQNUM should read database conditionally

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

Once the recovery starts and databases are frozen, then all the record
access is postponed till the recovery is complete except reading the
database sequence number.  Database access for reading sequence number
is done via a control which does not check if the databases are frozen
or not.

If the database is frozen and if the freeze transaction is not started
(this can happen when a node is inactive, or during recovery when the
database is frozen but the transaction has not yet started), then trying
to read sequence number will cause ctdb daemon to deadlock.

Before reading the sequence number, check if the database access is
allowed.

Signed-off-by: Amitay Isaacs <amitay@gmail.com>
Reviewed-by: Martin Schwenke <martin@meltin.net>

ctdb-daemon: Add a function to check if db access is allowed

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

Signed-off-by: Amitay Isaacs <amitay@gmail.com>
Reviewed-by: Martin Schwenke <martin@meltin.net>

ctdb-tests: Fix ctdb test binary name in path testing

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

Signed-off-by: Amitay Isaacs <amitay@gmail.com>
Reviewed-by: Martin Schwenke <martin@meltin.net>

ctdb-tests: Wait up to 30 seconds for process to be registered in ctdbd

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

This avoids a potential race where the client is not properly
registered before "ctdb process-exists" is called.

Signed-off-by: Martin Schwenke <martin@meltin.net>
Reviewed-by: Amitay Isaacs <amitay@gmail.com>