Internal DNS handling overhaul + TCP + truncated reply support

.drone.jsonnet

@@ -291,7 +291,7 @@ local clang(version) = debian_pipeline(
 local full_llvm(version) = debian_pipeline(
   'Debian sid/llvm-' + version,
   docker_base + 'debian-sid-clang',
-  deps=default_deps(add=['clang-' + version, ' lld-' + version, ' libc++-' + version + '-dev', 'libc++abi-' + version + '-dev', 'libngtcp2-crypto-gnutls-dev', 'libngtcp2-dev'],
+  deps=default_deps(add=['clang-' + version, ' lld-' + version, ' libc++-' + version + '-dev', 'libc++abi-' + version + '-dev', 'libunwind-' + version + '-dev', 'libngtcp2-crypto-gnutls-dev', 'libngtcp2-dev'],
                     remove='g++'),
   oxen_repo=[],
   cmake_extra='-DCMAKE_C_COMPILER=clang-' + version +

src/CMakeLists.txt

@@ -156,11 +156,12 @@ if (SROUTER_FULL)
   # parse modify and reconstitute dns wire proto, dns queries and RR
   target_sources(session-router-dns PRIVATE
     dns/encode.cpp
+    dns/handler.cpp
+    dns/listener.cpp
     dns/message.cpp
     dns/platform.cpp
     dns/question.cpp
     dns/rr.cpp
-    dns/server.cpp
   )
 
   # platform specific bits and bobs for setting dns

src/config/config.cpp

@@ -1035,6 +1035,8 @@ namespace srouter
             MultiValue,
             Comment{
                 "Address to bind to for handling DNS requests.",
+                "",
+                "Can be specified multiple times to bind to multiple addresses; can be set to empty to disable.",
             },
             [this, parse_addr_for_dns](std::string arg) {
                 if (not arg.empty())

src/dns/encode.cpp

@@ -46,36 +46,84 @@ namespace srouter::dns
         return name;
     }
 
-    size_t encode_name(std::span<std::byte> buf, std::string_view name)
+    void encode_name(std::span<std::byte>& buf, std::string_view name, prev_names_t& prev_names, uint16_t& buf_offset)
     {
-        auto orig = buf.size();
         if (name.size() && name.back() == '.')
             name.remove_suffix(1);
 
-        for (auto part : srouter::split(name, "."))
+        // Look for a previously used suffix of this name.  For instance, if we have a response
+        // consisting of:
+        //
+        // localhost.sesh IN CNAME mylongpubkey.sesh
+        // foo.mylongpubkey.sesh IN AAAA 1:2:3::4
+        //
+        // then when we repeat the question itself (IN AAAA localhost.sesh) we echo that question
+        // back into the response as the 16 bytes:
+        //     \x09localhost\x04sesh\x00
+        // Suppose that this was written at location Z in the DNS message, this creates two
+        // pointable addresses:
+        // - "localhost.sesh" -> Z
+        // - "sesh" -> Z+10
+        //
+        // Then we come to the answers, and for the first "localhost.sesh" value, we can simply
+        // write that as a single pointer [Z] (where the pointer is a 16-bit, big-endian value with
+        // the highest two bits set and the remaining 14 bits set to "Z").
+        //
+        // Then we get to "mylongpubkey.sesh" and we can encode that as:
+        //
+        //     \x34mylongpubkey[pointer to Z+10]
+        //
+        // This also creates a new pointable address:
+        // - "mylongpubkey.sesh" -> Y
+        //
+        // Then we come to foo.mylongpubkey.sesh and we can encode this as:
+        //
+        // - \x03foo[pointer to Y]
+        //
+        // i.e. we only need 6 bytes for this address instead of 1+3+1+52+1+4+1=63 bytes that we
+        // would need for the uncompressed version.
+        //
+        // Although this compression is optional, given how frequently we reuse long session router
+        // names (particularly for something like SRV records where a name can be repeated multiple
+        // times), and the DNS response size limit of 512 bytes, we implement that here.
+
+        for (size_t pos = name.empty() ? std::string::npos : 0; pos != std::string_view::npos;)
         {
+            std::string_view check = name.substr(pos);
+            if (auto it = prev_names.find(check); it != prev_names.end())
+            {
+                if (buf.size() < 2)
+                    throw std::out_of_range{"Buffer too small"};
+                uint16_t ptr = uint16_t{0b11000000'00000000} | it->second;
+                oxenc::write_host_as_big(ptr, buf.data());
+                buf = buf.subspan(2);
+                buf_offset += 2;
+                // A pointer is terminal (i.e. no nullptr to add), so we're done.
+                return;
+            }
+
+            auto next = name.find('.', pos + 1);
+            auto part = next == std::string_view::npos ? check : name.substr(pos, next - pos);
+
             size_t l = part.size();
             if (l > 63 || l >= buf.size())
-                return false;
-            buf.front() = static_cast<std::byte>(l);
+                throw std::out_of_range{"Buffer too small"};
+            buf.front() = static_cast<std::byte>(l);  // Length prefix
             std::memcpy(buf.data() + 1, part.data(), part.size());
+            prev_names.emplace(std::string{check}, static_cast<uint16_t>(buf_offset));
             buf = buf.subspan(1 + part.size());
+            buf_offset += 1 + part.size();
+
+            pos = next == std::string_view::npos ? next : next + 1;
         }
+
+        // If we get here we wrote all the pieces without pointing at anything, so we need to append
+        // a null byte to terminate the name:
         if (buf.empty())
-            return false;
+            throw std::out_of_range{"Buffer too small"};
         buf.front() = std::byte{0};
         buf = buf.subspan(1);
-        return orig - buf.size();
-    }
-
-    bool write_name_into(std::span<std::byte>& buf, std::string_view name)
-    {
-        if (auto s = encode_name(buf, name))
-        {
-            buf = buf.subspan(s);
-            return true;
-        }
-        return false;
+        buf_offset++;
     }
 
     std::optional<std::variant<ipv4, ipv6>> decode_ptr(std::string_view name)

src/dns/encode.hpp

@@ -6,45 +6,59 @@
 
 #include <concepts>
 #include <optional>
+#include <stdexcept>
 #include <string>
 #include <utility>
 
 namespace srouter::dns
 {
-    /// Writes the encoded version of DNS name `name` into buf, and returns how many bytes of buf
-    /// were written.  If buf is too small to store the encoded name, returns 0.
-    size_t encode_name(std::span<std::byte> buf, std::string_view name);
+    // Custom hasher to let us look up a string_view key in a string-keyed unordered map:
+    struct transparent_string_hash
+    {
+        using is_transparent = void;
+        [[nodiscard]] size_t operator()(std::string_view txt) const { return std::hash<std::string_view>{}(txt); }
+    };
+
+    using prev_names_t = std::unordered_map<std::string, uint16_t, transparent_string_hash, std::equal_to<>>;
 
-    /// Same as encode_name, except that instead of returning the written size, on success it mutates the span
-    /// to drop the written prefix.  Returns true (and prefix-drops the written part of the span) on success,
-    /// false on failure.  Note that the failure case can still partially write into span.
-    bool write_name_into(std::span<std::byte>& buf, std::string_view name);
+    /// Writes the encoded version of DNS name `name` into buf, mutating buf to eliminate the
+    /// written bytes.  Throws if buf is too small to store the encoded name.
+    ///
+    /// prev_names contains pointer values relative to the start of the message, used for name
+    /// compression, and buf_offset contains the relative positive of the beginning of buf to the
+    /// start of the message.  New names added here should be added into it so that later repeated
+    /// names (or name suffixes) can use compression.
+    void encode_name(std::span<std::byte>& buf, std::string_view name, prev_names_t& prev_names, uint16_t& buf_offset);
 
     /// decode name from buffer, mutating the buffer to begin just past the extracted name.  Return
-    /// nullopt (without mutating buf) on failure.
+    /// nullopt (without mutating buf) on failure.  Does not currently support compressed names (but
+    /// those are not typically used in questions).
     std::optional<std::string> extract_name(std::span<const std::byte>& buf);
 
     /// Encodes an integer in big-endian order into the buffer, mutating the span to start just
-    /// after the written integer.  Returns true on success, false if the span was too small.
+    /// after the written integer.  Throws if buf is too small.  Returns sizeof(T) (i.e. the amount
+    /// written into the buffer), for convenience.
     template <std::unsigned_integral T>
-    bool write_int_into(std::span<std::byte>& buf, T value)
+    size_t write_int_into(std::span<std::byte>& buf, T value)
     {
         if (buf.size() < sizeof(T))
-            return false;
+            throw std::out_of_range{"Buffer too small"};
         oxenc::write_host_as_big(value, buf.data());
         buf = buf.subspan(sizeof(T));
-        return true;
+        return sizeof(T);
     }
 
-    // Calls write_int_info multiple times with the given integers.  Returns true (and modifies buf)
-    // if all success.  If any fail then false is returned and buf is left unchanged.
+    // Calls write_int_info multiple times with the given integers.  Throws if the buffer is too
+    // small.  Returns the total size of the given integers (i.e. the number of bytes written to
+    // buf), for convenience.
     template <std::unsigned_integral... T>
-    bool write_ints_into(std::span<std::byte>& buf, T... values)
+    size_t write_ints_into(std::span<std::byte>& buf, T... values)
     {
-        if (buf.size() < (0 + ... + sizeof(T)))
-            return false;
+        // NB: it's tempting to want to use `return (0 + ... + write_int_into())` here, but
+        // left-to-right evaluation of + operands isn't guaranteed, and that could put things into
+        // buf in the wrong order.  With , as used here it is guaranteed (similarly to || or &&).
         ((void)write_int_into(buf, values), ...);
-        return true;
+        return (0 + ... + sizeof(T));
     }
 
     /// Extracts a big-endian integer of the given type from the buffer, mutating the span to start
@@ -72,23 +86,6 @@ namespace srouter::dns
         return true;
     }
 
-    // Takes some object T with an `size_t encode(buf)` function (such as various classes in this
-    // dns code) and attempts to call it with the given buffer.  If it returns success (non-0) then
-    // this mutates `buf` to skip the written data and returns true; on failure it returns false.
-    template <typename T>
-    bool encode_into(std::span<std::byte>& buf, const T& thing)
-    {
-        if (auto written = thing.encode(buf))
-        {
-            buf = buf.subspan(written);
-            return true;
-        }
-        return false;
-    }
-
-    // Writes encoded rr data into buf, mutating buf to point beyond the written data.  Returns
-    // false (without mutating buf) if buf is too short; true on success.
-    bool write_rdata_into(std::span<std::byte>& buf, std::span<const std::byte> rdata);
     // Extracts encoded rr data from buf, mutating buf to point beyond the extracted data.  Returns
     // nullopt (without mutating buf) on error, the vector of decoded data on success.
     std::optional<std::vector<std::byte>> extract_rdata(std::span<const std::byte>& buf);

src/dns/flags.hpp

@@ -0,0 +1,22 @@
+#pragma once
+
+#include <cstdint>
+
+namespace srouter::dns
+{
+    constexpr uint16_t flags_QR = 1 << 15;
+    constexpr uint16_t flags_AA = 1 << 10;
+    constexpr uint16_t flags_TC = 1 << 9;
+    constexpr uint16_t flags_RD = 1 << 8;
+    constexpr uint16_t flags_RA = 1 << 7;
+
+    constexpr uint16_t flags_RCODE_mask = ~uint16_t{0b1111};
+
+    constexpr uint16_t RCODE_NxDomain = 3;
+    constexpr uint16_t RCODE_ServFail = 2;
+    constexpr uint16_t RCODE_FormErr = 1;
+    constexpr uint16_t RCODE_NoError = 0;
+
+    inline constexpr uint16_t set_rcode(uint16_t flags, uint16_t rcode) { return (flags & flags_RCODE_mask) | rcode; }
+
+}  // namespace srouter::dns