Skip to content

Commit

Permalink
resolve: Don't install individual servers via resolvconf
Browse files Browse the repository at this point in the history 
The resolvconf implementation provided by systemd via resolvectl strips
everything after the interface name, so each additional server that's
installed replaces the previous one.  And even for other resolvconf
implementations installing them individually doesn't seem necessary as
we track and refcount them anyway.

Closes #1353
  • Loading branch information
@tobiasbrunner
tobiasbrunner committed Dec 19, 2022
1 parent bd6014a commit 17fd304
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 56 deletions.
13 changes: 6 additions & 7 deletions conf/plugins/resolve.opt
View file
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
charon.plugins.resolve.file = /etc/resolv.conf
File where to add DNS server entries if not using resolvconf(8).

charon.plugins.resolve.resolvconf.iface_prefix = lo.ipsec
Prefix used for interface names sent to resolvconf(8).
charon.plugins.resolve.resolvconf.iface = lo.ipsec
Interface name/protocol sent to resolvconf(8).

Prefix used for interface names sent to **resolvconf**(8). The nameserver
address is appended to this prefix to make it unique. The result has to be
a valid interface name according to the rules defined by resolvconf. Also,
it should have a high priority according to the order defined in
**interface-order**(5).
The interface name and protocol sent to **resolvconf**(8). This has to be a
valid interface name according to the rules defined by resolvconf. Also, it
should have a high priority according to the order defined in
**interface-order**(5) if relevant on the system.

charon.plugins.resolve.resolvconf.path = /sbin/resolvconf
Path/command for resolvconf(8).
Expand Down
93 changes: 44 additions & 49 deletions src/libcharon/plugins/resolve/resolve_handler.c
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2012-2016 Tobias Brunner
* Copyright (C) 2012-2022 Tobias Brunner
* Copyright (C) 2009 Martin Willi
*
* Copyright (C) secunet Security Networks AG
Expand Down Expand Up @@ -29,8 +29,8 @@
/* path to resolvconf executable */
#define RESOLVCONF_EXEC "/sbin/resolvconf"

/* default prefix used for resolvconf interfaces (should have high prio) */
#define RESOLVCONF_PREFIX "lo.ipsec"
/* default interface/protocol used for resolvconf (should have high prio) */
#define RESOLVCONF_IFACE "lo.ipsec"

typedef struct private_resolve_handler_t private_resolve_handler_t;

Expand All @@ -55,9 +55,9 @@ struct private_resolve_handler_t {
char *resolvconf;

/**
* Prefix to be used for interface names sent to resolvconf
* Interface name sent to resolvconf
*/
char *iface_prefix;
char *iface;

/**
* Mutex to access file exclusively
Expand Down Expand Up @@ -184,39 +184,33 @@ static void remove_nameserver(private_resolve_handler_t *this, host_t *addr)
}

/**
* Add or remove the given nameserver by invoking resolvconf.
* Install the given nameservers by invoking resolvconf. If the array is empty,
* remove the config.
*/
static bool invoke_resolvconf(private_resolve_handler_t *this, host_t *addr,
bool install)
static bool invoke_resolvconf(private_resolve_handler_t *this, array_t *servers)
{
process_t *process;
dns_server_t *dns;
FILE *shell;
char buf[BUF_LEN];
int in, out, retval;

if (snprintf(buf, sizeof(buf), "%H", addr) >= sizeof(buf))
{
return FALSE;
}
translate(buf, ".:", "__");

/* we use the nameserver's IP address as part of the interface name to
* make them unique */
process = process_start_shell(NULL, install ? &in : NULL, &out, NULL,
"2>&1 %s %s %s%s", this->resolvconf,
install ? "-a" : "-d", this->iface_prefix, buf);
int in, out, retval, i;

process = process_start_shell(NULL, array_count(servers) ? &in : NULL, &out,
NULL, "2>&1 %s %s %s", this->resolvconf,
array_count(servers) ? "-a" : "-d", this->iface);
if (!process)
{
return FALSE;
}
if (install)
if (array_count(servers))
{
shell = fdopen(in, "w");
if (shell)
{
DBG1(DBG_IKE, "installing DNS server %H via resolvconf", addr);
fprintf(shell, "nameserver %H\n", addr);
for (i = 0; i < array_count(servers); i++)
{
array_get(servers, i, &dns);
fprintf(shell, "nameserver %H\n", dns->server);
}
fclose(shell);
}
else
Expand All @@ -229,7 +223,7 @@ static bool invoke_resolvconf(private_resolve_handler_t *this, host_t *addr,
}
else
{
DBG1(DBG_IKE, "removing DNS server %H via resolvconf", addr);
DBG1(DBG_IKE, "removing DNS servers via resolvconf");
}
shell = fdopen(out, "r");
if (shell)
Expand Down Expand Up @@ -262,15 +256,7 @@ static bool invoke_resolvconf(private_resolve_handler_t *this, host_t *addr,
{
close(out);
}
if (!process->wait(process, &retval) || retval != EXIT_SUCCESS)
{
if (install)
{ /* revert changes when installing fails */
invoke_resolvconf(this, addr, FALSE);
return FALSE;
}
}
return TRUE;
return process->wait(process, &retval) && retval == EXIT_SUCCESS;
}

METHOD(attribute_handler_t, handle, bool,
Expand Down Expand Up @@ -302,22 +288,27 @@ METHOD(attribute_handler_t, handle, bool,
this->mutex->lock(this->mutex);
if (array_bsearch(this->servers, addr, dns_server_find, &found) == -1)
{
INIT(found,
.server = addr->clone(addr),
.refcount = 1,
);
array_insert_create(&this->servers, ARRAY_TAIL, found);
array_sort(this->servers, dns_server_sort, NULL);

if (this->resolvconf)
{
handled = invoke_resolvconf(this, addr, TRUE);
DBG1(DBG_IKE, "installing DNS server %H via resolvconf", addr);
handled = invoke_resolvconf(this, this->servers);
}
else
{
handled = write_nameserver(this, addr);
}
if (handled)
if (!handled)
{
INIT(found,
.server = addr->clone(addr),
.refcount = 1,
);
array_insert_create(&this->servers, ARRAY_TAIL, found);
array_sort(this->servers, dns_server_sort, NULL);
array_remove(this->servers, ARRAY_TAIL, NULL);
found->server->destroy(found->server);
free(found);
}
}
else
Expand Down Expand Up @@ -369,17 +360,19 @@ METHOD(attribute_handler_t, release, void,
}
else
{
array_remove(this->servers, idx, NULL);
found->server->destroy(found->server);
free(found);

if (this->resolvconf)
{
invoke_resolvconf(this, addr, FALSE);
DBG1(DBG_IKE, "removing DNS server %H via resolvconf", addr);
invoke_resolvconf(this, this->servers);
}
else
{
remove_nameserver(this, addr);
}
array_remove(this->servers, idx, NULL);
found->server->destroy(found->server);
free(found);
}
}
this->mutex->unlock(this->mutex);
Expand Down Expand Up @@ -495,9 +488,11 @@ resolve_handler_t *resolve_handler_create()
.resolvconf = lib->settings->get_str(lib->settings,
"%s.plugins.resolve.resolvconf.path",
NULL, lib->ns),
.iface_prefix = lib->settings->get_str(lib->settings,
.iface = lib->settings->get_str(lib->settings,
"%s.plugins.resolve.resolvconf.iface",
lib->settings->get_str(lib->settings,
"%s.plugins.resolve.resolvconf.iface_prefix",
RESOLVCONF_PREFIX, lib->ns),
RESOLVCONF_IFACE, lib->ns), lib->ns),
);

if (!this->resolvconf && stat(RESOLVCONF_EXEC, &st) == 0)
Expand Down

0 comments on commit 17fd304

Please sign in to comment.