Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

posix: multiple resolv.conf entries, avoid ipv6 failure on dns functions #405

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

posix: multiple resolv.conf entries, avoid ipv6 failure on dns functions #405

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d47064e
posix: multiple resolv.conf entries, avoid ipv6 failure on dns functions
AtieP Apr 9, 2022
2ac2797
posix: missing break on get_nameserver()
AtieP Apr 9, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 44 additions & 8 deletions options/posix/generic/lookup.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ namespace {
constexpr unsigned int RECORD_A = 1;
constexpr unsigned int RECORD_CNAME = 5;
constexpr unsigned int RECORD_PTR = 12;

struct nameserver_data default_nameserver("127.0.0.1", AF_INET);
}

static frg::string<MemoryAllocator> read_dns_name(char *buf, char *&it) {
Expand Down Expand Up @@ -86,10 +88,27 @@ int lookup_name_dns(struct lookup_result &buf, const char *name,
// for dns
sin.sin_port = htons(53);

auto nameserver = get_nameserver();
if (!inet_aton(nameserver ? nameserver->name.data() : "127.0.0.1", &sin.sin_addr)) {
mlibc::infoLogger() << "lookup_name_dns(): inet_aton() failed!" << frg::endlog;
return -EAI_SYSTEM;
int nameserver_idx = 0;
while (true) {
auto nameserver = get_nameserver(nameserver_idx++);
if (!nameserver) {
// no more nameservers in resolf.conf,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another style note: comments should be formally written. So with proper punctuation and capitalization.

// use default
if (!inet_pton(default_nameserver.af, default_nameserver.name.data(), &sin.sin_addr)) {
mlibc::infoLogger() << "lookup_name_dns(): no nameservers available!" << frg::endlog;
return -EAI_SYSTEM;
}
break;
}

// TODO: ipv6
if (nameserver->af != AF_INET)
continue;

if (!inet_pton(nameserver->af, nameserver->name.data(), &sin.sin_addr))
continue;

break;
}

int fd = socket(AF_INET, SOCK_DGRAM, 0);
Expand Down Expand Up @@ -215,10 +234,27 @@ int lookup_addr_dns(frg::span<char> name, frg::array<uint8_t, 16> &addr, int fam
// for dns
sin.sin_port = htons(53);

auto nameserver = get_nameserver();
if (!inet_aton(nameserver ? nameserver->name.data() : "127.0.0.1", &sin.sin_addr)) {
mlibc::infoLogger() << "lookup_name_dns(): inet_aton() failed!" << frg::endlog;
return -EAI_SYSTEM;
int nameserver_idx = 0;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like this routine can be split up into a function.

while (true) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also this while loop doesn't need to be a while(true) loop, you can also write while(nameserver = get_nameserver()).

auto nameserver = get_nameserver(nameserver_idx++);
if (!nameserver) {
// no more nameservers in resolf.conf,
// use default
if (!inet_pton(default_nameserver.af, default_nameserver.name.data(), &sin.sin_addr)) {
mlibc::infoLogger() << "lookup_addr_dns(): no nameservers available!" << frg::endlog;
return -EAI_SYSTEM;
}
break;
}

// TODO: ipv6
if (nameserver->af != AF_INET)
continue;

if (!inet_pton(nameserver->af, nameserver->name.data(), &sin.sin_addr))
continue;

break;
}

int fd = socket(AF_INET, SOCK_DGRAM, 0);
Expand Down
27 changes: 23 additions & 4 deletions options/posix/generic/resolv_conf.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,20 @@
#include <mlibc/allocator.hpp>
#include <stdio.h>
#include <ctype.h>
#include <arpa/inet.h>
#include <netinet/in.h>
#include <sys/socket.h>

namespace mlibc {

frg::optional<struct nameserver_data> get_nameserver() {
frg::optional<struct nameserver_data> get_nameserver(int idx) {
auto file = fopen("/etc/resolv.conf", "r");
if (!file)
return frg::null_opt;

char line[128];
struct nameserver_data ret;
int cur = 0;
while (fgets(line, 128, file)) {
char *pos;
if (!strchr(line, '\n') && !feof(file)) {
Expand All @@ -20,15 +24,30 @@ frg::optional<struct nameserver_data> get_nameserver() {
continue;
}

// TODO(geert): resolv.conf can actually have multiple nameservers
// but we just pick the first one for now
if (!strncmp(line, "nameserver", 10) && isspace(line[10])) {
if (cur != idx) {
cur++;
continue;
}

char *end;
for (pos = line + 11; isspace(*pos); pos++);
for (end = pos; *end && !isspace(*end); end++);
*end = '\0';
ret.name = frg::string<MemoryAllocator>(
auto str = frg::string<MemoryAllocator>(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this change?

pos, end - pos, getAllocator());


struct in_addr ipv4;
struct in6_addr ipv6;
if (inet_pton(AF_INET, str.data(), &ipv4))
ret.af = AF_INET;
else if (inet_pton(AF_INET6, str.data(), &ipv6))
ret.af = AF_INET6;
else
break;

ret.name = str;
break;
}
}
Expand Down
6 changes: 5 additions & 1 deletion options/posix/include/mlibc/resolv_conf.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,15 @@ namespace mlibc {
struct nameserver_data {
nameserver_data()
: name(getAllocator()) {}
nameserver_data(const char *name_, int af_)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a really minor nitpick, but we don't do this in the managarm code style, nor is it necessary.

: name(name_, getAllocator()), af(af_) {}

frg::string<MemoryAllocator> name;
int af;
// for in the future we can also store options here
};

frg::optional<struct nameserver_data> get_nameserver();
frg::optional<struct nameserver_data> get_nameserver(int idx);

} // namespace mlibc

Expand Down