GitLab

There's no need to try to set the engine/engine2 parameters, they're
automatically set.

Support tested with :
- eap-tls on libvirt/kvm virtual machine and real system
- wpa2-eap-peap-tls on real system. (TODO: setting test VM with mac80211_hwsim)
The certificate key is protected by tpm2. No pin.

Modifications done to  libnm-core/nm-setting-8021x.c :
o verify_tls() to make sure that if engine_id/engine2_id are specified
  key_id/key2_id are specified too. If engine_id/engine2_id not specified, behaves
  as before.
o need_secrets_tls() modified to not look for a passphrase for a certificate if
  an engine id is set for phase 1 or phase 2.
o verify_ttls() to work in my phase 2 peap-tls case. Could have used a new fonction
  but was a little bit easier to adapt verify_ttls(). The function nows check:
  - we're using phase2 auth or autheap
  - there's an identity set
  - in case of ttls(), check that anonymous identity is set.

Example of 802-1x section for ethernet eap-tls case:
[802-1x]
ca-cert=/home/rtp/ca.pem
client-cert=/home/rtp/tpm2/csr2/client-tpm-qemu.crt
eap=tls;
identity=nm-tpm2
phase1-engine-id=tpm2tss
phase1-key-id=/home/rtp/tpm2/csr2/pri_pub_blob.key
Signed-off-by: Arnaud Patard <apatard@hupstream.com>

The connection type can be NULL.

Fixes: e1ec22f7 ('cli: cleanup setting default interface-name')

The script is run for every dispatcher event. Most of the events are not
actually relevant, and we just need to determine that there is nothing
to do and quit.

Avoid calling "dirname" and "basename".

The supported ifcfg-file has a very specific form. We can just check
(and parse) it in one got regular expression in bash.

A shell script is executed line-by-line. Note that for most dispatcher
events, "10-ifcfg-rh-routes.sh" has nothing to do and will just quit.
Move those checks earlier, to avoid bash executing the code that won't
be needed most of the time.

found by cppcheck

[src/devices/nm-device.c:3032] -> [src/devices/nm-device.c:3025]: (warning) Either the condition '!handle' is redundant or there is possible null pointer dereference: handle.

https://github.com/NetworkManager/NetworkManager/pull/352

NetworkManager/NetworkManager!296

The previous commit marks all synchronous libnm API as deprecated.
In practice, the macro _NM_DEPRECATED_SYNC_METHOD expands to
nothing, because there is no immediate urgency to force users
to migrate.

However nm_client_check_connectivity() is especially bad: it
makes a synchronous call and then updates the content of the
cache artificially. Usually, NMClient's cache of D-Bus objects
is only updated by "PropertiesChanged" D-Bus signals.
nm_client_check_connectivity() instead will act on the response to
the "CheckConnectivity" D-Bus call -- a response that is picked
out of order from the ordered sequence of messages --  and will
update the cache instead of honoring the usual "PropertiesChanged"
signal.

I think such behavior is fundamentally broken. For a trivial property like
NM_CLIENT_CONNECTIVITY such behavior is odd at best. Note how applying
this approach to other functions (like nm_client_deactivate_connection(),
which would affect a much larger state) would not be feasible.

I also imagine it to be complicate to preserve this behavior when
reworking libnm, as I plan to do.

See also commit b799de28 ('libnm: update property in the manager
after connectivity check'), which introduced this behavior to "fix"
bgo#784629.

Note that D-Bus is fundamentally asynchronous. Doing blocking calls
on top of D-Bus is odd, especially for libnm's NMClient. That is because
NMClient essentially is a client-side cache of the objects from the D-Bus
interface. This cache should be filled exclusively by (asynchronous) D-Bus
events (PropertiesChanged). So, making a blocking D-Bus call means to wait
for a response and return it, while queuing all messages that are received
in the meantime.
Basically there are three ways how a synchronous API on NMClient could behave:

 1) the call just calls g_dbus_connection_call_sync(). This means
    that libnm sends a D-Bus request via GDBusConnection, and blockingly
    waits for the response. All D-Bus messages that get received in the
    meantime are queued in the GMainContext that belongs to NMClient.
    That means, none of these D-Bus events are processed until we
    iterate the GMainContext after the call returns. The effect is,
    that NMClient (and all cached objects in there) are unaffected by
    the D-Bus request.
    Most of the synchronous API calls in libnm are of this kind.
    The problem is that the strict ordering of D-Bus events gets
    violated.
    For some API this is not an immediate problem. Take for example
    nm_device_wifi_request_scan(). The call merely blockingly tells
    NetworkManager to start scanning, but since NetworkManager's D-Bus
    API does not directly expose any state that tells whether we are
    currently scanning, this out of order processing of the D-Bus
    request is a small issue.
    The problem is more obvious for nm_client_networking_set_enabled().
    After calling it, NM_CLIENT_NETWORKING_ENABLED is still unaffected
    and unchanged, because the PropertiesChanged signal from D-Bus
    is not yet processed.
    This means, while you make such a blocking call, NMClient's state
    does not change. But usually you perform the synchronous call
    to change some state. In this form, the blocking call is not useful,
    because NMClient only changes the state after iterating the GMainContext,
    and not after the blocking call returns.

 2) like 1), but after making the blocking g_dbus_connection_call_sync(),
    update the NMClient cache artificially. This is what
    nm_manager_check_connectivity() does, to "fix" bgo#784629.
    This also has the problem of out-of-order events, but it kinda
    solves the problem of not changing the state during the blocking
    call. But it does so by hacking the state of the cache. I think
    this is really wrong because the state should only be updated from
    the ordered stream of D-Bus messages (PropertiesChanged signal and
    similar). When libnm decides to modify the state, there may be already
    D-Bus messages queued that affect this very state.

 3) instead of calling g_dbus_connection_call_sync(), use the
    asynchronous g_dbus_connection_call(). If we would use a sepaate
    GMainContext for all D-Bus related calls, we could ensure that
    while we block for the response, we iterate that internal main context.
    This might be nice, because all events are processed in order and
    after the blocking call returns, the NMClient state is up to date.
    The are problems however: current blocking API does not do this,
    so it's a significant change in behavior. Also, it might be
    unexpected to the user that during the blocking call the entire
    content of NMClient's cache might change and all pointers to the
    cache might be invalidated. Also, of course NMClient would invoke
    signals for all the changes that happen.
    Another problem is that this would be more effort to implement
    and it involves a small performance overhead for all D-Bus related
    calls (because we have to serialize all events in an internal
    GMainContext first and then invoke them on the caller's context).
    Also, if the users wants this behavior, they could implement it themself
    by running libnm in their own GMainContext. Note that libnm might
    have bugs to make that really working, but that should be fixed
    instead of adding such synchrnous API behavior.

Read also [1], for why blocking calls are wrong.

[1] https://smcv.pseudorandom.co.uk/2008/11/nonblocking/

So, all possible behaviors for synchronous API have severe behavioural
issues.  Mark all this API as deprecated. Also, this serves the purpose of
identifying blocking D-Bus calls in libnm.

Note that "deprecated" here does not really mean that the API is going
to be removed. We don't break API. The user may:

  - continue to use this API. It's deprecated, awkward and discouraged,
    but if it works, by all means use it.

  - use asynchronous API. That's the only sensible way to use D-Bus.
    If libnm lacks a certain asynchronous counterpart, it should be
    added.

  - use GDBusConnection directly. There really isn't anything wrong
    with D-Bus or GDBusConnection. This deprecated API is just a wrapper
    around g_dbus_connection_call_sync(). You may call it directly
    without feeling dirty.

---

The only other remainging API is the synchronous GInitable call for
NMClient. That is an entirely separate beast and not particularly
wrong (from an API point of view).

Note that synchronous API in NMSecretAgentOld, NMVpnPluginOld and
NMVpnServicePlugin as not deprecated here. These types are not part
of the D-Bus cache and while they have similar issues, it's less severe
because they have less state.

When opening a merge request from a fork of NetworkManager, then the
pipeline runs with the a checkout of the fork. That means, checkpatch
would compare the branch against "master" (or "nm-x-y" stable branches)
of the fork, instead of upstream.

That doesn't seem too useful. Instead, also add upstream NetworkManager
as git remote, fetch the branches, and use the branches from there as
base for checkpatch.

NetworkManager/NetworkManager#255

Note the "to" in the output:

  $ LANG=de_DE.UTF-8 nmcli
  eth0: verbunden to Wired Connection 1
        "Intel Ethernet"
  ...

NetworkManager/NetworkManager#246

```bash

readarray -d '' FILES < <(
  git ls-files -z \
    ':(exclude)po' \
    ':(exclude)shared/c-rbtree' \
    ':(exclude)shared/c-list' \
    ':(exclude)shared/c-siphash' \
    ':(exclude)shared/c-stdaux' \
    ':(exclude)shared/n-acd' \
    ':(exclude)shared/n-dhcp4' \
    ':(exclude)src/systemd/src' \
    ':(exclude)shared/systemd/src' \
    ':(exclude)m4' \
    ':(exclude)COPYING*'
  )

sed \
  -e 's/^\(--\|#\| \*\) *\(([cC]) *\)\?Copyright \+\(\(([cC])\) \+\)\?\(\(20\|19\)[0-9][0-9]\) *[-–] *\(\(20\|19\)[0-9][0-9]\) \+\([^ ].*\)$/\1 C1pyright#\5 - \7#\9/' \
  -e 's/^\(--\|#\| \*\) *\(([cC]) *\)\?Copyright \+\(\(([cC])\) \+\)\?\(\(20\|19\)[0-9][0-9]\) *[,] *\(\(20\|19\)[0-9][0-9]\) \+\([^ ].*\)$/\1 C2pyright#\5, \7#\9/' \
  -e 's/^\(--\|#\| \*\) *\(([cC]) *\)\?Copyright \+\(\(([cC])\) \+\)\?\(\(20\|19\)[0-9][0-9]\) \+\([^ ].*\)$/\1 C3pyright#\5#\7/' \
  -e 's/^Copyright \(\(20\|19\)[0-9][0-9]\) \+\([^ ].*\)$/C4pyright#\1#\3/' \
  -i \
  "${FILES[@]}"

echo ">>> untouched Copyright lines"
git grep Copyright "${FILES[@]}"

echo ">>> Copyright lines with unusual extra"
git grep '\<C[0-9]pyright#' "${FILES[@]}" | grep -i reserved

sed \
  -e 's/\<C[0-9]pyright#\([^#]*\)#\(.*\)$/Copyright (C) \1 \2/' \
  -i \
  "${FILES[@]}"

```

NetworkManager/NetworkManager!298

The Bluetooth DUN device's NMModem would signal the reset of ifindex to zero
when it's disconnected and the NMDeviceBt would accordingly update the
bluetooth device's ip ifindex. This is not okay since commit ab457830
('device: refactor nm_device_set_ip_ifindex() and set_ip_iface()') which,
although claiming to be a refactoring, made such use of
nm_device_set_ip_ifindex() illegal. Resetting the ifindex is anyway not
necessary, since it's taken care of _cleanup_generic_post().

Let's leave the ifindex alone once the device is activated, in a manner
analogous to what NMDeviceModem.

Fixes: ab457830 ('device: refactor nm_device_set_ip_ifindex() and set_ip_iface()')
Fixes: 78ca2a70 ('device: don't set invalid ip-iface'):

Useful for quickly testing Bluetooth DUN support. Duplicates some
modemu.pl logic, but hey...

NetworkManager/NetworkManager!297

Otherwise, the script tries to run

  dbus-run-session -- exec ...

which fails (because `exec` is a shell command, not a program).
After the failure, the code falls through to run the test under
valgrind.

Fixes: 6a58c55c ('run-nm-test: Just use exec instead of running and exiting')

Merge a subset of the patches from !263.

NetworkManager/NetworkManager!263

Cleanup the code removing the deprecated GSimpleAsyncResult

If a new command was requested while a client was in the process of being
created we were just requesting a new client.

This was causing leak, so let's strongly ensure this is not the case.

If we failed on process wait, we didn't close the pipes and no clear output
of what happened was exposed.

So use a finally stanza to close the pipes and print stdout and stderr
on failure.

When a test is going to be run under valgrind we set NM_TEST_UNDER_VALGRIND
so that we can properly check whether this is happening.

Network Cost [1] is a vendor-specific information element defined by
Microsoft and used to advertise the cost of Wi-Fi networks to clients.

We can use it together with the ANDROID_METERED mechanism to
automatically set the metered flag on the device.

[1] https://docs.microsoft.com/en-us/windows-hardware/drivers/mobilebroadband/network-cost-information-element

NetworkManager/NetworkManager#200

"ubuntu:devel" ships iproute2 version "5.2.0-1ubuntu1". This has a well known
bug that prevents it from creating IP tunnels during the unit tests.

We already workaround that on Debian. Add the same workaround to match the
Ubuntu package.

NetworkManager/NetworkManager!256

The devices tests' meson build files include only the build of a
single executable file and its execution as a test unit.

This has been moved to the devices' main meson build files so this
files can be removed.

In 878d4963 a new `nm-bt-test` helper program was added. However,
although `autotools` build steps were included, meson build steps
were not.

This add meson's build steps.

There are different enum files created that make use of different
template files. However, `mkenums_simple` method allows the creation
of the same enum files without the need of template files.

The creation of the `nm-core-enum-types` and
`nm-core-tests-enum-types` use now `mkenums_simple` so template
files are now unnecessary.

Some variables belong to variables in their correspondent pkg-config
file.

These variables have been renamed to `dependency_variable` to
reflect the dependency and variables from pkg-config files they are
related to.

Some of these has also been fixed to use paths relative to
installation prefix.

The test unit name string is used in different place so it has been
replaced by a variable.

The `nm-service-providers.c` source file is appended by using a
`files` generated object.

These tests are already working since 19a718bc so `FIXME` comments
are not needed anymore.

the `doc_module` variable has been removed. It was created because
its used in the autotools build file but actually `nm_name` variable
can be used easily.

Different objects used in the documentation target have been grouped
together.

The content file `version.xml`, and different build files are now
added properly.

the `doc_module` variable has been removed. It was created because
its used in the autotools build file but actually `libnm_name`
variable can be used easily.

Different objects used in the documentation target have been grouped
together.

The content file `version.xml` is now added properly.

Qt dependencies have been moved to the main build file where the
rest of dependencies are located. This makes it easier to find them.

The included directories has also reviewed and removed the
unnecessary ones.

The dependencies used in the build of the `nmtui` executable and the
`libnmt-newt` library have been reviewed.

The compiler flags used in common by them has also been moved to a
`common_c_flags` variable to avoid any confussion.

The dependencies used in the build of `nmcli` has been reviewed and
removed the unnecessary ones. The used compiler flags has also been
moved to one line.

The build file in the `client` `common` directory has been improved
by grouping the objects used in properties and by reviewing the
dependencies used by tests built. Finally the indentation has also
been fixed.