summaryrefslogtreecommitdiff
path: root/source4/torture/drs
Commit message (Collapse)AuthorAgeFilesLines
* CVE-2018-10918: cracknames: Fix DoS (NULL pointer de-ref) when not ↵Andrew Bartlett2018-08-111-0/+38
| | | | | | | | | | | | | | servicePrincipalName is set on a user This regression was introduced in Samba 4.7 by bug 12842 and in master git commit eb2e77970e41c1cb62c041877565e939c78ff52d. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13552 CVE-2018-10918: Denial of Service Attack on AD DC DRSUAPI server. Signed-off-by: Andrew Bartlett <abartlet@samba.org> Reviewed-by: Gary Lockyer <gary@catalyst.net.nz>
* tests/replica_sync_rodc: Test conflict handling on an RODCGarming Sam2018-02-271-0/+156
| | | | | | | | | | | | | | | | There are two cases we are interested in: 1) RODC receives two identical DNs which conflict 2) RODC receives a rename to a DN which already exists Currently these issues are ignored, but the UDV and HWM are being updated, leading to objects/updates being skipped. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13269 Signed-off-by: Garming Sam <garming@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abartlet@samba.org> (cherry picked from commit 45d19167d52e42bd2f9369dbe37a233902cc81b0)
* tests/drs_base: Allow the net drs replicate to try with a single objectGarming Sam2018-02-271-1/+4
| | | | | | | | | | This eventually passes down the replicate single object exop. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13269 Signed-off-by: Garming Sam <garming@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abartlet@samba.org> (cherry picked from commit ff9e63f976ef76f7f70221d4f6276e221ecd167f)
* tests/replica_sync: Add some additional replication in setUpGarming Sam2018-02-271-0/+2
| | | | | | | | | | This should avoid some failures due to stale objects. Bug: https://bugzilla.samba.org/show_bug.cgi?id=13269 Signed-off-by: Garming Sam <garming@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abartlet@samba.org> (cherry picked from commit 19fcd872ec76afffbc4952266fdfad9a352c4871)
* s4:torture/samba_tool_drs: demote the test dc at the end of ↵Stefan Metzmacher2018-01-131-0/+3
| | | | | | | | | test_samba_tool_replicate_local() Otherwise this taints other tests which might follow. Signed-off-by: Stefan Metzmacher <metze@samba.org> Reviewed-by: Ralph Boehme <slow@samba.org>
* schema: Make writing indices flag an enum for a new stateGarming Sam2017-11-241-1/+1
| | | | | | | | | | In schema_load_init, we find that the writing of indices is not locked in any way. This leads to race conditions. To resolve this, we need to have a new state (SCHEMA_COMPARE) which can report to the caller that we need to open a transaction to write the indices. Signed-off-by: Garming Sam <garming@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abartlet@samba.org>
* smbtorture: Remove an unused variableVolker Lendecke2017-11-141-2/+0
| | | | | | | | Signed-off-by: Volker Lendecke <vl@samba.org> Reviewed-by: Jeremy Allison <jra@samba.org> Autobuild-User(master): Jeremy Allison <jra@samba.org> Autobuild-Date(master): Tue Nov 14 03:55:37 CET 2017 on sn-devel-144
* selftest: Print link meta-data when developer debugging is usedTim Beale2017-10-201-3/+15
| | | | | | | | | | | | | For Windows, DRS is the only way to see the RMD_VERSION of a link, or to tell what inactive links the DC. Add some debug to display this information. By default, this debug is turned off. Signed-off-by: Tim Beale <timbeale@catalyst.net.nz> Reviewed-by: Garming Sam <garming@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abartlet@samba.org> Autobuild-User(master): Andrew Bartlett <abartlet@samba.org> Autobuild-Date(master): Fri Oct 20 08:01:35 CEST 2017 on sn-devel-144
* selftest: Add test for initial link attribute RMD_VERSION valueTim Beale2017-10-201-5/+32
| | | | | | | | | | | | While testing link conflicts I noticed that links on Windows start from a different RMD_VERSION compared to Samba. This adds a simple test to highlight the problem. 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: Handle single-valued conflicts for an existing linkTim Beale2017-10-201-5/+2
| | | | | | | | | | | | | 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>
* replmd: Partial fix for single-valued link conflictTim Beale2017-10-201-5/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
* selftest: Add conflict test where the single-valued link already existsTim Beale2017-10-201-0/+59
| | | | | | | | | | | | As well as testing scenarios where both variants of the link are new, we should also check the case where the received link already exists on the DC as an inactive (i.e. previously deleted) link. 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>
* selftest: Add test for deleted single-valued link conflictTim Beale2017-10-201-0/+104
| | | | | | | | | | | | | | | | | | Currently we're only testing the case where the links have been modified independently on 2 different DCs and both the links are active. We also want to test the case where one link is active and the other is deleted. Technically, this isn't really a conflict - the links involve different target DNs, and the end result is still only one active link. It's still probably worth having these tests to prove that fixing bug 13055 doesn't break 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>
* selftest: Make sure single-link conflict retains the deleted linkTim Beale2017-10-201-2/+47
| | | | | | | | | | | | | | | | | | There should only ever be one active value for a single-valued link attribute. When a conflict occurs the 'losing' value should still be present, but should be marked as deleted. This change is just making the test criteria stricter to make sure that we fix the bug correctly. Note that the only way to query the deleted link attributes present is to send a DRS request. 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>
* selftest: Add sanity-check RODC can't use cache to reveal secretsTim Beale2017-10-141-0/+33
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Bug 12977 highlighted that Samba only checks exop GetNcChanges requests once, when they're first received. This makes sense because valid exop requests should only ever involve a single request. For regular (non-exop) GetNcChanges requests, the server stores a cache of the object GUIDs to return. What we don't want to happen is for a malicious/compromised RODC to use this cache to circumvent privilege checks, and receive secrets that it's normally not permitted to access (e.g. the administrator's password). The specific scenario we're concerned about is: - The RODC sends a regular GetNcChanges request for all objects (without secrets). (This causes the server to build its GUID array cache). - The RODC then sends a follow-on request for the next chunk, but sets the REPL_SECRET exop this time. The only thing inadvertently preventing Samba from leaking secrets in this case is updating msDS-RevealedUsers for auditing. It's possible that a future code change may alter the codepath and open up a security-hole without realizing. This patch adds a test case so if that ever did happen, the selftests would detect the problem. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12977 Signed-off-by: Tim Beale <timbeale@catalyst.net.nz> Reviewed-by: Garming Sam <garming@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abartlet@samba.org>
* selftest: Add test for a re-animated object conflictTim Beale2017-09-261-0/+63
| | | | | | | | | | | | | | | | | Added a test to simulate a user accidentally being deleted and 2 different admins trying to resolve the problem simultaneously - one by re-animating the object and one by just creating a new object with the same name. Currently this test fails on Samba because it chooses the higher version number as the winner instead of the latest change. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13039 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>
* selftest: Windows resolves object conflicts differently to SambaTim Beale2017-09-261-0/+80
| | | | | | | | | | | | | | | | | | | | | | | While testing link conflicts I noticed that Windows resolves conflicts differently to Samba. Samba considers the version number first when resolving the conflict, whereas Windows always takes the latest change. The existing object conflict test cases didn't detect this problem because they were both modifying the object the same number of times (so they had the same version number). I've added new tests that highlight the problem. They are basically the same as the existing rename tests, except that only one DC does the rename. Samba will always pick the renamed object as the winner, whereas Windows picks the most recent change. I've marked this test as a known fail for now. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13039 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>
* selftest: replica_sync did not fully cleanup if test failedTim Beale2017-09-261-11/+15
| | | | | | | | | | | | | | | | | | | Normally the replica_sync tests do the cleanup at the end of the test case, rather than in the tearDown(). However, if the tests don't run to completion (because they fail), then the objects may not get cleaned up properly, which causes the tests to fail on the 2nd test-env. The problem is the object deletion only occurs on DC2 and it relies on replication to propagate the deletion to DC1. Presumably this propagation could be missed because the tests are repeatedly turning off inbound replication on both DCs. This patch changes the tearDown() so it tries to delete the objects off both DCs, which appears to fix the problem. 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>
* selftest: Add some tests for linked attribute conflictsTim Beale2017-09-181-0/+498
| | | | | | | | | | | | | | | | | | | | | | | | | | 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
* replmd: Allow missing targets if GET_TGT has already been setTim Beale2017-09-181-1/+23
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
* getncchanges.py: Add a multi-valued linked attribute testTim Beale2017-09-181-0/+53
| | | | | | | | | | | | | | 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>
* getncchanges.py: Add a test for dropped cross-partition linksTim Beale2017-09-181-30/+146
| | | | | | | | | | | | | | | | | | | | | | 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>
* getncchanges.py: Add test for replicating reanimated objectsTim Beale2017-09-181-10/+114
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
* getncchanges.py: Add tests for object deletion during replicationTim Beale2017-09-181-0/+60
| | | | | | | | | 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_TGTTim Beale2017-09-181-24/+43
| | | | | | | | | | 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 combinedTim Beale2017-09-182-4/+138
| | | | | | | | | | | | | | | 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 replicationTim Beale2017-09-181-0/+47
| | | | | | | | | | | | | | | | | 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 casesTim Beale2017-09-182-29/+212
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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 testsTim Beale2017-09-182-5/+18
| | | | | | | | | | | | | | | | | | | | | | | | | 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>
* selftest: Use a unique(ish) OU for every run of getnc_unprivTim Beale2017-08-291-2/+9
| | | | | | | | | | | | | | | | | | | An intermittent problem I noticed with tests in the past is that the setup can fail to create the base OU because it already exists. I believe this is because the previous testenv DC has replicated out the test object, but not its deletion at the point that the next testenv DC starts running the test. This only seemed to happen very occassionally (I haven't seen it happen with getnc_unpriv yet, but I also haven't run it through the autobuild yet). Using same randomness in the test OU should help avoid this sort of problem, and it matches what some other replication tests do. 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-drsuapi/selftest: Add extra tests for invalid DNsTim Beale2017-08-291-5/+34
| | | | | | | | | | | | Add some test cases to check for requests for invalid/non-existent DNs. This exercises the first return case added in commit: s4-drsuapi: Refuse to replicate an NC is that not actually an NC I've also updated the error code returned here to match Windows. Signed-off-by: Tim Beale <timbeale@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abartlet@samba.org> Reviewed-by: Garming Sam <garming@catalyst.net.nz>
* selftest: Update getnc_unpriv tests to pass against SambaTim Beale2017-08-291-20/+28
| | | | | | | | | | | | | | | | In general Windows seems to return BAD_DN rather than ACCESS_DENIED for an unprivileged user. In the the long-term, it's unrealistic to think that Samba and Windows will agree exactly on every error code returned. So for the tests to be maintainable and pass against Windows and Samba, they need to handle differences in expected errors. To get around this problem, I've changed the expected_error to be a set, so that multiple error codes (one for Microsoft, one for Samba) can be specified for each test case. This approach also highlights the cases where Microsoft and Samba currently differ. Signed-off-by: Andrew Bartlett <abartlet@samba.org> Signed-off-by: Tim Beale <timbeale@catalyst.net.nz> Reviewed-by: Garming Sam <garming@catalyst.net.nz>
* selftest: GetNCChanges can 'accept' a repeated bad requestTim Beale2017-08-291-0/+7
| | | | | | | | | | | | | | | | | | In theory, if we send the exact same rejected request again, we should get the same response back from the DC. However, we don't - the request is accepted if we send it a second time. This patch updates the repl_rodc test to demonstrate the problem (which now causes the test to fail). Note that although the bad GetNCChanges request is not rejected outright, the response that gets sent back is empty - it has no objects in it, so it's not an actual security hole. It is annoying problem for writing self-tests though. Signed-off-by: Tim Beale <timbeale@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abartlet@samba.org> Reviewed-by: Garming Sam <garming@catalyst.net.nz>
* selftest: Extend further getnc_unpriv tests to pass against windows 2012R2Tim Beale2017-08-291-31/+181
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | An important change in this patch is changing the ACE type from A (Allow) to AO (Object Allow) as that will then respect the supplied GUID, which we also make use the constant from the security.idl. This reworks the tests to check replication with users with the following rights: - only GET_CHANGES - only GET_ALL_CHANGES - both GET_CHANGES and GET_ALL_CHANGES - no rights We basically want to test various different GetNCChanges requests against each type of user rights, and the only difference is the error/success value we get back. I've structured the tests this way, so that we have 4 test_repl_xyz_userpriv() functions (to cover each of the above user rights cases), and each test sends the same series of GetNCChanges requests of varying validity. Currently all these tests fail against Samba because Samba sends different error codes to Windows. Signed-off-by: Andrew Bartlett <abartlet@samba.org> Signed-off-by: Tim Beale <timbeale@catalyst.net.nz> Reviewed-by: Garming Sam <garming@catalyst.net.nz>
* selftest: Confirm privileged replication of an OU is not permittedAndrew Bartlett2017-08-291-0/+24
| | | | | | Signed-off-by: Andrew Bartlett <abartlet@samba.org> Signed-off-by: Tim Beale <timbeale@catalyst.net.nz> Reviewed-by: Garming Sam <garming@catalyst.net.nz>
* selftest: Move get_partial_attribute_set() to DrsBaseTestCaseAndrew Bartlett2017-08-292-6/+7
| | | | | | Signed-off-by: Andrew Bartlett <abartlet@samba.org> Reviewed-by: Stefan Metzmacher <metze@samba.org> Reviewed-by: Garming Sam <garming@catalyst.net.nz>
* selftest: encrypt the LDAP connection in drs_base.pyAndrew Bartlett2017-08-291-1/+3
| | | | | | Signed-off-by: Andrew Bartlett <abartlet@samba.org> Reviewed-by: Stefan Metzmacher <metze@samba.org> Reviewed-by: Garming Sam <garming@catalyst.net.nz>
* s4-drsuapi: Refuse to replicate an NC is that not actually an NCAndrew Bartlett2017-08-291-6/+1
| | | | | | | | This prevents replication of an OU, you must replicate a whole NC per Windows 2012R2 Signed-off-by: Andrew Bartlett <abartlet@samba.org> Signed-off-by: Tim Beale <timbeale@catalyst.net.nz> Reviewed-by: Garming Sam <garming@catalyst.net.nz>
* s4-drsuapi: Avoid segfault when replicating as a non-admin with ↵Andrew Bartlett2017-08-291-0/+116
| | | | | | | | | | | | | | | | GUID_DRS_GET_CHANGES Users who are not administrator do not get b_state->sam_ctx_system filled in. We should probably use the 'sam_ctx' variable in all cases (instead of b_state->sam_ctx*), but I'll make this change in a separate patch, so that the bug fix remains independent from other tidy-ups. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12946 Signed-off-by: Andrew Bartlett <abartlet@samba.org> Signed-off-by: Tim Beale <timbeale@catalyst.net.nz> Reviewed-by: Garming Sam <garming@catalyst.net.nz>
* getncchanges.py: Add test for GET_ANC and linked attributesTim Beale2017-08-182-39/+157
| | | | | | | | | | | | | | | | | | | | | Add a basic test that when we use GET_ANC and the parents have linked attributes, then we receive all the expected links and all the expected objects by the end of the test. This extends the test code to track what linked attributes get received and check whether they match what's present on the DC. Also made some minor cleanups to store the received objects/links each time we successfully receive a GETNCChanges response (this saves the test case having to repeat this code every time). Note that although this test involves linked attributes, it shouldn't exercise the GET_TGT case at all. 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
* getncchanges.py: Add GET_ANC replication test caseTim Beale2017-08-182-8/+212
| | | | | | | | | | | | | | | | | | | | | | | | | | This test: - creates blocks of parent/child objects - modifies the parents, so the child gets received first in the replication (which means the client has to use GET_ANC) - checks that we always receive the parent before the child (if not, it either retries with GET_ANC, or asserts if GET_ANC is already set) - modifies the parent objects to change their USN while the replication is in progress - checks that all expected objects are received by the end of the test I've added a repl_get_next() function to help simulate a client's behaviour - if it encounters an object it doesn't know the parent of, then it retries with GET_ANC. Also added some debug to drs_base.py that developers can turn on to make it easier to see what objects we're actually receiving in the responses. 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
* getncchanges.py: Add a new test for replicationTim Beale2017-08-182-19/+198
| | | | | | | | | | | | | | | | | | | | | | This adds a new test to check that if objects are modified during a replication, then those objects don't wind up missing from the replication data. Note that when this scenario occurs, samba returns the objects in a different order to Windows. This test doesn't care what order the replicated objects get returned in, so long as they all have been received by the end of the test. As part of this, I've refactored _check_replication() in drs_base.py so it can be reused in new tests. In these cases, the objects are split up over multiple different chunks. So asserting that the objects are returned in a specific order makes it difficult to run the same test on both Samba and Windows. 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
* getnc_exop.py: Fix typo in function nameTim Beale2017-07-282-8/+8
| | | | | | | | This drove me crazy when I tried to search for it. Signed-off-by: Tim Beale <timbeale@catalyst.net.nz> Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abartlet@samba.org>
* selftest: Use get_creds_ccache_name() in fsmo.pyAndrew Bartlett2017-07-281-3/+2
| | | | | | | This avoids a new kinit for every role transfer Signed-off-by: Andrew Bartlett <abartlet@samba.org> Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
* selftest: Add and use new helper function get_creds_ccache_name()Andrew Bartlett2017-07-281-3/+1
| | | | | Signed-off-by: Andrew Bartlett <abartlet@samba.org> Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
* selftest: Use new --krb5-ccache in drs_base.pyAndrew Bartlett2017-07-281-2/+8
| | | | | | | | This means that instead of doing a new kinit, the process-wide ccache is re-used, which is much faster. Signed-off-by: Andrew Bartlett <abartlet@samba.org> Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
* selftest: Port DrsBaseTestCase._{en,dis}able_all_repl() to self.runsubcmd()Andrew Bartlett2017-07-281-17/+13
| | | | | | | This avoids forking a subprocess with self.check_run() Signed-off-by: Andrew Bartlett <abartlet@samba.org> Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
* selftest: Port DrsBaseTestCase._disable_inbound_repl() to self.runsubcmd()Andrew Bartlett2017-07-281-2/+5
| | | | | | | This avoids forking a subprocess with self.check_run() Signed-off-by: Andrew Bartlett <abartlet@samba.org> Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
* selftest: Port DrsBaseTestCase._enable_inbound_repl() to self.runsubcmd()Andrew Bartlett2017-07-281-2/+5
| | | | | | | This avoids forking a subprocess with self.check_run() Signed-off-by: Andrew Bartlett <abartlet@samba.org> Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
* selftest: Port DrsBaseTestCase._net_drs_replicate() to self.runsubcmd()Andrew Bartlett2017-07-281-7/+19
| | | | | | | This avoids forking a subprocess with self.check_run() Signed-off-by: Andrew Bartlett <abartlet@samba.org> Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
View Lorry Controller Status