Bug 14044 - ldap search filter eratically don't work, samba-tool does not display IPV6 PTR records properly
Summary: ldap search filter eratically don't work, samba-tool does not display IPV6 PT...
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: AD: LDB/DSDB/SAMDB (show other bugs)
Version: 4.13.3
Hardware: x64 Linux
: P5 normal (vote)
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-07-22 23:57 UTC by Russell Thamm
Modified: 2021-03-31 09:00 UTC (History)
3 users (show)

See Also:


Attachments
untested fix (compiles though) (3.35 KB, patch)
2021-03-03 06:52 UTC, Douglas Bagnall
no flags Details
backport for 4.12, 4.13, 4.14 (12.50 KB, patch)
2021-03-11 12:56 UTC, Björn Jacke
dbagnall: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Russell Thamm 2019-07-22 23:57:49 UTC
samba-tool and RSAT DNS do not display IPv6 PTR records properly. I have seen this using samba 4.8.0 and 4.10.4 running on CentOS 7 with internal DNS.

[root@bilbo user]# samba-tool dns add localhost 4.e.7.c.9.3.0.b.4.7.6.0.7.8.d.f.ip6.arpa 5.5.0.0.0.0.0.0.0.0.0.0.0.0.0.0 PTR brutus.dev.local
Password for [administrator@DEV.LOCAL]:
Record added successfully

[root@bilbo user]# nslookup fd87:0674:b039:c7e4::55
Server:		131.185.87.4
Address:	131.185.87.4#53

5.5.0.0.0.0.0.0.0.0.0.0.0.0.0.0.4.e.7.c.9.3.0.b.4.7.6.0.7.8.d.f.ip6.arpa	name = brutus.dev.local.

[root@bilbo user]# samba-tool dns query localhost 4.e.7.c.9.3.0.b.4.7.6.0.7.8.d.f.ip6.arpa @ PTR
Password for [administrator@DEV.LOCAL]:
  Name=, Records=0, Children=0
  Name=0, Records=0, Children=1

For the above record, RSAT DNS shows a folder labeled 0.

To view the PTR records, I have to use ldbsearch.

ldbsearch -H /usr/local/samba/private/sam.ldb -b "DC=DomainDnsZones,DC=dev,DC=local" "(objectclass=dnsNode)" --show-binary

# record 50
dn: DC=5.5.0.0.0.0.0.0.0.0.0.0.0.0.0.0,DC=4.e.7.c.9.3.0.b.4.7.6.0.7.8.d.f.ip6.arpa,CN=MicrosoftDNS,DC=DomainDnsZones,DC=dev,DC=local
objectClass: top
objectClass: dnsNode
instanceType: 4
whenCreated: 20190718060248.0Z
whenChanged: 20190718060248.0Z
uSNCreated: 117101
uSNChanged: 117101
showInAdvancedViewOnly: TRUE
name: 5.5.0.0.0.0.0.0.0.0.0.0.0.0.0.0
objectGUID: 674f6822-4a99-4f24-bfd5-4b2fe1308826
dnsRecord:     NDR: struct dnsp_DnssrvRpcRecord
        wDataLength              : 0x0014 (20)
        wType                    : DNS_TYPE_PTR (12)
        version                  : 0x05 (5)
        rank                     : DNS_RANK_ZONE (240)
        flags                    : 0x0000 (0)
        dwSerial                 : 0x0000000b (11)
        dwTtlSeconds             : 0x00000384 (900)
        dwReserved               : 0x00000000 (0)
        dwTimeStamp              : 0x0037fb7e (3668862)
        data                     : union dnsRecordData(case 12)
        ptr                      : brutus.dev.local

objectCategory: CN=Dns-Node,CN=Schema,CN=Configuration,DC=dev,DC=local
dc: 5.5.0.0.0.0.0.0.0.0.0.0.0.0.0.0
distinguishedName: DC=5.5.0.0.0.0.0.0.0.0.0.0.0.0.0.0,DC=4.e.7.c.9.3.0.b.4.7.6.0.7.8.d.f.ip6.arpa,CN=MicrosoftDNS,DC=DomainDnsZones,DC=dev,DC=local
Comment 1 Amit Kumar 2019-09-16 08:04:43 UTC
# samba-tool dns zonecreate localhost test-dns-zone -U Administrator
# samba-tool dns zonelist localhost -U Administrator
..
  pszZoneName                 : test-dns-zone
  Flags                       : DNS_RPC_ZONE_DSINTEGRATED DNS_RPC_ZONE_UPDATE_SECURE 
  ZoneType                    : DNS_ZONE_TYPE_PRIMARY
  Version                     : 50
  dwDpFlags                   : DNS_DP_AUTOCREATED DNS_DP_DOMAIN_DEFAULT DNS_DP_ENLISTED 
  pszDpFqdn                   : DomainDnsZones.samdom.amitexample.com
..

To keep note for myself, will be working
Comment 2 Amit Kumar 2019-09-24 07:57:08 UTC
1. Added a IPv6 PTR record. zone=test-dns-zone, record-name=ptr-record1
samba-tool dns add -h
Usage: samba-tool dns add <server> <zone> <name> <A|AAAA|PTR|CNAME|NS|MX|SRV|TXT> <data>

# samba-tool dns add localhost test-dns-zone ptr-record1 PTR sambadom.amitexample.com -U Administrator
Record added successfully


2. Wanted to query same PTR record, But cannot find.
# samba-tool dns query localhost test-dns-zone -h
Usage: samba-tool dns query <server> <zone> <name> <A|AAAA|CNAME|MX|NS|SOA|SRV|TXT|ALL> [options]
Query a name.

# samba-tool dns query localhost test-dns-zone ptr-record1 ALL -U Administrator
  Name=, Records=1, Children=0
    PTR: sambadom.amitexample.com (flags=f0, serial=2, ttl=900)


Findings:
a. samba-tool dns query does not 'PTR' record search command.
b. Also, record entry is not shown by query ALL.
Comment 3 Mikhail 2020-09-28 04:34:07 UTC
Zones "in-addr.arpa" and "ip6.arpa" cannot contain anything other than PTR records.

When a reverse zone is created for network /16 for ipv4 or any net for ipv6, a symbol "dot" appears in the ptr record.

The Samba DNS RPC server is trying to create a hierarchical structure based on symbol "dot" when enumerate records.

The DNS service returns the correct data. But RPC server return wrong data and it is impossible to manage the structure of the DNS either through the Windows applet or through the samba-tool.

Sorry for my English
Comment 4 Björn Jacke 2020-12-21 15:36:40 UTC
(In reply to Mikhail from comment #3)
> Zones "in-addr.arpa" and "ip6.arpa" cannot contain anything other than PTR
> records.

this is not right, also a reverse zone can have other record types than PTR.
For example, if you add another nameserver for the reverse zone, then MMC will ask you to ass a glue record, if you do that, you will not only have an NS record but also an A/AAAA record for the new nameserver being added there, a DN like this:

dn: DC=othernameserver.int.example.de,DC=92.10.in-addr.arpa,CN=MicrosoftDNS,DC=DomainDnsZones,DC=int,DC=example,DC=de

(In MS AD the othernameserver.int.example.de would also have a trailing fqdn ".")

But yes, a valid PTR record would have to be added like this:

samba-tool dns  add dc1 0.0.0.0.5.5.5.5.0.c.e.f.ip6.arpa 1.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.5.5.5.5.0.c.e.f.ip6.arpa PTR v6host1 -P
Comment 5 Björn Jacke 2020-12-21 15:41:24 UTC
Back the the main problem:

MMC tries to make a query like this:

# ldbsearch -H /var/lib/samba/private/sam.ldb -b "DC=0.0.0.0.5.5.5.5.0.c.e.f.ip6.arpa,CN=MicrosoftDNS,DC=DomainDnsZones,DC=int,DC=example,DC=de" '(&(objectClass=dnsNode)(dc=*.0.0.0)(!(dNSTombstoned=TRUE)))'  dc
# returned 0 records

yes, it gives no results ... while with a modified search filer removing the (dc=*.0.0.0):


# ldbsearch -H /var/lib/samba/private/sam.ldb -b "DC=0.0.0.0.5.5.5.5.0.c.e.f.ip6.arpa,CN=MicrosoftDNS,DC=DomainDnsZones,DC=int,DC=example,DC=de" '(&(objectClass=dnsNode)(!(dNSTombstoned=TRUE)))'  dc
# record 1
dn: DC=@,DC=0.0.0.0.5.5.5.5.0.c.e.f.ip6.arpa,CN=MicrosoftDNS,DC=DomainDnsZones,DC=int,DC=example,DC=de
dc: @

# record 2
dn: DC=1.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0,DC=0.0.0.0.5.5.5.5.0.c.e.f.ip6.arpa,CN=MicrosoftDNS,DC=DomainDnsZones,DC=int,DC=example,DC=de
dc: 1.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0

# record 3
dn: DC=1.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0,DC=0.0.0.0.5.5.5.5.0.c.e.f.ip6.arpa,CN=MicrosoftDNS,DC=DomainDnsZones,DC=int,DC=example,DC=de
dc: 1.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0

It looks like we don't correctly handle the LDAP search filter here.
Comment 6 Björn Jacke 2021-02-26 17:26:52 UTC
a search filter for

"(dc=*.0)"

works well

But as soon as you add another 0 it fails.

"(dc=*0.0)"

returns no result.

I can reproduce the same with the description attribute on a user:

description: 1.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0

in this case it works well with a search filter of

"(description=*.0.0)"

but not any more with a search filter of

"(description=*0.0.0)"

What the heck?
Comment 7 Björn Jacke 2021-02-26 17:53:37 UTC
also works with letters :-)

description: 1.0.0.0.0.0.0.0.0.0.0aaaaaaaaaaaa

(description=*aaaaa) search filter (5 chars at the end) fails - less works

6 chars works again, 7, 8, 9, 10, 11 fails, 12 works again
Comment 8 Douglas Bagnall 2021-03-03 02:34:49 UTC
(In reply to Björn Jacke from comment #7)

There is a logic error in ldb_wildcard_compare().

For example with the "1.0.0.0.0.0.0.0.0.0.0aaaaaaaaaaaa" case, searching for "(description=*aaaaa)":


When we get to here, p = "AAAAAAAAAAAA", cnk.data is "AAAAA". (12 and 5 'A's respectively).

			uint8_t *g;
			uint8_t *end = val.data + val.length;
			do { /* greedy */

				/*
				 * haystack is a valid pointer in val
				 * because the memmem() can only
				 * succeed if the needle (cnk.length)
				 * is <= haystacklen
				 *
				 * p will be a pointer at least
				 * cnk.length from the end of haystack
				 */
				uint8_t *haystack
					= p + cnk.length;
				size_t haystacklen
					= end - (haystack);

				g = memmem(haystack,
					   haystacklen,
					   (const uint8_t *)cnk.data,
					   cnk.length);
				if (g) {
					p = g;
				}
			} while(g);


so haystack is set to &p[5] ("AAAAAAA"), and haystacklen is 7.
cnk.data ("AAAAA") is found at the start of haystack; g is not NULL (it is haystack).
p is now set to g (== haystack == (old p + 5)).

looping around, haystack is moved along 5, so it is no &p[10], or "AA".

this time memmem must fail because haystacklen is smaller than cnk.length.


Your lengths of "a"s that work (1, 2, 3, 4, 6, 12) are divisors of 12.
If you had 11 "a"s, I think only 1 and 11 would work.

I think we need to step more carefully through, perhaps starting at the back (i.e. end - cnk.length), or using a proper byte by byte search, not repeated memmems.


This doesn't affect the "common" (read "tested") case where the strings don't contain repetitions like this.
Comment 9 Douglas Bagnall 2021-03-03 02:47:52 UTC
The last change here 745b99fc6b75db33cdb0a58df1a3f2a5063bc76e, in response to 
https://bugzilla.samba.org/show_bug.cgi?id=13773 would not have changed this but it did clarify what is wrong.
Comment 10 Douglas Bagnall 2021-03-03 04:22:30 UTC
(In reply to Douglas Bagnall from comment #8)
I thought about it a bit. The chunk should obviously just succeed as soon as it finds a match, not search for the last-most match, which it seems to want to do. I mean, why is there even a loop?

Finding the first-most match is necessary for more complicated searches, e.g. "(description=*0.0.*0.0*aaa*aaa*)" which should match the same string. Greedily consuming the matches will break that.
Comment 11 Douglas Bagnall 2021-03-03 06:05:35 UTC
(In reply to Douglas Bagnall from comment #10)
> why is there even a loop?

OK, this is just for the last one, without a trailing '*', in which case I think it must match exactly the last n bytes of the string. So again, why the loop?
Comment 12 Douglas Bagnall 2021-03-03 06:52:21 UTC
Created attachment 16493 [details]
untested fix (compiles though)
Comment 14 Douglas Bagnall 2021-03-03 20:36:29 UTC
Björn, I just got interested when you said "what the heck?" and I haven't really looked at the original report, which I see mentions samba-tool and RSAT DNS.

Is it fixed for those? I can't see where samba-tool is making a query like the MMC one.

Amit, Russell, are you able to reproduce your problems with the patch from https://gitlab.com/samba-team/samba/-/merge_requests/1817.patch ?
Comment 15 Samba QA Contact 2021-03-10 09:52:08 UTC
This bug was referenced in samba master:

cc098f1cad04b2cfec4ddd6b2511cd5a600f31c6
33a95a1e75b85e9795c4490b78ead2162e2a1f47
fa93339978040eab52b2722c1716028b48d8d084
bb17b4e1bbd1f03445bb3ef8cfd5f33d5e49bccc
Comment 16 Björn Jacke 2021-03-11 12:56:09 UTC
Created attachment 16522 [details]
backport for 4.12, 4.13, 4.14
Comment 17 Douglas Bagnall 2021-03-11 21:03:13 UTC
I believe this is ready for the next 4.12, 4.13, 4.14.
Comment 18 Samba QA Contact 2021-03-24 08:56:22 UTC
This bug was referenced in samba v4-14-stable (Release samba-4.14.1):

ff12bd2fa126d105c2fbc62529d664324e22aeb3
Comment 19 Samba QA Contact 2021-03-24 08:57:27 UTC
This bug was referenced in samba v4-13-stable (Release samba-4.13.6):

99d849abc3be3e2b2f78c2eb15439b6d140d1fa4
Comment 20 Samba QA Contact 2021-03-24 08:58:45 UTC
This bug was referenced in samba v4-12-stable (Release samba-4.12.13):

c99c29e1e3490a559a16d2bd72b9aca80f1f1d38
Comment 21 Samba QA Contact 2021-03-24 10:37:51 UTC
This bug was referenced in samba v4-12-test:

c99c29e1e3490a559a16d2bd72b9aca80f1f1d38
Comment 22 Samba QA Contact 2021-03-24 10:53:43 UTC
This bug was referenced in samba v4-13-test:

99d849abc3be3e2b2f78c2eb15439b6d140d1fa4
Comment 23 Samba QA Contact 2021-03-24 10:56:38 UTC
This bug was referenced in samba v4-14-test:

ff12bd2fa126d105c2fbc62529d664324e22aeb3
Comment 24 Stefan Metzmacher 2021-03-24 11:04:12 UTC
Note the following additional releases were made in order to
have standalone ldb releases with the fixes and let Samba depend on
them when using system libraries:

samba-4.12.14 (with ldb-2.1.5)
samba-4.13.7  (with ldb-2.2.1)
samba-4.14.2  (with ldb-2.3.0)
Comment 25 Karolin Seeger 2021-03-31 09:00:11 UTC
Closing out bug report.

Thanks!