LookupServer: Introduce DNSName

This is a wrapper around a string representing a domain name (such as
"example.com"). It never has a trailing dot.

For now, this class doesn't do much except wrap the raw string. Subsequent
commits will add or move more functionality to it.

Userland/Services/LookupServer/CMakeLists.txt

@@ -3,6 +3,7 @@ compile_ipc(LookupClient.ipc LookupClientEndpoint.h)
 
 set(SOURCES
     DNSAnswer.cpp
+    DNSName.cpp
     DNSPacket.cpp
     LookupServer.cpp
     LookupServerEndpoint.h

Userland/Services/LookupServer/DNSAnswer.cpp

@@ -29,7 +29,7 @@
 
 namespace LookupServer {
 
-DNSAnswer::DNSAnswer(const String& name, u16 type, u16 class_code, u32 ttl, const String& record_data)
+DNSAnswer::DNSAnswer(const DNSName& name, u16 type, u16 class_code, u32 ttl, const String& record_data)
     : m_name(name)
     , m_type(type)
     , m_class_code(class_code)

Userland/Services/LookupServer/DNSAnswer.h

@@ -26,6 +26,7 @@
 
 #pragma once
 
+#include "DNSName.h"
 #include <AK/String.h>
 #include <AK/Types.h>
 
@@ -33,9 +34,9 @@ namespace LookupServer {
 
 class DNSAnswer {
 public:
-    DNSAnswer(const String& name, u16 type, u16 class_code, u32 ttl, const String& record_data);
+    DNSAnswer(const DNSName& name, u16 type, u16 class_code, u32 ttl, const String& record_data);
 
-    const String& name() const { return m_name; }
+    const DNSName& name() const { return m_name; }
     u16 type() const { return m_type; }
     u16 class_code() const { return m_class_code; }
     u32 ttl() const { return m_ttl; }
@@ -44,7 +45,7 @@ public:
     bool has_expired() const;
 
 private:
-    String m_name;
+    DNSName m_name;
     u16 m_type { 0 };
     u16 m_class_code { 0 };
     u32 m_ttl { 0 };

Userland/Services/LookupServer/DNSName.cpp

@@ -0,0 +1,39 @@
+/*
+ * Copyright (c) 2021, Sergey Bugaev <bugaevc@serenityos.org>
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright notice, this
+ *    list of conditions and the following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above copyright notice,
+ *    this list of conditions and the following disclaimer in the documentation
+ *    and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE
+ * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
+ * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+ * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include "DNSName.h"
+
+namespace LookupServer {
+
+DNSName::DNSName(const String& name)
+{
+    if (name.ends_with('.'))
+        m_name = name.substring(0, name.length() - 1);
+    else
+        m_name = name;
+}
+
+}

Userland/Services/LookupServer/DNSName.h

@@ -0,0 +1,42 @@
+/*
+ * Copyright (c) 2021, Sergey Bugaev <bugaevc@serenityos.org>
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright notice, this
+ *    list of conditions and the following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above copyright notice,
+ *    this list of conditions and the following disclaimer in the documentation
+ *    and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE
+ * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
+ * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+ * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#pragma once
+
+#include <AK/String.h>
+
+namespace LookupServer {
+
+class DNSName {
+public:
+    DNSName(const String&);
+    const String& as_string() const { return m_name; }
+
+private:
+    String m_name;
+};
+
+}

Userland/Services/LookupServer/DNSPacket.cpp

@@ -58,9 +58,6 @@ void DNSPacket::add_question(const String& name, u16 record_type, ShouldRandomiz
         builder.append(ch);
     }
 
-    if (name[name.length() - 1] != '.')
-        builder.append('.');
-
     m_questions.empend(builder.to_string(), record_type, (u16)C_IN);
 }
 
@@ -85,7 +82,7 @@ ByteBuffer DNSPacket::to_byte_buffer() const
 
     stream << ReadonlyBytes { &header, sizeof(header) };
     for (auto& question : m_questions) {
-        auto parts = question.name().split('.');
+        auto parts = question.name().as_string().split('.');
         for (auto& part : parts) {
             stream << (u8)part.length();
             stream << part.bytes();
@@ -95,7 +92,7 @@ ByteBuffer DNSPacket::to_byte_buffer() const
         stream << htons(question.class_code());
     }
     for (auto& answer : m_answers) {
-        auto parts = answer.name().split('.');
+        auto parts = answer.name().as_string().split('.');
         for (auto& part : parts) {
             stream << (u8)part.length();
             stream << part.bytes();

Userland/Services/LookupServer/DNSQuestion.h

@@ -26,12 +26,14 @@
 
 #pragma once
 
-#include <AK/String.h>
+#include "DNSName.h"
 #include <AK/Types.h>
 
+namespace LookupServer {
+
 class DNSQuestion {
 public:
-    DNSQuestion(const String& name, u16 record_type, u16 class_code)
+    DNSQuestion(const DNSName& name, u16 record_type, u16 class_code)
         : m_name(name)
         , m_record_type(record_type)
         , m_class_code(class_code)
@@ -40,20 +42,12 @@ public:
 
     u16 record_type() const { return m_record_type; }
     u16 class_code() const { return m_class_code; }
-    const String& name() const { return m_name; }
+    const DNSName& name() const { return m_name; }
-
-    bool operator==(const DNSQuestion& other) const
-    {
-        return m_name == other.m_name && m_record_type == other.m_record_type && m_class_code == other.m_class_code;
-    }
-
-    bool operator!=(const DNSQuestion& other) const
-    {
-        return !(*this == other);
-    }
 
 private:
-    String m_name;
+    DNSName m_name;
     u16 m_record_type { 0 };
     u16 m_class_code { 0 };
 };
+
+}

Userland/Services/LookupServer/LookupServer.cpp

@@ -113,17 +113,17 @@ void LookupServer::load_etc_hosts()
     }
 }
 
-Vector<String> LookupServer::lookup(const String& name, unsigned short record_type)
+Vector<String> LookupServer::lookup(const DNSName& name, unsigned short record_type)
 {
 #if LOOKUPSERVER_DEBUG
-    dbgln("Got request for '{}'", name);
+    dbgln("Got request for '{}'", name.as_string());
 #endif
 
     Vector<String> responses;
 
-    if (auto known_host = m_etc_hosts.get(name); known_host.has_value()) {
+    if (auto known_host = m_etc_hosts.get(name.as_string()); known_host.has_value()) {
         responses.append(known_host.value());
-    } else if (!name.is_empty()) {
+    } else if (!name.as_string().is_empty()) {
         for (auto& nameserver : m_nameservers) {
 #if LOOKUPSERVER_DEBUG
             dbgln("Doing lookup using nameserver '{}'", nameserver);
@@ -153,15 +153,15 @@ Vector<String> LookupServer::lookup(const String& name, unsigned short record_ty
     return move(responses);
 }
 
-Vector<String> LookupServer::lookup(const String& hostname, const String& nameserver, bool& did_get_response, unsigned short record_type, ShouldRandomizeCase should_randomize_case)
+Vector<String> LookupServer::lookup(const DNSName& name, const String& nameserver, bool& did_get_response, unsigned short record_type, ShouldRandomizeCase should_randomize_case)
 {
-    if (auto it = m_lookup_cache.find(hostname); it != m_lookup_cache.end()) {
+    if (auto it = m_lookup_cache.find(name.as_string()); it != m_lookup_cache.end()) {
         auto& cached_lookup = it->value;
         if (cached_lookup.question.record_type() == record_type) {
             Vector<String> responses;
             for (auto& cached_answer : cached_lookup.answers) {
 #if LOOKUPSERVER_DEBUG
-                dbgln("Cache hit: {} -> {}, expired: {}", hostname, cached_answer.record_data(), cached_answer.has_expired());
+                dbgln("Cache hit: {} -> {}, expired: {}", name.as_string(), cached_answer.record_data(), cached_answer.has_expired());
 #endif
                 if (!cached_answer.has_expired())
                     responses.append(cached_answer.record_data());
@@ -175,7 +175,7 @@ Vector<String> LookupServer::lookup(const String& hostname, const String& namese
     DNSPacket request;
     request.set_is_query();
     request.set_id(arc4random_uniform(UINT16_MAX));
-    request.add_question(hostname, record_type, should_randomize_case);
+    request.add_question(name.as_string(), record_type, should_randomize_case);
 
     auto buffer = request.to_byte_buffer();
 
@@ -219,7 +219,7 @@ Vector<String> LookupServer::lookup(const String& hostname, const String& namese
     if (response.code() == DNSPacket::Code::REFUSED) {
         if (should_randomize_case == ShouldRandomizeCase::Yes) {
             // Retry with 0x20 case randomization turned off.
-            return lookup(hostname, nameserver, did_get_response, record_type, ShouldRandomizeCase::No);
+            return lookup(name, nameserver, did_get_response, record_type, ShouldRandomizeCase::No);
         }
         return {};
     }
@@ -229,13 +229,17 @@ Vector<String> LookupServer::lookup(const String& hostname, const String& namese
         return {};
     }
 
+    // Verify the questions in our request and in their response match exactly, including case.
     for (size_t i = 0; i < request.question_count(); ++i) {
         auto& request_question = request.questions()[i];
         auto& response_question = response.questions()[i];
-        if (request_question != response_question) {
+        bool exact_match = request_question.class_code() == response_question.class_code()
+            && request_question.record_type() == response_question.record_type()
+            && request_question.name().as_string() == response_question.name().as_string();
+        if (!exact_match) {
             dbgln("Request and response questions do not match");
-            dbgln("   Request: name=_{}_, type={}, class={}", request_question.name(), response_question.record_type(), response_question.class_code());
+            dbgln("   Request: name=_{}_, type={}, class={}", request_question.name().as_string(), response_question.record_type(), response_question.class_code());
-            dbgln("  Response: name=_{}_, type={}, class={}", response_question.name(), response_question.record_type(), response_question.class_code());
+            dbgln("  Response: name=_{}_, type={}, class={}", response_question.name().as_string(), response_question.record_type(), response_question.class_code());
             return {};
         }
     }
@@ -258,7 +262,7 @@ Vector<String> LookupServer::lookup(const String& hostname, const String& namese
     if (!cacheable_answers.is_empty()) {
         if (m_lookup_cache.size() >= 256)
             m_lookup_cache.remove(m_lookup_cache.begin());
-        m_lookup_cache.set(hostname, { request.questions()[0], move(cacheable_answers) });
+        m_lookup_cache.set(name.as_string(), { request.questions()[0], move(cacheable_answers) });
     }
     return responses;
 }

Userland/Services/LookupServer/LookupServer.h

@@ -27,6 +27,7 @@
 #pragma once
 
 #include "DNSPacket.h"
+#include "DNSName.h"
 #include <LibCore/Object.h>
 
 namespace LookupServer {
@@ -38,13 +39,13 @@ class LookupServer final : public Core::Object {
 
 public:
     static LookupServer& the();
-    Vector<String> lookup(const String& name, unsigned short record_type);
+    Vector<String> lookup(const DNSName& name, unsigned short record_type);
 
 private:
     LookupServer();
 
     void load_etc_hosts();
-    Vector<String> lookup(const String& hostname, const String& nameserver, bool& did_get_response, unsigned short record_type, ShouldRandomizeCase = ShouldRandomizeCase::Yes);
+    Vector<String> lookup(const DNSName& hostname, const String& nameserver, bool& did_get_response, unsigned short record_type, ShouldRandomizeCase = ShouldRandomizeCase::Yes);
 
     struct CachedLookup {
         DNSQuestion question;