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

Jump to bottom

Bug fixes #484

Merged
merged 4 commits into from
Jun 27, 2024
Merged

Bug fixes #484

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ba21cfe
libnetplan: use more restrictive file permissions
daniloegea May 22, 2024
ee1703f
libnetplan: escape control characters
daniloegea May 29, 2024
85f1a6c
backends: escape file paths
daniloegea May 23, 2024
36010fa
backends: escape semicolons in service units
daniloegea May 24, 2024
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
95 changes: 48 additions & 47 deletions src/networkd.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,6 @@ STATIC void
write_link_file(const NetplanNetDefinition* def, const char* rootdir, const char* path)
{
GString* s = NULL;
mode_t orig_umask;

/* Don't write .link files for virtual devices; they use .netdev instead.
* Don't write .link files for MODEM devices, as they aren't supported by networkd.
Expand Down Expand Up @@ -374,9 +373,7 @@ write_link_file(const NetplanNetDefinition* def, const char* rootdir, const char
g_string_append_printf(s, "LargeReceiveOffload=%s\n",
(def->large_receive_offload ? "true" : "false"));

orig_umask = umask(022);
_netplan_g_string_free_to_file(s, rootdir, path, ".link");
umask(orig_umask);
_netplan_g_string_free_to_file_with_permissions(s, rootdir, path, ".link", "root", "root", 0640);
}

STATIC gboolean
Expand All @@ -394,7 +391,10 @@ write_regdom(const NetplanNetDefinition* def, const char* rootdir, GError** erro
g_string_append(s, "\n[Service]\nType=oneshot\n");
g_string_append_printf(s, "ExecStart="SBINDIR"/iw reg set %s\n", def->regulatory_domain);

_netplan_g_string_free_to_file(s, rootdir, path, NULL);
g_autofree char* new_s = _netplan_scrub_systemd_unit_contents(s->str);
g_string_free(s, TRUE);
s = g_string_new(new_s);
_netplan_g_string_free_to_file_with_permissions(s, rootdir, path, NULL, "root", "root", 0640);
_netplan_safe_mkdir_p_dir(link);
if (symlink(path, link) < 0 && errno != EEXIST) {
// LCOV_EXCL_START
Expand Down Expand Up @@ -586,7 +586,6 @@ STATIC void
write_netdev_file(const NetplanNetDefinition* def, const char* rootdir, const char* path)
{
GString* s = NULL;
mode_t orig_umask;

g_assert(def->type >= NETPLAN_DEF_TYPE_VIRTUAL);

Expand Down Expand Up @@ -685,11 +684,7 @@ write_netdev_file(const NetplanNetDefinition* def, const char* rootdir, const ch
default: g_assert_not_reached(); // LCOV_EXCL_LINE
}

/* these do not contain secrets and need to be readable by
* systemd-networkd - LP: #1736965 */
orig_umask = umask(022);
_netplan_g_string_free_to_file(s, rootdir, path, ".netdev");
umask(orig_umask);
_netplan_g_string_free_to_file_with_permissions(s, rootdir, path, ".netdev", "root", NETWORKD_GROUP, 0640);
}

STATIC void
Expand Down Expand Up @@ -833,7 +828,6 @@ _netplan_netdef_write_network_file(
g_autoptr(GString) network = NULL;
g_autoptr(GString) link = NULL;
GString* s = NULL;
mode_t orig_umask;

SET_OPT_OUT_PTR(has_been_written, FALSE);

Expand Down Expand Up @@ -1099,11 +1093,7 @@ _netplan_netdef_write_network_file(
if (network->len > 0)
g_string_append_printf(s, "\n[Network]\n%s", network->str);

/* these do not contain secrets and need to be readable by
* systemd-networkd - LP: #1736965 */
orig_umask = umask(022);
_netplan_g_string_free_to_file(s, rootdir, path, ".network");
umask(orig_umask);
_netplan_g_string_free_to_file_with_permissions(s, rootdir, path, ".network", "root", NETWORKD_GROUP, 0640);
}

SET_OPT_OUT_PTR(has_been_written, TRUE);
Expand All @@ -1114,8 +1104,8 @@ STATIC void
write_rules_file(const NetplanNetDefinition* def, const char* rootdir)
{
GString* s = NULL;
g_autofree char* path = g_strjoin(NULL, "run/udev/rules.d/99-netplan-", def->id, ".rules", NULL);
mode_t orig_umask;
g_autofree char* escaped_netdef_id = g_uri_escape_string(def->id, NULL, TRUE);
g_autofree char* path = g_strjoin(NULL, "run/udev/rules.d/99-netplan-", escaped_netdef_id, ".rules", NULL);

/* do we need to write a .rules file?
* It's only required for reliably setting the name of a physical device
Expand All @@ -1139,19 +1129,19 @@ write_rules_file(const NetplanNetDefinition* def, const char* rootdir)
g_string_append(s, "SUBSYSTEM==\"net\", ACTION==\"add\", ");

if (def->match.driver) {
g_string_append_printf(s,"DRIVERS==\"%s\", ", def->match.driver);
g_autofree char* driver = _netplan_scrub_string(def->match.driver);
g_string_append_printf(s,"DRIVERS==\"%s\", ", driver);
} else {
g_string_append(s, "DRIVERS==\"?*\", ");
}

if (def->match.mac)
g_string_append_printf(s, "ATTR{address}==\"%s\", ", def->match.mac);

g_string_append_printf(s, "NAME=\"%s\"\n", def->set_name);
g_autofree char* set_name = _netplan_scrub_string(def->set_name);
g_string_append_printf(s, "NAME=\"%s\"\n", set_name);

orig_umask = umask(022);
_netplan_g_string_free_to_file(s, rootdir, path, NULL);
umask(orig_umask);
_netplan_g_string_free_to_file_with_permissions(s, rootdir, path, NULL, "root", "root", 0640);
}

STATIC gboolean
Expand Down Expand Up @@ -1237,10 +1227,12 @@ append_wpa_auth_conf(GString* s, const NetplanAuthenticationSettings* auth, cons
}

if (auth->identity) {
g_string_append_printf(s, " identity=\"%s\"\n", auth->identity);
g_autofree char* identity = _netplan_scrub_string(auth->identity);
g_string_append_printf(s, " identity=\"%s\"\n", identity);
}
if (auth->anonymous_identity) {
g_string_append_printf(s, " anonymous_identity=\"%s\"\n", auth->anonymous_identity);
g_autofree char* anonymous_identity = _netplan_scrub_string(auth->anonymous_identity);
g_string_append_printf(s, " anonymous_identity=\"%s\"\n", anonymous_identity);
}

char* psk = NULL;
Expand Down Expand Up @@ -1278,19 +1270,23 @@ append_wpa_auth_conf(GString* s, const NetplanAuthenticationSettings* auth, cons
}
}
if (auth->ca_certificate) {
g_string_append_printf(s, " ca_cert=\"%s\"\n", auth->ca_certificate);
g_autofree char* ca_certificate = _netplan_scrub_string(auth->ca_certificate);
g_string_append_printf(s, " ca_cert=\"%s\"\n", ca_certificate);
}
if (auth->client_certificate) {
g_string_append_printf(s, " client_cert=\"%s\"\n", auth->client_certificate);
g_autofree char* client_certificate = _netplan_scrub_string(auth->client_certificate);
g_string_append_printf(s, " client_cert=\"%s\"\n", client_certificate);
}
if (auth->client_key) {
g_string_append_printf(s, " private_key=\"%s\"\n", auth->client_key);
g_autofree char* client_key = _netplan_scrub_string(auth->client_key);
g_string_append_printf(s, " private_key=\"%s\"\n", client_key);
}
if (auth->client_key_password) {
g_string_append_printf(s, " private_key_passwd=\"%s\"\n", auth->client_key_password);
}
if (auth->phase2_auth) {
g_string_append_printf(s, " phase2=\"auth=%s\"\n", auth->phase2_auth);
g_autofree char* phase2_auth = _netplan_scrub_string(auth->phase2_auth);
g_string_append_printf(s, " phase2=\"auth=%s\"\n", phase2_auth);
}
return TRUE;
}
Expand All @@ -1300,7 +1296,6 @@ STATIC void
write_wpa_unit(const NetplanNetDefinition* def, const char* rootdir)
{
g_autofree gchar *stdouth = NULL;
mode_t orig_umask;

stdouth = systemd_escape(def->id);

Expand All @@ -1319,18 +1314,20 @@ write_wpa_unit(const NetplanNetDefinition* def, const char* rootdir)
} else {
g_string_append(s, " -Dnl80211,wext\n");
}
orig_umask = umask(022);
_netplan_g_string_free_to_file(s, rootdir, path, NULL);
umask(orig_umask);

g_autofree char* new_s = _netplan_scrub_systemd_unit_contents(s->str);
g_string_free(s, TRUE);
s = g_string_new(new_s);
_netplan_g_string_free_to_file_with_permissions(s, rootdir, path, NULL, "root", "root", 0640);
}

STATIC gboolean
write_wpa_conf(const NetplanNetDefinition* def, const char* rootdir, GError** error)
{
GHashTableIter iter;
GString* s = g_string_new("ctrl_interface=/run/wpa_supplicant\n\n");
g_autofree char* path = g_strjoin(NULL, "run/netplan/wpa-", def->id, ".conf", NULL);
mode_t orig_umask;
g_autofree char* escaped_netdef_id = g_uri_escape_string(def->id, NULL, TRUE);
g_autofree char* path = g_strjoin(NULL, "run/netplan/wpa-", escaped_netdef_id, ".conf", NULL);

g_debug("%s: Creating wpa_supplicant configuration file %s", def->id, path);
if (def->type == NETPLAN_DEF_TYPE_WIFI) {
Expand All @@ -1347,14 +1344,16 @@ write_wpa_conf(const NetplanNetDefinition* def, const char* rootdir, GError** er
}
/* available as of wpa_supplicant version 0.6.7 */
if (def->regulatory_domain) {
g_string_append_printf(s, "country=%s\n", def->regulatory_domain);
g_autofree char* regdom = _netplan_scrub_string(def->regulatory_domain);
g_string_append_printf(s, "country=%s\n", regdom);
}
NetplanWifiAccessPoint* ap;
g_hash_table_iter_init(&iter, def->access_points);
while (g_hash_table_iter_next(&iter, NULL, (gpointer) &ap)) {
gchar* freq_config_str = ap->mode == NETPLAN_WIFI_MODE_ADHOC ? "frequency" : "freq_list";
g_autofree char* ssid = _netplan_scrub_string(ap->ssid);

g_string_append_printf(s, "network={\n ssid=\"%s\"\n", ap->ssid);
g_string_append_printf(s, "network={\n ssid=P\"%s\"\n", ssid);
if (ap->bssid) {
g_string_append_printf(s, " bssid=%s\n", ap->bssid);
}
Expand Down Expand Up @@ -1401,7 +1400,7 @@ write_wpa_conf(const NetplanNetDefinition* def, const char* rootdir, GError** er

/* wifi auth trumps netdef auth */
if (ap->has_auth) {
if (!append_wpa_auth_conf(s, &ap->auth, ap->ssid, error)) {
if (!append_wpa_auth_conf(s, &ap->auth, ssid, error)) {
g_string_free(s, TRUE);
return FALSE;
}
Expand All @@ -1423,9 +1422,7 @@ write_wpa_conf(const NetplanNetDefinition* def, const char* rootdir, GError** er
}

/* use tight permissions as this contains secrets */
orig_umask = umask(077);
_netplan_g_string_free_to_file(s, rootdir, path, NULL);
umask(orig_umask);
_netplan_g_string_free_to_file_with_permissions(s, rootdir, path, NULL, "root", "root", 0600);
return TRUE;
}

Expand All @@ -1446,7 +1443,8 @@ _netplan_netdef_write_networkd(
GError** error)
{
/* TODO: make use of netplan_netdef_get_output_filename() */
g_autofree char* path_base = g_strjoin(NULL, "run/systemd/network/10-netplan-", def->id, NULL);
g_autofree char* escaped_netdef_id = g_uri_escape_string(def->id, NULL, TRUE);
g_autofree char* path_base = g_strjoin(NULL, "run/systemd/network/10-netplan-", escaped_netdef_id, NULL);
SET_OPT_OUT_PTR(has_been_written, FALSE);

/* We want this for all backends when renaming, as *.link and *.rules files are
Expand All @@ -1468,8 +1466,8 @@ _netplan_netdef_write_networkd(
}

if (def->type == NETPLAN_DEF_TYPE_WIFI || def->has_auth) {
g_autofree char* link = g_strjoin(NULL, rootdir ?: "", "/run/systemd/system/systemd-networkd.service.wants/netplan-wpa-", def->id, ".service", NULL);
g_autofree char* slink = g_strjoin(NULL, "/run/systemd/system/netplan-wpa-", def->id, ".service", NULL);
g_autofree char* link = g_strjoin(NULL, rootdir ?: "", "/run/systemd/system/systemd-networkd.service.wants/netplan-wpa-", escaped_netdef_id, ".service", NULL);
g_autofree char* slink = g_strjoin(NULL, "/run/systemd/system/netplan-wpa-", escaped_netdef_id, ".service", NULL);
if (def->type == NETPLAN_DEF_TYPE_WIFI && def->has_match) {
g_set_error(error, NETPLAN_BACKEND_ERROR, NETPLAN_ERROR_UNSUPPORTED, "ERROR: %s: networkd backend does not support wifi with match:, only by interface name\n", def->id);
return FALSE;
Expand Down Expand Up @@ -1569,7 +1567,7 @@ _netplan_networkd_write_wait_online(const NetplanState* np_state, const char* ro
GString* content = g_string_new("[Unit]\n"
"ConditionPathIsSymbolicLink=/run/systemd/generator/network-online.target.wants/systemd-networkd-wait-online.service\n");
if (g_hash_table_size(non_optional_interfaces) == 0) {
_netplan_g_string_free_to_file(content, rootdir, override, NULL);
_netplan_g_string_free_to_file_with_permissions(content, rootdir, override, NULL, "root", "root", 0640);
g_hash_table_destroy(non_optional_interfaces);
return FALSE;
}
Expand All @@ -1593,7 +1591,10 @@ _netplan_networkd_write_wait_online(const NetplanState* np_state, const char* ro
}
g_string_append(content, "\n");

_netplan_g_string_free_to_file(content, rootdir, override, NULL);
g_autofree char* new_content = _netplan_scrub_systemd_unit_contents(content->str);
g_string_free(content, TRUE);
content = g_string_new(new_content);
_netplan_g_string_free_to_file_with_permissions(content, rootdir, override, NULL, "root", "root", 0640);
g_hash_table_destroy(non_optional_interfaces);
return TRUE;
}
Expand Down
2 changes: 2 additions & 0 deletions src/networkd.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
#include "netplan.h"
#include <glib.h>

#define NETWORKD_GROUP "systemd-network"

NETPLAN_INTERNAL gboolean
_netplan_netdef_write_networkd(
const NetplanState* np_state,
Expand Down
30 changes: 18 additions & 12 deletions src/nm.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -932,10 +932,11 @@ write_nm_conf_access_point(const NetplanNetDefinition* def, const char* rootdir,
g_datalist_foreach((GData**)&def->backend_settings.passthrough, write_fallback_key_value, kf);
}

g_autofree char* escaped_netdef_id = g_uri_escape_string(def->id, NULL, TRUE);
if (ap) {
g_autofree char* escaped_ssid = g_uri_escape_string(ap->ssid, NULL, TRUE);
/* TODO: make use of netplan_netdef_get_output_filename() */
conf_path = g_strjoin(NULL, "run/NetworkManager/system-connections/netplan-", def->id, "-", escaped_ssid, ".nmconnection", NULL);
conf_path = g_strjoin(NULL, "run/NetworkManager/system-connections/netplan-", escaped_netdef_id, "-", escaped_ssid, ".nmconnection", NULL);

g_key_file_set_string(kf, "wifi", "ssid", ap->ssid);
if (ap->mode < NETPLAN_WIFI_MODE_OTHER)
Expand Down Expand Up @@ -970,7 +971,7 @@ write_nm_conf_access_point(const NetplanNetDefinition* def, const char* rootdir,
}
} else {
/* TODO: make use of netplan_netdef_get_output_filename() */
conf_path = g_strjoin(NULL, "run/NetworkManager/system-connections/netplan-", def->id, ".nmconnection", NULL);
conf_path = g_strjoin(NULL, "run/NetworkManager/system-connections/netplan-", escaped_netdef_id, ".nmconnection", NULL);
if (def->has_auth) {
write_dot1x_auth_parameters(&def->auth, kf);
}
Expand Down Expand Up @@ -1084,28 +1085,30 @@ netplan_state_finish_nm_write(
GString *tmp = NULL;
guint unmanaged = nd->backend == NETPLAN_BACKEND_NM ? 0 : 1;

g_autofree char* netdef_id = _netplan_scrub_string(nd->id);
/* Special case: manage or ignore any device of given type on empty "match: {}" stanza */
if (nd->has_match && !nd->match.driver && !nd->match.mac && !nd->match.original_name) {
nm_type = type_str(nd);
g_assert(nm_type);
g_string_append_printf(nm_conf, "[device-netplan.%s.%s]\nmatch-device=type:%s\n"
"managed=%d\n\n", netplan_def_type_name(nd->type),
nd->id, nm_type, !unmanaged);
netdef_id, nm_type, !unmanaged);
}
/* Normal case: manage or ignore devices by specific udev rules */
else {
const gchar *prefix = "SUBSYSTEM==\"net\", ACTION==\"add|change|move\",";
const gchar *suffix = nd->backend == NETPLAN_BACKEND_NM ? " ENV{NM_UNMANAGED}=\"0\"\n" : " ENV{NM_UNMANAGED}=\"1\"\n";
g_string_append_printf(udev_rules, "# netplan: network.%s.%s (on NetworkManager %s)\n",
netplan_def_type_name(nd->type), nd->id,
netplan_def_type_name(nd->type), netdef_id,
unmanaged ? "deny-list" : "allow-list");
/* Match by explicit interface name, if possible */
if (nd->set_name) {
// simple case: explicit new interface name
g_string_append_printf(udev_rules, "%s ENV{ID_NET_NAME}==\"%s\",%s", prefix, nd->set_name, suffix);
g_autofree char* set_name = _netplan_scrub_string(nd->set_name);
g_string_append_printf(udev_rules, "%s ENV{ID_NET_NAME}==\"%s\",%s", prefix, set_name, suffix);
} else if (!nd->has_match) {
// simple case: explicit netplan ID is interface name
g_string_append_printf(udev_rules, "%s ENV{ID_NET_NAME}==\"%s\",%s", prefix, nd->id, suffix);
g_string_append_printf(udev_rules, "%s ENV{ID_NET_NAME}==\"%s\",%s", prefix, netdef_id, suffix);
}
/* Also, match by explicit (new) MAC, if available */
if (nd->set_mac && _is_valid_macaddress(nd->set_mac)) {
Expand All @@ -1121,9 +1124,10 @@ netplan_state_finish_nm_write(
// match on original name glob
// TODO: maybe support matching on multiple name globs in the future (like drivers)
g_string_append(udev_rules, prefix);
if (nd->match.original_name)
g_string_append_printf(udev_rules, " ENV{ID_NET_NAME}==\"%s\",", nd->match.original_name);

if (nd->match.original_name) {
g_autofree char* original_name = _netplan_scrub_string(nd->match.original_name);
g_string_append_printf(udev_rules, " ENV{ID_NET_NAME}==\"%s\",", original_name);
}
// match on (explicit) MAC address. Yes this would be unique on its own, but we
// keep it within the "full match" to make the logic more comprehensible.
if (nd->match.mac) {
Expand All @@ -1141,7 +1145,9 @@ netplan_state_finish_nm_write(
g_strfreev(split);
} else
drivers = g_strdup(nd->match.driver);
g_string_append_printf(udev_rules, " ENV{ID_NET_DRIVER}==\"%s\",", drivers);

g_autofree char* escaped_drivers = _netplan_scrub_string(drivers);
g_string_append_printf(udev_rules, " ENV{ID_NET_DRIVER}==\"%s\",", escaped_drivers);
g_free(drivers);
}
g_string_append(udev_rules, suffix);
Expand All @@ -1152,13 +1158,13 @@ netplan_state_finish_nm_write(

/* write generated NetworkManager drop-in config */
if (nm_conf->len > 0)
_netplan_g_string_free_to_file(nm_conf, rootdir, "run/NetworkManager/conf.d/netplan.conf", NULL);
_netplan_g_string_free_to_file_with_permissions(nm_conf, rootdir, "run/NetworkManager/conf.d/netplan.conf", NULL, "root", "root", 0640);
else
g_string_free(nm_conf, TRUE);

/* write generated udev rules */
if (udev_rules->len > 0)
_netplan_g_string_free_to_file(udev_rules, rootdir, "run/udev/rules.d/90-netplan.rules", NULL);
_netplan_g_string_free_to_file_with_permissions(udev_rules, rootdir, "run/udev/rules.d/90-netplan.rules", NULL, "root", "root", 0640);
else
g_string_free(udev_rules, TRUE);

Expand Down
Loading
Loading