Introduce a new ifcfg-rh variable ACD_TIMEOUT that stores the exact
value of ipv4.dad-timeout without rounding. We still write the
initscripts-compatible ARPING_WAIT variable, and read it when
ACD_TIMEOUT is missing.
Print a warning whenever we find a IP conflict on the network. In the
future we may export a flag on the device or send a signal so that
clients can notify the user of the conflict.
Don't return an error from nm_arping_manager_start_probe() since it is
currently useless and the arping-manager already prints the failure
reason. Also, drop a log print from add_address().
NMArpingManager previously spawned an arping process for each
probed/announced address and watched it. This has the disadvantage of
being inefficient and also that for small timeouts we can't be sure
that arping actually started the probe.
Switch to an implementation that doesn't need to spawn external
processes, by using the n-acd code [1] currently imported in our
source tree. The long term plan is that n-acd will become a shared
library we can link against.
The file is still called nm-arping-manager for lazyness, even if a
better name would be nm-acd-manager.
[1] https://github.com/nettools/n-acd/https://bugzilla.redhat.com/show_bug.cgi?id=1507864
During _new_active_connection() we just create the NMActiveConnection
instance to proceed with authorization. The caller might not even
authorize, so we must not touch the device yet.
Do that only later.
Often, functions perform a series of steps, and when they fail,
they bail out. It's simpler if the code is structured that way,
so you can read it from top to bottom and whenever something is
wrong, either return directly (or goto a cleanup label at the
bottom).
From the D-Bus layer, no specific-object is represented by "/". We
should early on normalize such values to NULL, and not expect or
handle them later (like during _new_active_connection()).
Merge _new_vpn_active_connection() into _new_active_connection(). It was the
only caller, and it is simpler to have all the code visible at one place.
That also shows, that the device argument is ignored and not handled.
Ensure that no device is specified for VPN type activations.
Also, in _add_and_activate_auth_done(), always steal the connection
from active's user-data. Otherwise, the lifetime of the connection
is extended until active gets destroyed. For example, if we would leak
active, we would also leak connection that way.
- pass is-vpn to _new_active_connection(). It is confusing that _new_active_connection()
would re-determine whether a connection is a VPN type, although that was already
established previously. The confusing part is: will they come to the
same result? Why? What if not?
Instead pass it as argument and assert that we got it right.
- the check for existing connections should look whether there is an existing
active connection of type NMVpnConnection. Instead, what matters is,
- do we have a connection of type VPN (otherwise, don't even bother
to search for existing-ac)
- is the connection already active?
Checking whether the connection is already active, and ask backwards
whether it's of type NMVpnConnection is odd, maybe even wrong in
some cases.
- there are only two callers of validate_activation_request(). One of them,
might already lookup the device before calling the validate function.
Safe to looking up again. But this is not only an optimization, more importantly,
it feels odd to first lookup a device, and then later look it up again. Are
we guaranteed to use the same path? Why? Just avoid that question.
- re-order some error checking for missing device, so that it is clearer.
- use cleanup attribute to handle return value and drop the "goto error".
NMDBusObject already gets this right, by calling nm_dbus_utils_get_property(),
which calls g_dbus_gvalue_to_gvariant(), which correctly converts NULL
object paths to "/".
We already rely on that elsewhere. No need for this workaround.
In the loop, we invoke callbacks. What the callbacks do, is out of control
of NMAuthManager. For example, they could cancel or schedule new
requests. Especially, cancelling invalidate the stored @safe pointer.
Fix that, by always iterate from the start of the list.
Fixes: d0563f0733
In the first loop, the element is removed only when the callback is
executed. The second loop never removes the current element. Use the
for_each macro for both.
Fixes: d0563f0733
Requesting broadcast replies from the DHCP server can be problematic in
filtered environments like some wireless networks. Don't override the
default of using unicast. This matches the behaviour of the external DHCP
clients.
https://github.com/NetworkManager/NetworkManager/pull/93
The list of calls contains two kinds of elements: (1) calls that don't
need a D-Bus request and are only waiting for the asynchronous
invocation of the callback in an idle function; (2) calls that need a
D-Bus request and are waiting for the D-Bus proxy.
When the proxy creation finishes, only (2) calls must be canceled (if
the creation failed) or started (if the proxy was created).
Fixes: 798b2a7527https://bugzilla.redhat.com/show_bug.cgi?id=1567807
Up to now, it was not visible on D-Bus whether a connection
was generated by NetworkManager and/or volatile.
That is for example interesting for firewalld, which aims
to store persistant configuration in NetworkManager's profile.
However, that doesn't make sense for external connections
(which are nm-generated & volatile). In fact, it probably
makes no sense for volatile connections in general, because
modifying them, likely makes them non-volatile (depending on
how the profile is modified).
Also, the Update2() D-Bus operation allows to carefully
make connections volatile and unsaved. As we have public
API to set these flags, we should also expose them on D-Bus.
Related: https://bugzilla.redhat.com/show_bug.cgi?id=1460295
The D-Bus interface already has a boolean property "Unsaved".
While that is nicer too look at (in the API), adding a new flag
is very cumbersome, and also has more overhead. For example,
it requires extending the D-Bus API, all the way down to libnm.
Add a flags argument, that will allow to add future boolean
flags easier.
For one, these flags are "internal" flags. Soon, we will gain
a new NMSettingsConnectionFlags type that is exported on D-Bus
and partly overlaps with these internal flags. However, then we
will need the "flags" properties to expose the public bits.
This property only exists because other parts are interested in
notification signals. Note that we encourage NMDbusObject types
to freeze/thaw property-changed notifications. As freezing the
notifications also delays the signals, this is not desired for
the purpose where internal users subscribe to the signal.
"NMSettingsConnectionFlags" was an internal enum. Soon, we will add such
a type in libnm. Avoid the naming conflict by renaming. The "Int" stands
for "internal".
We don't want to apped the value to @buf, we want to set it.
Also, if @buf happens to be uninitialized, g_strlcat() might
determine there is nothing to append and return the buffer unmodified.
Then, the (non NULL terminated) buffer might be printed.
Note that before recent refactoring, we effectively would only call
nm_auth_subject_to_string() on auth-subjects that were of type
UNIX-PROCESS. Hence, this bug came only to light very recently,
although it was present for a long time.
Fixes: eabe7d856c
We also don't emit the PropertiesChanged signal while connections are
not loaded. Maybe that is wrong, in any case, the property should agree
with the way how we emit notifications. So, for now, make the property
agree with not notifying about connections during startup.
Use the same form everywhere: "TRANSLATORS" instead of "Translators".
The manual also seems to prefer the upper-case form [1].
$ sed 's/\<Translators\>: /TRANSLATORS: /g' $(git grep -l Translators) -i
[1] https://www.gnu.org/software/gettext/manual/gettext.html
It makes sense to use NMAuthChain also when not attaching any user-data to
the chain. The main reason would be, the ability to schedule multiple permission
checks in parallel, and wait for them to complete together.
Only allocate the hash-table, when we really need it.
More of a proof of concept, how convenient (or not) it is to drop
NMAuthChain and use NMAuthManager's API directly. I think it's
reasonably nice.
As before, when asking for both general network-control permissions
and wifi-shared-permissions, we will not fail with
wifi-shared-permissions, as long as network-control check is still
pending. The effect is, that the error response preferably complains
about no permissions to network-control (in case both permissions
are missing).
One change in behavior is, if network-control authorization check
fails before wifi-shared-permissions, we declare the result and
cancel the pending wifi-shared-permissions. Previously, we would
have waited for both results. The change in behavior is not merely
that we declare the result earlier, but also that NMAuthManager will
actively send a "CancelCheckAuthorization" D-Bus call to cancel the
still pending wifi-shared-permissions check.
For one, we already do <trace> level logging inside NMAuthManager.
So, at trace level we have everything.
If a request fails, it's not up to NMAuthChain to log a warning.
Get rid of the NMAuthChain layer.
I think NMAuthChain only makes sense if we schedule multiple requests
together for the same topic. But NMSettingsConnection never does that:
each D-Bus request corresponds to only one polkit authorization request.
So, we can just call NMAuthManager directly.
