NM_DEVICE_MODEM_GET_PRIVATE() is based on _NM_GET_PRIVATE(), which has
some smarts to check the pointer type, but is fine with well-known parent
pointer types like "NMDevice *".
If the @append_force argument is set and the object is already in the
list, it must be moved at the end.
Fixes: 22edeb5b69 ('core: track addresses for NMIP4Config/NMIP6Config via NMDedupMultiIndex')
Add test to show a wrong result of ip_ipX_config_replace() due to a
bug in _nm_ip_config_add_obj(). When an address is added to the tail
of the index and another address with the same id already exists, the
existing object is left at the same place, breaking the order of
addresses.
Nowadays, we should prefer "/run" over "/var/run". When not specifying
during ./configure, autotools however still defaults to "/var/run".
This default is also visible in the pre-generated documenation, for
example `man NetworkManager.conf` says
Unless the symlink points to the internal file /run/NetworkManager/resolv.conf,
in which case the ...
It's important whether a setting is present or not. Keyfile writer
omits properties that have a default value, that means, if the setting
has all-default values, it would be dropped. For [proxy] that doesn't
really matter, because we tend to normalize it back. For some settings
it matters:
$ nmcli connection add type bluetooth con-name bt autoconnect no bluetooth.type dun bluetooth.bdaddr aa:bb:cc:dd:ee:ff gsm.apn a
Connection 'bt' (652cabd8-d350-4246-a6f3-3dc17eeb028f) successfully added.
$ nmcli connection modify bt gsm.apn ''
When storing this to keyfile, the [gsm] section was dropped
(server-side) and we fail an nm_assert() (omitted from the example
output below).
<error> [1566732645.9845] BUG: failure to normalized profile that we just wrote to disk: bluetooth: 'dun' connection requires 'gsm' or 'cdma' setting
<trace> [1566732645.9846] keyfile: commit: "/etc/NetworkManager/system-connections/bt.nmconnection": profile 652cabd8-d350-4246-a6f3-3dc17eeb028f (bt) written
<trace> [1566732645.9846] settings: update[652cabd8-d350-4246-a6f3-3dc17eeb028f]: update-from-dbus: update profile "bt"
<trace> [1566732645.9849] settings: storage[652cabd8-d350-4246-a6f3-3dc17eeb028f,3e504752a4a78fb3/keyfile]: change event with connection "bt" (file "/etc/NetworkManager/system-connections/>
<trace> [1566732645.9849] settings: update[652cabd8-d350-4246-a6f3-3dc17eeb028f]: updating connection "bt" (3e504752a4a78fb3/keyfile)
<debug> [1566732645.9857] ++ connection 'update connection' (0x7f7918003340/NMSimpleConnection/"bluetooth" < 0x55e1c52480e0/NMSimpleConnection/"bluetooth") [/org/freedesktop/NetworkManager>
<debug> [1566732645.9857] ++ gsm [ 0x55e1c5276f80 < 0x55e1c53205f0 ]
<debug> [1566732645.9858] ++ gsm.apn < 'a'
Of course, after reload the connection on disk is no loner valid.
Keyfile writer wrote an invalid setting.
# nmcli connection reload
Logfile:
<warn> [1566732775.4920] keyfile: load: "/etc/NetworkManager/system-connections/bt.nmconnection": failed to load connection: invalid connection: bluetooth: 'dun' connection requires 'gsm' or 'cdma' setting
...
<trace> [1566732775.5432] settings: update[652cabd8-d350-4246-a6f3-3dc17eeb028f]: delete connection "bt" (3e504752a4a78fb3/keyfile)
<debug> [1566732775.5434] Deleting secrets for connection /org/freedesktop/NetworkManager/Settings (bt)
<trace> [1566732775.5436] dbus-object[9a402fbe14c8d975]: unexport: "/org/freedesktop/NetworkManager/Settings/55"
I thought I would need this, but ended up not using it.
Anyway, it makes sense in general that the function can lookup
all relevant information, so merge it.
First of all, keyfile writer (and reader) are supposed to be able to store
every profile to disk and re-read a valid profile back. Note that the profile
might be modified in the process, for example, blob certificates are written
to a file. So, the result might no be exactly the same, but it must still be
valid (and should only diverge in expected ways from the original, like mangled
certificates).
Previously, we would re-read the profile after writing to disk. If that failed,
we would only fail an assertion but otherwise proceeed. It is a bug
after all. However, it's bad to check only after writing to file,
because it results in a unreadable profile on disk, and in the first
moment it appears that noting went wrong. Instead, we should fail early.
Note that nms_keyfile_reader_from_keyfile() must entirely operate on the in-memory
representation of the keyfile. It must not actually access any files on disk. Hence,
moving this check before writing the profile must work. Otherwise, that would be
a separate bug. Actually, keyfile reader and writer violate this. I
added FIXME comments for that. But it doesn't interfere with this
patch.
NM didn't support wpa-none for years because kernel drivers used to be
broken. Note that it wasn't even possible to *add* a connection with
wpa-none because it was rejected in nm_settings_add_connection_dbus().
Given that wpa-none is also deprecated in wpa_supplicant and is
considered insecure, drop altogether any reference to it.
This file causes a crash [1], add it to the tests.
Note that the test only check parsing the file and the
crash happens in the "upper" layers. So, it's not really
a test for the crash. But at least have such a file in
our repository.
[1] https://gitlab.freedesktop.org/NetworkManager/NetworkManager/issues/235
There is really no reason for this to be a macro. Our hash-related
helpers (like nm_hash_update_val()) are macros because they do some
shenigans to accept arguments of different (compile-time) types. But
the arguments for nm_hash_obfuscate_ptr() are well known and expected
of a certain form.
Note that with "-O2" some quick testing shows that the compiler no
longer inlines the function. But I guess that's fine, probably the
compiler knows best anyway.
Logging pointer values might reveal information that can be used to defeat
ASLR. We should avoid that.
On the other hand, it's useful to tag a logging message with the pointer
value of the "source" of the message. It helps to correlate messages and
search for relevant messages in the log.
As a compromise, use NM_HASH_OBFUSCATE_PTR(), like we do at several places
already. For example, we also log
<debug> [1566550899.7901] setup NMPlatform singleton (29a6af9867f2e5d0)
This obfuscated value is a 64 bit unsigned integer with the siphash24
hash of the raw value with a randomized seed. Of course, contrary to the
pointer value, there is a tiny chance that two different pointers hash
to the same identifier. However, that seems unlikely enough to be of no
concern. Note that this pointer value is only logged to aid debugging.
It is sufficiently unlikely that this causes confusion.
One other downside of printed the obfuscated value, is that you can no
longer read the pointer from the log and use it in gdb directly. That
might be sometimes convenient, but making this impossible is kinda the
purpose of this change.
As such, nm_log_ptr() becomes a bit of a misnomer. But not too bad, it
still is a good name. For example, if we wanted we could redefine the
NM_HASH_OBFUSCATE_PTR* macros when building "--with-more-asserts".
Previously we only cared whether supplicant is build with support for
FT. In that case we would pass FT-PSK to supplicant, like
Config: added 'key_mgmt' value 'WPA-PSK WPA-PSK-SHA256 FT-PSK'
Supplicant would then always try FT with preference, regardless whether
the interface/driver support it. That results in a failure to associate, if
the driver does not support it.
NetworkManager[1356]: <info> [1566296144.9940] Config: added 'key_mgmt' value 'WPA-PSK WPA-PSK-SHA256 FT-PSK'
...
wpa_supplicant[1348]: wlan0: WPA: AP key_mgmt 0x42 network profile key_mgmt 0x142; available key_mgmt 0x42
wpa_supplicant[1348]: wlan0: WPA: using KEY_MGMT FT/PSK
...
wpa_supplicant[1348]: * akm=0xfac04
...
kernel: ERROR @wl_set_key_mgmt :
kernel: invalid cipher group (1027076)
Since we pass a list of acceptable "key_mgmt" options to supplicant,
FT-PSK should not be used when supplicant knows it's not supported.
That is a supplicant bug.
Regardless, work around it by checking the per-interface capability, and
avoid it if support is apparently not present.
Systemd v243 is complaining about the wrong substitution there. That is
sort of harmless, because systemd-udevd in that version doesn't need the
rule anyway. But still fix it, to avoid a warning.
Also, newer udevd's $PATH doesn't include sbin. That is also okay,
because we don't need the rule to actually work there. But fix it
anyway.
https://bugzilla.redhat.com/show_bug.cgi?id=1740655
Various cleanups:
- after detecting the modifier, remove it from the string right away.
It's redundant and confusing to do it later.
- rename variables and move to inner scope.
- don't use g_str_split() to split the property name at the
first dot. strchr() is sufficient.
Also, now that we strip the modifier from option early, they start also
working for aliases. There is no need to not support (or behave
differently) w.r.t. whether aliases support modifiers or not.
This fixes:
$ nmcli connection modify r +ip4 192.168.5.2/24
Error: invalid <setting>.<property> 'ip4'.
The enum values are unique throughout the source code so they
can easier be searched (e.g. with grep), compared to '\0'. It
is often interesting where a certain modifier is used, so searching
the source code is important to give relevant results.
Also, the modifier is really an enum and we shouldn't misuse char type.
If that would be a good idea in general, we wouldn't need any enums
at all. But we use them for good reasons.
Let's enable the option to use IWD as an alternative to wpa_supplicant
for Wi-Fi support. People have been asking for this, it works, and is well
maintained.
