DNS message parsing fixes and cleanup

Fixes:

- tighten reserved name detection to not match fooloki.loki, but instead
  only match "foo.loki.loki" and "loki.loki" (and similar for reserved
  name "snode.loki").
- IPv6 PTR parsing was completely broken.
- Added tests for the above two issues.

Cleanups:

- Eliminate llarp::dns::Name_t typedef for std::string
- Use optional return instead of bool + output param
- Use string_views; we were doing a *lot* of string substr's during
  parsing, each of which allocates a new string.
- Use fmt instead of stringstream
- Simplify IPv4 PTR parsing
pull/1962/head
Jason Rhinelander 2 years ago
parent 12653d4ac2
commit 9ea82edc07
No known key found for this signature in database
GPG Key ID: C4992CE7A88D4262

@ -218,7 +218,7 @@ namespace llarp
rec.ttl = ttl; rec.ttl = ttl;
std::array<byte_t, 512> tmp = {{0}}; std::array<byte_t, 512> tmp = {{0}};
llarp_buffer_t buf(tmp); llarp_buffer_t buf(tmp);
if (EncodeName(&buf, name)) if (EncodeNameTo(&buf, name))
{ {
buf.sz = buf.cur - buf.base; buf.sz = buf.cur - buf.base;
rec.rData.resize(buf.sz); rec.rData.resize(buf.sz);
@ -243,7 +243,7 @@ namespace llarp
rec.ttl = ttl; rec.ttl = ttl;
std::array<byte_t, 512> tmp = {{0}}; std::array<byte_t, 512> tmp = {{0}};
llarp_buffer_t buf(tmp); llarp_buffer_t buf(tmp);
if (EncodeName(&buf, name)) if (EncodeNameTo(&buf, name))
{ {
buf.sz = buf.cur - buf.base; buf.sz = buf.cur - buf.base;
rec.rData.resize(buf.sz); rec.rData.resize(buf.sz);
@ -268,7 +268,7 @@ namespace llarp
rec.ttl = ttl; rec.ttl = ttl;
std::array<byte_t, 512> tmp = {{0}}; std::array<byte_t, 512> tmp = {{0}};
llarp_buffer_t buf(tmp); llarp_buffer_t buf(tmp);
if (EncodeName(&buf, name)) if (EncodeNameTo(&buf, name))
{ {
buf.sz = buf.cur - buf.base; buf.sz = buf.cur - buf.base;
rec.rData.resize(buf.sz); rec.rData.resize(buf.sz);
@ -294,7 +294,7 @@ namespace llarp
std::array<byte_t, 512> tmp = {{0}}; std::array<byte_t, 512> tmp = {{0}};
llarp_buffer_t buf(tmp); llarp_buffer_t buf(tmp);
buf.put_uint16(priority); buf.put_uint16(priority);
if (EncodeName(&buf, name)) if (EncodeNameTo(&buf, name))
{ {
buf.sz = buf.cur - buf.base; buf.sz = buf.cur - buf.base;
rec.rData.resize(buf.sz); rec.rData.resize(buf.sz);
@ -346,7 +346,7 @@ namespace llarp
target = srv.target; target = srv.target;
} }
if (not EncodeName(&buf, target)) if (not EncodeNameTo(&buf, target))
{ {
AddNXReply(); AddNXReply();
return; return;

@ -1,22 +1,20 @@
#define __USE_MINGW_ANSI_STDIO 1
#include "name.hpp" #include "name.hpp"
#include <llarp/net/net.hpp> #include <llarp/net/net.hpp>
#include <llarp/net/ip.hpp> #include <llarp/net/ip.hpp>
#include <llarp/util/str.hpp> #include <llarp/util/str.hpp>
#include <oxenc/hex.h>
#include <algorithm>
#include <sstream>
namespace llarp namespace llarp
{ {
namespace dns namespace dns
{ {
bool std::optional<std::string>
DecodeName(llarp_buffer_t* buf, Name_t& name, bool trimTrailingDot) DecodeName(llarp_buffer_t* buf, bool trimTrailingDot)
{ {
if (buf->size_left() < 1) if (buf->size_left() < 1)
return false; return std::nullopt;
std::stringstream ss; auto result = std::make_optional<std::string>();
auto& name = *result;
size_t l; size_t l;
do do
{ {
@ -25,31 +23,26 @@ namespace llarp
if (l) if (l)
{ {
if (buf->size_left() < l) if (buf->size_left() < l)
return false; return std::nullopt;
ss << Name_t((const char*)buf->cur, l); name.append((const char*)buf->cur, l);
ss << "."; name += '.';
} }
buf->cur = buf->cur + l; buf->cur = buf->cur + l;
} while (l); } while (l);
name = ss.str();
/// trim off last dot /// trim off last dot
if (trimTrailingDot) if (trimTrailingDot)
name = name.substr(0, name.find_last_of('.')); name.pop_back();
return true; return result;
} }
bool bool
EncodeName(llarp_buffer_t* buf, Name_t name) EncodeNameTo(llarp_buffer_t* buf, std::string_view name)
{ {
std::stringstream ss; if (name.size() && name.back() == '.')
if (name.size() && name[name.size() - 1] == '.') name.remove_suffix(1);
ss << name.substr(0, name.size() - 1);
else
ss << name;
std::string part; for (auto part : llarp::split(name, "."))
while (std::getline(ss, part, '.'))
{ {
size_t l = part.length(); size_t l = part.length();
if (l > 63) if (l > 63)
@ -60,7 +53,7 @@ namespace llarp
return false; return false;
if (l) if (l)
{ {
memcpy(buf->cur, part.data(), l); std::memcpy(buf->cur, part.data(), l);
buf->cur += l; buf->cur += l;
} }
else else
@ -71,8 +64,8 @@ namespace llarp
return true; return true;
} }
bool std::optional<huint128_t>
DecodePTR(Name_t name, huint128_t& ip) DecodePTR(std::string_view name)
{ {
bool isV6 = false; bool isV6 = false;
auto pos = name.find(".in-addr.arpa"); auto pos = name.find(".in-addr.arpa");
@ -82,56 +75,62 @@ namespace llarp
isV6 = true; isV6 = true;
} }
if (pos == std::string::npos) if (pos == std::string::npos)
return false; return std::nullopt;
std::string sub = name.substr(0, pos + 1); name = name.substr(0, pos + 1);
const auto numdots = std::count(sub.begin(), sub.end(), '.'); const auto numdots = std::count(name.begin(), name.end(), '.');
if (numdots == 4 && !isV6) if (numdots == 4 && !isV6)
{ {
uint8_t a, b, c, d; std::array<uint8_t, 4> q;
pos = sub.find('.'); for (int i = 3; i >= 0; i--)
d = atoi(sub.substr(0, pos).c_str()); {
sub = sub.substr(pos + 1); pos = name.find('.');
pos = sub.find('.'); if (!llarp::parse_int(name.substr(0, pos), q[i]))
c = atoi(sub.substr(0, pos).c_str()); return std::nullopt;
sub = sub.substr(pos + 1); name.remove_prefix(pos + 1);
pos = sub.find('.'); }
b = atoi(sub.substr(0, pos).c_str()); return net::ExpandV4(llarp::ipaddr_ipv4_bits(q[0], q[1], q[2], q[3]));
sub = sub.substr(pos + 1);
pos = sub.find('.');
a = atoi(sub.substr(0, pos).c_str());
ip = net::ExpandV4(llarp::ipaddr_ipv4_bits(a, b, c, d));
return true;
} }
if (numdots == 32 && isV6) if (numdots == 32 && name.size() == 64 && isV6)
{ {
size_t idx = 0; // We're going to convert from nybbles a.b.c.d.e.f.0.1.2.3.[...] into hex string
uint8_t lo, hi; // "badcfe1032...", then decode the hex string to bytes.
auto* ptr = (uint8_t*)&ip.h; std::array<char, 32> in;
while (idx < 16) auto in_pos = in.data();
for (size_t i = 0; i < 64; i += 4)
{ {
pos = sub.find('.'); if (not(oxenc::is_hex_digit(name[i]) and name[i + 1] == '.'
lo = (*sub.substr(0, pos).c_str()) - 'a'; and oxenc::is_hex_digit(name[i + 2]) and name[i + 3] == '.'))
sub = sub.substr(pos + 1); return std::nullopt;
pos = sub.find('.');
hi = (*sub.substr(0, pos).c_str()) - 'a'; // Flip the nybbles because the smallest one is first
sub = sub.substr(pos + 1); *in_pos++ = name[i + 2];
ptr[idx] = lo | (hi << 4); *in_pos++ = name[i];
++idx;
} }
return true; assert(in_pos == in.data() + in.size());
} huint128_t ip;
static_assert(in.size() == 2 * sizeof(ip.h));
// our string right now is the little endian representation, so load it as such on little
// endian, or in reverse on big endian.
if constexpr (oxenc::little_endian)
oxenc::from_hex(in.begin(), in.end(), reinterpret_cast<uint8_t*>(&ip.h));
else
oxenc::from_hex(in.rbegin(), in.rend(), reinterpret_cast<uint8_t*>(&ip.h));
return false; return ip;
}
return std::nullopt;
} }
bool bool
NameIsReserved(Name_t name) NameIsReserved(std::string_view name)
{ {
const std::vector<std::string_view> reserved_names = { const std::vector<std::string_view> reserved_names = {
"snode.loki"sv, "loki.loki"sv, "snode.loki."sv, "loki.loki."sv}; ".snode.loki"sv, ".loki.loki"sv, ".snode.loki."sv, ".loki.loki."sv};
for (const auto& reserved : reserved_names) for (const auto& reserved : reserved_names)
{ {
if (ends_with(name, reserved)) if (ends_with(name, reserved)) // subdomain foo.loki.loki
return true;
if (name == reserved.substr(1)) // loki.loki itself
return true; return true;
} }
return false; return false;

@ -4,26 +4,25 @@
#include <llarp/util/buffer.hpp> #include <llarp/util/buffer.hpp>
#include <string> #include <string>
#include <optional>
namespace llarp namespace llarp
{ {
namespace dns namespace dns
{ {
using Name_t = std::string; /// decode name from buffer; return nullopt on failure
std::optional<std::string>
/// decode name from buffer DecodeName(llarp_buffer_t* buf, bool trimTrailingDot = false);
bool
DecodeName(llarp_buffer_t* buf, Name_t& name, bool trimTrailingDot = false);
/// encode name to buffer /// encode name to buffer
bool bool
EncodeName(llarp_buffer_t* buf, Name_t name); EncodeNameTo(llarp_buffer_t* buf, std::string_view name);
bool std::optional<huint128_t>
DecodePTR(Name_t name, huint128_t& ip); DecodePTR(std::string_view name);
bool bool
NameIsReserved(Name_t name); NameIsReserved(std::string_view name);
} // namespace dns } // namespace dns
} // namespace llarp } // namespace llarp

@ -27,7 +27,7 @@ namespace llarp
bool bool
Question::Encode(llarp_buffer_t* buf) const Question::Encode(llarp_buffer_t* buf) const
{ {
if (!EncodeName(buf, qname)) if (!EncodeNameTo(buf, qname))
return false; return false;
if (!buf->put_uint16(qtype)) if (!buf->put_uint16(qtype))
return false; return false;
@ -37,7 +37,9 @@ namespace llarp
bool bool
Question::Decode(llarp_buffer_t* buf) Question::Decode(llarp_buffer_t* buf)
{ {
if (!DecodeName(buf, qname)) if (auto name = DecodeName(buf))
qname = *std::move(name);
else
{ {
llarp::LogError("failed to decode name"); llarp::LogError("failed to decode name");
return false; return false;

@ -34,7 +34,7 @@ namespace llarp
return qname == other.qname && qtype == other.qtype && qclass == other.qclass; return qname == other.qname && qtype == other.qtype && qclass == other.qclass;
} }
Name_t qname; std::string qname;
QType_t qtype; QType_t qtype;
QClass_t qclass; QClass_t qclass;

@ -24,7 +24,7 @@ namespace llarp
, rData(std::move(other.rData)) , rData(std::move(other.rData))
{} {}
ResourceRecord::ResourceRecord(Name_t name, RRType_t type, RR_RData_t data) ResourceRecord::ResourceRecord(std::string name, RRType_t type, RR_RData_t data)
: rr_name{std::move(name)} : rr_name{std::move(name)}
, rr_type{type} , rr_type{type}
, rr_class{qClassIN} , rr_class{qClassIN}
@ -35,7 +35,7 @@ namespace llarp
bool bool
ResourceRecord::Encode(llarp_buffer_t* buf) const ResourceRecord::Encode(llarp_buffer_t* buf) const
{ {
if (not EncodeName(buf, rr_name)) if (not EncodeNameTo(buf, rr_name))
return false; return false;
if (!buf->put_uint16(rr_type)) if (!buf->put_uint16(rr_type))
{ {
@ -113,12 +113,10 @@ namespace llarp
{ {
if (rr_type != qTypeCNAME) if (rr_type != qTypeCNAME)
return false; return false;
Name_t name;
llarp_buffer_t buf(rData); llarp_buffer_t buf(rData);
if (not DecodeName(&buf, name)) if (auto name = DecodeName(&buf))
return false; return name->rfind(tld) == name->size() - tld.size() - 1;
return name.find(tld) != std::string::npos return false;
&& name.rfind(tld) == (name.size() - tld.size()) - 1;
} }
} // namespace dns } // namespace dns

@ -22,7 +22,7 @@ namespace llarp
ResourceRecord(const ResourceRecord& other); ResourceRecord(const ResourceRecord& other);
ResourceRecord(ResourceRecord&& other); ResourceRecord(ResourceRecord&& other);
explicit ResourceRecord(Name_t name, RRType_t type, RR_RData_t rdata); explicit ResourceRecord(std::string name, RRType_t type, RR_RData_t rdata);
bool bool
Encode(llarp_buffer_t* buf) const override; Encode(llarp_buffer_t* buf) const override;
@ -39,7 +39,7 @@ namespace llarp
bool bool
HasCNameForTLD(const std::string& tld) const; HasCNameForTLD(const std::string& tld) const;
Name_t rr_name; std::string rr_name;
RRType_t rr_type; RRType_t rr_type;
RRClass_t rr_class; RRClass_t rr_class;
RR_TTL_t ttl; RR_TTL_t ttl;

@ -196,10 +196,9 @@ namespace llarp
// always hook ptr for ranges we own // always hook ptr for ranges we own
if (msg.questions[0].qtype == dns::qTypePTR) if (msg.questions[0].qtype == dns::qTypePTR)
{ {
huint128_t ip; if (auto ip = dns::DecodePTR(msg.questions[0].qname))
if (!dns::DecodePTR(msg.questions[0].qname, ip)) return m_OurRange.Contains(*ip);
return false; return false;
return m_OurRange.Contains(ip);
} }
if (msg.questions[0].qtype == dns::qTypeA || msg.questions[0].qtype == dns::qTypeCNAME if (msg.questions[0].qtype == dns::qTypeA || msg.questions[0].qtype == dns::qTypeCNAME
|| msg.questions[0].qtype == dns::qTypeAAAA) || msg.questions[0].qtype == dns::qTypeAAAA)
@ -217,8 +216,8 @@ namespace llarp
{ {
if (msg.questions[0].qtype == dns::qTypePTR) if (msg.questions[0].qtype == dns::qTypePTR)
{ {
huint128_t ip; auto ip = dns::DecodePTR(msg.questions[0].qname);
if (!dns::DecodePTR(msg.questions[0].qname, ip)) if (not ip)
return false; return false;
if (ip == m_IfAddr) if (ip == m_IfAddr)
{ {
@ -227,7 +226,7 @@ namespace llarp
} }
else else
{ {
auto itr = m_IPToKey.find(ip); auto itr = m_IPToKey.find(*ip);
if (itr != m_IPToKey.end() && m_SNodeKeys.find(itr->second) != m_SNodeKeys.end()) if (itr != m_IPToKey.end() && m_SNodeKeys.find(itr->second) != m_SNodeKeys.end())
{ {
RouterID them = itr->second; RouterID them = itr->second;

@ -479,25 +479,25 @@ namespace llarp
const auto& answer = msg.answers[0]; const auto& answer = msg.answers[0];
if (answer.HasCNameForTLD(".snode")) if (answer.HasCNameForTLD(".snode"))
{ {
dns::Name_t qname;
llarp_buffer_t buf(answer.rData); llarp_buffer_t buf(answer.rData);
if (not dns::DecodeName(&buf, qname, true)) auto qname = dns::DecodeName(&buf, true);
if (not qname)
return false; return false;
RouterID addr; RouterID addr;
if (not addr.FromString(qname)) if (not addr.FromString(*qname))
return false; return false;
auto replyMsg = std::make_shared<dns::Message>(clear_dns_message(msg)); auto replyMsg = std::make_shared<dns::Message>(clear_dns_message(msg));
return ReplyToSNodeDNSWhenReady(addr, std::move(replyMsg), false); return ReplyToSNodeDNSWhenReady(addr, std::move(replyMsg), false);
} }
else if (answer.HasCNameForTLD(".loki")) else if (answer.HasCNameForTLD(".loki"))
{ {
dns::Name_t qname;
llarp_buffer_t buf(answer.rData); llarp_buffer_t buf(answer.rData);
if (not dns::DecodeName(&buf, qname, true)) auto qname = dns::DecodeName(&buf, true);
if (not qname)
return false; return false;
service::Address addr; service::Address addr;
if (not addr.FromString(qname)) if (not addr.FromString(*qname))
return false; return false;
auto replyMsg = std::make_shared<dns::Message>(clear_dns_message(msg)); auto replyMsg = std::make_shared<dns::Message>(clear_dns_message(msg));
@ -745,20 +745,16 @@ namespace llarp
else if (msg.questions[0].qtype == dns::qTypePTR) else if (msg.questions[0].qtype == dns::qTypePTR)
{ {
// reverse dns // reverse dns
huint128_t ip = {0}; if (auto ip = dns::DecodePTR(msg.questions[0].qname))
if (!dns::DecodePTR(msg.questions[0].qname, ip))
{ {
msg.AddNXReply(); if (auto maybe = ObtainAddrForIP(*ip))
reply(msg); {
return true; var::visit([&msg](auto&& result) { msg.AddAReply(result.ToString()); }, *maybe);
reply(msg);
return true;
}
} }
if (auto maybe = ObtainAddrForIP(ip))
{
var::visit([&msg](auto&& result) { msg.AddAReply(result.ToString()); }, *maybe);
reply(msg);
return true;
}
msg.AddNXReply(); msg.AddNXReply();
reply(msg); reply(msg);
return true; return true;
@ -816,10 +812,9 @@ namespace llarp
// hook any ranges we own // hook any ranges we own
if (msg.questions[0].qtype == llarp::dns::qTypePTR) if (msg.questions[0].qtype == llarp::dns::qTypePTR)
{ {
huint128_t ip = {0}; if (auto ip = dns::DecodePTR(msg.questions[0].qname))
if (!dns::DecodePTR(msg.questions[0].qname, ip)) return m_OurRange.Contains(*ip);
return false; return false;
return m_OurRange.Contains(ip);
} }
} }
for (const auto& answer : msg.answers) for (const auto& answer : msg.answers)

@ -90,11 +90,19 @@ TEST_CASE("Test Get Subdomains" , "[dns]")
TEST_CASE("Test PTR records", "[dns]") TEST_CASE("Test PTR records", "[dns]")
{ {
llarp::huint128_t ip = {0};
llarp::huint128_t expected = llarp::huint128_t expected =
llarp::net::ExpandV4(llarp::ipaddr_ipv4_bits(10, 10, 10, 1)); llarp::net::ExpandV4(llarp::ipaddr_ipv4_bits(10, 10, 10, 1));
CHECK(llarp::dns::DecodePTR("1.10.10.10.in-addr.arpa.", ip)); auto ip = llarp::dns::DecodePTR("1.10.10.10.in-addr.arpa.");
CHECK(ip == expected); CHECK(ip);
CHECK(*ip == expected);
expected.h.upper = 0x0123456789abcdefUL;
expected.h.lower = 0xeeee888812341234UL;
ip = llarp::dns::DecodePTR("4.3.2.1.4.3.2.1.8.8.8.8.e.e.e.e.f.e.d.c.b.a.9.8.7.6.5.4.3.2.1.0.ip6.arpa.");
CHECK(ip);
CHECK(oxenc::to_hex(std::string_view{reinterpret_cast<char*>(&expected.h), 16}) ==
oxenc::to_hex(std::string_view{reinterpret_cast<char*>(&ip->h), 16}));
CHECK(*ip == expected);
} }
TEST_CASE("Test Serialize Header", "[dns]") TEST_CASE("Test Serialize Header", "[dns]")
@ -123,13 +131,12 @@ TEST_CASE("Test Serialize Header", "[dns]")
TEST_CASE("Test Serialize Name" , "[dns]") TEST_CASE("Test Serialize Name" , "[dns]")
{ {
const llarp::dns::Name_t name = "whatever.tld"; const std::string name = "whatever.tld";
const llarp::dns::Name_t expected = "whatever.tld."; const std::string expected = "whatever.tld.";
llarp::dns::Name_t other;
std::array<byte_t, 1500> data{}; std::array<byte_t, 1500> data{};
llarp_buffer_t buf(data); llarp_buffer_t buf(data);
CHECK(llarp::dns::EncodeName(&buf, name)); CHECK(llarp::dns::EncodeNameTo(&buf, name));
buf.cur = buf.base; buf.cur = buf.base;
@ -147,8 +154,9 @@ TEST_CASE("Test Serialize Name" , "[dns]")
CHECK(buf.base[11] == 'l'); CHECK(buf.base[11] == 'l');
CHECK(buf.base[12] == 'd'); CHECK(buf.base[12] == 'd');
CHECK(buf.base[13] == 0); CHECK(buf.base[13] == 0);
CHECK(llarp::dns::DecodeName(&buf, other)); auto other = llarp::dns::DecodeName(&buf);
CHECK(expected == other); CHECK(other);
CHECK(expected == *other);
} }
TEST_CASE("Test serialize question", "[dns]") TEST_CASE("Test serialize question", "[dns]")
@ -191,3 +199,20 @@ TEST_CASE("Test Encode/Decode RData" , "[dns]")
CHECK(llarp::dns::DecodeRData(&buf, other_rdata)); CHECK(llarp::dns::DecodeRData(&buf, other_rdata));
CHECK(rdata == other_rdata); CHECK(rdata == other_rdata);
} }
TEST_CASE("Test reserved names", "[dns]")
{
using namespace llarp::dns;
CHECK(NameIsReserved("loki.loki"));
CHECK(NameIsReserved("loki.loki."));
CHECK(NameIsReserved("snode.loki"));
CHECK(NameIsReserved("snode.loki."));
CHECK(NameIsReserved("foo.loki.loki"));
CHECK(NameIsReserved("foo.loki.loki."));
CHECK(NameIsReserved("bar.snode.loki"));
CHECK(NameIsReserved("bar.snode.loki."));
CHECK_FALSE(NameIsReserved("barsnode.loki."));
CHECK_FALSE(NameIsReserved("barsnode.loki"));
CHECK_FALSE(NameIsReserved("alltheloki.loki"));
CHECK_FALSE(NameIsReserved("alltheloki.loki."));
}

Loading…
Cancel
Save