Merge "dns_responder: Truncate UDP response which is larger than 512 bytes" (c90b4a07) · Commits · e / os / android_packages_modules_DnsResolver

tests/dns_responder/dns_responder.cpp

+79 −18

Original line number	Diff line number	Diff line
		@@ -623,6 +623,15 @@ void DNSResponder::clearQueries() {
		queries_.clear();
		}

		bool DNSResponder::hasOptPseudoRR(DNSHeader* header) const {
		if (header->additionals.empty()) return false;

		// OPT RR may be placed anywhere within the additional section. See RFC 6891 section 6.1.1.
		auto found = std::find_if(header->additionals.begin(), header->additionals.end(),
		[](const auto& a) { return a.rtype == ns_type::ns_t_opt; });
		return found != header->additionals.end();
		}

		void DNSResponder::requestHandler() {
		epoll_event evs[EPOLL_MAX_EVENTS];
		while (true) {
		@@ -713,15 +722,7 @@ bool DNSResponder::handleDNSRequest(const char* buffer, ssize_t len, int protoco

		// Make the response. The query has been read into \|header\| which is used to build and return
		// the response as well.
		switch (mapping_type_) {
		case MappingType::DNS_HEADER:
		return makeResponseFromDnsHeader(&header, response, response_len);
		case MappingType::BINARY_PACKET:
		return makeResponseFromBinaryPacket(&header, response, response_len);
		case MappingType::ADDRESS_OR_HOSTNAME:
		default:
		return makeResponse(&header, response, response_len);
		}
		return makeResponse(&header, protocol, response, response_len);
		}

		bool DNSResponder::addAnswerRecords(const DNSQuestion& question,
		@@ -841,7 +842,66 @@ bool DNSResponder::makeErrorResponse(DNSHeader* header, ns_rcode rcode, char* re
		return writePacket(header, response, response_len);
		}

		bool DNSResponder::makeResponse(DNSHeader* header, char* response, size_t* response_len) const {
		bool DNSResponder::makeTruncatedResponse(DNSHeader* header, char* response,
		size_t* response_len) const {
		// Build a minimal response for non-EDNS response over UDP. Truncate all stub RRs in answer,
		// authority and additional section. EDNS response truncation has not supported here yet
		// because the EDNS response must have an OPT record. See RFC 6891 section 7.
		header->answers.clear();
		header->authorities.clear();
		header->additionals.clear();
		header->qr = true;
		header->tr = true;
		return writePacket(header, response, response_len);
		}

		bool DNSResponder::makeResponse(DNSHeader* header, int protocol, char* response,
		size_t* response_len) const {
		char buffer[4096];
		size_t buffer_len = sizeof(buffer);
		bool ret;

		switch (mapping_type_) {
		case MappingType::DNS_HEADER:
		ret = makeResponseFromDnsHeader(header, buffer, &buffer_len);
		break;
		case MappingType::BINARY_PACKET:
		ret = makeResponseFromBinaryPacket(header, buffer, &buffer_len);
		break;
		case MappingType::ADDRESS_OR_HOSTNAME:
		default:
		ret = makeResponseFromAddressOrHostname(header, buffer, &buffer_len);
		}

		if (!ret) return false;

		// Return truncated response if the built non-EDNS response size which is larger than 512 bytes
		// will be responded over UDP. The truncated response implementation here just simply set up
		// the TC bit and truncate all stub RRs in answer, authority and additional section. It is
		// because the resolver will retry DNS query over TCP and use the full TCP response. See also
		// RFC 1035 section 4.2.1 for UDP response truncation and RFC 6891 section 4.3 for EDNS larger
		// response size capability.
		// TODO: Perhaps keep the stub RRs as possible.
		// TODO: Perhaps truncate the EDNS based response over UDP. See also RFC 6891 section 4.3,
		// section 6.2.5 and section 7.
		if (protocol == IPPROTO_UDP && buffer_len > kMaximumUdpSize &&
		!hasOptPseudoRR(header) /* non-EDNS */) {
		LOG(INFO) << "Return truncated response because original response length " << buffer_len
		<< " is larger than " << kMaximumUdpSize << " bytes.";
		return makeTruncatedResponse(header, response, response_len);
		}

		if (buffer_len > *response_len) {
		LOG(ERROR) << "buffer overflow on line " << __LINE__;
		return false;
		}
		memcpy(response, buffer, buffer_len);
		*response_len = buffer_len;
		return true;
		}

		bool DNSResponder::makeResponseFromAddressOrHostname(DNSHeader* header, char* response,
		size_t* response_len) const {
		for (const DNSQuestion& question : header->questions) {
		if (question.qclass != ns_class::ns_c_in && question.qclass != ns_class::ns_c_any) {
		LOG(INFO) << "unsupported question class " << question.qclass;
		@@ -894,9 +954,9 @@ bool DNSResponder::makeResponseFromDnsHeader(DNSHeader* header, char* response,
		LOG(INFO) << "no mapping found for " << name << " " << dnstype2str(qtype)
		<< ", couldn't build a response from DNSHeader mapping";

		// Note that do nothing as makeResponse() if no mapping is found. It just changes the QR
		// flag from query (0) to response (1) in the query. Then, send the modified query back as
		// a response.
		// Note that do nothing as makeResponseFromAddressOrHostname() if no mapping is found. It
		// just changes the QR flag from query (0) to response (1) in the query. Then, send the
		// modified query back as a response.
		header->qr = true;
		}
		return writePacket(header, response, response_len);
		@@ -932,9 +992,9 @@ bool DNSResponder::makeResponseFromBinaryPacket(DNSHeader* header, char* respons
		// TODO: handle correctly. See also TODO in addAnswerRecords().
		// TODO: Perhaps dump packet content to indicate which query failed.
		LOG(INFO) << "no mapping found, couldn't build a response from BinaryPacket mapping";
		// Note that do nothing as makeResponse() if no mapping is found. It just changes the QR
		// flag from query (0) to response (1) in the query. Then, send the modified query back as
		// a response.
		// Note that do nothing as makeResponseFromAddressOrHostname() if no mapping is found. It
		// just changes the QR flag from query (0) to response (1) in the query. Then, send the
		// modified query back as a response.
		header->qr = true;
		return writePacket(header, response, response_len);
		}
		@@ -1049,8 +1109,9 @@ void DNSResponder::handleQuery(int protocol) {
		LOG(ERROR) << method_str << " failed for " << host_str;
		}
		// Test that the response is actually a correct DNS message.
		// TODO: Make DNS message test to support name compression. Or it throws a warning for
		// a valid DNS message with name compression while the raw packet mapping is used.
		// TODO: Perhaps make DNS message validation to support name compression. Or it throws
		// a warning for a valid DNS message with name compression while the binary packet mapping
		// is used.
		const char* response_end = response + len;
		DNSHeader header;
		const char* cur = header.read(response, response_end);

tests/dns_responder/dns_responder.h

+18 −5

Original line number	Diff line number	Diff line
		@@ -34,6 +34,10 @@
		// Default TTL of the DNS answer record.
		constexpr unsigned kAnswerRecordTtlSec = 5;

		// The maximum UDP response size in bytes the DNS responder allows to send. It is used in non-EDNS
		// case. See RFC 1035 section 4.2.1.
		constexpr unsigned kMaximumUdpSize = 512;

		namespace test {

		struct DNSName {
		@@ -224,6 +228,9 @@ class DNSResponder {

		void requestHandler();

		// Check if any OPT Pseudo RR in the additional section.
		bool hasOptPseudoRR(DNSHeader* header) const;

		// Parses and generates a response message for incoming DNS requests.
		// Returns false to ignore the request, which might be due to either parsing error
		// or unresponsiveness.
		@@ -235,14 +242,20 @@ class DNSResponder {
		bool generateErrorResponse(DNSHeader* header, ns_rcode rcode, char* response,
		size_t* response_len) const;

		// TODO: Change writePacket, makeErrorResponse and
		// makeResponse{, FromDnsHeader, FromBinaryPacket} to use C++ containers instead of the unsafe
		// pointer + length buffer.
		// TODO: Change writePacket, makeErrorResponse, makeTruncatedResponse and
		// makeResponse{, FromAddressOrHostname, FromDnsHeader, FromBinaryPacket} to use C++ containers
		// instead of the unsafe pointer + length buffer.
		bool writePacket(const DNSHeader* header, char* response, size_t* response_len) const;
		// Build an error response with a given rcode.
		bool makeErrorResponse(DNSHeader* header, ns_rcode rcode, char* response,
		size_t* response_len) const;
		// Build a response from mapping {ADDRESS_OR_HOSTNAME, DNS_HEADER, BINARY_PACKET}.
		bool makeResponse(DNSHeader* header, char* response, size_t* response_len) const;
		// Build a truncated response.
		bool makeTruncatedResponse(DNSHeader* header, char* response, size_t* response_len) const;
		// Build a response.
		bool makeResponse(DNSHeader* header, int protocol, char* response, size_t* response_len) const;
		// Helper for building a response from mapping {ADDRESS_OR_HOSTNAME, DNS_HEADER, BINARY_PACKET}.
		bool makeResponseFromAddressOrHostname(DNSHeader* header, char* response,
		size_t* response_len) const;
		bool makeResponseFromDnsHeader(DNSHeader* header, char* response, size_t* response_len) const;
		bool makeResponseFromBinaryPacket(DNSHeader* header, char* response,
		size_t* response_len) const;

tests/resolv_integration_test.cpp

+43 −2

Original line number	Diff line number	Diff line
		@@ -3762,7 +3762,8 @@ enum class CallType { GETADDRINFO, GETHOSTBYNAME };
		class ResolverParameterizedTest : public ResolverTest,
		public testing::WithParamInterface<CallType> {
		protected:
		void VerifyQueryHelloExampleComV4(const test::DNSResponder& dns, const CallType calltype) {
		void VerifyQueryHelloExampleComV4(const test::DNSResponder& dns, const CallType calltype,
		const bool verifyNumQueries = true) {
		if (calltype == CallType::GETADDRINFO) {
		const addrinfo hints = {.ai_family = AF_INET, .ai_socktype = SOCK_DGRAM};
		ScopedAddrinfo result = safe_getaddrinfo("hello", nullptr, &hints);
		@@ -3778,7 +3779,7 @@ class ResolverParameterizedTest : public ResolverTest,
		} else {
		FAIL() << "Unsupported call type: " << static_cast<uint32_t>(calltype);
		}
		EXPECT_EQ(1U, GetNumQueries(dns, kHelloExampleCom));
		if (verifyNumQueries) EXPECT_EQ(1U, GetNumQueries(dns, kHelloExampleCom));
		}
		};

		@@ -3992,3 +3993,43 @@ TEST_P(ResolverParameterizedTest, MessageCompression) {
		VerifyQueryHelloExampleComV4(dns, calltype);
		}
		}

		TEST_P(ResolverParameterizedTest, TruncatedResponse) {
		const auto& calltype = GetParam();

		const size_t kMaxmiumLabelSize = 63; // see RFC 1035 section 2.3.4.
		const std::string kDomainName = ".example.com.";
		const std::string kCnameA = std::string(kMaxmiumLabelSize, 'a') + kDomainName;
		const std::string kCnameB = std::string(kMaxmiumLabelSize, 'b') + kDomainName;
		const std::string kCnameC = std::string(kMaxmiumLabelSize, 'c') + kDomainName;
		const std::string kCnameD = std::string(kMaxmiumLabelSize, 'd') + kDomainName;

		// Build a response message which exceeds 512 bytes by CNAME chain.
		//
		// Ignoring the other fields of the message, the response message has 8 CNAMEs in 5 answer RRs
		// and each CNAME has 77 bytes as the follows. The response message at least has 616 bytes in
		// answer section and has already exceeded 512 bytes totally.
		//
		// The CNAME is presented as:
		// 0 1 64 65 72 73 76 77
		// +---+--........--+---+---+---+---+---+---+---+---+---+---+---+---+---+
		// \| 63\| {x, .., x} \| 7 \| e \| x \| a \| m \| p \| l \| e \| 3 \| c \| o \| m \| 0 \|
		// +---+--........--+---+---+---+---+---+---+---+---+---+---+---+---+---+
		// ^-- x = {a, b, c, d}
		//
		const std::vector<DnsRecord> records = {
		{kHelloExampleCom, ns_type::ns_t_cname, kCnameA},
		{kCnameA, ns_type::ns_t_cname, kCnameB},
		{kCnameB, ns_type::ns_t_cname, kCnameC},
		{kCnameC, ns_type::ns_t_cname, kCnameD},
		{kCnameD, ns_type::ns_t_a, kHelloExampleComAddrV4},
		};
		test::DNSResponder dns;
		StartDns(dns, records);
		ASSERT_TRUE(mDnsClient.SetResolversForNetwork());

		// Expect UDP response is truncated. The resolver retries over TCP. See RFC 1035 section 4.2.1.
		VerifyQueryHelloExampleComV4(dns, calltype, false);
		EXPECT_EQ(1U, GetNumQueriesForProtocol(dns, IPPROTO_UDP, kHelloExampleCom));
		EXPECT_EQ(1U, GetNumQueriesForProtocol(dns, IPPROTO_TCP, kHelloExampleCom));
		}