Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

dnsresult: key to lowercase letters #181

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

dnsresult: key to lowercase letters #181

wants to merge 1 commit into from

Conversation

jfagf
Copy link

@jfagf jfagf commented Jun 8, 2021

With some routers, the DNS query provides a mixed uppercase and lowercase answer, e.g. for _sip._tcp.tel.t-online.de you get the answer for _sIp._tcp.TeL.t-oNLinE.De
The transport key check then fails.
First and foremost, this can be observed with an IPv6 DNS implementation in the router

sgodin
sgodin reviewed Jun 9, 2021
View reviewed changes
Copy link
Member

@sgodin sgodin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this!

It seems this change isn't 100% though.

  1. It would be great to find a reference in the RFC's that indicates it is acceptable to treat a NAPTR replacement field and an SRV key field as case insensitive. RFC2782 indicates clearly that the service and proto parts of the SRV are case insensitive, but it doesn't say this for the Name portion (Page 1 and 2). This seems to indicate that we shouldn't be modifying it to lowercase. Do you see more definitive references to the contrary elsewhere? Really it seems odd that a DNS server would give you a NAPTR record that points to an SRV record and that SRV record is returned in a different case than the server returned in the NAPTR record.
  2. If we find it's legal to do the case insensitive thing. Line 1341, naptr.replacement is what get's used at the index into the mTopOrderedNAPTRs map (line 1365). We should probably be lowercasing that too. If the DNS server returns mixed case there and we are just lowercasing the SRV result, then it won't match.
  3. If we find it's legal to do the case insensitive thing. There is a 2nd find call for the mTopOrderedNAPTRs in DnsResult::primeResults, line 673. We should probably lowercase that as well.

@jfagf
Copy link
Author

jfagf commented Jun 10, 2021

Thank you for your quick response,
we will work on the topic again and then let you know.

@jfagf
Copy link
Author

jfagf commented Jun 22, 2021 

        ▪ You're right, RFC2782 clearly states that the service and proto parts of the SRV are case insensitive. There is no information for the name part.
        ▪ However, in this case the key consists of service, proto and name.
        ▪ Only Service and Proto are used to determine the transport (srv.transport) and the comparison should therefore always be made regardless of capitalization.
        ▪ So that the key is not changed permanently, I suggest performing the comparisons with a temporary variable.
        ▪ A test with NAPTRs also showed a dependency on the spelling there. But the comparison does not only include service and prototype, but also the name.
        ▪ In my test, NAPTR would not work, see appendix jftest
        ▪ The search in mTopOrderedNAPTRs.find with srv.key is not successful, the search with skey (lowercase) is successful


        ▪ DEBUG | 20210622-152217.051 | resipcm | RESIP:DNS | 3070046752 | DnsResult.cxx:261 | DnsResult::lookup sip:tel.t-online.de
        ▪ STACK | 20210622-152217.051 | resipcm | RESIP:DNS | 3070046752 | dns/DnsStub.cxx:466 | DNS query of:tel.t-online.de NAPTR
        ▪ DEBUG | 20210622-152217.051 | resipcm | RESIP:DNS | 3070046752 | dns/DnsStub.cxx:73 | NAPTR Result: tel.t-online.de (NAPTR)--> o=10 p=0 s=SIPS+D2T, tel.t-online.de (NAPTR)--> o=30 p=0 s=SIP+D2T, tel.t-online.de (NAPTR)--> o=20 p=0 s=SIP+D2U
        ▪ STACK | 20210622-152217.051 | resipcm | RESIP:DNS | 3070046752 | DnsResult.cxx:1466 | Received NAPTR result for: sip: target=tel.t-online.de
        ▪ STACK | 20210622-152217.051 | resipcm | RESIP:DNS | 3070046752 | DnsResult.cxx:1467 | DnsResult::onDnsResult() 0
        ▪ STACK | 20210622-152217.051 | resipcm | RESIP:DNS | 3070046752 | DnsResult.cxx:1367 | Received NAPTR record: key=tel.t-online.de order=10 pref=0 flags=s service=SIPS+D2T regex= ->  replacement=_sips._tcp.tel.t-online.de
        ▪ STACK | 20210622-152217.051 | resipcm | RESIP:DNS | 3070046752 | DnsResult.cxx:1367 | Received NAPTR record: key=tel.t-online.de order=30 pref=0 flags=s service=SIP+D2T regex= ->  replacement=_sip._tcp.tel.t-online.de
        ▪ STACK | 20210622-152217.051 | resipcm | RESIP:DNS | 3070046752 | DnsResult.cxx:1367 | Received NAPTR record: key=tel.t-online.de order=20 pref=0 flags=s service=SIP+D2U regex= ->  replacement=_sip._udp.tel.t-online.de
        ▪ STACK | 20210622-152217.051 | resipcm | RESIP:DNS | 3070046752 | DnsResult.cxx:1397 | NAPTR record is supported and matches highes priority order. doing SRV query: key=tel.t-online.de order=20 pref=0 flags=s service=SIP+D2U regex= ->  replacement=_sip._udp.tel.t-online.de
        ▪ STACK | 20210622-152217.052 | resipcm | RESIP:DNS | 3070046752 | dns/DnsStub.cxx:466 | DNS query of:_sip._udp.tel.t-online.de SRV
        ▪ DEBUG | 20210622-152217.052 | resipcm | RESIP:DNS | 3070046752 | dns/DnsStub.cxx:67 | SRV Result: _Sip._Udp.tel.T-ONLine.de (SRV) --> p=30 w=0 d-epp-110.edns.t-ipnet.de:5060, _Sip._Udp.tel.T-ONLine.de (SRV) --> p=20 w=0 h2-epp-110.edns.t-ipnet.de:5060, _Sip._Udp.tel.T-ONLine.de (SRV) --> p=10 w=0 m-epp-110.edns.t-ipnet.de:5060
        ▪ STACK | 20210622-152217.052 | resipcm | RESIP:DNS | 3070046752 | DnsResult.cxx:1101 | Received SRV result for: tel.t-online.de
        ▪ STACK | 20210622-152217.052 | resipcm | RESIP:DNS | 3070046752 | DnsResult.cxx:1104 | DnsResult::onDnsResult() 0 status=0
        ▪ INFO | 20210622-152217.052 | resipcm | RESIP:DNS | 3070046752 | DnsResult.cxx:1135 | jftest not found srv.key _Sip._Udp.tel.T-ONLine.de
        ▪ INFO | 20210622-152217.052 | resipcm | RESIP:DNS | 3070046752 | DnsResult.cxx:1146 | jftest found skey _sip._udp.tel.t-online.de
        ▪ INFO | 20210622-152217.052 | resipcm | RESIP:DNS | 3070046752 | DnsResult.cxx:1135 | jftest not found srv.key _Sip._Udp.tel.T-ONLine.de
        ▪ INFO | 20210622-152217.052 | resipcm | RESIP:DNS | 3070046752 | DnsResult.cxx:1146 | jftest found skey _sip._udp.tel.t-online.de
        ▪ INFO | 20210622-152217.052 | resipcm | RESIP:DNS | 3070046752 | DnsResult.cxx:1135 | jftest not found srv.key _Sip._Udp.tel.T-ONLine.de
        ▪ INFO | 20210622-152217.052 | resipcm | RESIP:DNS | 3070046752 | DnsResult.cxx:1146 | jftest found skey _sip._udp.tel.t-online.de
        ▪ STACK | 20210622-152217.052 | resipcm | RESIP:DNS | 3070046752 | DnsResult.cxx:655 | Priming [key=_Sip._Udp.tel.T-ONLine.de t=UDP p=10 w=0 port=5060 target=m-epp-110.edns.t-ipnet.de, key=_Sip._Udp.tel.T-ONLine.de t=UDP p=20 w=0 port=5060 target=h2-epp-110.edns.t-ipnet.de, key=_Sip._Udp.tel.T-ONLine.de t=UDP p=30 w=0 port=5060 target=d-epp-110.edns.t-ipnet.de]
        ▪ STACK | 20210622-152217.052 | resipcm | RESIP:DNS | 3070046752 | DnsResult.cxx:771 | cumulative weight = 0 selected=-1
        ▪ STACK | 20210622-152217.058 | resipcm | RESIP:DNS | 3070046752 | DnsResult.cxx:806 | SRV: [key=_Sip._Udp.tel.T-ONLine.de t=UDP p=20 w=0 port=5060 target=h2-epp-110.edns.t-ipnet.de, key=_Sip._Udp.tel.T-ONLine.de t=UDP p=30 w=0 port=5060 target=d-epp-110.edns.t-ipnet.de]
        ▪ STACK | 20210622-152217.058 | resipcm | RESIP:DNS | 3070046752 | DnsResult.cxx:663 | Primed with SRV=key=_Sip._Udp.tel.T-ONLine.de t=UDP p=10 w=0 port=5060 target=m-epp-110.edns.t-ipnet.de

@jfagf
Copy link
Author

jfagf commented Jun 24, 2021

One more note

From the rfc regulations one can see:

rfc3403 4.1:

  • REPLACEMENT
    A which is the next domain name to query for
    depending on the potential values found in the flags field.
  • and as used here are defined in RFC 1035 [7].

rfc 1035 2.3.3. Character case

  • For all parts of the DNS that are part of the official protocol, all
    comparisons between character strings (e.g., labels, domain names, etc.) are done in a case-insensitive manner

I think that means that the entire srv.key can be treated as case insensitive.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sgodin sgodin sgodin left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@jfagf @sgodin