diff --git a/netwerk/dns/TRR.cpp b/netwerk/dns/TRR.cpp index 25489193adc4..b8b6820bb7c3 100644 --- a/netwerk/dns/TRR.cpp +++ b/netwerk/dns/TRR.cpp @@ -485,11 +485,71 @@ TRR::PassQName(unsigned int &index) return NS_OK; } +// GetQname: retrieves the qname (stores in 'aQname') and stores the index +// after qname was parsed into the 'aIndex'. + +nsresult +TRR::GetQname(nsAutoCString &aQname, unsigned int &aIndex) +{ + uint8_t clength = 0; + unsigned int cindex = aIndex; + unsigned int loop = 128; // a valid DNS name can never loop this much + unsigned int endindex = 0; // index position after this data + do { + if (cindex >= mBodySize) { + LOG(("TRR: bad cname packet\n")); + return NS_ERROR_ILLEGAL_VALUE; + } + clength = static_cast(mResponse[cindex]); + if ((clength & 0xc0) == 0xc0) { + // name pointer, get the new offset (14 bits) + if ((cindex +1) >= mBodySize) { + return NS_ERROR_ILLEGAL_VALUE; + } + // extract the new index position for the next label + uint16_t newpos = (clength & 0x3f) << 8 | mResponse[cindex+1]; + if (!endindex) { + // only update on the first "jump" + endindex = cindex + 2; + } + cindex = newpos; + continue; + } else if (clength & 0xc0) { + // any of those bits set individually is an error + LOG(("TRR: bad cname packet\n")); + return NS_ERROR_ILLEGAL_VALUE; + } else { + cindex++; + } + if (clength) { + if (!aQname.IsEmpty()) { + aQname.Append("."); + } + if ((cindex + clength) > mBodySize) { + return NS_ERROR_ILLEGAL_VALUE; + } + aQname.Append((const char *)(&mResponse[cindex]), clength); + cindex += clength; // skip label + } + } while (clength && --loop); + + if (!loop) { + LOG(("TRR::DohDecode pointer loop error\n")); + return NS_ERROR_ILLEGAL_VALUE; + } + if (!endindex) { + // there was no "jump" + endindex = cindex; + } + aIndex = endindex; + return NS_OK; +} + // // DohDecode() collects the TTL and the IP addresses in the response // nsresult -TRR::DohDecode() +TRR::DohDecode(nsCString &aHost) { // The response has a 12 byte header followed by the question (returned) // and then the answer. The answer section itself contains the name, type @@ -509,7 +569,7 @@ TRR::DohDecode() nsAutoCString host; nsresult rv; - LOG(("doh decode %s %d bytes\n", mHost.get(), mBodySize)); + LOG(("doh decode %s %d bytes\n", aHost.get(), mBodySize)); mCname.Truncate(); @@ -519,7 +579,7 @@ TRR::DohDecode() } uint8_t rcode = mResponse[3] & 0x0F; if (rcode) { - LOG(("TRR Decode %s RCODE %d\n", mHost.get(), rcode)); + LOG(("TRR Decode %s RCODE %d\n", aHost.get(), rcode)); return NS_ERROR_FAILURE; } @@ -556,7 +616,8 @@ TRR::DohDecode() answerRecords, mBodySize, host.get(), index)); while (answerRecords) { - rv = PassQName(index); + nsAutoCString qname; + rv = GetQname(qname, index); if (NS_FAILED(rv)) { return rv; } @@ -610,94 +671,70 @@ TRR::DohDecode() return NS_ERROR_ILLEGAL_VALUE; } - // RDATA - // - A (TYPE 1): 4 bytes - // - AAAA (TYPE 28): 16 bytes - // - NS (TYPE 2): N bytes + if (qname.Equals(aHost)) { - switch(TYPE) { - case TRRTYPE_A: - if (RDLENGTH != 4) { - LOG(("TRR bad length for A (%u)\n", RDLENGTH)); - return NS_ERROR_UNEXPECTED; - } - rv = mDNS.Add(TTL, mResponse, index, RDLENGTH, - mAllowRFC1918); - if (NS_FAILED(rv)) { - LOG(("TRR:DohDecode failed: local IP addresses or unknown IP family\n")); - return rv; - } - break; - case TRRTYPE_AAAA: - if (RDLENGTH != 16) { - LOG(("TRR bad length for AAAA (%u)\n", RDLENGTH)); - return NS_ERROR_UNEXPECTED; - } - rv = mDNS.Add(TTL, mResponse, index, RDLENGTH, - mAllowRFC1918); - if (NS_FAILED(rv)) { - LOG(("TRR got unique/local IPv6 address!\n")); - return rv; - } - break; + // RDATA + // - A (TYPE 1): 4 bytes + // - AAAA (TYPE 28): 16 bytes + // - NS (TYPE 2): N bytes - case TRRTYPE_NS: - break; - case TRRTYPE_CNAME: - if (mCname.IsEmpty()) { - uint8_t clength = 0; - unsigned int cindex = index; - unsigned int loop = 128; // a valid DNS name can never loop this much - do { - if (cindex >= mBodySize) { - LOG(("TRR: bad cname packet\n")); - return NS_ERROR_ILLEGAL_VALUE; - } - clength = static_cast(mResponse[cindex]); - if ((clength & 0xc0) == 0xc0) { - // name pointer, get the new offset (14 bits) - if ((cindex +1) >= mBodySize) { - return NS_ERROR_ILLEGAL_VALUE; - } - // extract the new index position for the next label - uint16_t newpos = (clength & 0x3f) << 8 | mResponse[cindex+1]; - cindex = newpos; - continue; - } else if (clength & 0xc0) { - // any of those bits set individually is an error - LOG(("TRR: bad cname packet\n")); - return NS_ERROR_ILLEGAL_VALUE; - } else { - cindex++; - } - if (clength) { - if (!mCname.IsEmpty()) { - mCname.Append("."); - } - if ((cindex + clength) > mBodySize) { - return NS_ERROR_ILLEGAL_VALUE; - } - mCname.Append((const char *)(&mResponse[cindex]), clength); - cindex += clength; // skip label - } - } while (clength && --loop); - - if (!loop) { - LOG(("TRR::DohDecode pointer loop error\n")); - return NS_ERROR_ILLEGAL_VALUE; + switch(TYPE) { + case TRRTYPE_A: + if (RDLENGTH != 4) { + LOG(("TRR bad length for A (%u)\n", RDLENGTH)); + return NS_ERROR_UNEXPECTED; } + rv = mDNS.Add(TTL, mResponse, index, RDLENGTH, + mAllowRFC1918); + if (NS_FAILED(rv)) { + LOG(("TRR:DohDecode failed: local IP addresses or unknown IP family\n")); + return rv; + } + break; + case TRRTYPE_AAAA: + if (RDLENGTH != 16) { + LOG(("TRR bad length for AAAA (%u)\n", RDLENGTH)); + return NS_ERROR_UNEXPECTED; + } + rv = mDNS.Add(TTL, mResponse, index, RDLENGTH, + mAllowRFC1918); + if (NS_FAILED(rv)) { + LOG(("TRR got unique/local IPv6 address!\n")); + return rv; + } + break; - LOG(("TRR::DohDecode CNAME host %s => %s\n", - host.get(), mCname.get())); + case TRRTYPE_NS: + break; + case TRRTYPE_CNAME: + if (mCname.IsEmpty()) { + nsAutoCString qname; + unsigned int qnameindex = index; + rv = GetQname(qname, qnameindex); + if (NS_FAILED(rv)) { + return rv; + } + if(!qname.IsEmpty()) { + mCname = qname; + LOG(("TRR::DohDecode CNAME host %s => %s\n", + host.get(), mCname.get())); + } else { + LOG(("TRR::DohDecode empty CNAME for host %s!\n", + host.get())); + } + } + else { + LOG(("TRR::DohDecode CNAME - ignoring another entry\n")); + } + break; + default: + // skip unknown record types + LOG(("TRR unsupported TYPE (%u) RDLENGTH %u\n", TYPE, RDLENGTH)); + break; } - else { - LOG(("TRR::DohDecode CNAME - ignoring another entry\n")); - } - break; - default: - // skip unknown record types - LOG(("TRR unsupported TYPE (%u) RDLENGTH %u\n", TYPE, RDLENGTH)); - break; + } + else { + LOG(("TRR asked for %s data but got %s\n", aHost.get(), qname.get())); } index += RDLENGTH; @@ -832,10 +869,21 @@ nsresult TRR::On200Response() { // decode body and create an AddrInfo struct for the response - nsresult rv = DohDecode(); + nsresult rv = DohDecode(mHost); if (NS_SUCCEEDED(rv)) { if (!mDNS.mAddresses.getFirst() && !mCname.IsEmpty()) { + nsCString cname = mCname; + LOG(("TRR: check for CNAME record for %s within previous response\n", + cname.get())); + rv = DohDecode(cname); + if (NS_SUCCEEDED(rv) && mDNS.mAddresses.getFirst()) { + LOG(("TRR: Got the CNAME record without asking for it\n")); + ReturnData(); + return NS_OK; + } + // restore mCname as DohDecode() change it + mCname = cname; if (!--mCnameLoop) { LOG(("TRR::On200Response CNAME loop, eject!\n")); } else { diff --git a/netwerk/dns/TRR.h b/netwerk/dns/TRR.h index d982809ca38e..3df505bf3ed8 100644 --- a/netwerk/dns/TRR.h +++ b/netwerk/dns/TRR.h @@ -143,7 +143,8 @@ private: nsresult SendHTTPRequest(); nsresult DohEncode(nsCString &target); nsresult PassQName(unsigned int &index); - nsresult DohDecode(); + nsresult GetQname(nsAutoCString &aQname, unsigned int &aIndex); + nsresult DohDecode(nsCString &aHost); nsresult ReturnData(); nsresult FailData(); nsresult DohDecodeQuery(const nsCString &query, diff --git a/netwerk/test/unit/test_trr.js b/netwerk/test/unit/test_trr.js index 05a1bddc8558..40203cc1c240 100644 --- a/netwerk/test/unit/test_trr.js +++ b/netwerk/test/unit/test_trr.js @@ -199,7 +199,7 @@ function test3() prefs.setCharPref("network.trr.uri", "https://foo.example.com:" + h2Port + "/dns-auth"); prefs.setCharPref("network.trr.credentials", "user:password"); test_answer = "127.0.0.1"; - listen = dns.asyncResolve("auth.example.com", 0, listenerFine, mainThread, defaultOriginAttributes); + listen = dns.asyncResolve("bar.example.com", 0, listenerFine, mainThread, defaultOriginAttributes); } // verify failing credentials in DOH request @@ -284,7 +284,8 @@ function test10() } catch (e) { // NS_ERROR_UNKNOWN_HOST exception is expected do_test_finished(); - run_dns_tests(); + do_timeout(200, run_dns_tests); + //run_dns_tests(); } } @@ -403,12 +404,30 @@ function test20() // TRR-shadow and a CNAME loop function test21() { - prefs.setIntPref("network.trr.mode", 4); // TRR-first + prefs.setIntPref("network.trr.mode", 4); // shadow prefs.setCharPref("network.trr.uri", "https://foo.example.com:" + h2Port + "/dns-cname-loop"); test_answer = "127.0.0.1"; listen = dns.asyncResolve("test21.example.com", 0, listenerFine, mainThread, defaultOriginAttributes); } +// verify that basic A record name mismatch gets rejected. Gets the same DOH +// response back as test1 +function test22() +{ + prefs.setIntPref("network.trr.mode", 3); // TRR-only to avoid native fallback + prefs.setCharPref("network.trr.uri", "https://foo.example.com:" + h2Port + "/dns"); + listen = dns.asyncResolve("mismatch.example.com", 0, listenerFails, mainThread, defaultOriginAttributes); +} + +// TRR-only, with a CNAME response with a bundled A record for that CNAME! +function test23() +{ + prefs.setIntPref("network.trr.mode", 3); // TRR-only + prefs.setCharPref("network.trr.uri", "https://foo.example.com:" + h2Port + "/dns-cname-a"); + test_answer = "9.8.7.6"; + listen = dns.asyncResolve("cname-a.example.com", 0, listenerFine, mainThread, defaultOriginAttributes); +} + var tests = [ test1, test1b, test2, @@ -433,6 +452,8 @@ var tests = [ test1, test19, test20, test21, + test22, + test23, testsDone ]; diff --git a/testing/xpcshell/moz-http2/moz-http2.js b/testing/xpcshell/moz-http2/moz-http2.js index 4d966d0ec6fb..f39083fcac19 100644 --- a/testing/xpcshell/moz-http2/moz-http2.js +++ b/testing/xpcshell/moz-http2/moz-http2.js @@ -549,6 +549,47 @@ function handleRequest(req, res) { res.end(""); return; + } + else if (u.pathname === "/dns-cname-a") { + // test23 asks for cname-a.example.com + // this responds with a CNAME to here.example.com *and* an A record + // for here.example.com + var content; + + content = new Buffer("0000" + + "0100" + + "0001" + // QDCOUNT + "0002" + // ANCOUNT + "00000000" + // NSCOUNT + ARCOUNT + "07636E616D652d61" + // cname-a + "076578616D706C6503636F6D00" + // .example.com + "00010001" + // question type (A) + question class (IN) + + // answer record 1 + "C00C" + // name pointer to cname-a.example.com + "0005" + // type (CNAME) + "0001" + // class + "00000037" + // TTL + "0012" + // RDLENGTH + "0468657265" + // here + "076578616D706C6503636F6D00" + // .example.com + + // answer record 2, the A entry for the CNAME above + "0468657265" + // here + "076578616D706C6503636F6D00" + // .example.com + "0001" + // type (A) + "0001" + // class + "00000037" + // TTL + "0004" + // RDLENGTH + "09080706", // IPv4 address + "hex"); + res.setHeader('Content-Type', 'application/dns-udpwireformat'); + res.setHeader('Content-Length', content.length); + res.writeHead(200); + res.write(content); + res.end(""); + return; + } else if (u.pathname === "/dns-cname-loop") { // asking for cname.example.com @@ -611,9 +652,17 @@ function handleRequest(req, res) { // confirm.example.com has NS entry ns.example.com var content= new Buffer("00000100000100010000000007636F6E6669726D076578616D706C6503636F6D0000020001C00C00020001000000370012026E73076578616D706C6503636F6D010A00", "hex"); ns_confirm++; + } else if (2 >= ns_confirm) { + // next response: 10b-100.example.com has AAAA entry 1::FFFF + + // we expect two requests for this name (A + AAAA), respond identically + // for both and expect the client to reject the wrong one + var content= new Buffer("000001000001000100000000" + "073130622d313030" + + "076578616D706C6503636F6D00001C0001C00C001C00010000003700100001000000000000000000000000FFFF", "hex"); + ns_confirm++; } else { - // next response: wrong.example.com has AAAA entry 1::FFFF - var content= new Buffer("0000010000010001000000000577726F6E67076578616D706C6503636F6D00001C0001C00C001C00010000003700100001000000000000000000000000FFFF", "hex"); + // everything else is just wrong + return; } res.setHeader('Content-Type', 'application/dns-udpwireformat'); res.setHeader('Content-Length', content.length);