Bug 39806 - DDNS PTR update fails if IP is reused by other client
DDNS PTR update fails if IP is reused by other client
Status: CLOSED FIXED
Product: UCS
Classification: Unclassified
Component: Samba4
UCS 4.2
Other Linux
: P5 normal with 1 vote (vote)
: UCS 4.2-0-errata
Assigned To: Lukas Oyen
Arvid Requate
:
: 34910 (view as bug list)
Depends on:
Blocks: 49593 34910
  Show dependency treegraph
 
Reported: 2015-11-05 19:57 CET by Arvid Requate
Modified: 2021-04-21 12:30 CEST (History)
5 users (show)

See Also:
What kind of report is it?: Bug Report
What type of bug is this?: 5: Major Usability: Impairs usability in key scenarios
Who will be affected by this bug?: 3: Will affect average number of installed domains
How will those affected feel about the bug?: 2: A Pain – users won’t like this once they notice it
User Pain: 0.171
Enterprise Customer affected?: Yes
School Customer affected?: Yes
ISV affected?:
Waiting Support:
Flags outvoted (downgraded) after PO Review:
Ticket number: 2015100821000533, 2017022021000745, 2017052321000298
Bug group (optional):
Max CVSS v3 score:
oyen: Patch_Available+


Attachments
DNS_TYPE_PTR record (42.66 KB, text/plain)
2015-11-05 19:57 CET, Arvid Requate
Details
DNS_TYPE_TOMBSTONE record (42.66 KB, text/plain)
2015-11-05 19:58 CET, Arvid Requate
Details
Delete tombstoned objects if mismatched access rights. (2.94 KB, patch)
2017-04-12 13:58 CEST, Lukas Oyen
Details | Diff
Escalate priviledges if maschine name matches. (4.00 KB, patch)
2017-04-12 13:58 CEST, Lukas Oyen
Details | Diff
Escalate priviledges if maschine name matches. (4.01 KB, patch)
2017-05-09 15:56 CEST, Lukas Oyen
Details | Diff
this script adds the SID of a client in the nTsecuritydescriptor (1.14 KB, application/x-shellscript)
2017-05-12 16:03 CEST, Christina Scheinig
Details
8829: Escalate priviledges if maschine name matches. (4.31 KB, patch)
2017-06-14 18:57 CEST, Lukas Oyen
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Arvid Requate univentionstaff 2015-11-05 19:57:00 CET
We had a report of a Samba/AD-domain where PTR records registered by Windows clients where not updated properly (Ticket# 2015100821000533). In that case the issues showed up at an increased rate, because apparently the DHCP server used by the clients was configured to issue dynamic leases.

During the UCS 4.1 product tests I just saw that Windows clients are actually behave like good citizens by removing the registred PTR records from DNS in case they change the IP. This works when manually changing a static client IP as well as when the client obtains a new IP from DHCP during boot.

But: On the Samba/AD server side, the LDAP objects are still there, only the dnsRecord attribute changed from DNS_TYPE_PTR to DNS_TYPE_TOMBSTONE. That way, the bind9 nameserver doesn't find them any longer as PTR records. Fine. Problem is that the obejcts ntsecuritydescriptor still says that the object belongs to the client that initially registered it. I'll attach the two LDIFs before and after DDNS removal.

When a different Windows client wants to register a PTR record for that address later, the DDNS update is denied by the bind9 server:
==============================================================================
Nov  5 19:22:42 master100 named[2492]: samba_dlz: starting transaction on zone 8.200.10.in-addr.
arpa
Nov  5 19:22:42 master100 named[2492]: client 10.200.8.238#51169: update '8.200.10.in-addr.arpa/
IN' denied
Nov  5 19:22:42 master100 named[2492]: samba_dlz: cancelling transaction on zone 8.200.10.in-add
r.arpa
Nov  5 19:22:42 master100 named[2492]: samba_dlz: starting transaction on zone 8.200.10.in-addr.
arpa
Nov  5 19:22:42 master100 named[2492]: samba_dlz: disallowing update of signer=WIN7PRO2\$\@AR41S
4PT1.QA name=238.8.200.10.in-addr.arpa type=PTR error=insufficient access rights
Nov  5 19:22:42 master100 named[2492]: client 10.200.8.238#58965: updating zone '8.200.10.in-add
r.arpa/NONE': update failed: rejected by secure update (REFUSED)
Nov  5 19:22:42 master100 named[2492]: samba_dlz: cancelling transaction on zone 8.200.10.in-addr.arpa
==============================================================================

Maybe it would be possible to adjust the dlz_bind9 plugin to detect this case, remove the tombstone (with local system level access via LDAPI) and then try the update again, all directly during that DDNS update call.

A mechanism like that could also help us resolve systematic problems like the one in Bug 33637 and in Bug 33638. Some good security checks would be required.
Comment 1 Arvid Requate univentionstaff 2015-11-05 19:57:43 CET
Created attachment 7258 [details]
DNS_TYPE_PTR record
Comment 2 Arvid Requate univentionstaff 2015-11-05 19:58:11 CET
Created attachment 7259 [details]
DNS_TYPE_TOMBSTONE record
Comment 3 Arvid Requate univentionstaff 2015-11-05 20:07:52 CET
Note: Things would be a lot easier (especially for Bug 33637 and Bug 33638) when both, the S4-Connector and the bind9 dlz module would perform their operations as S-1-5-18 ("local system", and that's actually close to the truth).

This SID is e.g. present in the ntSecurityDescriptor of the DNS record objects, with access_mask 0x000f01ff (full control).
Comment 4 Arvid Requate univentionstaff 2017-03-20 20:18:05 CET
Ok, apart from the general remark of Comment 3, we should check how Sambas own DNS implementation handles this. Grepping for DNS_TYPE_TOMBSTONE finds:

source4/rpc_server/dnsserver/dnsdata.c
source4/dns_server/dns_update.c
source4/dns_server/dnsserver_common.c


Here is a bit of rationale behind this record type:
https://bugzilla.samba.org/show_bug.cgi?id=10749
Comment 5 Stefan Gohmann univentionstaff 2017-03-24 14:10:42 CET
Re-tagging as bug because it seems to be a problem in several customer environments.
Comment 6 Lukas Oyen univentionstaff 2017-04-12 13:58:08 CEST
Created attachment 8772 [details]
Delete tombstoned objects if mismatched access rights.
Comment 7 Lukas Oyen univentionstaff 2017-04-12 13:58:35 CEST
Created attachment 8773 [details]
Escalate priviledges if maschine name matches.
Comment 8 Lukas Oyen univentionstaff 2017-04-12 13:59:04 CEST
Attached are patches for samba in two parts. The first one handles the update of
tombstoned objects. The second modifies the Samba DLZ modul to allow updates to
forward-zones if the requesting maschine has the same name, but insufficient
access rights (due to prior modifications by the S4-Connector).

Some background for the first case: [1] describes the behaviour of the MS
implementation. This can't be found in any of the official documentation
([MS-DNSP] or [MS-ADTS]). We have two cases: a) "Updates while the record is
`dnsTombstoned`", b) "Updates while the record is dnsTombstoned, but the SID of
the maschine has changed.".

The common Samba DNS code (used by the DLZ modul and the build-in DNS server),
consists of primarily two functions `dns_common_lookup()` and
`dns_common_replace()`. Both handle tombstoned objects, `dns_common_replace()`
resurrects them if necessary. Case a) is therefore handled.

Case b) is neither handled by the build-in DNS server, nor by the DLZ modul. The
first attached patch implements deleting the tombstoned object if an updates
with insufficient access rights is requested.

The second patch implements a Univention specific case: If a maschine tries to
access a forward-zone without the proper access-rights, but the FQDN as computed
from the DN and the actual FQDN of the requesting maschine match, a modification
is allowed and the privileges for this operation are escalated to system-rights.

[1]: https://blogs.technet.microsoft.com/isrpfeplat/2010/09/23/dns-scavenging-internals-or-what-is-the-dnstombstoned-attribute-for-ad-integrated-zones/
Comment 9 Arvid Requate univentionstaff 2017-04-12 14:03:53 CEST
Bug 41190 might be Case a. We should also test the status of Bug 41190 once this one is fixed.
Comment 10 Lukas Oyen univentionstaff 2017-05-09 15:56:08 CEST
Created attachment 8829 [details]
Escalate priviledges if maschine name matches.

Updated 2nd patch: Fix a typo.
Comment 11 Christina Scheinig univentionstaff 2017-05-12 16:03:52 CEST
Created attachment 8846 [details]
this script adds the SID of a client in the nTsecuritydescriptor

IMHO the customer has the same problem for some clients in his environment. The script we provided (fix_ownership_of_dns_hostrecord.sh) does not fit in here.
More information in Ticket# 2017051221000102
Comment 12 Arvid Requate univentionstaff 2017-06-12 21:34:42 CEST
Ok, the first patch works:

> attachment 8772 [details]
> Delete tombstoned objects if mismatched access rights.

Maybe we could even simplify it, because https://blogs.technet.microsoft.com/isrpfeplat/2010/09/23/dns-scavenging-internals-or-what-is-the-dnstombstoned-attribute-for-ad-integrated-zones/ says:
==============================================================================
Note: The default behavior for Windows 2008 R2 was modified and will be acting as if the SID of the machine has changed regardless of whether it’s true or not. Meaning when a record update is sent to a DNS server hosted on a Windows 2008 R2 Domain Controller and the record’s dnsTombstone=True the record is deleted regardless of the permissions issue described earlier.
==============================================================================

So, I guess we could simply adjust the first patch, from

  if (ldb_ret == LDB_ERR_INSUFFICIENT_ACCESS_RIGHTS && is_tombstoned) {
         ldb_ret = ldb_delete(state->samdb, dn);

to

  if (is_tombstoned) {
         ldb_ret = ldb_delete(state->samdb, dn);

But that's a minor thing. If we want to upstream the patch, we should simply let them decide, both implementations would work for us.
Comment 13 Arvid Requate univentionstaff 2017-06-12 21:38:31 CEST
The second patch on the other hand needs some adjustment:

> attachment 8829 [details]
> Escalate priviledges if maschine name matches.

It checks if (name == fqdn) but "name" is is not "the actual FQDN of the requesting machine": I wrote a script to nsupdate a tombstoned machine DNS record simply as a normal user, and it worked even though the username differed from the machine FQDN. In my test "name" was the fqdn of the DNS record that the nsupdate request was referring to (visible in /var/log/syslog).


I think we would have to ldb_search for the sAMAccountName/userPrincipalName of the authenticated identity and check if the returned object has a dNSHostName attribute that matches the "name" requested in the update. That's a bit more tricky, because the Kerbers Principal of the authenticated updater can differ from userPrincipalName attributes found in the Active Directory data (e.g. MACHINE$@REALM.EXAMPLE). We'll have to scan the Samba sources to see how this can be done.

Maybe we should split this patch off to a separate bug, because it is independent of the tombstone issue and we wont be able to upstream it.
Comment 14 Lukas Oyen univentionstaff 2017-06-13 14:17:14 CEST
(In reply to Arvid Requate from comment #12)
> Ok, the first patch works:
> [...]
> So, I guess we could simply adjust the first patch, from
> 
>   if (ldb_ret == LDB_ERR_INSUFFICIENT_ACCESS_RIGHTS && is_tombstoned) {
>          ldb_ret = ldb_delete(state->samdb, dn);
> 
> to
> 
>   if (is_tombstoned) {
>          ldb_ret = ldb_delete(state->samdb, dn);

I'd rather not, as `dsdb_check_access_on_dn()` and `dsdb_check_access_on_dn_internal()` can also return `LDB_ERR_OPERATIONS_ERROR` which would signal an internal error (error parsing the GUID, ..).
Comment 15 Lukas Oyen univentionstaff 2017-06-14 18:57:10 CEST
Created attachment 8925 [details]
8829: Escalate priviledges if maschine name matches.

Update of the second patch.

This version gets the account name of the user that signed the request against bind9 (`session_info->info->account_name`), checks if it is a machine account (ends with `$`) and if it matches the `dc` attribute of the dns-record object.
Comment 16 Arvid Requate univentionstaff 2017-06-15 16:01:36 CEST
Ok works.
Comment 18 Lukas Oyen univentionstaff 2017-06-22 14:26:24 CEST
*** Bug 34910 has been marked as a duplicate of this bug. ***