This is actually trying *too* hard to prevent DNS leaks, breaking normal
expected use of split DNS. Let systemd-resolved handle sending our DNS
queries to the right place instead.
It's true that NetworkManager is trying to emulate the behavior of
wg-quick here, and wg-quick uses 'resolvconf -x' to attempt to set
"exclusive" DNS. But with systemd-resolved this is implemented by
setting a ~. routing domain for the Wireguard interface. That is a
*really* big hammer already, since Domain=~. overrides +DefaultRoute,
ensuring most DNS queries can only go to other interfaces with Domain=~.
NetworkManager follows systemd-resolved's recommended convention by only
applying Domain=~. to other "privacy VPNs" since 1.26.6. Setting DNS
priority only prevents *domain-specific* "leaks", which are almost
always desired. For example, it prevents using both the Wireguard VPN
and a corporate VPN at the same time.
Note that all of the justification behind !688 applies here as well.
See-also: https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/688https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/585https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/901
This is confusing Coverity:
Error: RESOURCE_LEAK (CWE-772): [#def249] [important]
NetworkManager-1.31.5/src/libnmc-base/nm-secret-agent-simple.c:810: alloc_fn: Storage is returned from allocation function "g_string_free".
NetworkManager-1.31.5/src/libnmc-base/nm-secret-agent-simple.c:810: var_assign: Assigning: "auth_dialog_request_str" = storage returned from "g_string_free(auth_dialog_request, 0)".
NetworkManager-1.31.5/src/libnmc-base/nm-secret-agent-simple.c:822: noescape: Resource "auth_dialog_request_str" is not freed or pointed-to in "g_output_stream_write_async".
NetworkManager-1.31.5/src/libnmc-base/nm-secret-agent-simple.c:822: noescape: Resource "auth_dialog_request_str" is not freed or pointed-to in "g_output_stream_write_async".
NetworkManager-1.31.5/src/libnmc-base/nm-secret-agent-simple.c:838: leaked_storage: Variable "auth_dialog_request_str" going out of scope leaks the storage it points to.
# 836| data);
# 837|
# 838|-> return TRUE;
# 839| }
# 840|
Maybe this works better to avoid the warning. At least, it also
documents it better to the reader.
Found by Coverity:
Error: RESOURCE_LEAK (CWE-772): [#def271] [important]
NetworkManager-1.31.3/src/libnmc-base/nm-secret-agent-simple.c:874: alloc_fn: Storage is returned from allocation function "nm_utils_ssid_to_utf8".
NetworkManager-1.31.3/src/libnmc-base/nm-secret-agent-simple.c:874: var_assign: Assigning: "ssid_utf8" = storage returned from "nm_utils_ssid_to_utf8(g_bytes_get_data(ssid, NULL), g_bytes_get_size(ssid))".
NetworkManager-1.31.3/src/libnmc-base/nm-secret-agent-simple.c:877: noescape: Resource "ssid_utf8" is not freed or pointed-to in "g_strdup_printf".
NetworkManager-1.31.3/src/libnmc-base/nm-secret-agent-simple.c:882: leaked_storage: Variable "ssid_utf8" going out of scope leaks the storage it points to.
# 880|
# 881| if (!add_wireless_secrets(request, secrets))
# 882|-> goto out_fail;
# 883| } else if (nm_connection_is_type(request->connection, NM_SETTING_WIRED_SETTING_NAME)) {
# 884| title = _("Wired 802.1X authentication");
Error: RESOURCE_LEAK (CWE-772): [#def272] [important]
NetworkManager-1.31.3/src/libnmc-base/nm-secret-agent-simple.c:874: alloc_fn: Storage is returned from allocation function "nm_utils_ssid_to_utf8".
NetworkManager-1.31.3/src/libnmc-base/nm-secret-agent-simple.c:874: var_assign: Assigning: "ssid_utf8" = storage returned from "nm_utils_ssid_to_utf8(g_bytes_get_data(ssid, NULL), g_bytes_get_size(ssid))".
NetworkManager-1.31.3/src/libnmc-base/nm-secret-agent-simple.c:877: noescape: Resource "ssid_utf8" is not freed or pointed-to in "g_strdup_printf".
NetworkManager-1.31.3/src/libnmc-base/nm-secret-agent-simple.c:883: leaked_storage: Variable "ssid_utf8" going out of scope leaks the storage it points to.
# 881| if (!add_wireless_secrets(request, secrets))
# 882| goto out_fail;
# 883|-> } else if (nm_connection_is_type(request->connection, NM_SETTING_WIRED_SETTING_NAME)) {
# 884| title = _("Wired 802.1X authentication");
# 885| msg = g_strdup_printf(_("Secrets are required to access the wired network %s"),
Fixes: 3fbabde4c3 ('libnm-core: replace GByteArray with pointer + length in some APIs')
- use strstrip() to remove leading and trailing whitespace
- use _nm_utils_ascii_str_to_int64() for parsing numeric values
like -1, 0 and 1. In particular, this now also allows passing
the numeric values.
- also accept "default" as valid value for NM_TERNARY_DEFAULT.
With this change, nmc_string_to_ternary() can also parse everything that
we commonly and currently parse with _nm_utils_enum_from_str_full()
and NM_TYPE_TERNARY. This will allow to configure ternary values in
a more flexible way.
- use strstrip() to remove leading and trailing whitespace
- use _nm_utils_ascii_str_to_int64() for parsing numeric values
like 0 and 1. The difference is small, for one, it also accepts
hex numbers like 0x1. More interestingly, it uses our common
number parsing function, and we will later do the same for
parsing ternaries.
g_set_error(error, 1, 0, ...) is not right. "1" is not a valid GQuark,
we should initialize proper error instances.
Use nm_utils_error_set() for that.
Also, the code previously hacked the numeric value "1" to indicate
ambiguous text. Add and use a new error code NM_UTILS_ERROR_AMBIGUOUS
for that.
With a const argument, we can make variables static const,
which means the linker loads the memory as read only.
Also, use NM_CAST_STRV_CC() macro, which casts the argument
accordingly.
