From a9ec2225a5769be903f8cc19b50025d83b94f4ac Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Sun, 26 Jan 2020 13:45:15 +0100 Subject: [PATCH] LookupServer: Retry with 0x20 randomization turned off on EREFUSED Apparently some authoritative servers don't handle 0x20 randomization well and may send EREFUSED. Retry with randomization turned off then. Reference: https://github.com/dns-violations/dns-violations/blob/master/2017/DVE-2017-0006.md More work towards #10. --- Servers/LookupServer/DNSRequest.cpp | 16 +++++++++------- Servers/LookupServer/DNSRequest.h | 7 ++++++- Servers/LookupServer/DNSResponse.cpp | 4 ++++ Servers/LookupServer/DNSResponse.h | 16 ++++++++++++++++ Servers/LookupServer/LookupServer.cpp | 13 +++++++++++-- Servers/LookupServer/LookupServer.h | 3 ++- 6 files changed, 48 insertions(+), 11 deletions(-) diff --git a/Servers/LookupServer/DNSRequest.cpp b/Servers/LookupServer/DNSRequest.cpp index f70fe02aff..ef1048ccca 100644 --- a/Servers/LookupServer/DNSRequest.cpp +++ b/Servers/LookupServer/DNSRequest.cpp @@ -13,22 +13,24 @@ DNSRequest::DNSRequest() { } -void DNSRequest::add_question(const String& name, u16 record_type) +void DNSRequest::add_question(const String& name, u16 record_type, ShouldRandomizeCase should_randomize_case) { ASSERT(m_questions.size() <= UINT16_MAX); if (name.is_empty()) return; - // Randomize the 0x20 bit in every ASCII character. StringBuilder builder; for (size_t i = 0; i < name.length(); ++i) { u8 ch = name[i]; - if (isalpha(ch)) { - if (arc4random_uniform(2)) - ch |= 0x20; - else - ch &= ~0x20; + if (should_randomize_case == ShouldRandomizeCase::Yes) { + // Randomize the 0x20 bit in every ASCII character. + if (isalpha(ch)) { + if (arc4random_uniform(2)) + ch |= 0x20; + else + ch &= ~0x20; + } } builder.append(ch); } diff --git a/Servers/LookupServer/DNSRequest.h b/Servers/LookupServer/DNSRequest.h index 4a73b35718..f702a4ede3 100644 --- a/Servers/LookupServer/DNSRequest.h +++ b/Servers/LookupServer/DNSRequest.h @@ -12,11 +12,16 @@ #define T_PTR 12 #define T_MX 15 +enum class ShouldRandomizeCase { + No = 0, + Yes +}; + class DNSRequest { public: DNSRequest(); - void add_question(const String& name, u16 record_type); + void add_question(const String& name, u16 record_type, ShouldRandomizeCase); const Vector& questions() const { return m_questions; } diff --git a/Servers/LookupServer/DNSResponse.cpp b/Servers/LookupServer/DNSResponse.cpp index 4d86dcb528..5be6fba62f 100644 --- a/Servers/LookupServer/DNSResponse.cpp +++ b/Servers/LookupServer/DNSResponse.cpp @@ -44,6 +44,10 @@ Optional DNSResponse::from_raw_response(const u8* raw_data, size_t DNSResponse response; response.m_id = response_header.id(); + response.m_code = response_header.response_code(); + + if (response.code() != DNSResponse::Code::NOERROR) + return response; size_t offset = sizeof(DNSPacket); diff --git a/Servers/LookupServer/DNSResponse.h b/Servers/LookupServer/DNSResponse.h index 2e0da23044..078e007a98 100644 --- a/Servers/LookupServer/DNSResponse.h +++ b/Servers/LookupServer/DNSResponse.h @@ -26,10 +26,26 @@ public: return m_answers.size(); } + enum class Code : u8 { + NOERROR = 0, + FORMERR = 1, + SERVFAIL = 2, + NXDOMAIN = 3, + NOTIMP = 4, + REFUSED = 5, + YXDOMAIN = 6, + XRRSET = 7, + NOTAUTH = 8, + NOTZONE = 9, + }; + + Code code() const { return (Code)m_code; } + private: DNSResponse() {} u16 m_id { 0 }; + u8 m_code { 0 }; Vector m_questions; Vector m_answers; }; diff --git a/Servers/LookupServer/LookupServer.cpp b/Servers/LookupServer/LookupServer.cpp index ae7605d76f..4cf7dfc36a 100644 --- a/Servers/LookupServer/LookupServer.cpp +++ b/Servers/LookupServer/LookupServer.cpp @@ -161,7 +161,7 @@ void LookupServer::service_client(RefPtr socket) } } -Vector LookupServer::lookup(const String& hostname, bool& did_timeout, unsigned short record_type) +Vector LookupServer::lookup(const String& hostname, bool& did_timeout, unsigned short record_type, ShouldRandomizeCase should_randomize_case) { if (auto it = m_lookup_cache.find(hostname); it != m_lookup_cache.end()) { auto& cached_lookup = it->value; @@ -172,7 +172,7 @@ Vector LookupServer::lookup(const String& hostname, bool& did_timeout, u } DNSRequest request; - request.add_question(hostname, record_type); + request.add_question(hostname, record_type, should_randomize_case); auto buffer = request.to_byte_buffer(); @@ -216,6 +216,15 @@ Vector LookupServer::lookup(const String& hostname, bool& did_timeout, u dbgprintf("LookupServer: ID mismatch (%u vs %u) :(\n", response.id(), request.id()); return {}; } + + if (response.code() == DNSResponse::Code::REFUSED) { + if (should_randomize_case == ShouldRandomizeCase::Yes) { + // Retry with 0x20 case randomization turned off. + return lookup(hostname, did_timeout, record_type, ShouldRandomizeCase::No); + } + return {}; + } + if (response.question_count() != request.question_count()) { dbgprintf("LookupServer: Question count (%u vs %u) :(\n", response.question_count(), request.question_count()); return {}; diff --git a/Servers/LookupServer/LookupServer.h b/Servers/LookupServer/LookupServer.h index 0ef7d29322..5c8120cd4c 100644 --- a/Servers/LookupServer/LookupServer.h +++ b/Servers/LookupServer/LookupServer.h @@ -26,6 +26,7 @@ #pragma once +#include "DNSRequest.h" #include #include #include @@ -44,7 +45,7 @@ public: private: void load_etc_hosts(); void service_client(RefPtr); - Vector lookup(const String& hostname, bool& did_timeout, unsigned short record_type); + Vector lookup(const String& hostname, bool& did_timeout, unsigned short record_type, ShouldRandomizeCase = ShouldRandomizeCase::Yes); int make_dns_request_socket(sockaddr_in& dst_addr);