Most string properties can be either %NULL (unset) or a non-empty
string.
For a few properties, like "gsm.apn", also the empty word is a valid
value. That makes it problematic to use from nmcli, because
nmcli connection modify "$PROFILE" gsm.apn ""
means to reset the default (NULL). How to configure the empty word?
For the APN, "" has a specific meaning, distinct from NULL, so we
need to be able to represent that.
The other problem with nmcli is that
nmcli -g gsm.apn connection show "$PROFILE"
is supposed to give you a value that you an set again, like
X="$(nmcli -g gsm.apn connection show "$PROFILE"; echo x)"
nmcli connection modify "$PROFILE2" gsm.apn "${X%$'\n'x}"
but for %NULL and "" the output would be the same.
The "solution" to that is interpreting "" as NULL (like we always did)
and a non-empty string that contains all whitespace, like a string with
one whitespace less. This way, all values can be expressed.
Note that in case of "gsm.apn", the string is anyway internally
normalized with g_strstrip(), so a string with all whitespace was
not expressable.
This was currently unused, because actually no property of type string
had handle_emptyunuset set.
Fixes: e9ee4e39f1 ('cli: handle string properties that can both be empty and %NULL')
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 ""
This will be used for NMTernary properties. The get() method
is still the same as for _pt_gobject_enum, but the setter
and complete functions are more flexible to also allow yes/true
and any unique abbreviations.
- use strstrip() to remove leading and trailing whitespace
- use _nm_utils_ascii_str_to_int64() for parsing numeric values
like -1, 0 and 1. In particular, this now also allows passing
the numeric values.
- also accept "default" as valid value for NM_TERNARY_DEFAULT.
With this change, nmc_string_to_ternary() can also parse everything that
we commonly and currently parse with _nm_utils_enum_from_str_full()
and NM_TYPE_TERNARY. This will allow to configure ternary values in
a more flexible way.
- use strstrip() to remove leading and trailing whitespace
- use _nm_utils_ascii_str_to_int64() for parsing numeric values
like 0 and 1. The difference is small, for one, it also accepts
hex numbers like 0x1. More interestingly, it uses our common
number parsing function, and we will later do the same for
parsing ternaries.
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.
When supported by the D-Bus daemon, it's better to have service files
in /usr rather than in /etc. Change the path for RHEL 8.
See also commit ef8c292881 ('contrib/rpm: install D-Bus service
files to /usr if we can').
wpa-eap-suite-b-192 key-mgmt method uses special values for "pairwise"
and "group" ciphers, we can also handle that a few lines underneath
where those are set to make this a bit easier to read.
We currently set the supplicants PMF config (ieee80211w) inside an if
block that tries to detect whether WPA is used. That if-block doesn't
include the "wpa-eap-suite-b-192" case because we want special
"pairwise" and "group" handling for wpa-eap-suite-b-192. This means
we're currently missing to enable PMF in the "wpa-eap-suite-b-192" case,
even though it's set to REQUIRED.
Fix it by moving the "pairwise" and "group" special-casing down a bit so
we can include "wpa-eap-suite-b-192" in the "Only WPA-specific things
when using WPA" check, that will make sure ieee80211w gets set in the
wpa-eap-suite-b-192 case.
A small bug sneaked into commit 3ef3733c81 ('wireless-security: ensure
Mesh networks can't use anything but SAE') during review: Instead of
allowing only "sae" or "none" as key-mgmt, we now disallow "sae" and
"none", but allow anything else. This is obviously not what was
intended, so fix the check.
Also move the valid_key_mgmt check back up to where it was before that
commit, it seems we want to apply that check in all cases.
We only set the "ieee80211w" option in the wpa_supplicant config in case
we're using WPA (see the if-block underneath), otherwise the value of
"pmf" is completely ignored. That means the override here (in case WPA
isn't used) isn't getting applied anyway, so just remove it.
It looks a bit weird on the first glance that we do nothing when
NM_SETTING_WIRELESS_SECURITY_PMF_OPTIONAL is used. The reason for this
is that we already intialize the global option "pmf" of wpa_supplicant
to "1" (optional), so add a brief comment about that here.
ResolveAddress is a simple D-Bus API. One might think that it does not
require much logic, and we would just make the D-Bus call directly.
However, we want to ensure that we always sent our latest DNS
configuration to systemd-resolved first, before making any D-Bus request
to resolve a name.
Also, as we D-Bus activate the service and watch the name owner, we
want to handle that. If a user resolves a name while we are still D-Bus
activating, then we need to queue the request.
This adds some complexity in the implementation. But the benefit is that
the API becomes simple from the user's point of view. The queuing and
avoiding race conditions is handled internally.
The implementation does not use standard glib patterns with GAsyncResult
GTask and GCancellable. I find implementing them correctly is more
effort and leads to a less nice API. That might be fine for public API,
which should follow glib style. But this is entirely internal, and
returning a NMDnsSystemdResolvedResolveHandle that allows cancellation
seems more convenient to me.
As we already track the current name owner, it seems wrong to make
requests to the well-known name. It theoretically opens a race
where we would talk to the wrong systemd-resolved instance.
So far, we didn't care and it didn't make much of a difference.
Still, let's talk to the process that we expect.
For infiniband, request_broadcast is automatically (and always) enabled.
Otherwise, we usually don't enable it, and (unlike systemd-networkd),
there is currently no configuration option to enable it.
Still honor the new udev property that can indicate to enable the flag
per device.
See-also: https://github.com/systemd/systemd/pull/ ### 19346
The DHCP client has potentially a large number of options,
including boolean options (flags). It is cumbersome to implement
them one by one. Instead, make more prominent use of NMDhcpClientFlags.
Previously, we used nm_udev_utils_property_as_boolean(), which was
taken from g_udev_device_get_property_as_boolean(). That function
accepts "1" and "true" (with ASCII case insensitive).
When we parse a flag, there is no need to reject "no", "yes" or
"on"/"off" as invalid (and thus return FALSE). We have a boolean
parse method _nm_utils_ascii_str_to_bool(), which parses everything
that nm_udev_utils_property_as_boolean() accepts, and more.
Be liberal in what we accept, so use our general parse function.
g_key_file_set_comment() has slightly weird API that will fail to set a
comment if it doesn't find the group. This is the case here since we
haven't set any strings under the [Security] group yet.
Fixing this is kind of ugly, so for now just don't add that comment in
the case where we don't have the [Security] group.
There are cases where the settings didn't actually change and we just
want to ensure NM and iwd settings are in sync (one such case would be
when a setting was changed while iwd wasn't running, here we want to
synchronize all settings when starting up iwd).
We're already doing this and calling sett_conn_changed() from
mirror_connection() on all connections after adding an interface, the
actual settings synchronization code doesn't get executed though because
we're passing 0 as update_reason, which means we bail out early from
sett_conn_changed().
To make sure we actually update the iwd network configurations in that
case, too, pass UPDATE_REASON_UPDATE_NON_SECRET as the update reason to
sett_conn_changed(), which is the correct update reason in this case.
Quite obviously, we want to update the AutoConnect setting of the iwd
network in case the NM and iwd settings differ, not if they are the
same. So check for unequality here instead of equality, which fixes the
AutoConnect setting's synchronization.
Fixes: 4229c97012 ('iwd: Mirror NM connections to IWD network config files'):
The macro always overwrites LIBS and the result is that every binary
links against libreadline.
Instead, save the library to READLINE_LIBS.
See also: 94274c6fcd ('build: fix wrongly linking against libreadline in all applications')
Fixes: af360238be ('m4/ax_lib_readline.m4: Update after running aclocal')
Apparently moving secrets between priv->connection and
priv->system_secrets in the various places throughout
NMSettingsConnection is no longer needed and has no effect on the
state of the D-Bus object or the gobject visible from outside. It
seems that it was needed for the secrets handling in NMDevice
subclasses before the introduction of the applied connection concept
but now nm_connection_need_secrets() is called in those subclasses
directly on the applied connection object and the secrets obtained
from multiple nm_settings_connection_get_secrets calls are also
collected directly in the applied connection's settings.
Drop the system secrets cache mechanism as a cause of a minor memory
overhead, some code overhead and also a source of some unneeded gobject
signals as the connection settings were being updated.
Note: the NM_SETTINGS_CONNECTION_UPDATE_REASON_CLEAR_SYSTEM_SECRETS and
NM_SETTINGS_CONNECTION_UPDATE_REASON_RESET_SYSTEM_SECRETS flags in the
SettingsConnection update signals appear to only have been used by the
SettingsConnection internally to keep priv->system_secrets up to date.
They can have potential other uses in the handlers of those signals so I
kept them. Unlike some of the other NMSettingsConnectionUpdateReason
values these are actual update *reasons* and not flags telling the
settings backends how to handle a specific change in the settings.
All usages of nm_settings_connection_clear_secrets() outside of the
NMSettingsConnection implementation were setting the
clear_cached_system_secrets parameter to FALSE which meant that the
operation was a no-op since the system-secrets cache kept a copy of the
secrets being cleared and any access to the SettingsConnection through
the D-Bus API or the class methods would behave the same as without the
call, with the exception of directly reading the settings from the
result of nm_settings_connection_get_connection(). The calls would
still generate D-Bus and gobject signals however, which were redundant.
Drop the method and its calls from the rest of NM code as not needed and
potentially confusing. The comments preceding these calls implied that
they were needed so that the next activation attempt would be forced to
use nm_settings_connection_get_secrets() but this was the case probably
only before the applied connection concept was introduced.
Also drop two nm_active_connection_clear_secrets() uses in
NMVpnConnection, right before the teardown of the active connection,
that could only possibly have any effect if they affected the
NMSettingsConnection, but as mentioned earlier the
nm_settings_connection_clear_secrets() use inside
nm_active_connection_clear_secrets() didn't do anything and is now
removed.
The one internal use of nm_active_connection_clear_secrets() in the
D-Bus ClearSecrets() implementation is inlined.
