An active connection started by user could be queued in state UNKNOWN
which means the device hasn't started activating yet. Eventually it
will, and we don't want to cancel the user activation because of an
internal autoconnection attempt.
There was a special case for ensuring that the device's configuration
doesn't disappear when some factory is too late at recognizing the
device is just a component of another one.
It was always a bad idea. If the device already had an active
connection (such as for a generated default wired connection), it would
remain around, with a dangling reference to the device.
This effectively reverts commit 5ad69cb29b ('core: remove child devices
without deconfiguring them (bgo #738479)'). It's okay to do so, because
we now wouldn't deconfigure the device upon its removal anyway.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/151/
There are two similar cases where the caller attaches the requested
permission to the auth-chain. There the tag "perm" is used.
Rename for consistency.
Out of the 33 callers of nm_auth_chain_add_call(), the permission
argument is:
- 29 times a C string literal like NM_AUTH_PERMISSION_NETWORK_CONTROL.
- 3 times assign a string that is in fact a static string (it's just
not a string literal)
- only NMManager's device_auth_request_cb() passes a permission of
(possibly) non static origin. But it already duplicates the string
for it's own purposes and attaches it as user-data to the
NMAuthChain.
There really is no need to duplicate the string.
Replace nm_auth_chain_add_call() by a macro that ensures that the
permission string is a C literal.
Rename nm_auth_chain_add_call() to nm_auth_chain_add_call_unsafe() to
indicate that the lifetime of the permission argument is now the
responsibility of the caller.
NMAuthChain usually requests several permissions at once. Hence, an error
argument in the overall callback does not make sense, because you
wouldn't know which request failed.
If at all, it could only mean that the overall request failed (like an
D-Bus failure communicating to D-Bus *for all permisssions*),
but we don't need to handle that specially. In fact, we don't really care
why permission was not granted, whether it's due to an error or legitimate
reasons.
The error in the callback was always set to %NULL. Remove it.
NMAuthChain is not ref-counted.
You may call nm_auth_chain_destroy() once before the callback
gets invoked. This destroys the auth-chain instance right away.
You may call nm_auth_chain_destroy() once from inside the callback.
This basically has no effect but is allowed for convenince.
All this does is remembering that destroy was called and asserts that
destroy gets called at most once.
After the callback returns, the auth-chain will always be destroyed.
That means, generally there is no need to call nm_auth_chain_destroy()
from inside the callback.
Remove that code, and refactor some code to return early (where it makes
sense).
This function was left as a reminder now for 9 years. Get rid of it.
Related: 8310593ce4 ('core: ignore authorization for sleep/wake requests (but restrict to root) (rh #638640)')
Only read the keyfile databases once and cache them for the remainder of
the program.
- this avoids the overhead of opening the file over and over again.
- it also avoids the data changing without us expecting it. The state
files are internal and we don't support changing it outside of
NetworkManager. So in the base case we read the same data over
and over. In the worst case, we read different data but are not
interested in handling the changes.
- only write the file when the content changes or before exiting
(normally).
- better log what is happening.
- our state files tend to grow as we don't garbage collect old entries.
Keeping this all in memory might be problematic. However, the right
solution for this is that we come up with some form of garbage
collection so that the state files are reaonsably small to begin with.
If a user disables networking, we consider that as an indication that
also software devices must be disconnected. OTOH, we don't want to
destroy them for external events as a system suspend.
When networking is disabled at NM startup we unmanage all devices
(including software ones) due to SLEEPING. After networking gets
enabled again we must clear the unmanaged-sleeping flag on software
devices.
"libnm-core" implements common functionality for "NetworkManager" and
"libnm".
Note that clients like "nmcli" cannot access the internal API provided
by "libnm-core". So, if nmcli wants to do something that is also done by
"libnm-core", , "libnm", or "NetworkManager", the code would have to be
duplicated.
Instead, such code can be in "libnm-libnm-core-{intern|aux}.la".
Note that:
0) "libnm-libnm-core-intern.la" is used by libnm-core itsself.
On the other hand, "libnm-libnm-core-aux.la" is not used by
libnm-core, but provides utilities on top of it.
1) they both extend "libnm-core" with utlities that are not public
API of libnm itself. Maybe part of the code should one day become
public API of libnm. On the other hand, this is code for which
we may not want to commit to a stable interface or which we
don't want to provide as part of the API.
2) "libnm-libnm-core-intern.la" is statically linked by "libnm-core"
and thus directly available to "libnm" and "NetworkManager".
On the other hand, "libnm-libnm-core-aux.la" may be used by "libnm"
and "NetworkManager".
Both libraries may be statically linked by libnm clients (like
nmcli).
3) it must only use glib, libnm-glib-aux.la, and the public API
of libnm-core.
This is important: it must not use "libnm-core/nm-core-internal.h"
nor "libnm-core/nm-utils-private.h" so the static library is usable
by nmcli which couldn't access these.
Note that "shared/nm-meta-setting.c" is an entirely different case,
because it behaves differently depending on whether linking against
"libnm-core" or the client programs. As such, this file must be compiled
twice.
(cherry picked from commit af07ed01c0)
From the files under "shared/nm-utils" we build an internal library
that provides glib-based helper utilities.
Move the files of that basic library to a new subdirectory
"shared/nm-glib-aux" and rename the helper library "libnm-core-base.la"
to "libnm-glib-aux.la".
Reasons:
- the name "utils" is overused in our code-base. Everything's an
"utils". Give this thing a more distinct name.
- there were additional files under "shared/nm-utils", which are not
part of this internal library "libnm-utils-base.la". All the files
that are part of this library should be together in the same
directory, but files that are not, should not be there.
- the new name should better convey what this library is and what is isn't:
it's a set of utilities and helper functions that extend glib with
funcitonality that we commonly need.
There are still some files left under "shared/nm-utils". They have less
a unifying propose to be in their own directory, so I leave them there
for now. But at least they are separate from "shared/nm-glib-aux",
which has a very clear purpose.
(cherry picked from commit 80db06f768)
Currently, if user configuration or settings specify that a software
device is unmanaged, for example:
[device-bond-unmanaged]
match-device=interface-name:bond*
managed=0
or
[keyfile]
unmanaged-devices=interface-name:bond*
and there is a connection for the device with autoconnect=yes, NM
creates the platform link and a realized device in unmanaged
state. Fix this, the device should not be realized if it is unmanaged.
https://bugzilla.redhat.com/show_bug.cgi?id=1679230
When a device is removed (like when the user unplugs a usb network
device) the device object is removed, so it doesn't emit a notify signal
for a change in its connectivity and so, device_connectivity_changed
is not called. This means that nobody updates the global connectivity
value which is potentially wrong if the device was the one providing
network connectivity.
Since device_connectivity_changed's first two parameters aren't actually
used and are there just for the signal to be able to be connected, I
moved the code from device_connectivity_changed to a new
update_connectivity_value function that just takes a NMManager
parameter and also call it from remove_device.
[thaller@redhat.com: fix coding style regarding whitespace]
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/issues/141https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/101
Add a "a{sv}" output argument to "AddAndActivate2" D-Bus API.
"AddAndActivate2" replaces "AddAndActivate" with more options.
It also has a dictionary argument to be forward compatible so that we
hopefully won't need an "AddAndActivate3". However, it lacked a similar
output dictionary. Add it for future extensibility. I think this is
really to workaround a shortcoming of D-Bus, which does provide strong
typing and type information about its API, but does not allow to extend
an existing API in a backward compatible manner. So we either resort to
Method(), Method2(), Method3() variants, or a catch-all variant with a
generic "a{sv}" input/output argument.
In libnm, rename "nm_client_add_and_activate_connection_options()" to
"nm_client_add_and_activate_connection2()". I think libnm API should have
an obvious correspondence with D-Bus API. Or stated differently, if
"AddAndActivateOptions" would be a better name, then the D-Bus API should
be renamed. We should prefer one name over the other, but regardless
of which is preferred, the naming for D-Bus and libnm API should
correspond.
In this case, I do think that AddAndActivate2() is a better name than
AddAndActivateOptions(). Hence I rename the libnm API.
Also, unless necessary, let libnm still call "AddAndActivate" instead of
"AddAndActivate2". Our backward compatibility works the way that libnm
requires a server version at least as new as itself. As such, libnm
theoretically could assume that server version is new enough to support
"AddAndActivate2" and could always use the more powerful variant.
However, we don't need to break compatibility intentionally and for
little gain. Here, it's easy to let libnm also handle old server API, by
continuing to use "AddAndActivate" for nm_client_add_and_activate_connection().
Note that during package update, we don't restart the currently running
NetworkManager instance. In such a scenario, it can easily happen that
nmcli/libnm is newer than the server version. Let's try a bit harder
to not break that.
Changes as discussed in [1].
[1] https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/37#note_79876
Don't return success from a nested code path. Handle all errors
first, and return early. Well, we cannot really return right away
because we need to handle the failure. So, at least, check for errors
and "goto fail".
When we agregate the connectivity state, only devices that
have the best default route should be considered.
Since we do connectivity checking per-device, the per-device check
does not care whether traffic to the internet is really routed via this
device.
But when talking about the global connectivity state, we care mostly
about the (best) default route. So, we should not allow a device with
worse or now default route, to contribute its connectivity state.
Fixes: 6b7e9f9b22
Since we determine the connectivity state of each device individually,
the global connectivity state is an aggregate of all these states.
I am not sure about considering here devices that don't have the (best)
default route for their respective address family. But anyway.
When we aggregate the best connectivity, we chose the numerical largest
value. That is wrong, because PORTAL is numerically smaller than
LIMITED.
That means, if you have two devices, one with connectivity LIMITED and
one with connectivity PORTAL, then LIMITED wrongly wins.
Fixes: 6b7e9f9b22https://bugzilla.redhat.com/show_bug.cgi?id=1619873
By setting "connection.permissions", a profile is restricted to a
particular user.
That means for example, that another user cannot see, modify, delete,
activate or deactivate the profile. It also means, that the profile
will only autoconnect when the user is logged in (has a session).
Note that root is always able to activate the profile. Likewise, the
user is also allowed to manually activate the own profile, even if no
session currently exists (which can easily happen with `sudo`).
When the user logs out (the session goes away), we want do disconnect
the profile, however there are conflicting goals here:
1) if the profile was activate by root user, then logging out the user
should not disconnect the profile. The patch fixes that by not
binding the activation to the connection, if the activation is done
by the root user.
2) if the profile was activated by the owner when it had no session,
then it should stay alive until the user logs in (once) and logs
out again. This is already handled by the previous commit.
Yes, this point is odd. If you first do
$ sudo -u $OTHER_USER nmcli connection up $PROFILE
the profile activates despite not having a session. If you then
$ ssh guest@localhost nmcli device
you'll still see the profile active. However, the moment the SSH session
ends, a session closes and the profile disconnects. It's unclear, how to
solve that any better. I think, a user who cares about this, should not
activate the profile without having a session in the first place.
There are quite some special cases, in particular with internal
activations. In those cases we need to decide whether to bind the
activation to the profile's visibility.
Also, expose the "bind" setting in the D-Bus API. Note, that in the future
this flag may be modified via D-Bus API. Like we may also add related API
that allows to tweak the lifetime of the activation.
Also, I think we broke handling of connection visiblity with 37e8c53eee
"core: Introduce helper class to track connection keep alive". This
should be fixed now too, with improved behavior.
Fixes: 37e8c53eeehttps://bugzilla.redhat.com/show_bug.cgi?id=1530977
NMKeepAlive is a proper GObject type, with a specific API that on the one
end allows to configure watches/bindings, and on the other end exposes
and is-alive property and the owner instance. That's great, as NMActiveConnection
is not concerned with either end (moving complexity away from
"nm-active-connection.c") and as we later can reuse NMKeepAlive with
NMSettingsConnection.
However, we don't need to wrap this API by NMActiveConnection. Doing so
means, we need to expose all the watch/bind functions also as part of
NMActiveConnection API.
The only ugliness here is, that NMPolicy subscribes to property changed
signal of the keep alive instance, which would fail horribly if
NMActiveConnection ever decides to swap the keep alive instance (in
which case NMPolicy would have to disconnect the signal, and possibly
reconnect it to another NMKeepAlive instance). We avoid that by just not
doing that and documenting it.
We should not blindly change the device's state. Instead, call
nm_device_disconnect_active_connection() which will figure out whether
the device state needs to change. Note that it is very possible that
the active-connection instance is still queued, not yet queued, or
already disconnected. nm_device_disconnect_active_connection() does
the right thing in all cases.
nm_device_disconnect_active_connection() is generally useful and a prefered
form to fail an active connection. The device's state-change reason is important,
so it needs to be injected.
remove_device() is also called when the device has no longer a valid
ifindex and so device_is_wake_on_lan() must do an extra check to avoid
the following assertion:
nmp_cache_lookup_entry_link: assertion 'ifindex > 0' failed
0 _g_log_abort () from target:/lib64/libglib-2.0.so.0
1 g_logv () from target:/lib64/libglib-2.0.so.0
2 g_log () from target:/lib64/libglib-2.0.so.0
3 nmp_cache_lookup_entry_link (cache=0xb858f0, ifindex=ifindex@entry=0) at ../src/platform/nmp-object.c:1713
4 nmp_cache_lookup_link (cache=<optimized out>, ifindex=ifindex@entry=0) at ../src/platform/nmp-object.c:1728
5 nm_platform_link_get_obj (self=self@entry=0xb85840, ifindex=ifindex@entry=0, visible_only=visible_only@entry=1) at ../src/platform/nm-platform.c:759
6 nm_platform_link_get (self=self@entry=0xb85840, ifindex=ifindex@entry=0) at ../src/platform/nm-platform.c:784
7 nm_platform_link_get_type (self=self@entry=0xb85840, ifindex=ifindex@entry=0) at ../src/platform/nm-platform.c:1065
8 link_get_wake_on_lan (platform=0xb85840, ifindex=0) at ../src/platform/nm-linux-platform.c:6963
9 nm_platform_link_get_wake_on_lan (self=self@entry=0xb85840, ifindex=0) at ../src/platform/nm-platform.c:1705
10 device_is_wake_on_lan (platform=0xb85840, device=<optimized out>) at ../src/nm-manager.c:1617
11 remove_device (self=0xbd0060, device=<optimized out>, device@entry=0xd298c0, quitting=quitting@entry=0, allow_unmanage=allow_unmanage@entry=1)
12 device_removed_cb (device=0xd298c0, user_data=0xbd0060) at ../src/nm-manager.c:1698
13 _g_closure_invoke_va () from target:/lib64/libgobject-2.0.so.0
14 g_signal_emit_valist () from target:/lib64/libgobject-2.0.so.0
15 g_signal_emit () from target:/lib64/libgobject-2.0.so.0
16 available_connections_check_delete_unrealized_on_idle (user_data=0xd298c0) at ../src/devices/nm-device.c:4446
Fixes: ca3bbede74
The problem is that updating the metered value of a shared connection is
not implemented. The user needs to fully reactivate the profile for changes
to take effect.
That is unfortunate, especially because reapplying the route metric
works in other other cases.
Add a new CON_DEFAULT() macro that places a property name into a
special section used at runtime to check whether it is a supported
connection default.
Unfortunately, this mechanism doesn't work for plugins so we have to
enumerate the connection defaults from plugins in the daemon using
another CON_DEFAULT_NOP() macro.
Correct the spelling across the *entire* tree, including translations,
comments, etc. It's easier that way.
Even the places where it's not exposed to the user, such as tests, so
that we learn how is it spelled correctly.
g_object_set_property() cannot fail nor signal an error reason when
invalid arguments are passed. Use our wrapper nm_g_object_set_property()
instead.
Note that the input argument comes from untrusted (although authenticated)
source.
"bind" specifically binds the lifetime of the activation (NMActiveConnection).
In combination with "persist=volatile", the lifetime of the NMSettingsConnection
is indirectly bound to the NMActiveConnection. But still these concepts make sense
independently.
In the future, it may make sense to also bind the lifetime of the NMSettingsConnection
to the D-Bus client. Hence, rename the option to allow for the distinction.
Also, belatedly fix libnm comment about "bind" only working with
"persist" "volatile".
Fixes: eb883e34a5
- use nm_streq() instead of g_strcmp0(). I think streq() is easier
to understand.
- the strings that are checked here must never be %NULL, because they come
from string variants. Use nm_streq() instead of nm_streq0() or g_strcmp0().
- don't add a "." to the GError messages. GError messages are commonly
embedded in a larger message, and shoult not themself contain the dot.
Previously, @bind_lifetime was a string. While parsing the @options, we
would set the string to the content of the parsed GVariant. Note that
the GVariant is unrefed before we access @bind_lifetime, thus it's
not guaranteed that it will still exist.
Arguably, the string GVariant's lifetime is tied to the @options
dictionary, so indeed it lives long enough. But that is not-obviously
the case.
Fix that by using a boolean instead. Also, rename @bind_lifetime to
@bind_dbus_client.
For one, there was a bug here: we cannot "goto error" without setting
the @error variable.
Anyway, restricting "bind" "dbus-client" only to profiles that are
"persist" mode "volatile" seems wrong. The "bind" option as it is,
limits the lifetime of the active-connection. This has no direct relation
with the lifetime of the setting-connection. Indeed, if the
settings-connection's lifetime is itself set to "volatile", then
it will indeed go away with the active-connection. However, these
two concepts are not strictly related.
In the future, we might add an option to limite the lifetime of
a settings-connection to a D-Bus client ("bind-setting"). Possibly
we should thus rename "bind" to "bind-activation", to make the
distinction clearer.
This allows binding the lifetime of the created connection to the
existance of the requesting dbus client. This feature is useful if one
has a service specific connection (e.g. P2P wireless) which will not be
useful without the specific service.
This is simply a mechanism to ensure proper connection cleanup if the
requesting service has a failure.
This option allows setting the rules for how long the connection should
be stored. Valid values are "disk" (the default), "memory" and
"volatile". If "memory" or "volatile" is selected, the connection will
not be saved to disk and with "volatile" it will be automatically
removed when it is deactivated again.
This adds a new routine to be able to handle an arbitrary set of further
options for AddAndActivateConnection. Note that no options are accepted
for now.
When a device is unmanaged, an explicit activation request can
still activate it. In particular, that is the case for
$ nmcli connection up "$PROFILE" ifname "$DEVICE"
It is also the case, for plain
$ nmcli connection up "$PROFILE"
where NetworkManager searches for a suitable device -- depending on
multi-connect setting of the profile.
The idea is, that a profile with "multi-connect=single" is expected
to sufficently and uniquely match a device, based on matching properties
like "connection.interface-name". In that case, an explicit activation
request from the user shows the intent to manage the device.
Note that it's hard to understand whether the profile really uniquely
selects a particular device. For example, if the profile doesn't specify
"connection.interface-name", it might still uniquely identify
an ethernet device, if you only have one such device.
On the other hand, with "connection.multi-connect" other than "single",
it is very much expected that the profile does not strictly match
one device.
Change the behavior here for multi-connect profiles. This allows the
user to block individual devices from activation via
$ nmcli device set "$DEVICE" managed not
A subsequent
$ nmcli connection up "$MULTI_PROFILE"
will not consider "$DEVICE" as suitable candidate for activation.
Likewise, in the future we may want to add a
$ nmcli connection up --all "$MULTI_PROFILE"
command, to activate the profile on all suitable device.
In that case again, unmanaged devices probably also should be skipped
for multi-connect profiles.
https://bugzilla.redhat.com/show_bug.cgi?id=1639254
With
$ nmcli connection up "$PROFILE" ifname "$DEVICE"
it's clear that the user means the particular device. That also
is taken as a indication to make $DEVICE as managed, in case it was
unmanaged before. So, this command implies a previous
$ nmcli device set $DEVICE managed yes
On the other hand, if the user just issues
$ nmcli connection up "$PROFILE"
without a particular device, then we should prefer devices which
are marked as managed instead of unmanaged once.
Likewise, we should consider the device's state when selecting
a device. This means, when activating a profile which is activatable on
multiple devices, it will now prefer devices which are not already
active. The exception to this is that if the profile itself is already
active (and multi-connect "single"), then it will prefer to re-activate
the profile on the same device. This was done previously already. What's
new is that if the the profile is not multi-connect "single", the said
exception no longer applies, and we prefer to activate the profile on a
hitherto unactivated device.
https://bugzilla.redhat.com/show_bug.cgi?id=1639254https://github.com/NetworkManager/NetworkManager/pull/232
