Bug 41190 - DDNS update of Samba/AD doesn't delete dnsRecord from dNSTombstoned Objects
DDNS update of Samba/AD doesn't delete dnsRecord from dNSTombstoned Objects
Status: CLOSED FIXED
Product: UCS
Classification: Unclassified
Component: Samba4
UCS 4.2
Other Linux
: P5 normal (vote)
: UCS 4.2-0-errata
Assigned To: Lukas Oyen
Arvid Requate
:
: 40967 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2016-05-03 14:12 CEST by Arvid Requate
Modified: 2017-09-12 11:02 CEST (History)
1 user (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?: 2: Will only affect a few 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.114
Enterprise Customer affected?: Yes
School Customer affected?:
ISV affected?:
Waiting Support:
Flags outvoted (downgraded) after PO Review:
Ticket number: 2016033021000182
Bug group (optional): Troubleshooting
Max CVSS v3 score:
oyen: Patch_Available+


Attachments
refresh_dns_records_for_host (1.15 KB, text/plain)
2016-05-03 14:12 CEST, Arvid Requate
Details
script_and_output (5.76 KB, text/plain)
2017-05-09 14:55 CEST, Lukas Oyen
Details
41190-dns-correct-tombstoned-461.patch (13.39 KB, patch)
2017-05-10 14:29 CEST, Lukas Oyen
Details | Diff
41190-dns-correct-tombstoned-461.patch (13.37 KB, patch)
2017-05-12 15:17 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 2016-05-03 14:12:58 CEST
Created attachment 7635 [details]
refresh_dns_records_for_host

Ticket#: 2016033021000182 shows a case where the DDNS update of Samba/AD doesn't delete dnsRecord from Objects marked as dNSTombstoned: TRUE. In the case of the ticket the customer reported S4-Connector rejects for DNS-Records of MAC clients:

=========================================================================
03.05.2016 13:57:57,58 LDAP        (PROCESS): sync to ucs:   [           dns] [    modify] relativeDomainName=macbook-teil,zonename=domain.local,cn=dns,dc=domain,dc=local         
03.05.2016 13:57:57,682 LDAP        (ERROR  ): Unknown Exception during sync_to_ucs          
03.05.2016 13:57:57,682 LDAP        (ERROR  ): Traceback (most recent call last):
  File "/usr/lib/pymodules/python2.7/univention/s4connector/__init__.py", line 1438, in sync_to_ucs
    result = self.property[property_type].ucs_sync_function(self, property_type, object)
  File "/usr/lib/pymodules/python2.7/univention/s4connector/s4/dns.py", line 1438, in con2ucs
    ucs_host_record_create(s4connector, object)
  File "/usr/lib/pymodules/python2.7/univention/s4connector/s4/dns.py", line 876, in ucs_host_record_create
    newRecord.modify()         
  File "/usr/lib/pymodules/python2.7/univention/admin/handlers/__init__.py", line 363, in modify
    return self._modify(modify_childs,ignore_license=ignore_license)
  File "/usr/lib/pymodules/python2.7/univention/admin/handlers/__init__.py", line 972, in _modify       self.lo.modify(self.dn, ml, ignore_license=ignore_license)
  File "/usr/lib/pymodules/python2.7/univention/admin/uldap.py", line 423, in modify
    raise univention.admin.uexceptions.ldapError(_err2str(msg), original_exception=msg)
ldapError: Type or value exists: aRecord: value #204 provided more than once
========================================================================

The first impression was, that the MACs didn't take care while updating their DNS-record and would only always add new records. But the initial version attached nsupdate script didn't fix the issue either. The other mysterious point was, that the clients DNS address was not resolvable via DNS lookup!

Finally the point seems to be, that the dlz_bind9 module doesn't delete entries from objects marked as "dNSTombstoned: TRUE" -- but it allows to add entries to the object. This results in an unhealthy accumulation of junk values in the Samba/AD dnsRecord attribute.

The attached updated version of the refresh_dns_records_for_host script clears the dNSTombstoned attribute before doing the nsupdate cleanup of duplicate dnsRecord values. But we should fix this in the dlz_bind9 module.
Comment 1 Arvid Requate univentionstaff 2016-05-03 14:17:45 CEST
*** Bug 40967 has been marked as a duplicate of this bug. ***
Comment 2 Lukas Oyen univentionstaff 2017-05-09 14:55:24 CEST
Created attachment 8828 [details]
script_and_output

I am able to produce the duplicate `dnsRecord` entries in an object, if this
object is `dnsTombstoned=True`. The problem is not limited to MacOS, I am able
to reproduce the duplicate entries with a Windows client or pure `nsupdate`.

To reproduce, create a `host_record` via udm, set it to `dnsTombstoned=True` and
perform modifications per `nsupdate`. This will create duplicate `dnsRecord`
entries. See attached script and output.

The Samba code uses the function `dns_common_replace()` to update objects. This
expects a `DNS_TYPE_TOMBSTONE` marker `dnsRecord` in addition to
`dnsTombstoned=True` (more specifically: `dnsTombstoned=True` is set if a marker
`dnsRecord` exists or existed, otherwise `dnsTombstoned` is not touched). Here
the marker never existed and there is no reason to touch the `dnsTombstoned`
attribute. `dns_common_lookup()` only looks at the `dnsTombstoned` attribute on
the other hand.

[MS-DNSP] specifies, that a `DNS_TYPE_ZERO` record must be created whenever
`dnsTombstoned` is set to TRUE. This `DNS_TYPE_ZERO` translates to
`DNS_TYPE_TOMBSTONE` in Samba. A state where `dnsTombstoned=True`, but no marker
`dnsRecord` exists is therefore invalid.

But: I tried to reproduce the problem with a Windows Server 2012 and Windows8
client. A host record with `dnsRecord` manually set to TRUE, will be
corrected by the server upon modification by the client. A valid object with
`dnsTombstoned=False` and no marker or duplicate `dnsRecord` is restored.

Two questions arise:
1) Is this a bug in Samba and how difficult is the fix?
2) Under what circumstances does the S4Connector produce this invalid state
   and how difficult is the fix?

I would answer the first part of 1) with yes and will work on a patch for Samba.
Comment 3 Arvid Requate univentionstaff 2017-05-09 15:07:58 CEST
I agree. For the second part there is Bug 39161, at least.
Comment 4 Lukas Oyen univentionstaff 2017-05-10 14:29:04 CEST
Created attachment 8836 [details]
41190-dns-correct-tombstoned-461.patch

The attached patch fixes the issue in `dns_common_replace()` (and wrapping functions) by introducing a new parameter `tombstone`, marking if the object was tombstoned. A modification that revives the object now correctly sets `dnsTombstoned=FALSE`.

With this patches applied, compiled and installed, the previously attached test-script produces the desired output (snippet):

dn: DC=test,DC=loyen.intranet,CN=MicrosoftDNS,DC=DomainDnsZones,DC=loyen,DC=intranet
objectClass: top
objectClass: dnsNode
instanceType: 4
whenCreated: 20170510122133.0Z
uSNCreated: 3817
showInAdvancedViewOnly: TRUE
name: test
objectGUID: 1fba2c33-45d4-4d2c-9df7-a6d05009b79e
objectCategory: CN=Dns-Node,CN=Schema,CN=Configuration,DC=loyen,DC=intranet
dc: test
dnsRecord:     NDR: struct dnsp_DnssrvRpcRecord
        wDataLength              : 0x0004 (4)
        wType                    : DNS_TYPE_A (1)
        version                  : 0x05 (5)
        rank                     : DNS_RANK_ZONE (240)
        flags                    : 0x0000 (0)
        dwSerial                 : 0x0000001d (29)
        dwTtlSeconds             : 0x00000384 (900)
        dwReserved               : 0x00000000 (0)
        dwTimeStamp              : 0x00000000 (0)
        data                     : union dnsRecordData(case 1)
        ipv4                     : 10.200.46.59

dnsRecord:     NDR: struct dnsp_DnssrvRpcRecord
        wDataLength              : 0x0004 (4)
        wType                    : DNS_TYPE_A (1)
        version                  : 0x05 (5)
        rank                     : DNS_RANK_ZONE (240)
        flags                    : 0x0000 (0)
        dwSerial                 : 0x0000001d (29)
        dwTtlSeconds             : 0x00000384 (900)
        dwReserved               : 0x00000000 (0)
        dwTimeStamp              : 0x0037b09c (3649692)
        data                     : union dnsRecordData(case 1)
        ipv4                     : 10.200.46.58

dNSTombstoned: FALSE
whenChanged: 20170510122142.0Z
uSNChanged: 3822
distinguishedName: DC=test,DC=loyen.intranet,CN=MicrosoftDNS,DC=DomainDnsZones,DC=loyen,DC=intranet
Comment 5 Lukas Oyen univentionstaff 2017-05-12 15:17:13 CEST
Created attachment 8845 [details]
41190-dns-correct-tombstoned-461.patch

Updated version of the patch, now tested against (the relevant) Samba-Torture-Tests and `ucs-test-samba4`/`ucs-test-s4connector`.
Comment 6 Arvid Requate univentionstaff 2017-06-13 18:48:13 CEST
Ok, it works. I've committed your patch as

  patches/samba/4.2-0-0-ucs/2:4.6.1-1-errata4.2-0/34_samba_dns_tomstone.quilt

and I've built the package with that patch and created an advisory:

  ucs-4.2-0/doc/errata/staging/samba.yaml

If you think that this is everything that should be done, you may just set the bug to resolved, and I'll "continue" with the QA.
Comment 7 Lukas Oyen univentionstaff 2017-06-14 14:14:15 CEST
(In reply to Arvid Requate from comment #6)
> Ok, it works. I've committed your patch as
> [...]
> If you think that this is everything that should be done, you may just set
> the bug to resolved, and I'll "continue" with the QA.

Thank you!

With `patches/samba/4.2-0-0-ucs/2:4.6.1-1-errata4.2-0/34_samba_dns_tomstone.quilt` the commit-messages, which might be useful in the future, are lost. But as long as this Bugzilla is active one should be able to refer to this bug.
Comment 8 Arvid Requate univentionstaff 2017-06-14 19:11:50 CEST
> With `patches/samba/4.2-0-0-ucs/2:4.6.1-1-errata4.2-0/34_samba_dns_tomstone.quilt` the commit-messages, which might be useful in the future, are lost.

AFAICS there have been no commit messages in attachment 8845 [details], otherwise I would have preserved them.