Commit graph

23477 commits

Author SHA1 Message Date
Beniamino Galvani
2c97ae435e dhcp: systemd: relicense as LGPL
Soon a new DHCP backend will be added that will take code from the
internal one. Change its license to LGPL so that the whole new backend
code can also be LGPL, which is the preferred license for new
NetworkManager code.

Acked-by: Dan Williams <dcbw@redhat.com>
Acked-by: Dan Winship <danw@redhat.com>
Acked-by: Lubomir Rintel <lkundrak@v3.sk>
Acked-by: Thomas Haller <thaller@redhat.com>
2019-06-27 17:08:37 +02:00
Thomas Haller
8a8e894f80 core: add and use nm_keep_alive_destroy()
When we are done with a NMKeepAlive instance, we always should do
three things:

  - unset the owner
  - disarm (freeze) the keep-alive
  - give up our reference.

Add and use nm_keep_alive_destroy() that does this.
2019-06-27 13:25:40 +02:00
Thomas Haller
dd5c88b1cc settings: merge branch 'th/various-settings-cleanup-3'
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/187
2019-06-26 18:10:17 +02:00
Thomas Haller
7b6f1c2d90 tools: export more symbols from NetworkManager binary to plugins
Plugins also may use nmtst_*() functions (when built with --with-more-asserts)
or c_list_*(). Whitelist them too.
2019-06-26 12:26:11 +02:00
Thomas Haller
e36cf1e890 ifcfg-rh: add allow_filename_cb() argument to write-ifcfg-rh function
The function determines the filename automatically, but we
need to blacklist certain names.

That is, because NetworkManager keeps a list of loaded files
in memory. When writing a new file, we really want to choose
a filename that is not yet taken. For that we must not only
consider files on disk, but also files that existed on the last
time of loading.
2019-06-26 12:26:11 +02:00
Thomas Haller
e5b21344c5 ifcfg-rh: cleanup utils_detect_ifcfg_path()
- avoid cloing the basename. Determining the basename can be done conveniently
  with strrchr().
- use cleanup macro for temporary variable.
- while in practice it should not happen, check that the colon in the name
  of alias file names is not followed by another '/'.
2019-06-26 12:26:11 +02:00
Thomas Haller
a4642c78f7 settings: cache agent and system secrets in a GVariant only
We don't need the full NMConnection.
2019-06-26 12:26:11 +02:00
Thomas Haller
f8a20d0a09 manager: don't consider by-user change flag for creating unrealized device
This effectively reverts commit [1].

The by-user argument is not very clear what it means. Is a "nmcli
connection load $FILENAME" a user-action? How about reload?

I don't know whether the problem that this was supposed to fix is still
present. But in any case, the condition here seems not right. It's
already hard to understand when and how we generate unrealized devices.

If the condition from commit [1] should be prevented, then it must happen
somehow differently. In the example, the offending connection is a generated
volatile profile with the device being sys-face-state "external". Of course,
we should not generate devices for such profiles nor autoactivating them.
So adding a device for a volatile connection is always wrong. Don't do that,
which should avoid the original problem.

[1] commit a8a4eb1418 ('manager: don't create the virtual devices on all connection changes')
2019-06-26 12:26:11 +02:00
Thomas Haller
d1f269ab36 core: ensure normalized connection during add-and-activate
nm_connection_verify() returns success for fully valid (normalized)
connections and also connections that are NM_SETTING_VERIFY_NORMALIZABLE.

We really want to fully normalize the profiles during add-and-activate.
2019-06-26 12:26:11 +02:00
Thomas Haller
6d4592354f settings: reorder checks in _delete_volatile_connection_do() to perform cheaper check first
Checking whether a settings connection is still tracked is a simple
c_list_is_empty(). It's faster, so do it first.
2019-06-26 12:26:11 +02:00
Thomas Haller
50e193d12c settings: use NMCListElem instead of DeleteVolatileConnectionData for tracking connections to delete
For tracking a CList of one pointer we have NMCListElem API. We don't need
to implement our own struct to hold the list pointers and the data pointer.
2019-06-26 12:26:11 +02:00
Thomas Haller
eed4b5253f settings: don't implement settings plugins as singletons
The settings plugins are created by NMSettings when the plugin
gets loaded. There is no need for these instances to be singletons
or to have a singleton getter.

Also, while in practice we create a settings plugin instance of
each type only once, there is nothing that would prevent creating
multiple instances. Hence, having a singleton getter is not right.

What is however useful, is to track them and block shutdown
via nm_shutdown_wait_obj_register*(). While the actual waiting
is not yet implemented, we should mark the plugin instances to
block shutdown (in the future).
2019-06-26 12:26:11 +02:00
Thomas Haller
a410873461 core: add flag to nm_shutdown_wait_obj_register_full() for freeing allcated message string
In fact, nm_shutdown_wait_obj_register*() API is still not implemented
and registering an object has no effect currently. That is, blocking
shutdown and waiting for instances to be destroyed during shutdown
is not yet implemented. Still, we already implement the API so that
components can register themself to block the shutdown. The point is
of course, that the callers already use this API, although it's not yet
implemented.

Anyway, sometimes the message string is not static. Add an option to
pass an allocated string and let the string be destroyed when no longer
needed.
2019-06-26 12:26:11 +02:00
Thomas Haller
02a0967520 libnm: fix setting error for nm_connection_update_secrets()
By convention, a function that indicates failure *MUST* set
an error.

Also, an error can only be set once.
2019-06-26 12:26:11 +02:00
Thomas Haller
d704f02119 libnm: workaround assertion failure for nmtst_connection_assert_unchanging() when disposing connection
nmtst_connection_assert_unchanging() registers to the changed signals
and asserts that they are not invoked. The purpose is that sometimes
we want to keep a reference to an NMConnection and be sure that it does
not get modified. This allows everybody to keep a reference to the very
same connection instance without cloning it -- provided they too promise
not to change it. This assert is to ensure that.

Note that NMSimpleConnection.dispose() clears the secrets and thus upon
destruction the assertion fails. At that point, the assertion is no longer
relevant, because the purpose was to ensure that no alive instances gets
modified. While destroying the instance, it's fine to modify it (nobody should
have a reference to it anymore).

This avoids the assertion failure when destroying a NMSimpleConnection with secrets
that is set with nmtst_connection_assert_unchanging().
2019-06-26 09:53:54 +02:00
Thomas Haller
b9587008fc shared: add nm_clear_error() and patch g_clear_error() to use this inlinable variant 2019-06-26 09:53:54 +02:00
Thomas Haller
03b8eb124e shared/glib: unconditionally redefine g_object_ref()/g_object_ref_sink() as typesafe macro 2019-06-26 09:53:54 +02:00
Thomas Haller
02ac5693d3 shared: add nm_utils_file_stat() util
A small convenience function to call stat(). The difference is that the
function returns an error code.
2019-06-26 09:53:54 +02:00
Thomas Haller
ec707f56c1 shared: add nm_utils_hashtable_same_keys() util 2019-06-26 09:53:54 +02:00
Thomas Haller
fcaf7994f2 shared: allow nm_c_list_move_*() API also to move from one list to another
Previously, nm_c_list_move_*() only allowed to move element inside the
same list. Relax that, it works just the same list to move the element
from one list into a different list.
2019-06-26 09:53:54 +02:00
Thomas Haller
bf6e902c90 CONTRIBUTING: update section about assertions in NetworkManager 2019-06-26 09:53:54 +02:00
Beniamino Galvani
e4ce9bd7af device: set IPv6 token only when necessary
Setting the IPv6 token triggers a new router solicitation from kernel
and so we should avoid when not strictly necessary.

https://mail.gnome.org/archives/networkmanager-list/2019-May/msg00004.html
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/issues/179
2019-06-26 09:04:00 +02:00
Lubomir Rintel
a5dd31afeb contrib/checkpatch: allow empty lines within continuations
This chunk from nm-device.c is, in fact, okay:

               |<-tab->nm_assert (   !new_config
               |<-tab->           || (   new_config
               |<-tab->               && ({
               |<-tab->                    int ip_ifindex = ...
 empty line -> |
               |<-tab->                    (   ip_ifindex > 0
               |<-tab->                     && ip_ifindex == ...
               |<-tab->                  })));
2019-06-25 20:27:39 +02:00
Lubomir Rintel
da312e6220 contrib/checkpatch: be a bit stricter about whitespace
In continations (that use spaces for alignment), don't allow the number
of leading tabs to change. Previously only removal of tabs was
disallowed, but addition doesn't make sense either, as only spaces
should be used for further alignemnt.

This catches situations like this:

  |<-tab->all_work_and_no_play (makes,
  |<-tab->                      jack,
  |<-tab-><-tab->               a dull boy);
2019-06-25 20:27:39 +02:00
Lubomir Rintel
5ff19ea8d2 contrib/checkpatch: discourage g_assert*() 2019-06-25 20:27:39 +02:00
Thomas Haller
16dbe0a573 core: avoid plain pointer values in logging output
Logging pointer values is useful to identify the object in the logging message.
But plain pointer values also can be used to defeat ASLR and should not be logged.

Instead, print NM_HASH_OBFUSCATE_PTR() value, which is a 64 bit number based on
the pointer value and some random seed. A minor problem is that there is still the
chance of duplicates, albeit small.
2019-06-25 13:26:37 +02:00
Thomas Haller
1ed338fe5e CONTRIBUTING: reword "novel contributions" to "new contributions"
The meaning of "novel" and "new" here is the same, but "novel" als
has a meaning related to patents. So avoid that confusion.
2019-06-24 09:27:03 +02:00
Thomas Haller
bcbc39b240 settings/ibft: merge branch 'th/drop-ibft-settings-plugin'
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/186
2019-06-20 17:27:02 +02:00
Thomas Haller
74641be816 settings: drop ibft settings plugin
The functionality of the ibft settings plugin is now handled by
nm-initrd-generator. There is no need for it anymore, drop it.

Note that ibft called iscsiadm, which requires CAP_SYS_ADMIN to work
([1]). We really want to drop this capability, so the current solution
of a settings plugin (as it is implemented) is wrong. The solution
instead is nm-initrd-generator.

Also, on Fedora the ibft was disabled and probably on most other
distributions as well. This was only used on RHEL.

[1] https://bugzilla.redhat.com/show_bug.cgi?id=1371201#c7
2019-06-20 16:06:44 +02:00
Thomas Haller
f182d4fa20 shared/tests: add test for nm_utils_bin2hexstr_full() 2019-06-19 15:49:57 +02:00
Thomas Haller
637c785f4e shared: fix nm_utils_bin2hexstr_full() for buffers of length zero 2019-06-19 15:30:55 +02:00
Beniamino Galvani
4dce38c37f connectivity: merge branch 'bg/concheck-issue181'
Don't start connectivity check on unconfigured devices.

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/issues/181
2019-06-18 16:09:40 +02:00
Thomas Haller
5a416a9da1 Revert "Coerce connectivity "LIMITED" to "NONE" when device is disconnected"
NMConnectivity can now distinguish between LIMITED and NONE connectivity
and it does so based on whether IP addresses and routes are configured.

Previously, NMConnectivity would not differenciate between limited and
no connectivity, which is why NMDevice added some additional logic on top
to coerce LIMITED to NONE (if the device is not logically connected).

But note that the connectivity state (whether a network is reachable on
an interface) depends on what is configured in kernel and whether the
internet is reachable on that interface. It does not depend on the
logical device state.

On the other hand, whether the device is configured in a manner to have
connectivity depends on the logical state of the device (as NetworkManager
is configuring the device).

So, in many cases, the logical state and the connectivity state agree now,
but for the right reasons.

This reverts commit 4c4dbcb78d.
2019-06-18 15:49:09 +02:00
Thomas Haller
b626baa313 connectivity: make platform argument to nm_connectivity_check_start() optional
The platform is used to detect whether to skip the connectivity check right away.
It should be an optional argument, so one could avoid this pre-check.
2019-06-18 15:49:09 +02:00
Thomas Haller
19c957f091 connectivity: simplify passing result to idle handler 2019-06-18 15:49:09 +02:00
Thomas Haller
4001aee370 connectivity: remove unused error varialbe in _idle_cb() 2019-06-18 15:49:09 +02:00
Beniamino Galvani
91d447df19 device: don't start connectivity check on unconfigured devices
If the interface has no carrier, no addresses or no routes there is no
point in starting a connectivity check on it because it will fail.
Moreover, doing the check on a device without routes causes the
addition of a negative entry in the ARP table for each of the
addresses associated with the connectivity check host; this can lead
to poor network performances.

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/issues/181
2019-06-18 15:49:09 +02:00
Lubomir Rintel
11d59de600 build/autotools: generate "config-extra.h" via makefile "config-extra.h.mk"
When the code that generates "config-extra.h" changes, we want to regenerate
the file. Move that code to a separate makefile so we can add a
dependency.

Otherwise, we'd had to depend on "Makefile", which itself is generated by
Makefile.am.

Also, depend on "config.h" to regenerate it when ./configure runs and
touches that header. This may not cover all cases where ./configure's
configuration changes and a regeneration would be due. But such is life.

Also, most components depend on this header, so let various .dirstamp
files depend on it, so we are sure to build this first. That because,
autotools generates dependencies for header files automatically, but
that requires that the header file exist. Such automatic dependencies
don't work out-of-the-box for generated headers.

Co-authored-by: Thomas Haller <thaller@redhat.com>
2019-06-17 17:42:09 +02:00
Thomas Haller
721f238946 build/autotools: depend "config-extra.h" on "config.h"
"config-extra.h" is really just like "config.h", except it works around some
limitations of autoconf.

If we depend on "Makefile", any changes to "Makefile.am" will cause a full
rebuild. We want to avoid that.

Instead, depend on "config.h". That one only changes when configure runs
again. And that's the better dependancy, because "config-extra.h" is
generated based on informations generated by configure (despite being
generated by "Makefile").
2019-06-17 13:00:37 +02:00
Thomas Haller
7ed1fc817f Revert "build: only update config-extra.h if it changes"
Not touching "config-extra.h" means that the target is rebuild every
time (because the timestampt does not get updated). On the other hand,
touching it will cause a full rebuild (which we often want to avoid).

The right solution is instead to depend on "config.h", which will be
done next.

This reverts commit 14271d84a0.
2019-06-17 12:54:04 +02:00
Thomas Haller
57431d872c settings: merge branch 'th/various-settings-cleanup-2'
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/184
2019-06-17 12:12:17 +02:00
Thomas Haller
5b7f6421c7 keyfile: rework selecting path name in nms_keyfile_writer_connection() and add callback to reject filenames
The previous logic seems complicated to me. I even think it is wrong.
Rework it, I think this makes sense.

Also, previously the existing path was used if the file didn't exist.
I think that is wrong. If for force a rename, then the filename must
not be used even if the file currently does not exist.

Also add an "allow_filename_cb" argument, to reject filenames that
are blacklisted.
2019-06-17 12:12:02 +02:00
Thomas Haller
71928a3e5c settings: avoid cloning the connection to maintain agent-owned secrets 2019-06-17 12:12:02 +02:00
Thomas Haller
1de36fad51 libnm: add NM_CONNECTION_SERIALIZE_WITH_SECRETS_AGENT_OWNED serialization flag
At various places we only want to serialize agent-owned secrets. Without this
flag, we need to clone the setting first, then drop the secrets, then serialize
to D-Bus. Add a serialization flag to avoid that.

The name ("with") and the meaning of the flag is chosen in a way, that
there could be multiple such flags (NM_CONNECTION_SERIALIZE_WITH_SECRETS_NOT_REQUIRED),
and specifying at least one of them, would have the meaning to whitelist
flags of this kind. Specifying non of these "with" flags would have the
meaning of specifying *all*. Currently there is only one kind, so the name
and meaning is slightly counter intuitive.
2019-06-17 12:12:02 +02:00
Thomas Haller
a17453913c settings: add _nm_connection_clear_secrets_by_secret_flags() function to simplify clearing secrets 2019-06-17 12:12:02 +02:00
Thomas Haller
396b188697 settings: pass const strv plugins array to load_plugins() 2019-06-17 12:12:02 +02:00
Thomas Haller
a56fb02af6 settings: avoid emiting notify::unmanaged-specs for NMSettings if there are no changes 2019-06-17 12:12:02 +02:00
Thomas Haller
408a453bee settings: track keyfile plugin explicitly in NMSettings
The keyfile plugin is special. For one, NetworkManager will always load
it.

In the future, only this plugin should handle in-memory connections.
In-memory connections are kinda special, and we don't need general
plugins to be concerned about them. They should be handled by keyfile
plugin.

But then NMSettings needs to have a reference to the keyfile plugin
instance at hand.
2019-06-17 12:12:02 +02:00
Thomas Haller
45013bfbff libnm: cleanup _nm_connection_ensure_normalized() and split nm_connection_normalize()
- in _nm_connection_ensure_normalized() allow also to only check that
  the UUID is as expected, without really resetting it.

- split the normalization part out of nm_connection_normalize() and
  reuse it in _nm_connection_ensure_normalized(). As we already verified
  the connnection, we know that normalization is due and don't need to
  verify again.
2019-06-17 12:12:02 +02:00
Thomas Haller
1cc4a8b6a9 shared: add nm_utils_g_slist_strlist_cmp() util
Usually we avoid GSList, because I think it's not a great data type.
Anyway, our match-specs are just a GSList of strings, so we need some
API to handle them.
2019-06-17 12:12:02 +02:00