NetworkManager (the daemon) has no defined working directory, so
it can only handle absolute path names. This is in general and also for
the LoadConnections() D-Bus call.
That means, nmcli should make relative paths absolute.
We don't use g_canonicalize_filename() because that also cleans up
double slash and "/./". I don't think we should do that in this case, we
should only prepend $PWD to make the path absolute.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/794
While both functions are basically the same, the majority of the time
we use g_snprintf(). There is no strong reason to prefer one or the
other, but let's keep using one variant.
For IPv4, the order is not like for IPv6. Of course not.
Fixes: 7aa4ad0fa2 ('nmcli/docs: better describe ipv[46].addresses in `man nm-settings-nmcli`')
Edit nmcli command to show additional information about the routes
(both route4 and route6).
If there is information about next hop or metric in the route
structure it will be shown in addition to destination and prefix.
Coverity warns about this:
Error: RESOURCE_LEAK (CWE-772):
NetworkManager-1.32.4/src/nmcli/agent.c:87: alloc_fn: Storage is returned from allocation function "g_strdup".
NetworkManager-1.32.4/src/nmcli/agent.c:87: var_assign: Assigning: "pre_input_deftext" = storage returned from "g_strdup(secret->value)".
NetworkManager-1.32.4/src/nmcli/agent.c:87: overwrite_var: Overwriting "pre_input_deftext" in "pre_input_deftext = g_strdup(secret->value)" leaks the storage that "pre_input_deftext" points to.
# 85| /* Prefill the password if we have it. */
# 86| rl_startup_hook = set_deftext;
# 87|-> pre_input_deftext = g_strdup(secret->value);
# 88| }
# 89| if (secret->no_prompt_entry_id)
Error: RESOURCE_LEAK (CWE-772):
NetworkManager-1.32.4/src/nmcli/common.c:712: alloc_fn: Storage is returned from allocation function "g_strdup".
NetworkManager-1.32.4/src/nmcli/common.c:712: var_assign: Assigning: "nmc_rl_pre_input_deftext" = storage returned from "g_strdup(secret->value)".
NetworkManager-1.32.4/src/nmcli/common.c:712: overwrite_var: Overwriting "nmc_rl_pre_input_deftext" in "nmc_rl_pre_input_deftext = g_strdup(secret->value)" leaks the storage that "nmc_rl_pre_input_deftext" points to.
# 710| /* Prefill the password if we have it. */
# 711| rl_startup_hook = nmc_rl_set_deftext;
# 712|-> nmc_rl_pre_input_deftext = g_strdup(secret->value);
# 713| }
# 714| }
Naming is important, because the name of a thing should give you a good
idea what it does. Also, to find a thing, it needs a good name in the
first place. But naming is also hard.
Historically, some strv helper API was named as nm_utils_strv_*(),
and some API had a leading underscore (as it is internal API).
This was all inconsistent. Do some renaming and try to unify things.
We get rid of the leading underscore if this is just a regular
(internal) helper. But not for example from _nm_strv_find_first(),
because that is the implementation of nm_strv_find_first().
- _nm_utils_strv_cleanup() -> nm_strv_cleanup()
- _nm_utils_strv_cleanup_const() -> nm_strv_cleanup_const()
- _nm_utils_strv_cmp_n() -> _nm_strv_cmp_n()
- _nm_utils_strv_dup() -> _nm_strv_dup()
- _nm_utils_strv_dup_packed() -> _nm_strv_dup_packed()
- _nm_utils_strv_find_first() -> _nm_strv_find_first()
- _nm_utils_strv_sort() -> _nm_strv_sort()
- _nm_utils_strv_to_ptrarray() -> nm_strv_to_ptrarray()
- _nm_utils_strv_to_slist() -> nm_strv_to_gslist()
- nm_utils_strv_cmp_n() -> nm_strv_cmp_n()
- nm_utils_strv_dup() -> nm_strv_dup()
- nm_utils_strv_dup_packed() -> nm_strv_dup_packed()
- nm_utils_strv_dup_shallow_maybe_a() -> nm_strv_dup_shallow_maybe_a()
- nm_utils_strv_equal() -> nm_strv_equal()
- nm_utils_strv_find_binary_search() -> nm_strv_find_binary_search()
- nm_utils_strv_find_first() -> nm_strv_find_first()
- nm_utils_strv_make_deep_copied() -> nm_strv_make_deep_copied()
- nm_utils_strv_make_deep_copied_n() -> nm_strv_make_deep_copied_n()
- nm_utils_strv_make_deep_copied_nonnull() -> nm_strv_make_deep_copied_nonnull()
- nm_utils_strv_sort() -> nm_strv_sort()
Note that no names are swapped and none of the new names existed
previously. That means, all the new names are really new, which
simplifies to find errors due to this larger refactoring. E.g. if
you backport a patch from after this change to an old branch, you'll
get a compiler error and notice that something is missing.
The libreadline starting from version 6 is licensed as GPLv3. For some
use cases it is not acceptable to use this license.
In the NetworkManager the libreadline is used by nmcli.
This change allows using libedit instead of libreadline.
Following adjustments were made:
1. The history_set_history_state() is not supported in the libedit.
Instead, the where_history() with remove_history() were used to remove
the history content if needed.
2. rl_complete_with_tilde_expansion - it is the binary flag used only
when one wants to have the expansion support. The libedit is not
supporting and hence exporting this flag.
When one wants to compile the nmcli with libedit (GPLv2 replacement of
libreadline) the rl_completion_display_matches_hook hook shall be left
untouched (as NULL) as it is not supported in libedit.
The rl_startup_hook function has different prototype in libreadline and
in the libedit.
To fix this issue, arguments of hook function has been wrapped to C
preprocessor macro and properly adjusted.
Sort entries in src/nmcli/devices.c (on line 5023 - 5036)
by alphabetical order. The Order is violated in cases where
the sort would affect previous behaviour.
Example: `nmcli d d` is still shortcut for `nmcli device disconnect`
instead of `nmcli device delete`.
nmcli now accepts `nmcli device up|down` which works the same way as
`nmcli device connect|disconnect`
I also edited man pages of nmcli with new options.
$ nmcli connection add type dummy con-name x autoconnect no ipv4.method disabled ipv6.method disabled ifname d0
$ mcli connection up x ifname bogus
Connection successfully activated (D-Bus active path: /org/freedesktop/NetworkManager/ActiveConnection/12)
This is not right. A non-existing ifname argument was simply ignored
and nmcli would tell NetworkManager to activate the profile (on any
device).
Instead, if the user specifies a device argument, also for a virtual
type, it must exist.
Note that usually for virtual devices (like 'dummy'), the device
in fact does not exist, so passing `ifname` is likely to fail.
If the device already exists, then the command is no going to work
as expected, with
$ mcli connection up x ifname d0
succeeding, while
$ mcli connection up x ifname d1
fails (as intended) with
Error: device 'd1' not compatible with connection 'x': The interface names of the device and the connection didn't match..
$ nmcli connection add type ethernet con-name x autoconnect no ipv4.method disabled ipv6.method disabled
$ nmcli connection up x ifname bogus
Error: device 'bogus' not compatible with connection 'x'.
Better would be:
Error: device 'bogus' not found for connection 'x'.
Add a new property to specify the minimum time interval in
milliseconds for which dynamic IP configuration should be tried before
the connection succeeds.
This property is useful for example if both IPv4 and IPv6 are enabled
and are allowed to fail. Normally the connection succeeds as soon as
one of the two address families completes; by setting a required
timeout for e.g. IPv4, one can ensure that even if IP6 succeeds
earlier than IPv4, NetworkManager waits some time for IPv4 before the
connection becomes active.
NetworkManager supports a very limited set of qdiscs. If users want to
configure a unsupported qdisc, they need to do it outside of
NetworkManager using tc.
The problem is that NM also removes all qdiscs and filters during
activation if the connection doesn't contain a TC setting. Therefore,
setting TC configuration outside of NM is hard because users need to
do it *after* the connection is up (for example through a dispatcher
script).
Let NM consider the presence (or absence) of a TC setting in the
connection to determine whether NM should configure (or not) qdiscs
and filters on the interface. We already do something similar for
SR-IOV configuration.
Since new connections don't have the TC setting, the new behavior
(ignore existing configuration) will be the default. The impact of
this change in different scenarios is:
- the user previously configured TC settings via NM. This continues
to work as before;
- the user didn't set any qdiscs or filters in the connection, and
expected NM to clear them from the interface during activation.
Here there is a change in behavior, but it seems unlikely that
anybody relied on the old one;
- the user didn't care about qdiscs and filters; NM removed all
qdiscs upon activation, and so the default qdisc from kernel was
used. After this change, NM will not touch qdiscs and the default
qdisc will be used, as before;
- the user set a different qdisc via tc and NM cleared it during
activation. Now this will work as expected.
So, the new default behavior seems better than the previous one.
https://bugzilla.redhat.com/show_bug.cgi?id=1928078
Introducing ethtool PAUSE support with:
* ethtool.pause-autoneg on/off
* ethtool.pause-rx on/off
* ethtool.pause-tx on/off
Limitations:
* When `ethtool.pause-autoneg` is set to true, the `ethtool.pause-rx`
and `ethtool.pause-tx` will be ignored. We don't have warning for
this yet.
Unit test case included.
Signed-off-by: Gris Ge <fge@redhat.com>
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/829
Found by Coverity:
Error: RESOURCE_LEAK (CWE-772): [#def297] [important]
NetworkManager-1.31.3/src/nmcli/devices.c:4610: alloc_fn: Storage is returned from allocation function "nm_utils_ssid_to_utf8".
NetworkManager-1.31.3/src/nmcli/devices.c:4610: var_assign: Assigning: "ssid" = storage returned from "nm_utils_ssid_to_utf8(g_bytes_get_data(ssid_bytes, NULL), g_bytes_get_size(ssid_bytes))".
NetworkManager-1.31.3/src/nmcli/devices.c:4612: noescape: Resource "ssid" is not freed or pointed-to in "g_print".
NetworkManager-1.31.3/src/nmcli/devices.c:4642: noescape: Resource "ssid" is not freed or pointed-to in "string_append_mecard".
NetworkManager-1.31.3/src/nmcli/devices.c:4654: leaked_storage: Variable "ssid" going out of scope leaks the storage it points to.
# 4652|
# 4653| g_print("\n");
# 4654|-> }
# 4655|
# 4656| static gboolean
Fixes: 7061341a41 ('cli: add "nmcli d wifi show"')
The key-mgmt property of NMSettingWirelessSecurity is slightly confusing
when you know there's also a wpa_supplicant configuration option called
"key_mgmt". Our property is not the same as that supplicant option even
though they do have things in common. NMs key-mgmt is not exactly meant
to configure which AKM suites you want to use, but rather which method
of wifi security is being used (so "wpa2+wpa3 personal", "wpa3 personal
only" or "wpa3 enterprise only").
Try to make this a bit clearer in the documentation of the property by
rewriting it and listing those security methods.
Invoke Reload() asynchronously and run the main loop. In this way, the
polkit agent built into nmcli can be used to authenticate the reload
request.
Fixes: 5afcf9c045 ('cli: add 'general reload' command')
Property aliases should really just be shortcuts for one fully spelled
out property (sometimes, they do more like "master").
Anyway, we must also handle resetting the value, otherwise:
$ nmcli connection add type gsm apn ""
will still result in "gsm.apn=internet", unlike
$ nmcli connection add type gsm gsm.apn ""
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.
This patch is introducing the wired setting accept-all-mac-addresses
property. The value corresponds to the kernel flag IFF_PROMISC.
When accept-all-mac-address is enabled, the interface will accept all
the packets without checking the destination mac address.
Signed-off-by: Fernando Fernandez Mancera <ffmancera@riseup.net>
This patch is introducing NM_DEVICE_INTERFACE_FLAG_PROMISC in
interface_flags. The flag represents IFF_PROMISC kernel flag.
Signed-off-by: Fernando Fernandez Mancera <ffmancera@riseup.net>
