This was previously tracked via a signal "scanning-prohibited".
However, I think it was buggy, because the signal didn't specify
a GSignalAccumulator, so when a NMDeviceOlpcMesh registered a handler,
NMDeviceWifi.scanning_prohibited() was ignored.
In theory, a GObject signal decouples the target and source of the
signal and is more abstract. But more abstraction is worse, if there
is exactly one target who cares about this signal: the OLPC mesh.
And that target is well known at compile time. So, don't pretend that
NMDeviceWifi or NMDeviceOlpcMesh aren't aware that they are together in
this.
Another downside of the signal is that you don't know when scanning gets
unblocked. You can only poll and asked whether it is blocked, but there
was no mechanism how NMDeviceWifi would be notified when scanning is
no longer blocked.
Rework this. Instead, the OLPC mesh explicitly registers and unregisters
its blocking state with nm_device_wifi_scanning_prohibited_track().
It feels better to first parse input arguments before authenticating.
One argument for otherwise would be that we shouldn't reveal any
information about the request before authenticating it. Meaning: every
request (even with invalid arguments) should fail with
permission-denied.
However, I prefer this for minor reasons:
- what makes a valid request is no secret. And if somebody makes an
invalid request, it should fail with invalid-arguments first.
- we possibly can short cut the expensive authentication process, where
we ask PolicyKit.
- by extracting the options variant early and only pass on the SSIDs
array, we handle the encoding of the options array earlier and where
it belongs: closer to the D-Bus request that defines the meaning of
the argument.
Also, change the failure reason to return invalid-argument.
This was first introduced by commit 4ed4b491fa ('2005-12-31 Dan
Williams <dcbw@redhat.com>'), a very long time ago.
It got reworked several times, but I don't think this code makes sense
anymore. So, if nm_platform_wifi_get_quality() returns an error, we
would ignore it for three times, until we would set the strength to the
error code (presumably -1). Why? If we cannot read the strength via
nl80211/WEXT, then we should just keep whatever we got from supplicant.
Drop this.
Also, only accept the percentage if it is in a valid range from 0 to
100%. If the driver (or platform code) gives us numbers out of that
range, we have no idea what their meaning is. In that case, the value
must be fixed in the lower layers, that knows how to convert the value
from the actual meaning to the requested percentage.
In NMSupplicantInterface, we determine whether we currently are scanning
both on the "scanning" supplicant state and the "Scanning" property.
Extend that. If we currently are scanning and are about to clear the
scanning state, then pretend to still scan as long as we are still
initializing BSS instances. What otherwise happens is that we declare
that we finished scanning, but the NMWifiAP instances are not yet ready.
The result is, that `nmcli device wifi` will already start printing the
scan list, when we didn't yet fully process all access points.
Now, _notify_maybe_scanning() will delay switching the scanning state to
disabled, as long as we have BSS initializing (bss_initializing_lst_head).
Also, ignore the "ScanDone" signal. It's redundant to the "Scanning"
property anyway.
Also, only set priv->last_scan_msec when we switch the scanning state
off. That is the right (and only) place where the last-scan timestamp
needs updating.
Certain properties (for example "scanning") are combined from multiple
other properties. So, we want to notify a changed signal, exactly when
something relevant changes. We also may not want to emit a signal while
we are still in the middle of changing multiple properties together.
Only at certain places we want to check and emit the signal.
Simplify the implementation for that by tracking the property value that
we currently expose, and keeping state about when it changes.
- add more code comments
- refactor the code flow in _get_hash_key_init() to follow a simpler
code path.
- use c_siphash_hash() instead of 3 separate steps.
- Drop "?: static_seed" from nm_hash_static(). It's not useful, because
the only _get_hash_key() for which _get_hash_key()^static_seed is zero
is ~static_seed. That means, only one value of all the static seeds
can result in zero here. At that point, we can just coerce that value
to 3679500967u directly.
Meson 0.54.0 requires ninja-1.7 ([1]).
On Ubuntu 16.04, we now would get meson 0.54.0 via pip3, but ninja-1.5.1 via
apt. That doesn't work anymore.
We could install ninja via pip3, but of course, doing that on other
Debian/Ubuntu versions fails due to ... I don't even want to know.
So, instead use an old meson version on Ubuntu 16.04, which is
known to still work with the ninja provided by the packaging system.
We anyway don't want to test the same meson/ninja versions on all our
Ubuntu/Debian images. The point of having different images is to build
with different software versions. If `pip3 install` gives us the same
everywhere, it isn't very useful.
https://mesonbuild.com/Release-notes-for-0-54-0.html#ninja-version-requirement-bumped-to-17
error: bare words are no longer supported, please use "...": "x" != x
error: ^
error: /root/nm-build/NetworkManager/contrib/fedora/rpm/NetworkManager.20200402-030113.Hk7EGs/SPECS/NetworkManager.spec:32: bad %if condition: "x" != x
ERROR: rpmbuild FAILED
Sometimes these function may set errno to unexpected values like EAGAIN.
This causes confusion. Avoid that by using our own wrappers that retry
in that case. For example, in rhbz#1797915 we have failures like:
errno = 0;
v = g_ascii_strtoll ("10", 0, &end);
if (errno != 0)
g_assert_not_reached ();
as g_ascii_strtoll() would return 10, but also set errno to EAGAIN.
Work around that by using wrapper functions that retry. This certainly
should be fixed in glib (or glibc), but the issues are severe enough to
warrant a workaround.
Note that our workarounds are very defensive. We only retry 2 times, if
we get an unexpected errno value. This is in the hope to recover from
a spurious EAGAIN. It won't recover from other errors.
https://bugzilla.redhat.com/show_bug.cgi?id=1797915
Be more graceful and allow whitespaces around the floating point number
for DEVTIMEOUT. Note that _nm_utils_ascii_str_to_int64() is already graceful
against whitespace, so also be it with the g_ascii_strtod() code path.
g_ascii_strtoull() returns a guint64, which is very wrong to directly pass
to the variadic argument list of g_object_set(). We expect a guint there
and need to cast.
While at it, use _nm_utils_ascii_str_to_int64() to parse and validate the input.
Before:
It writes files out in a .ini-style format in
/etc/NetworkManager/system-connections/.
...
For security, it will ignore files that are readable or
writable by any user or group other than 'root' since private
keys...
After:
The files are in a .ini-style format and located in
/etc/NetworkManager/system-connections/,
/usr/lib/NetworkManager/system-connections/ and
/run/NetworkManager/system-connections/.
...
For security, it will ignore files that are readable or
writable by any user other than 'root' since private keys...
https://bugzilla.redhat.com/show_bug.cgi?id=1819259
Scenario:
- have ethernet connection as unmanaged.
- create (or have) a suitable profile for the connection.
- make the device as managed. No default wired connection gets
created.
- delete the profile.
Note that NMManager does in manager_device_state_changed():
»···if (NM_IN_SET (new_state,
»··· NM_DEVICE_STATE_UNAVAILABLE,
»··· NM_DEVICE_STATE_DISCONNECTED))
»···»···nm_settings_device_added (priv->settings, device);
that means, when the device the next time goes through
UNAVAILABLE/DISCONNECTED states, we will suddenly create the
default "Wired connection 1" profile.
That doesn't seem right. When a device is suitable to have a
default-wired connection, we should only check once whether to
create it. We should not retry that later. The !no-auto-default
mechanism exists so we can start NetworkManager without a profile for
the device. It doesn't mean that we later one (after previously deciding
not to create a profile), we still create it.
https://bugzilla.redhat.com/show_bug.cgi?id=1687937https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/450
$ meson -Dmore_asserts=0 meson-build
$ ninja -C meson-build
[712/859] Compiling C object 'src/initrd/b383957@@nmi-core@sta/nmi-cmdline-reader.c.o'.
../src/initrd/nmi-cmdline-reader.c: In function ‘nmi_cmdline_reader_parse’:
../src/initrd/nmi-cmdline-reader.c:871:4: warning: ‘s_ip’ may be used uninitialized in this function [-Wmaybe-uninitialized]
871 | nm_setting_ip_config_add_dns (s_ip, ns);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../src/initrd/nmi-cmdline-reader.c:835:21: note: ‘s_ip’ was declared here
835 | NMSettingIPConfig *s_ip;
| ^~~~
Fixes: 25a2b6e14f ('initrd: rework command line parsing')
When starting with a locked modem, it may take some time for the user to
enter the PIN code, leading to the secrets request timing out. In that
case, we want the connection activation to be retried automatically once
the modem is unlocked, which can't be achieved if we propagate the error,
as the device will change state to 'failed'.
This patch ignores the 'no-secrets' error, as it means either the
request has timed out, or the user cancelled the request without
notifying NetworkManager. By doing this, we allow the connection to be
re-activated once the modem is unlocked.
Signed-off-by: Arnaud Ferraris <arnaud.ferraris@collabora.com>
When the modem is unlocked externally to NetworkManager, it is kept in
the 'need-auth' state. The current connection activation continues as
if nothing had changed, and the secrets request for the PIN code (which
is no longer necessary) eventually times out. The device state is then
changed to 'failed', meaning there won't be a new try at activating the
default connection automatically.
In order to prevent this, and retry activating the default connection
when the modem gets unlocked, we change the device state to
'deactivating' when we identify the modem has been unlocked externally.
Signed-off-by: Arnaud Ferraris <arnaud.ferraris@collabora.com>
The 'default_connection' created by the command line parser has
multiple purposes. It's the connection created for 'ip=' arguments
without command line, but is also created when there is a 'bootdev='
or for 'nameserver=' and no other connection exists at the moment the
argument is parsed. This is confusing and leads to a result that
depends on the order of parameters. For example:
$ /usr/libexec/nm-initrd-generator -c connections -- bootdev=eth1 ip=eth0:dhcp
$ ls connections/
default_connection.nmconnection eth0.nmconnection
$ /usr/libexec/nm-initrd-generator -c connections -- ip=eth0:dhcp bootdev=eth1
$ ls connections/
eth0.nmconnection eth1.nmconnection
Make this more explicit by tracking 'bootdev_connection' and
'default_connection' individually.
Also fix handling of 'nameserver', 'rd.peerdns' and 'rd.route'
arguments. First process all connections, and then set those
properties. In particular, now nameservers are applied to all
connections.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/issues/391
