When nmcli needs secrets for a connection it asks them for every known
setting. nmtui is a bit smarter and asks them only for settings that
actually exist in the connection. Make a step further and let clients
ask secrets only for setting that exist *and* have any secret
property. This decreases the number of D-Bus calls when editing or
showing a connection with secrets.
https://bugzilla.redhat.com/show_bug.cgi?id=1506536https://github.com/NetworkManager/NetworkManager/pull/327
(cherry picked from commit 5b5a768b69)
Support importing ".conf" files as `wg-quick up` supports it.
`wg-quick` parses several options under "[Interface]" and
passes the remainder to `wg setconf`.
The PreUp/PreDown/PostUp/PostDown options are of course not supported.
"Table" for the moment behaves different.
(cherry picked from commit a3a8583c31)
@secrets is unreferenced at the end of request_secrets_from_ui() and
so try_spawn_vpn_auth_helper() must take a reference to it.
Fixes: 1a0fc8d437
(cherry picked from commit b57a3a4cc6)
Rework the explicit implementation of NM_SETTING_802_1X_PASSWORD_RAW
handling to generically handle GBytes properties.
Note that the NM_SETTING_802_1X_PASSWORD_RAW setter accepts a legacy
format where hex-words are separated by space. I don't think we want
to support this format for new options.
So, there are two possibilities:
1) either leave _set_fcn_802_1x_password_raw() as-is, with the special
handling.
2) interpret a property-data gobject_bytes.legacy_format.
1) seems to make more sense, because there is only one such property,
and we won't use this for new properties. However let's do 2), because
it shows nicely the two styles side-by-side. In other words, let's
password-raw also be a _pt_gobject_bytes typed property, with some
special legacy handling. Instead, of having it an entirely separate
property type (with a different setter implementation). I think it's
better to have the parts where they differ pushed down (the "stack") as
much as possible.
- it's less lines of code (for the caller).
- it's a function that can be easier unit-tested on its own.
Possibly there are already other unit-tests that cover it.
- it's more efficient than the GString based implementation.
- it reuses our one and only bin-to-hexstr implementation.
For now only add the core settings, no peers' data.
To support peers and the allowed-ips of the peers is more complicated
and will be done later. It's more complicated because these are nested
lists (allowed-ips) inside a list (peers). That is quite unusual and to
conveniently support that in D-Bus API, in keyfile format, in libnm,
and nmcli, is a effort.
Also, it's further complicated by the fact that each peer has a secret (the
preshared-key). Thus we probably need secret flags for each peer, which
is a novelty as well (until now we require a fixed set of secrets per
profile that is well known).
When asking for the preshared-key for WireGuard peers, the secret request
will be very verbose with redundant information. Allow suppressing the entry
id from the prompt.
It's not really used, but we shouldn't just forget about it.
Currently, we fill requests only based on the connection-type, ignoring
the setting-name. I guess, the concept of requesting secrets for a setting
is utterly broken. But equally broken it is to just look at the connection
(type). At least, don't just throw parts of the request away but keep
it.
Currently, default-routes cannot be added like regular static-routes
as ipv4.routes setting.
Instead, one has to configure "ipv4.gateway" and "ipv4.never-default".
That of course should be fixed, for example to configure a default-route
in different routing tables.
As it is, both nmcli's parse function and libnm's
NMSettingIPConfig:verify() functions reject default-routes.
But nmcli goes way beyond that, it also rejects all networks with
"0.0.0.0"/"::" even if their prefix length is not zero. Such routes are
not default-routes, and nmcli has no business rejecting them. The
correct way for checking for a default-route is to check the prefix-length
for zero.
Drop the wrong validation in nmcli.
Note, it may still not be the best idea to add catch-all routes like
"0.0.0.0/1" and "128.0.0.0/1". It just defeats what counts as a default-route.
NM has other means (like configuring the route-metric) to handle routing
in face of multiple interfaces. But sure, whatever works for you.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/issues/114https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/75
This adds support for configuring the Wi-Fi connections to use SAE. SAE
is a password-based authentication mechanism that replaces WPA-PSK in
WPA3-Personal.
The pass phrase is still stored in the "psk" property, with some
limitations lifted.
The generic connection validation produces a good result:
Error: failed to modify 802-11-wireless-security.psk: ':(' is not a valid PSK.
vs.:
Error: Failed to add 'wifi666' connection: 802-11-wireless-security.psk: property is invalid
Like also done for autotools, create and use intermediate libraries
from "shared/nm-utils/".
Also, replace "shared_dep" by "shared_nm_utils_base_dep". We don't
need super fine-grained selection of what we link. We can always
link in "shared/libnm-utils-base.a", and let the linker throw away
unsed parts.
The callback must be invoked, as also documented.
Otherwise, the tracked info gets leaked.
Let NMSecretAgentOld (the caller) be a bit resilient against
bugs in the client, and avoid a crash by prematurely remove
the request-info from the pending list. That does not fully
workaround the bug (it leads to a leak), but at least it does
not cause other "severe" issues.
The leak was present earlier as well.
NMSecretAgentOld's get_secrets_cb() gets this right and takes
a floating reference. So this was correct.
However, make this a bit more robust, and don't pass on
floating references. This was, we don't require the callee
to consume the reference.
Most of the times we actually need a NMSecretAgentSimple typed pointer.
This way, need need to cast less.
But even if we would need to cast more, it's better to have pointers
point to the actual type, not merely to avoid shortcomings of C.
No caller cared about the NM_SECRET_AGENT_ERROR_AGENT_CANCELED reason.
In particular, because previously the requests would keep the secret-agent
instance alive, and this never happend.
Also, NM_SECRET_AGENT_ERROR_AGENT_CANCELED precicley exists for
NMSecretAgentOld:cancel_get_secrets() (as documented). During finalize
we are not cancelled -- at least not the same way as
cancel_get_secrets(). Setting NM_SECRET_AGENT_ERROR_AGENT_CANCELED
is wrong.
Anyway, we have a default error for such cases already.
The code was correct. But it's hard to follow when and whether
the child-watch-id was destroyed at the right time.
Instead, always let _auth_dialog_data_free() clear the signal handlers.
Don't let RequestData keep the parent NMSecretAgentSimple instance
alive. Previously, the code in finalize() was never actually reached.
Also, move the final callback from finalize() to dispose(). It feels
wrong to invoke callbacks from finalize().
We must actually cancel the GCancellable. Otherwise, the pending async
operations are not cancelled. _auth_dialog_write_done() doesn't care
about that, but _auth_dialog_read_done() does. It must not touch the
destroyed data, after the operation is cancelled.
Note that previously the @requests hash took the request-id as key and
the RequestData as value. Likewise, the destroy functions of the head
would destroy the key and the value.
However, RequestData also had a field "request_id". But that pointer was
not owned (nor freed) by the RequestData structure. Instead, it was
relied that the hash kept the request-id alive long enough.
That is confusing. Let RequestData own the request-id.
Also, we don't need to track a separate key. Just move the request-id
as first filed in RequestData, and use compare/hash functions that
handle that correctly (nm_pstr_*()).
Code that is internal to a source file should not have a "nm" prefix.
That is what differenciates it from declarations in header files. It
makes it clearer that these names are only defined in the current file.
Also, our implementations of virtual functions shall have the same
name as the function pointer of the VTable (or at least, it shouldn't
have a "nm" prefix).
