IF YOU WOULD LIKE TO GET AN ACCOUNT, please write an
email to Administrator. User accounts are meant only to access repo
and report issues and/or generate pull requests.
This is a purpose-specific Git hosting for
BaseALT
projects. Thank you for your understanding!
Только зарегистрированные пользователи имеют доступ к сервису!
Для получения аккаунта, обратитесь к администратору.
We can't remove the same thing twice in the same message.
Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Because we already have a sorted parsed_dn list, this is a simple
linear scan.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13095
Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
We should not be able to introduce duplicate links using MOD_REPLACE.
It turns out we could and weren't testing.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13095
Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
replmd_build_la_val() is creating a new link attribute. In this case,
the RMD_ORIGINATING_USN and RMD_LOCAL_USN are always going to be the
same thing, so we don't need to pass them in as 2 separate parameters.
This isn't required for any bug fix, but is just a general code
tidy-up.
Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Garming Sam <garming@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
replmd_build_la_val() and replmd_set_la_val() are pretty much identical.
Keep the replmd_build_la_val() API (as it makes it clearer we're
creating a new linked attribute), but replace the code with a call to
replmd_set_la_val().
This isn't required for any bug fix, but is just a general tidy-up to
avoid code duplication.
Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Garming Sam <garming@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
The initial value for RMD_VERSION is one on Windows. The MS-DRSR spec
states the following in section 5.11 AttributeStamp:
dwVersion: A 32-bit integer. Set to 1 when a value for the attribute is
set for the first time. On each subsequent originating update, if the
current value of dwVersion is less than 0xFFFFFFFF, then increment it
by 1; otherwise set it to 0
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13059
Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Garming Sam <garming@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
replmd_build_la_val() is used to populate a new link attribute value
from scratch. The version parameter is always passed in as the initial
value (zero), and deleted is always passed in as false.
For cases (like replication) where we want to set version/deleted to
something other than the defaults, we can use replmd_set_la_val()
instead.
This patch changes these 2 parameters to variables instead.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13059
Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Garming Sam <garming@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Now that the code is all in one place we can refactor it to make it
slightly more readable.
- added more code comments
- tweaked the 'no conflict' return logic to try to make what it's checking
for more obvious
- removed conflict_pdn (we can just use active_pdn instead)
- added a placeholder variable and tweaked a parameter name
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13055
Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Garming Sam <garming@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Return immediately if there's no conflict, which reduces nesting.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13055
Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Garming Sam <garming@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Link conflict handling is a corner-case. The logic in
replmd_process_linked_attribute() is already reasonably busy/complex.
Split out the handling of link conflicts into a separate function so
that it doesn't detract from the core replmd_process_linked_attribute()
logic too much.
This refactor should not alter functionality.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13055
Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Garming Sam <garming@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Currently the code only handles the case where the received link
attribute is a new link (i.e. pdn == NULL). As well as this, we need to
handle the case where the conflicting link already exists, i.e. it's a
deleted link that has been re-added on another DC.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13055
Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Garming Sam <garming@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
The previous patch to handle link conflicts was simply overriding the
received information and marking the link as deleted. We should be doing
this as a separate operation to make it clear what has happened, and so
that the new (i.e. inactive) link details get replicated out.
This patch changes it so that when a conflict occurs, we immediately
overwrite the received information to mark it as deleted, and to update
the version/USN/timestamp/originating_invocation_id to make it clear
that this is a new change.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13055
Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Garming Sam <garming@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
replmd_set_la_val() and replmd_build_la_val() are almost identical. When
we were processing the replicated link attributes we were calling one
function if the link was new, and a different one if the link existed.
I think we should be able to get away with using replmd_set_la_val() in
both cases.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13055
Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Garming Sam <garming@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
All the other talloc_asprintf()s in this function use the mem_ctx, but
for some reason the vstring was using the dsdb_dn->dn. This probably
isn't a big deal, but might have unintentional side-effects.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13055
Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Garming Sam <garming@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
These two functions are almost identical. The main difference between
them is the RMD_ADDTIME. replmd_set_la_val() tries to use the
RMD_ADDTIME of the old_dsdb_dn. Whereas replmd_build_la_val() always
uses the time passed in.
Change replmd_set_la_val() so it can accept a NULL old_dsdb_dn (i.e. if
it's a new linked attribute that's being set). If so, it'll end up using
the nttime parameter passed in, same as replmd_build_la_val() does.
Also update replmd_process_linked_attribute (which used to use
replmd_build_la_val()) to now pass in a NULL old_dsdb_dn. There
shouldn't be a difference in behaviour either way, but this exercises
the code change.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13055
Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Garming Sam <garming@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
If 2 DCs independently set a single-valued linked attribute to differing
values, Samba should be able to resolve this problem when replication
occurs.
If the received information is better, then we want to set the existing
link attribute in our DB as inactive.
If our own information is better, then we still want to add the received
link attribute, but mark it as inactive so that it doesn't clobber our
own link.
This still isn't a complete solution. When we add the received attribute
as inactive, we really should be incrementing the version, updating the
USN, etc. Also this only deals with the case where the received link is
completely new (i.e. a received link conflicting with an existing
inactive link isn't handled).
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13055
Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Garming Sam <garming@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
This is the first part of the fix for resolving a single-valued link
conflict.
When processing the replication data for a linked attribute, if we don't
find a match for the link target value, check if the link is a
single-valued attribute and it currently has an active link. If so, then
use the active link instead.
This change means we delete the existing active link (and backlink)
before adding the new link. This prevents the failure in the subsequent
dsdb_check_single_valued_link() check that was happening previously
(because the link would end up with 2 active values).
This is only a partial fix. It stops replication from failing completely
if we ever hit this situation (which means the test is no longer
hitting an assertion when replicating). However, ideally the existing
active link should be retained and just marked as deleted (with this
change, the existing link is overwritten completely).
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13055
Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Garming Sam <garming@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
The previous refactor makes it obvious that we aren't actually using
this variable for anything.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13055
Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Garming Sam <garming@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
This is precursor work for supporting single-link conflicts.
Split out the code to check if the link update is newer. It's now safe
to call this from the main codepath. This also means we can combine the 2
calls to get the seqnum into a single common call.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13055
Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Garming Sam <garming@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
The code to add the backlink is the same in both the 'if' and the 'else'
case, so move it outside the if-else block.
(We're going to rework this block of code quite a bit in order to
support single-value linked attribute conflicts, aka bug #13055).
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13055
Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Garming Sam <garming@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Refactor the process model code to allow the addition of a prefork
process model.
- Add a process context to contain process model specific state
- Add a service details structure to allow service to indicate which
process model options they can support.
In the new code the services advertise the features they support to the
process model. The process model context is plumbed through to allow the
process model to keep track of the supported options, and any state
the process model may require.
Signed-off-by: Gary Lockyer <gary@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Garming Sam <garming@catalyst.net.nz>
This problem was noticed when 2 DCs added the same linked attribute at
roughly the same time. One DC would have a later timestamp than the
other, so it would re-apply the same link information. However, when it
did this, replmd_update_la_val() would incorrectly increment the
RMD_VERSION for the attribute. We then end up with one DC having a
higher RMD_VERSION than the others (and it doesn't replicate the new
RMD_VERSION out).
During replication RMD_VERSION is used to determine whether a linked
attribute is old (and should be ignored), or whether the information is
new and should be applied to the DB. This RMD_VERSION discrepancy could
potentially cause a subsequent linked attribute update to be ignored.
Normally when a local DB operation is performed, we just pass in a
version of zero and get replmd_update_la_val() to increment what's
already in the DB. However, we *never* want this to happen during
replication - we should always use the version we receive from the peer
DC.
This patch fixes the problem by separating the API into two:
- replmd_update_la_val(): we're updating a linked attribute in the DB,
and so as part of this operation we always want to increment the
version number (the version no longer need to be passed in because
we can work it out from the existing DB entry).
- replmd_set_la_val(): we want to set a linked attribute to use the
exact values we're telling it, including the version. This is what
replication needs to use.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13038
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): Andrew Bartlett <abartlet@samba.org>
Autobuild-Date(master): Tue Sep 26 09:36:48 CEST 2017 on sn-devel-144
Confusing these two concepts is not a good idea, SAMDB_INDEXING_VERSION refers to
a change in a Samba rule to canonicalise one of our attributes, not the
in-DB index format.
As we already change @INDEXLIST in this version, this commit
is at no extra cost.
Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Garming Sam <garming@catalyst.net.nz>
This is optional, but only to aid the downgrade script (and in case
there is some major issue found with it). We don't support that mode,
as that would require us to test and maintain multiple code paths and
not optimise queries to be GUID centric.
Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Garming Sam <garming@catalyst.net.nz>
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
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
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>
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>
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>
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>
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>
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>
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>
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>
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>
This avoids printing un-encrypted secret values in logs
Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
The code makes sure we are backwards compatible. It will first check if
we still have files in the private directory, if yes it will use those.
If the the file is not in the private directory it will try the binddns
dir.
Signed-off-by: Andreas Schneider <asn@samba.org>
Reviewed-by: Andrew Bartlet <abartlet@samba.org>
This drove me to strace before I understood what it really meant.
Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
This looks like a footnote, but is actually where the default password rules are applied.
Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
A is for Allow, OA is for Object Allow, which means check the GUID.
The previous ACE allowed all access, which was not the intention.
Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Garming Sam <garming@catalyst.net.nz>
We do not like fixed passwords
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12946
Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Garming Sam <garming@catalyst.net.nz>
Previously Samba would just drop cross-partition links where the link
target object is unknown. Instead, what we want to do is try to add the
forward link for the GUID specified. We can't add the backlink because
we don't know the target, however, dbcheck should be able to fix any
missing backlinks.
The new behaviour should now mean dbcheck will detect the problem and be
able to fix it. It's still not ideal, but it's better than dropping the
link completely.
I've updated the log so that it has higher severity and tells the user
what they need to do to fix it.
These changes now mean that the selftests now detect an error - instead
of completely dropping the serverReference, we now have a missing
backlink. I've updated the selftests to fix up any missing
serverReference backlinks before running dbcheck.
Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Garming Sam <garming@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12972
We are going to end up supporting 2 different server schemes:
A. the old/default behaviour of sending all the linked attributes last,
at the end of the replication cycle.
B. the new/Microsoft way of sending the linked attributes interleaved
with the source/target objects.
Normally if we're talking to a server using the old scheme-A, we won't
ever use the GET_TGT flag. However, there are a couple of cases where
it can happen:
- A link to a new object was added during the replication cycle.
- An object was deleted while the replication was in progress (and
the linked attribute got queued before the object was deleted).
Talking to an Samba DC running the old scheme will just cause it to
start the replication cycle from scratch again, which is fairly
harmless. However, there is a chance that the same thing can happen
again, in which case the replication cycle will fail (because GET_TGT
was already set).
Even if we're using the new scheme (B), we could still potentially hit
this case, as we can still queue up linked attributes between requests
(group memberships can be larger than what can fit into a single
replication chunk).
If GET_TGT is set in the GetNcChanges request, then the local copy of
the target object should always be up-to-date when we process the linked
attribute. So if we still think the target object is deleted/recycled at
this point, then it's safe to ignore the linked attribute (because we
know our local copy is up-to-date). This logic matches the MS spec logic
in ProcessLinkValue().
Not failing the replication cycle may be beneficial if we're trying to
do a full-sync of a large database. Otherwise it might be time-consuming
and frustrating to repeat the sync unnecessarily.
Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Garming Sam <garming@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12972
The server-side can potentially send the linked attribute before the
target-object. This happens on Microsoft, and will happen on Samba once
server-side GET_TGT support is added. In these cases there is a hole
where the Samba client can silently drop the linked attribute.
If the old copy of the target object was deleted/recycled, then the
client can receive the new linked attribute before it realizes the target
has now been reincarnated. It silently ignores the linked attribute,
thinking its receiving out of date information, when really it's the
client's copy of the target object that's out of date.
In this case we want to retry with the GET_TGT flag set, which will
force the updated version of the target object to be sent along with the
linked attribute. This deleted/recycled target case is the main reason
that Windows added the GET_TGT flag.
If the server sends all the links at the end, instead of along with the
source object, then this case can still be hit. If so, it will cause the
server to restart the replication from the beginning again. This is
probably preferential to silently dropping links.
Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Garming Sam <garming@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12972
There was a bug in my previous patch where the code would verify
*all* links in the list, rather than just the ones that are new. And it
would do this for every replication chunk it received, regardless of
whether there were actually any links in that chunk.
The problem is by the time we want to verify the attributes, we don't
actually know which attributes are new. We can fix this by moving where
we store the linked attributes from the start of processing the
replication chunk to the end of processing the chunk. We can then verify
the new linked attributes at the same time we store them.
Longer-term we may want to try to apply the linked attribute at this
point. This would save looking up the source/target objects twice, but
it makes things a bit more complicated (attributes will usually apply at
this point *most* of the time, but not *all* the time).
Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Garming Sam <garming@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12972
Windows replication can send the linked attribute before it sends the
source object. The MS-DRSR spec says that in this case the client should
resend the GetNCChanges request with the GET_ANC flag set. In my testing
this resolves the problem - Windows will include the source object for the
linked attribute in the same replication chunk.
This problem doesn't happen with Samba-to-Samba replication, because the
source object for the linked attribute is guaranteed to have already been
sent.
Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Garming Sam <garming@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12972
- Update IDL comments to include Microsoft reference doc
- Add support for sending v10 GetNCChanges request (needed for the
GET_TGT flag, which is in the new 'more_flags' field)
- Update to also set the GET_TGT flag in the same place we were setting
GET_ANC (I split this logic out into a separate function).
- The state struct now needs to hold a 'more_flags' field as well (this
flag is different to the GET_ANC replica flag)
Note that using the GET_TGT when replicating from a Windows DC could be
highly inefficient. Because Samba keeps the GET_TGT flag set throughout
the replication cycle, it will basically receive a repeated object from
Windows for every single linked attribute that it receives.
I believe Windows behaviour only expects the client to set the GET_TGT
flag when it actually needs to (i.e. when it receives a target object it
doesn't know about), rather than throughout the replication cycle.
However, this approach won't work with Samba-to-Samba replication,
because when the server receives the GET_TGT flag it restarts the
replication cycle from scratch. So if we only set the GET_TGT flag when
the client encountered an unknown target then Samba-to-Samba could
potentially get into an endless replication loop.
Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Garming Sam <garming@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12972
Currently we only check that the target object is known at the end of
the transaction (i.e. the .prepare_commit hook). It's too late at this
point to resend the request with GET_TGT. Move this processing earlier
on, after we've applied all the objects (i.e. off the .extended hook).
In reality, we need to perform the checks at both points. I've
split the common code that gets the source/target details out of the
la_entry into a helper function. It's not the greatest function ever,
but seemed to make more sense than duplicating the code.
Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Garming Sam <garming@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12972
If the DRS client received a linked attribute that it couldn't resolve
the target for, then it would just ignore that link and keep going. That
link would then be lost forever (although a full-sync would resolve
this). Instead of silently ignoring the link, fail the transaction.
This *can* happen on Samba, but it is unusual. The target object and
linked-attribute would need to be added while a replication is still in
progress. It can also happen fairly easily when talking to a Windows DC.
There are two import exceptions to this:
1). Linked attributes that span partitions. We can never guarantee that
we will have received the target object, because it may be in a partition
we haven't replicated yet. Samba doesn't have a great way of handling
this currently, but we shouldn't fail the replication (because that breaks
basic join tests). Just skip that linked attribute and hope that a
subsequent full-sync will fix it.
(I queried Microsoft and they said resolving cross-partition linked
attributes is a implementation-specific problem to solve. GET_TGT won't
resolve it)
2). When the replication involves a subset of objects, e.g.
critical-only. In these cases, we don't increase the highwater-mark, so
it is probably not such a dire problem if we don't add the link. In the
case of critical-only, we will do a subsequent full sync which will then
add the links.
Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Garming Sam <garming@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12972
