Commit graph

17296 commits

Author SHA1 Message Date
Íñigo Huguet
bdbcda1e22 vpn: handle hint tags in the daemon
Commit 345bd1b187 ('libnmc: fix secrets request on 2nd stage of 2FA
authentication') and commit 27c701ebfb ('libnmc: allow user input in
ECHO mode for 2FA challenges') introduced 2 new tags that hints for the
secret agents can have as prefix.

These tags were processed (and removed) in the secret agents, not in the
daemon. This is wrong because a system with an updated VPN plugin but a
not yet updated secret agent (like nm-plasma) will fail: it won't remove
the prefix and the daemon will save the secret with the prefix, i.e.
"x-dynamic-challenge:challenge-response" instead of just
"challenge-response". Then, VPN plugins doesn't recognize it, failing the
profile's activation. This is, in fact, an API break.

Also, if the VPN connection already existed before updating NM and the
VPN plugin, the secret flags are not added to the profile (they are only
added when the profile is created or modified). This causes the user's
first time response is saved to the profile, so the activation fails the
second and next times.

See:
- https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/1536
- https://gitlab.gnome.org/GNOME/NetworkManager-openvpn/-/issues/142

Anyway, in a good design the daemon should contain almost all the logic
and the clients should keep as simple as possible. Fix above's problems
by letting the daemon to receive the secret names with the prefix
already included. The daemon will strip it and will know what it means.

Note that this is done only in the functions that saves the secrets from
the data received via D-Bus. For example, nm_setting_vpn_add_secret
doesn't need to do it because this value shouldn't come from VPN
plugin's hints.

(cherry picked from commit 0583e1f843)
(cherry picked from commit 574741783c)
2024-06-18 16:06:13 +02:00
Beniamino Galvani
83847cc621 vpn: allow IP configurations with routes and without addresses
Usually, when the method is "auto" we want to avoid configuring routes
until the automatic method completes. To achieve that, we clear the
"allow_routes_without_address" flag of l3cds when the method is "auto".

For VPNs, IP configurations with only routes are perfectly valid,
therefore set the flag.

(cherry picked from commit d1ffdb28eb)
(cherry picked from commit 5b4ed809cc)
2024-06-17 15:37:08 +02:00
Beniamino Galvani
e008ec7345 core: add nm_l3_config_data_set_allow_routes_without_address()
Add a function to set the allow-routes-without-address flag for
l3cds. It will be used in the next commit.

(cherry picked from commit a3ce13c947)
(cherry picked from commit 5fa063f90d)
2024-06-17 15:37:08 +02:00
Beniamino Galvani
ea731bba9b core: rename l3cd's "dhcp_enabled" to "allow_routes_without_address"
The name "dhcp_enabled" is misleading because the flag is set for
method=auto, which doesn't necessarily imply DHCP. Also, it doesn't
convey what the flag is used for. Rename it to
"allow_routes_without_address".

(cherry picked from commit b31febea22)
(cherry picked from commit 6897b6ecfd)
2024-06-17 15:37:08 +02:00
Beniamino Galvani
476a9553f6 vpn: allow IP configurations without addresses
An IPv4-over-IPv6 (or vice-versa) IPsec VPN can return IP
configurations with routes and without addresses. For example, in this
scenario:

         +---------------+         +---------------+
         |  fd01::10/64  <-- VPN -->  fd02::20/64  |
         |     host1     |         |     host2     |
         +-------^-------+         +-------^-------+
                 |                         |
         +-------v-------+         +-------v-------+
         |    subnet1    |         |    subnet2    |
         | 172.16.1.0/24 |         | 172.16.2.0/24 |
         +---------------+         +---------------+

host1 and host2 establish a IPv6 tunnel which encapsulates packets
between the two IPv4 subnets. Therefore, in routed mode, host1 will
need to configure a route like "172.16.2.0/24 via ipsec1" even if the
host doesn't have any IPv4 address on the VPN interface.

Accept IP configurations without address from the VPN; only check that
the address and prefix are sane if they are provided.

(cherry picked from commit 97f185e1f8)
(cherry picked from commit 518b7c5bd5)
2024-06-17 15:37:08 +02:00
Gris Ge
91f63030b1 checkpoint: fix port reactivation when controller is deactivating
Problem:

    Given a OVS port with `autoconnect-ports` set to default or false,
    when reactivation required for checkpoint rollback,
    previous activated OVS interface will be in deactivate state after
    checkpoint rollback.

The root cause:

    The `activate_stage1_device_prepare()` will mark the device as
    failed when controller is deactivating or deactivated.
    In `activate_stage1_device_prepare()`, the controller device is
    retrieved from NMActiveConnection, it will be NULL when NMActiveConnection
    is in deactivated state. This will cause device been set to
    `NM_DEVICE_STATE_REASON_DEPENDENCY_FAILED` which prevent all follow
    up `autoconnect` actions.

Fix:
    When noticing controller is deactivating or deactivated with reason
    `NM_DEVICE_STATE_REASON_NEW_ACTIVATION`, use new function
    `nm_active_connection_set_controller_dev()` to wait on controller
    device state between NM_DEVICE_STATE_PREPARE and
    NM_DEVICE_STATE_ACTIVATED. After that, use existing
    `nm_active_connection_set_controller()` to use new
    NMActiveConnection of controller to move on.

Resolves: https://issues.redhat.com/browse/RHEL-31972

Signed-off-by: Gris Ge <fge@redhat.com>
(cherry picked from commit a68d2fd780)
(cherry picked from commit 4726822fb0)
2024-06-01 17:18:58 +02:00
Thomas Haller
3f3f446f6e core: workaround "-Wnonnull-compare" warning in nm_lldp_neighbor_tlv_get_oui()
../src/libnm-lldp/nm-lldp-neighbor.c: In function ‘nm_lldp_neighbor_tlv_get_oui’:
  ../src/libnm-std-aux/nm-std-aux.h:191:12: error: ‘nonnull’ argument ‘oui’ compared to NULL [-Werror=nonnull-compare]
    191 |         if (expr)                      \
        |            ^
  ../src/libnm-std-aux/nm-std-aux.h:202:27: note: in expansion of macro ‘_NM_BOOLEAN_EXPR_IMPL’
    202 |                           _NM_BOOLEAN_EXPR_IMPL(NM_UNIQ, expr))
        |                           ^~~~~~~~~~~~~~~~~~~~~
  ../src/libnm-glib-aux/nm-macros-internal.h:1693:31: note: in expansion of macro ‘NM_BOOLEAN_EXPR’
   1693 | #define _G_BOOLEAN_EXPR(expr) NM_BOOLEAN_EXPR(expr)
        |                               ^~~~~~~~~~~~~~~
  /usr/include/glib-2.0/glib/gmacros.h:1244:43: note: in expansion of macro ‘_G_BOOLEAN_EXPR’
   1244 | #define G_LIKELY(expr) (__builtin_expect (_G_BOOLEAN_EXPR(expr), 1))
        |                                           ^~~~~~~~~~~~~~~
  /usr/include/glib-2.0/glib/gmessages.h:661:9: note: in expansion of macro ‘G_LIKELY’
    661 |     if (G_LIKELY (expr)) \
        |         ^~~~~~~~
  ../src/libnm-lldp/nm-lldp-neighbor.c:651:5: note: in expansion of macro ‘g_return_val_if_fail’
    651 |     g_return_val_if_fail(oui, -EINVAL);
        |     ^~~~~~~~~~~~~~~~~~~~

(cherry picked from commit a500538fb2)
2024-06-01 17:18:58 +02:00
Beniamino Galvani
ebf25794d9 checkpoint: preserve in-memory state of connections
If a connection is in-memory (i.e. has flag "unsaved"), after a
checkpoint and rollback it can be wrongly persisted to disk:

 - if the connection was modified and written to disk after the
   rollback, during the rollback we update it again with persist mode
   "keep", which keeps it on disk;

 - if the connection was deleted after the rollback, during the
   rollback we add it again with persist mode "to-disk".

Instead, remember whether the connection had the "unsaved" flag set
and try to restore the previous state.

However, this is not straightforward as there are 4 different possible
states for the settings connection: persistent; in-memory only;
in-memory shadowing a persistent file; in-memory shadowing a detached
persistent file (i.e. the deletion of the connection doesn't delete
the persistent file). Handle all those cases.

Fixes: 3e09aed2a0 ('checkpoint: add create, rollback and destroy D-Bus API')
(cherry picked from commit c979bfeb8b)
2024-05-23 13:46:24 +02:00
Beniamino Galvani
e5837aa1d3 settings: add nm_settings_connection_persist_mode_to_string()
(cherry picked from commit a48b7fe7b9)
2024-05-23 13:44:11 +02:00
Beniamino Galvani
9f992dd85e dcb: fix test compilation
GCC 14 with LTO generates the following warning:

  src/core/tests/test-dcb.c: In function 'test_dcb_cleanup':
  src/core/tests/test-dcb.c:283:5: error: array subscript _3 is outside array bounds of 'const char *[0:]' [-Werror=array-bounds=]
    283 |     g_assert_cmpstr(expected.cmds[expected.num], ==, NULL);
        |     ^
  src/core/tests/test-dcb.c:14:17: note: while referencing 'cmds'
     14 |     const char *cmds[];
        |                 ^
  src/core/tests/test-dcb.c:261:24: note: defined here 'expected'
    261 |     static DcbExpected expected = {
        |                        ^

Define the commands as a fixed array instead of flexible array member.

(cherry picked from commit 9ff7ff28fc)
2024-05-15 12:11:57 +02:00
Beniamino Galvani
efa1d0e173 libnm-core: avoid compiler warnings in team settings
GCC 14 with LTO complains with:

  In function 'nm_team_link_watcher_new_ethtool',
      inlined from 'nm_team_link_watcher_new_ethtool' at src/libnm-core-impl/nm-setting-team.c:106:1:
  src/libnm-core-impl/nm-setting-team.c:130:33: error: array subscript 'struct NMTeamLinkWatcher[0]' is partly outside array bounds of 'unsigned char[16]' [-Werror=array-bounds=]
    130 |     watcher->ref_count          = 1;
        |                                 ^
  src/libnm-core-impl/nm-setting-team.c:128:15: note: object of size 16 allocated by 'g_malloc'
    128 |     watcher = g_malloc(nm_offsetofend(NMTeamLinkWatcher, ethtool));
        |               ^

even if the warning is disabled via pragma directives in that
code. This looks like the following GCC bug:

  https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80922

saying

  We do not track warning options (and thus optimize pragmas /
  attributes) across LTO because they are not saved in the function
  specific optimization flag section.

We use a (NMTeamLinkWatcher *) to point to a memory area that is
shorter than the struct, because depending on the watcher type we need
to store different parameters; in this way we can save few bytes of
memory for some watcher types. However, this often breaks when
upgrading the compiler; instead just allocate the full struct.

(cherry picked from commit d369f55192)
2024-05-15 12:11:52 +02:00
Beniamino Galvani
78d5ba4d4f libnm-glib-aux: fix comments about UUID generation
Whether the length is supplied explicitly or implicitly (via -1), the
result is the same. Update the comment.

(cherry picked from commit fcea2f174e)
2024-05-15 12:11:42 +02:00
Beniamino Galvani
cbf20a2317 libnm-glib-aux: fix "maybe-uninitialized" error when generating UUID
GCC 14 complans with:

  src/libnm-glib-aux/nm-uuid.c: In function 'nm_uuid_generate_from_strings_strv':
  src/libnm-glib-aux/nm-uuid.c:492:12: error: '_1' may be used uninitialized [-Werror=maybe-uninitialized]
    492 |     return nm_uuid_generate_from_string_str(s, slen, uuid_type, type_args);
        |            ^
  src/libnm-glib-aux/nm-uuid.c:392:1: note: by argument 1 of type 'const char *' to 'nm_uuid_generate_from_string_str' declared here
    392 | nm_uuid_generate_from_string_str(const char   *s,
        | ^

"-Wmaybe-uninitialized" diagnoses passing pointers or references to
uninitialized memory to functions taking const-qualified arguments.

In this case, nm_uuid_generate_from_string_str()'s first argument is a
"const char *" and so the compiler expects that the string is always
initialized. However, it is not initialized when len is zero.

A non-null zero-length array can be specified in two ways: by setting
len to zero, or by setting len to -1 and having NULL as first
element. Handle both cases in the same way.

(cherry picked from commit 2386c0f52d)
2024-05-15 12:11:35 +02:00
Íñigo Huguet
15ffa8ec6f platform: avoid routes resync for routes that we don't track
When we recibe a Netlink message with a "route change" event, normally
we just ignore it if it's a route that we don't track (i.e. because of
the route protocol).

However, it's not that easy if it has the NLM_F_REPLACE flag because
that means that it might be replacing another route. If the kernel has
similar routes which are candidates for the replacement, it's hard for
NM to guess which one of those is being replaced (as the kernel doesn't
have a "route ID" or similar field to indicate it). Moreover, the kernel
might choose to replace a route that we don't have on cache, so we know
nothing about it.

It is important to note that we cannot just discard Netlink messages of
routes that we don't track if they has the NLM_F_REPLACE. For example,
if we are tracking a route with proto=static, we might receive a replace
message, changing that route to proto=other_proto_that_we_dont_track. We
need to process that message and remove the route from our cache.

As NM doesn't know what route is being replaced, trying to guess will
lead to errors that will leave the cache in an inconsistent state.
Because of that, it just do a cache resync for the routes.

For IPv4 there was an optimization to this: if we don't have in the
cache any route candidate for the replacement there are only 2 possible
options: either add the new route to the cache or discard it if we are
not interested on it. We don't need a resync for that.

This commit is extending that optimization to IPv6 routes. There is no
reason why it shouldn't work in the same way than with IPv4. This
optimization will only work well as long as we find potential candidate
routes in the same way than the kernel (comparing the same fields). NM
calls to this "comparing by WEAK_ID". But this can also happen with IPv4
routes.

It is worth it to enable this optimization because there are routing
daemons using custom routing protocols that makes tens or hundreds of
updates per second. If they use NLM_F_REPLACE, this caused NM to do a
resync hundreds of times per second leading to a 100% CPU usage:
https://issues.redhat.com/browse/RHEL-26195

An additional but smaller optimization is done in this commit: if we
receive a route message for routes that we don't track AND doesn't have
the NLM_F_REPLACE flag, we can ignore the entire message, thus avoiding
the memory allocation of the nmp_object. That nmp_object was going to be
ignored later, anyway, so better to avoid these allocations that, with
the routing daemon of the above's example, can happen hundreds of times
per second.

With this changes, the CPU usage doing `ip route replace` 300 times/s
drops from 100% to 1%. Doing `ip route replace` as fast as possible,
without any rate limitting, still keeps NM with a 3% CPU usage in the
system that I have used to test.

(cherry picked from commit 4d426f581d)
2024-05-13 13:33:06 +02:00
Wen Liang
86fbfb3806 device: use subnet when the applied connection has IPv6 method shared
We should use the IPv6 subnet when we have an applied connection
stored on the downlink device, and the IPv6 method should be "shared"
for that applied connection. It does not make sense to register l3cd
and set router config for ndisc instance when the downlink device is
already deactivated.

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1880
Resolves: https://issues.redhat.com/browse/RHEL-17350

Signed-off-by: Wen Liang <liangwen12year@gmail.com>
(cherry picked from commit 15901a7489)
2024-05-02 14:35:22 +02:00
Íñigo Huguet
2636797b0b man: fix missing deprecation message
In the gtkdoc comments, the text below tags like `Since: 1.2` is
discarded. In the property `autoconnect-slaves` a line indicating its
deprecation was below one of these tags. As a result, it was missing in
the man page. Fix it.

Fixes: 194455660d ('connection: deprecate NMSettingConnection autoconnect-slaves property')
(cherry picked from commit 7427e9d320)
2024-04-18 15:31:54 +02:00
Fernando Fernandez Mancera
2fac176986 libnm-lldp: use ETH_P_ALL instead of NM_ETHERTYPE_LLDP for the socket
When creating the socket for listening to LLDP frames we are setting
NM_ETHERTYPE_LLDP (0x88cc) as protocol. In most of the cases, that is
correct but when the interface is attached as a port to a OVS bridge,
kernel is not matching the protocol correctly. The reason might be that
some metadata is added to the packet, but we are not completely sure
about it.

Instead, we should use ETH_P_ALL to match all the protocols. Later, we
have a eBPF filter to drop the packet by multicast MAC address or
protocol. This is how lldpd is doing it for example.

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1903
(cherry picked from commit 9ac1d6e22b)
2024-04-05 11:52:18 +02:00
Beniamino Galvani
809c2d7c50 device: ignore error setting mac if it's a global special value
If the distro sets a global special value for the cloned MAC address
(for example, "stable-ssid") and the driver doesn't support changing
the MAC, all activations will fail on the interface unless users know
that they need to change the cloned MAC. Be more tolerant to errors in
case the MAC is global and special.

https://bugzilla.redhat.com/show_bug.cgi?id=2270062
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1898
(cherry picked from commit d534f984f7)
2024-03-29 14:00:58 +01:00
Beniamino Galvani
6f3739e76f manager: fix race condition while enumerating devices at startup
While enumerating devices at startup, we take a snapshot of existing
links from platform and we start creating device instances for
them. It's possible that in the meantime, while processing netlink
events in platform_link_added(), a link gets renamed. If that happens,
then we have two different views of the same ifindex: the cached link
from `links` and the link in platform.

This can cause issues: in platform_link_added() we create the device
with the cached name; then in NMDevice's constructor(), we look up
from platform the ifindex for the given name. Because of the rename,
this lookup can match a newly created, different link.

The end result is that the ifindex from the initial snapshot doesn't
get a NMDevice and is not handled by NetworkManager.

Fix this problem by fetching the latest version of the link from
platform to make sure we have a consistent view of the state.

https://issues.redhat.com/browse/RHEL-25808
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1897
(cherry picked from commit de130df3e2)
2024-03-26 10:53:55 +01:00
Beniamino Galvani
669bf33de3 nm-dispatcher: fix crash when parsing output dictionary
'stdout' is NULL when the script didn't write anything or failed.

Fixes the following crash detected by NMCI in test
'dispatcher_device_handler_dummy'.

  nm-dispatcher[936339]: g_strsplit: assertion 'string != NULL' failed

  build_result_options (nm-dispatcher)
  complete_request (nm-dispatcher)
  complete_script (nm-dispatcher)
  script_watch_cb (nm-dispatcher)
  g_child_watch_dispatch (libglib-2.0.so.0)
  g_main_dispatch (libglib-2.0.so.0)
  g_main_context_iterate (libglib-2.0.so.0)
  g_main_context_iteration (libglib-2.0.so.0)
  main (nm-dispatcher)
  __libc_start_main (libc.so.6)
  _start (nm-dispatcher)

Fixes: d72f26b875 ('dispatcher: read device-handler's stdout into a dictionary')

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1889
(cherry picked from commit e5c2c5f1c2)
2024-03-26 10:53:43 +01:00
Gris Ge
d3329f0599 ovs: Do not allow OVS bridge and port to be parent
When creating VLAN over OVS internal interface which holding the same
name as its controller OVS bridge, NetworkManager will fail with error:

    Error: Connection activation failed: br0.101 failed to create
    resources: cannot retrieve ifindex of interface br0 (Open vSwitch
    Bridge)

Expanded the `find_device_by_iface()` with additional argument
`child: NmConnection *` which will validate whether candidate is
suitable to be parent device.

In `nm_device_check_parent_connection_compatible()`, we only not allow OVS
bridge and OVS port being parent.

Resolves: https://issues.redhat.com/browse/RHEL-26753

Signed-off-by: Gris Ge <fge@redhat.com>
(cherry picked from commit 7096f52a59)
2024-03-20 19:37:09 +01:00
Gris Ge
69d5761fa8 checkpoint: Allow rollback on internal global DNS
With `NM_CHECKPOINT_CREATE_FLAG_TRACK_INTERNAL_GLOBAL_DNS` flag set on
checkpoint creation, the checkpoint rollback will restore the
global DNS in internal configure file
`/var/lib/NetworkManager/NetworkManager-intern.conf`.

If user has set global DNS in /etc folder, this flag will not take any
effect.

Resolves: https://issues.redhat.com/browse/RHEL-23446

Signed-off-by: Gris Ge <fge@redhat.com>
(cherry picked from commit 86d67da28d)
2024-03-20 19:35:42 +01:00
Beniamino Galvani
a2ca3515c8 l3cfg: fix handling of ipv6 hop limit
Fixes: 5c48c5d5d6 ('l3cfg: set IPv6 sysctls during NML3Cfg commit')

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/1497

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1883
(cherry picked from commit b185d21c95)
2024-03-14 08:35:35 +01:00
Thomas Haller
1803520a7a config/tests: fix test failure in "/config/set-values"
GKeyfile changed something about how to handle invalid escape sequences.
As we don't want to test GKeyfile (per-se), just adjust to test to not
hit the problem.

This would fail with glib2-2.79.1-1.fc40:

  # ./tools/run-nm-test.sh -m src/core/tests/config/test-config -p /config/set-values
  TAP version 13
  # random seed: R02Sb8afff1ec38ca5a1b7713e8c40eb4f56
  # Start of config tests
  # GLib-GIO-DEBUG: _g_io_module_get_default: Found default implementation local (GLocalVfs) for ?gio-vfs?
  # (src/core/tests/config/test-config.c:1107) invalid value in config-data .intern.with-whitespace.key2 = (null) (instead of " b c\,  d  ")
  ./tools/run-nm-test.sh: line 307: 245847 Trace/breakpoint trap   (core dumped) "${NMTST_DBUS_RUN_SESSION[@]}" "$TEST" "${TEST_ARGV[@]}"
  exec "src/core/tests/config/test-config" failed with exit code 133

(cherry picked from commit 7f2a32fa11)
2024-03-06 18:27:29 +01:00
Thomas Haller
52ab487fd6 core: ignore unused result warning of audit_log_user_message()
Affects build on rawhide (audit-libs-4.0-8.fc40):

    src/core/nm-audit-manager.c: In function 'nm_audit_log':
    src/core/nm-audit-manager.c:188:9: error: ignoring return value of 'audit_log_user_message' declared with attribute 'warn_unused_result' [-Werror=unused-result]
      188 |         audit_log_user_message(priv->auditd_fd,
          |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      189 |                                AUDIT_USYS_CONFIG,
          |                                ~~~~~~~~~~~~~~~~~~
      190 |                                build_message(&strbuf, BACKEND_AUDITD, fields),
          |                                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      191 |                                NULL,
          |                                ~~~~~
      192 |                                NULL,
          |                                ~~~~~
      193 |                                NULL,
          |                                ~~~~~
      194 |                                success);
          |                                ~~~~~~~~

86bfa9bf4f
(cherry picked from commit ced0cf8005)
2024-03-06 14:38:52 +01:00
Thomas Haller
df879e6950 build: workaround -Wno-calloc-transposed-args warning in systemd code
Upstream systemd fixed this compiler warning. What really needs to be
done, is re-importing the upstream code.

In the meantime, suppress the warning that hits on GCC 14.

This is a temporary workaround!

See-also: fdd84270df
(cherry picked from commit b1016e3be8)
2024-02-26 12:17:07 +01:00
Thomas Haller
4c66cce980 build: use "-Wno-nonnull-compare" for building systemd
systemd uses that too. We cannot enable compiler warnings that
upstream doesn't want to support.

See-also: b59bce308d
(cherry picked from commit ad22a96da9)
2024-02-26 12:09:09 +01:00
Stanislas FAYE
6e30e37ebe test: disable vlan_xgress unit test
Disable the vlan_xgress unit test which was failing.
This test is currently being work and is going to be fixed soon.
2024-02-22 12:29:12 +01:00
Beniamino Galvani
f9c0f7ae64 manager: make generic devices compatible with all link types
If a generic device is present and the name matches, it is compatible
with any link type.

For example, if a generic connection has a device-handler that creates
a dummy interface, the link is compatible with the NMDeviceGeneric.

(cherry picked from commit 5978fb2b27)
2024-02-21 11:49:21 +01:00
Beniamino Galvani
3bb34edc53 core: persist state of software generic devices across restarts
When a generic connection has a custom device-handler, it always
generates a NMDeviceGeneric, even when the link that gets created is
of a type natively supported by NM. On service restart, we need to
keep track that the device is generic or otherwise a different device
type will be instantiated.

(cherry picked from commit f2613be150)
2024-02-21 11:49:20 +01:00
Beniamino Galvani
f8e020c29e device: support creating generic devices via device-handler
If the device-handler of the generic connection is set, the connection
is virtual and the device is created by invoking the device-handler
via NetworkManager-dispatcher service.

With this change, a generic device now represents two different device
classes:

 - existing interfaces that are not natively supported or recognized
   by NetworkManager. Those devices have the `has_device_handler`
   property set to FALSE;

 - interfaces that are created by NM by invoking the device-handler;
   they have `has_device_handler` set to TRUE.

(cherry picked from commit df6c35ec75)
2024-02-21 11:49:19 +01:00
Beniamino Galvani
cc5ebf265d dispatcher: read device-handler's stdout into a dictionary
Device handlers need a way to pass data back to NetworkManager, such
as the ifindex and an error message. Allow them to return a dictionary
on standard output, where each line contains a "$key=$value" pair.
In the daemon, the dictionary is returned via the callback function.

(cherry picked from commit d72f26b875)
2024-02-21 11:49:17 +01:00
Beniamino Galvani
5c33b14fe0 dispatcher: support device-handler actions
"device-add" and "device-delete" actions are called for
device-handlers of generic devices. They differ from other actions in
the following aspects:

 - only one script is invoked, the one with name specified by the
   device-handler property;
 - the script is searched in the "device" subdirectory;
 - since there is only one script executed, the result and error
   string from that script are returned by NM in the callback function.

(cherry picked from commit ee5845063d)
2024-02-21 11:49:16 +01:00
Beniamino Galvani
220d42c6f8 core/dispatcher: prefer the Action2() method and fall back to Action()
Call the Action2() method first, and fall back to the old Action() if
the new one is not available. This allows full interoperability
between different versions of the dispatcher service and NM.

(cherry picked from commit 8fd0d39444)
2024-02-21 11:49:15 +01:00
Beniamino Galvani
ad5b3b38db dispatcher: add Action2() D-Bus method
Currently, the dispatcher service implements an Action() method to
dispatch events. In the next commits, we'll need to add new
parameters, which is not possible with the current signature.

Introduce a new Action2() method, similar to the existing one but with
the following changes:

 - it accepts an additional "options" input parameter of type a{sv};
 - for each script executed, it also returns a dictionary of type
   a{sv}.

The new parameters will allow to easily extend functionality in the
future without having to implement an Action3().

(cherry picked from commit abf0f03d25)
2024-02-21 11:49:14 +01:00
Beniamino Galvani
10041294fe dispatcher: refactor building the result
Introduce request_dbus_method_return() and call it whenever we need to
return a result. Don't collect the list of scripts in case the
parameters can't be parsed.

(cherry picked from commit 703efdfbbf)
2024-02-21 11:49:14 +01:00
Beniamino Galvani
1dbad7b5d0 core/dispatcher: refactor nm_dispatcher_need_device()
Remove the "nm_" prefix that is usually reserved for non-static
functions. Also, use NM_IN_SET.

(cherry picked from commit 98b73e88e6)
2024-02-21 11:49:13 +01:00
Beniamino Galvani
7e67091d5f core/dispatcher: factorize code
Move common code from nm_dispatcher_call_device() and
nm_dispatcher_call_device_sync() to a new function; it will also be
used in the next commits by a new variant of the function.

(cherry picked from commit fa6ce51a0b)
2024-02-21 11:49:12 +01:00
Beniamino Galvani
9322c3e9db libnm: add generic.device-handler property
Add a new "generic.device-handler" property that specifies the name of
a dispatcher script to be invoked to add and delete the interface for
this connection.

(cherry picked from commit e686ab35b3)
2024-02-21 11:49:11 +01:00
Beniamino Galvani
5dea7f068b dispatcher: pass user setting properties in the environment
Properties in the "user" setting are a convenient way to associate any
kind of user-provided metadata to connections.

However, nmcli doesn't support the user setting at the moment and
adding this feature requires a significant effort. Without nmcli
support, dispatcher scripts can only access user properties by either
parsing connection files or by using D-Bus (with or without libnm and
GObject introspection). Since both these solutions are not very
convenient, provide an alternative way: pass the properties as
environment variables.

(cherry picked from commit d7c311eb85)
2024-02-21 11:49:11 +01:00
Beniamino Galvani
0ca9caa32d core: move functions for env variable name encoding to libnm-glib-aux
They will be used by the dispatcher service.

(cherry picked from commit 38acb7a57d)
2024-02-21 11:49:10 +01:00
Beniamino Galvani
636928f889 dispatcher: remove trailing dot from error messages
The error messages are logged by the dispatcher and passed back to
NetworkManager which also logs them. NetworkManager log messages
usually don't end with a dot: remove it.

(cherry picked from commit 7b769e9e49)
2024-02-21 11:49:10 +01:00
Beniamino Galvani
cd1d803cb2 macsec: support the offload property
(cherry picked from commit 010c54dce9)
2024-02-21 11:48:43 +01:00
Beniamino Galvani
9a9267ad4e libnm,nmcli: add macsec.offload property
Introduce a new property to control the MACsec offload mode.

(cherry picked from commit aa418275cf)
2024-02-21 11:48:42 +01:00
Javier Sánchez Parra
6601e6bf48 nmtui: Add bond for creating bridge port interface
This commit adds the missing bond option in the nmtui interface for
creating a bridge port interface.

Resolves: https://issues.redhat.com/browse/RHEL-18158
(cherry picked from commit 279d559a12)
2024-02-21 11:48:21 +01:00
Beniamino Galvani
9a42722718 nmcli: fix crash in nmc_connection_check_deprecated()
It's not clear in which circumstances, but 'type' can be NULL as in
the following backtrace:

  nmc_connection_check_deprecated (c=c@entry=0x55d93f937610) at src/nmcli/connections.c:676
  connection_warnings (nmc=nmc@entry=0x55d93f5ae5e0 <nm_cli>, connection=connection@entry=0x55d93f937610) at src/nmcli/connections.c:5464
  add_connection_cb (client=<optimized out>, result=<optimized out>, user_data=0x55d93fc83820) at src/nmcli/connections.c:5510
  g_task_return_now (task=0x55d93fc86fd0 [GTask]) at ../gio/gtask.c:1361
  g_task_return (type=<optimized out>, task=0x55d93fc86fd0 [GTask]) at ../gio/gtask.c:1430
  g_task_return (task=0x55d93fc86fd0 [GTask], type=<optimized out>) at ../gio/gtask.c:1387
  _request_wait_complete () at /lib64/libnm.so.0
  _nm_client_notify_event_emit_parts () at /lib64/libnm.so.0
  _dbus_handle_changes_commit () at /lib64/libnm.so.0
  _nm_client_get_settings_call_cb () at /lib64/libnm.so.0
  _nm_client_dbus_call_simple_cb () at /lib64/libnm.so.0
  g_task_return_now (task=0x55d93f7bd6f0 [GTask]) at ../gio/gtask.c:1361
  g_task_return (type=<optimized out>, task=0x55d93f7bd6f0 [GTask]) at ../gio/gtask.c:1430
  g_task_return (task=0x55d93f7bd6f0 [GTask], type=<optimized out>) at ../gio/gtask.c:1387
  g_dbus_connection_call_done (source=<optimized out>, result=<optimized out>, user_data=0x55d93f7bd6f0) at ../gio/gdbusconnection.c:5895
  g_task_return_now (task=0x55d93f7bd7b0 [GTask]) at ../gio/gtask.c:1361
  complete_in_idle_cb (task=task@entry=0x55d93f7bd7b0) at ../gio/gtask.c:1375
  g_idle_dispatch (source=0x7f15b007c940, callback=0x7f15ca7e4850 <complete_in_idle_cb>, user_data=0x55d93f7bd7b0) at ../glib/gmain.c:6150
  g_main_dispatch (context=0x55d93f77cde0) at ../glib/gmain.c:3344
  g_main_context_dispatch_unlocked (context=0x55d93f77cde0) at ../glib/gmain.c:4152
  g_main_context_iterate_unlocked.isra.0 (context=0x55d93f77cde0, block=block@entry=1, dispatch=dispatch@entry=1, self=<optimized out>) at ../glib/gmain.c:4217
  g_main_loop_run (loop=0x55d93f7589b0) at ../glib/gmain.c:4419
  main (argc=19, argv=0x7fff77359138) at src/nmcli/nmcli.c:1044

Fixes: f377114d6e ('cli: connection: check for deprecated features')

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1872
(cherry picked from commit 416d596b31)
2024-02-21 11:48:11 +01:00
Sergey Koshelenko
d88e21ff91 ndisc: fix IPv6 address lifetimes computation
Background: when router sends router advertisement (RA) message,
NetworkManager processes it and passes data to a lower system layer.
Currently there is a problem that NetworkManager adds one second to both
valid lifetime and preferred lifetime. This happens because of the
algorithm in nm_ndisc_data_to_l3cd() function.

Let's look at an example: let current timestamp be 100450, so now_sec
variable is 100. At this moment RA message was received from the router.
The IPv6 address' valid lifetime is 200 seconds (for example), so
expiration timestamp (ndisc_addr->expiry_msec) is 300450. But after the
_nm_ndisc_lifetime_from_expiry() call, NMPlatformIP6Address lifetime
becomes 201 ((300450-(100*1000)+999)/1000). Which is wrong.

This commit fixes this behaviour by replacing
nm_utils_get_monotonic_timestamp_sec() with
nm_utils_get_monotonic_timestamp_msec() so that timestamps are
calculated more precisely.

Related issue: https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/1464
Merge request: https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1863

(cherry picked from commit 14e7220f5f)
2024-02-21 11:48:04 +01:00
Íñigo Huguet
27c701ebfb libnmc: allow user input in ECHO mode for 2FA challenges
Depending on the type of challenge used in the 2FA authentication, the
user input doesn't need to be hidden and sometimes it's even undesired
(it makes more difficult to enter the text).

Allow to VPN plugins to indicate that a secret that is being requested
is a 2FA challenge with ECHO mode enabled:
- When using auth dialog: accept a new option "ForceEcho" that can be
  set to TRUE to enable ECHO.
- When using the fallback method: recognize the prefix
  "x-dynamic-challenge-echo". This indicate both that ECHO should be enabled
  and that this is a 2FA challenge (see previous commit).

The correct way to enable echo mode from VPN plugins is doing both
things: pass the hint prefixed with "x-dynamic-challenge-echo" and add the
option "ForceEcho=true" for the auth dialog.

An attempt to support ECHO mode from NM-openvpn was made by passing
"IsSecret=false", but it didn't work because nm-secret-agent-simple
ignores returned values for which "IsSecret=false". It's not a good idea
to start accepting them because we could break other plugins, and anyway
the challenge response is actually a secret, so it is better to keep it
as such and add this new "ForceEcho" option.

This is backwards compatible because existing plugins were not using the
tag nor the auth dialog option. Withouth them, the previous behaviour is
preserved. On the contrary, plugins that want to use this new feature
will need to bump their NM version dependency because old daemons will
not handle correctly the prefix tag.

Secret agents will need to be updated to check secret->force_echo if
they want to support this feature. Until they update, the only drawback
is that ECHO mode will be ignored and the user's input will be hidden.

Updated nmcli and nmtui to support ECHO mode.

(cherry picked from commit 2ab56e82d4)
2024-02-21 11:31:49 +01:00
Íñigo Huguet
345bd1b187 libnmc: fix secrets request on 2nd stage of 2FA authentication
Clients using nm-secret-agent-simple always asked for some default VPN
secrets, which are dependent on the VPN service, when the auth dialog
can't be used and the fallback method is used instead.

When using 2FA this has to be avoided in the 2nd step because those
default secrets were already requested and validated in the 1st step.

Fix it by adding a new "x-dynamic-challenge" prefix tag that can be used
in the hints received from the VPN plugin. This tag indicates that we
are in the 2nd step of a 2FA authentication. This way we know that we
don't have to request the default secrets this time. Note that the tag
name doesn't explicitly mention VPNs so it can be reused for other type
of connections in the future.

As the default secrets were requested always unconditionally when using
the fallback method, there is no possible workaround to this problem
that avoids having to change libnm-client.

The change is backwards compatible because VPN plugins were not using
the tag and the previous behaviour does not change if the tag is not
used. However, VPN plugins that want to properly support 2FA
aunthentication will need to bump the NM version dependency because
old daemons won't handle properly a hint with the new prefix tag.

Finally, move the macro that defines the "x-vpn-message:" tag in a public
header so it is more visible for users. It has been renamed and prefixed
with the NM_ namespace so it shouldn't collide with macros defined in
the VPN plugins.

(cherry picked from commit c5f46bae43)
2024-02-21 11:31:48 +01:00
Íñigo Huguet
3df94a4f2e sriov: allow reading empty eswitch paramaters via Devlink
Probably not all drivers and devices return all parameters. Set them to
"unknown" if they are missing and let the caller to decide what to do.

In our case, if the sriov setting has a value different to "preserve" it
will try to set it (and will probably fail). But if the missing
parameter is set to "preserve" in the sriov setting we can continue,
just ignoring it.

(cherry picked from commit 7346c5b556)
2024-02-21 11:27:36 +01:00