NMSetting8021x has various utility functions to set
the certificate:
- nm_setting_802_1x_set_ca_cert()
- nm_setting_802_1x_set_client_cert()
- nm_setting_802_1x_set_private_key()
- nm_setting_802_1x_set_phase2_ca_cert()
- nm_setting_802_1x_set_phase2_client_cert()
- nm_setting_802_1x_set_phase2_private_key()
They support:
- accepting a plain PKCS11 URI, with scheme set to
NM_SETTING_802_1X_CK_SCHEME_PKCS11.
- accepting a filename, with scheme set to
NM_SETTING_802_1X_CK_SCHEME_BLOB or
NM_SETTING_802_1X_CK_SCHEME_PATH.
In the latter case, the function tries to load the file and verify it.
In case of the private-key setters, this also involves accepting a
password. Depending on whether the scheme is BLOB or PATH, the function
will either set the certificate to a PATH blob, or take the blob that
was read from file.
The functions seem misdesigned to me, because their behavior is
rather obscure. E.g. they behave fundamentally different, depending
on whether scheme is PKCS11 or BLOB/PATH.
Anyway, improve them:
- refactor the common code into a function _cert_impl_set(). Previously,
their non-trivial implementations were copy+pasted several times,
now they all use the same implementation.
- if the function is going to fail, don't touch the setting. Previously,
the functions would first clear the certificate before trying to
validate the input. It's more logical, that if a functions is going
to fail to check for failure first and don't modify the settings.
- not every blob can be represented. For example, if we have a blob
which starts with "file://", then there is no way to set it, simply
because we don't support a prefix for blobs (like "data:;base64,").
This means, if we try to set the certificate to a particular binary,
we must check that the binary is interpreted with the expected scheme.
Add this check.
It's only used for testing, so this change is not very relevant.
Anyway, I think our crypto code should succeed in not leaving
key material in memory. Refactor the code to do that, though,
how the pem file gets composed is quite a hack (for tests good
enough though).
nm_utils_rsa_key_encrypt() is internal API which is only uesd for testing.
Move it to nm-crypto.h (where it fits better) and rename it to make the
testing-aspect obvious.
The GBytes has a suitable cleanup function, which zeros the certificate
from memory.
Also, all callers that require the certificate, actually later converted
it into a GBytes anyway. This way, they can re-used the same instance
(avoiding an additionaly copying of the data), and they will properly
clear the memory when freed.
Follow our convention, that items in headers are all named with
an "NM" prefix.
Also, "nm-crypto-impl.h" contains internal functions that are to be implemented
by the corresponding crypto backends. Distinguish their names as well.
There are two aspects: the public crypto API that is provided by
"nm-crypto.h" header, and the internal header which crypto backends
need to implement. Split them.
There should be a clear distinction between whether an array
is a NUL terminated string or binary with a length.
crypto_md5_hash() is already complicated enough. Adjust it's
API to only support binary arguments, and thus have "guint8 *" type.
We already have nm_utils_str_utf8safe_escape() to convert a
NUL termianted string to an UTF-8 string. nm_utils_str_utf8safe_escape()
operates under the assumption, that the input strig is already valid UTF-8
and returns the input string verbatim. That way, in the common expected
cases, the string just looks like a regular UTF-8 string.
However, in case there are invalid UTF-8 sequences (or a backslash
escape characters), the function will use backslash escaping to encode
the input string as a valid UTF-8 sequence. Note that the escaped
sequence, can be reverted to the original non-UTF-8 string via
unescape.
An example, where this is useful are file names or interface names.
Which are not in a defined encoding, but NUL terminated and commonly ASCII or
UTF-8 encoded.
Extend this, to also handle not NUL terminated buffers. The same
applies, except that the process cannot be reverted via g_strcompress()
-- because the NUL character cannot be unescaped.
This will be useful to escape a Wi-Fi SSID. Commonly we expect the SSID
to be in UTF-8/ASCII encoding and we want to print it verbatim. Only
if that is not the case, we fallback to backslash escaping. However, the
orginal value can be fully recovered via unescape(). The difference
between an SSID and a filename is, that the former can contain '\0'
bytes.
Note that in NetworkManager API (D-Bus, libnm, and nmcli),
the features are called "feature-xyz". The "feature-" prefix
is used, because NMSettingEthtool possibly will gain support
for options that are not only -K|--offload|--features, for
example -C|--coalesce.
The "xzy" suffix is either how ethtool utility calls the feature
("tso", "rx"). Or, if ethtool utility specifies no alias for that
feature, it's the name from kernel's ETH_SS_FEATURES ("tx-tcp6-segmentation").
If possible, we prefer ethtool utility's naming.
Also note, how the features "feature-sg", "feature-tso", and
"feature-tx" actually refer to multiple underlying kernel features
at once. This too follows what ethtool utility does.
The functionality is not yet implemented server-side.
Add a new option that allows to activate a profile multiple times
(at the same time). Previoulsy, all profiles were implicitly
NM_SETTING_CONNECTION_MULTI_CONNECT_SINGLE, meaning, that activating
a profile that is already active will deactivate it first.
This will make more sense, as we also add more match-options how
profiles can be restricted to particular devices. We already have
connection.type, connection.interface-name, and (ethernet|wifi).mac-address
to restrict a profile to particular devices. For example, it is however
not possible to specify a wildcard like "eth*" to match a profile to
a set of devices by interface-name. That is another missing feature,
and once we extend the matching capabilities, it makes more sense to
activate a profile multiple times.
See also https://bugzilla.redhat.com/show_bug.cgi?id=997998, which
previously changed that a connection is restricted to a single activation
at a time. This work relaxes that again.
This only adds the new property, it is not used nor implemented yet.
https://bugzilla.redhat.com/show_bug.cgi?id=1555012
1) the command line gets shorter. I frequently run `make V=1` to see
the command line arguments for the compiler, and there is a lot
of noise.
2) define each of these variables at one place. This makes it easy
to verify that for all compilation units, a particular
define has the same value. Previously that was not obvious or
even not the case (see commit e5d1a71396
and commit d63cf1ef2f).
The point is to avoid redundancy.
3) not all compilation units need all defines. In fact, most modules
would only need a few of these defines. We aimed to pass the necessary
minium of defines to each compilation unit, but that was non-obvious
to get right and often we set a define that wasn't used. See for example
"src_settings_plugins_ibft_cppflags" which needlessly had "-DSYSCONFDIR".
This question is now entirely avoided by just defining all variables in
a header. We don't care to find the minimum, because every component
gets anyway all defines from the header.
4) this also avoids the situation, where a module that previously did
not use a particular define gets modified to require it. Previously,
that would have required to identify the missing define, and add
it to the CFLAGS of the complation unit. Since every compilation
now includes "config-extra.h", all defines are available everywhere.
5) the fact that each define is now available in all compilation units
could be perceived as a downside. But it isn't, because these defines
should have a unique name and one specific value. Defining the same
name with different values, or refer to the same value by different
names is a bug, not a desirable feature. Since these defines should
be unique accross the entire tree, there is no problem in providing
them to every compilation unit.
6) the reason why we generate "config-extra.h" this way, instead of using
AC_DEFINE() in configure.ac, is due to the particular handling of
autoconf for directory variables. See [1].
With meson, it would be trivial to put them into "config.h.meson".
While that is not easy with autoconf, the "config-extra.h" workaround
seems still preferable to me.
[1] https://www.gnu.org/software/autoconf/manual/autoconf-2.63/html_node/Installation-Directory-Variables.html
We commonly don't use the glib typedefs for char/short/int/long,
but their C types directly.
$ git grep '\<g\(char\|short\|int\|long\|float\|double\)\>' | wc -l
587
$ git grep '\<\(char\|short\|int\|long\|float\|double\)\>' | wc -l
21114
One could argue that using the glib typedefs is preferable in
public API (of our glib based libnm library) or where it clearly
is related to glib, like during
g_object_set (obj, PROPERTY, (gint) value, NULL);
However, that argument does not seem strong, because in practice we don't
follow that argument today, and seldomly use the glib typedefs.
Also, the style guide for this would be hard to formalize, because
"using them where clearly related to a glib" is a very loose suggestion.
Also note that glib typedefs will always just be typedefs of the
underlying C types. There is no danger of glib changing the meaning
of these typedefs (because that would be a major API break of glib).
A simple style guide is instead: don't use these typedefs.
No manual actions, I only ran the bash script:
FILES=($(git ls-files '*.[hc]'))
sed -i \
-e 's/\<g\(char\|short\|int\|long\|float\|double\)\>\( [^ ]\)/\1\2/g' \
-e 's/\<g\(char\|short\|int\|long\|float\|double\)\> /\1 /g' \
-e 's/\<g\(char\|short\|int\|long\|float\|double\)\>/\1/g' \
"${FILES[@]}"
A configuration with duplicate tc qdiscs and tfilters is not valid;
reject it in verify(). Note that nm_setting_tc_config_add_qdisc() and
nm_setting_tc_config_add_tfilter() can't add duplicate entries and so
the only way to achieve an invalid configuration is setting the
properties directly.
https://github.com/NetworkManager/NetworkManager/pull/95
Use two common defines NM_BUILD_SRCDIR and NM_BUILD_BUILDDIR
for specifying the location of srcdir and builddir.
Note that this is only relevant for tests, as they expect
a certain layout of the directories, to find files that concern
them.
There are multiple tests with the same in different directories; add a
unique prefix to test names so that it is clear from the output which
one is running.
Some targets are missing dependencies on some generated sources in
the meson port. These makes the build to fail due to missing source
files on a highly parallelized build.
These dependencies have been resolved by taking advantage of meson's
internal dependencies which can be used to pass source files,
include directories, libraries and compiler flags.
One of such internal dependencies called `core_dep` was already in
use. However, in order to avoid any confusion with another new
internal dependency called `nm_core_dep`, which is used to include
directories and source files from the `libnm-core` directory, the
`core_dep` dependency has been renamed to `nm_dep`.
These changes have allowed minimizing the build details which are
inherited by using those dependencies. The parallelized build has
also been improved.
Tests are commonly created via copy&paste. Hence, it's
better to express a certain concept explicitly via a function
or macro. This way, the implementation of the concept can be
adjusted at one place, without requiring to change all the callers.
Also, the macro is shorter, and brevity is better for tests
so it's easier to understand what the test does. Without being
bothered by noise from the redundant information.
Also, the macro knows better which message to expect. For example,
messages inside "src" are prepended by nm-logging.c with a level
and a timestamp. The expect macro is aware of that and tests for it
#define NMTST_EXPECT_NM_ERROR(msg) NMTST_EXPECT_NM (G_LOG_LEVEL_MESSAGE, "*<error> [*] "msg)
This again allows the caller to ignore this prefix, but still assert
more strictly.
Note that:
- we compile some source files multiple times. Most notably those
under "shared/".
- we include a default header "shared/nm-default.h" in every source
file. This header is supposed to setup a common environment by defining
and including parts that are commonly used. As we always include the
same header, the header must behave differently depending
one whether the compilation is for libnm-core, NetworkManager or
libnm-glib. E.g. it must include <glib/gi18n.h> or <glib/gi18n-lib.h>
depending on whether we compile a library or an application.
For that, the source files need the NETWORKMANAGER_COMPILATION #define
to behave accordingly.
Extend the define to be composed of flags. These flags are all named
NM_NETWORKMANAGER_COMPILATION_WITH_*, they indicate which part of the
build are available. E.g. when building libnm-core.la itself, then
WITH_LIBNM_CORE, WITH_LIBNM_CORE_INTERNAL, and WITH_LIBNM_CORE_PRIVATE
are available. When building NetworkManager, WITH_LIBNM_CORE_PRIVATE
is not available but the internal parts are still accessible. When
building nmcli, only WITH_LIBNM_CORE (the public part) is available.
This granularily controls the build.
Source files for enum types are generated by passing segments of the
source code of the files to the `glib-mkenums` command.
This patch removes those parameters where source code is used from
meson build files by moving those segmeents to template files.
https://mail.gnome.org/archives/networkmanager-list/2017-December/msg00057.html
binary-search can find an index of a matching entry in a sorted
list. However, if the list contains multiple entries that compare
equal, it can be interesting to find the first/last entry. For example,
if you want to append new items after the last.
Extend binary search to optionally continue the binary search
to determine the range that compares equal.
