Previously, both nm_setting_connection_add_permission() and the GObject
property setter would merely assert that the provided values are valid
(and otherwise don't do anything). That is bad for handling errors.
For example, we use the property setter to initialize the setting from
keyfile and GVariant (D-Bus). That means, if a user provides an invalid
permissions value, we would emit a g_critical() assertion failure, but
otherwise ignore the configuration. What we instead need to do is to
accept the value, and afterwards fail verification. That way, a proper error
message can be generated.
$ mcli connection add type ethernet autoconnect no ifname bogus con-name x connection.permissions 'bogus:'
(process:429514): libnm-CRITICAL **: 12:12:00.359: permission_new: assertion 'strchr (uname, ':') == NULL' failed
(process:429514): libnm-CRITICAL **: 12:12:00.359: nm_setting_connection_add_permission: assertion 'p != NULL' failed
Connection 'x' (2802d117-f84e-44d9-925b-bfe26fd85da1) successfully added.
$ $ nmcli -f connection.permissions connection show x
connection.permissions: --
While at it, also don't track the permissions in a GSList. Tracking one
permission in a GSList requires 3 allocations (one for the user string,
one for the Permission struct, and one for the GSList struct). Instead,
use a GArray. That is still not great, because GArray cannot be embedded
inside NMSettingConnectionPrivate, so tracking one permission also
requires 3 allocations (which is really a fault of GArray). So, GArray
is not better in the common case where there is only one permissions. But even
in the worst case (only one entry), GArray is no worse than GSList.
Also change the API of nm_setting_connection_add_permission().
Previously, the function would assert that the arguments are in
a certain form (strcmp (ptype, "user") == 0), but still document
the such behaviors like regular operation ("[returns] %FALSE if @ptype
or @pitem was invalid"). Don't assert against the function arguments.
Also, if you first set the user to "fo:o", then
nm_setting_connection_add_permission() would accept it -- only at
a later phase, the property setter would assert against such values.
Also, the function would return %FALSE both if the input value was
invalid (an error) and if the value already existed. I think the
function should not treat a duplicate entry like a badly formatted
input.
Now the function does much less asserting of the arguments, but will
return %FALSE only if the values are invalid. And it will silently ignore
duplicate entries.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/636
Compare to the connection's GetSettings() call, which is not protected
by policykit permissions. It only checks that the requesting user is
allowed according to "connection.permission".
Previously, device's GetAppliedConnection() requires "network-control"
permissions. This although it only reads a profile, without modifying
anything. That seems unnecessary, also because in the common case the
applied connection is identical to the current settings connection, and
the latter can be read without special permissions.
Don't require a special policykit permission to read the applied
connection.
https://bugzilla.redhat.com/show_bug.cgi?id=1882380
Run:
./contrib/scripts/nm-code-format.sh -i
./contrib/scripts/nm-code-format.sh -i
Yes, it needs to run twice because the first run doesn't yet produce the
final result.
Signed-off-by: Antonio Cardace <acardace@redhat.com>
Adding NML3Cfg support is a large effort that is done in parallel.
While already parts of the code is merged, it is not actually used
yet. Remove the parts from NMDevice that interact with NML3Cfg
before we actually start using it.
The point is that we might want to do a release before switching
over to the new way. For that release, we should not have the unused
code in NMDevice.
This patch will later be restored and extended.
But also take a reference whenever we have any configurations
registered. Registering a configuration means to automatically
keep the instance alive.
Any user must take care to unregister again when it no longer requires
the configuration.
Currently all NMDevice instance register to the platform change signals,
then if a signal for their IP ifindex appears, they schedule a task on
an idle handler. That is wasteful.
NML3Cfg already gets a notification on an idle handler and can just re-emit
it to the respective listeners.
With this, there is only one subscriber to the platform signals (NMNetns)
which then multiplexes the signals to the right NML3Cfg instances, and
further.
The caller may want to know the merge NML3ConfigData as it would be used
for the next commit, without already committing it.
Track both the NML3ConfigData instance that is merged from the
registered items, and the one that was used the last time during
commit.
It's often convenient not to require the caller to check for
%NULL. On the other hand, there are cases where having a %NULL instance
is a bug, so gracefully accepting %NULL might hide bugs.
Still, change it.
nm_utils_hexstr2bin_full() is our general hexstr to binary parsing
method. It uses (either mandatory or optional) delimiters. Before,
if delimiters are in use, it would accept individual hexdigits.
E.g. "a:b" would be accepted as "0a:0b:.
Add an argument that prevents accepting such single digits.
All our devices will return the same value on D-Bus: a "u" variant with zero value.
Since NMDBusObject caches all the property values, we can share the instance.
The "Ip4Address" property of "org.freedesktop.NetworkManager.Device"
interface is deprecated since version 0.9.9.1 (2013). Also, the property
is not exposed by libnm and generally not useful.
Drop the code to maintain it. The property still exists but always
returns 0 (0.0.0.0).
NML3ConfigData is a simple container structure that contains no logic.
Also, DHCP code already is intimately related to NMIP[46]Config (for
now, later that will be NML3ConfigData).
It makes sense that NMNDisc is aware of NML3ConfigData and knows how to
conver the RA data into an l3cd instance.
NMDevice currently configures use_tempaddr sysctl itself. Later,
NML3Cfg should do that, so we need to keep track of that as part
of the configuration.
It is bad style to rely on the last unref of an object for stopping
the operation. With a ref-counted object you should never rely on
anybody else still having (or not having) a reference. Hence, you
should not rely on stopping the ND during the last unref.
Add an explicit nm_ndisc_stop() function.
Previously, if we passed ra_timeout 0 to NMNDisc, then it would
calculate the effective timeout based on the router-solicitations
and the router-solicitation-interval.
The caller may want to know the used timeout, to also run its own timers
with the same timeout. Hence, it cannot leave this automatism internal
to NMNDisc.
nm_utils_inet4_ntop() is public API of libnm. Also, it accepts a
%NULL buffer to use a static buffer. That is error prone and we
should not use such convenience behavior for our own code.
