summaryrefslogtreecommitdiff
Commit message (Collapse)AuthorAgeFilesLines
* settings: return errno from nms_keyfile_nmmeta_write() for better loggingth/settings-improvementsThomas Haller2019-08-084-34/+42
| | | | | | | | | | | | I encountered a failure in the log <trace> [1564647990.7822] keyfile: commit: deleting nmmeta file "/etc/NetworkManager/system-connections/35370b0b-e53b-42ea-9fe3-f1b1d552343b.nmmeta" failed <trace> [1564647990.7822] keyfile: commit: deleting nmmeta file "/etc/NetworkManager/system-connections/35370b0b-e53b-42ea-9fe3-f1b1d552343b.nmmeta" simulated I think that was due to SELinux (rh #1738010). Let nms_keyfile_nmmeta_write() return an errno code so we can log more information about the failure.
* shared,all: return boolean success from nm_utils_file_get_contents()Thomas Haller2019-08-0813-180/+234
| | | | | | | | | | | | | | | | | | | | | | ... and nm_utils_fd_get_contents() and nm_utils_file_set_contents(). Don't mix negative errno return value with a GError output. Instead, return a boolean result indicating success or failure. Also, optionally - output GError - set out_errsv to the positive errno (or 0 on success) Obviously, the return value and the output arguments (contents, length, out_errsv, error) must all agree in their success/failure result. That means, you may check any of the return value, out_errsv, error, and contents to reliably detect failure or success. Also note that out_errsv gives the positive(!) errno. But you probably shouldn't care about the distinction and use nm_errno_native() either way to normalize the value.
* shared: let nm_utils_file_set_contents() return a errno error codeThomas Haller2019-08-087-36/+33
| | | | | | | | | | | | | | | | nm_utils_file_set_contents() is a re-implementation of g_file_set_contents(), as such it returned merely a boolean success value. It's sometimes interesting to get the native error code. Let the function deviate from glib's original g_file_set_contents() and return the error code (as negative value) instead. This requires all callers to change. Also, it's potentially a dangerous change, as this is easy to miss. Note that nm_utils_file_get_contents() also returns an errno, and already deviates from g_file_get_contents() in the same way. This patch resolves at least the inconsistency with nm_utils_file_get_contents().
* examples: improve usage/synposis for nm-update2.py and nm-add-connection2.pyThomas Haller2019-08-082-4/+14
|
* secret-agent: merge branch 'th/secret-agent-cleanup'Thomas Haller2019-08-087-478/+407
|\ | | | | | | https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/231
| * secret-agent: rework secret-agent to better handle service shutdownThomas Haller2019-08-083-313/+351
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The secret-agent D-Bus API knows 4 methods: GetSecrets, SaveSecrets, DeleteSecrets and CancelGetSecrets. When we cancel a GetSecrets request, we must issue another CancelGetSecrets to tell the agent that the request was aborted. This is also true during shutdown. Well, technically, during shutdown we anyway drop off the bus and it woudn't matter. In practice, I think we should get this right and always cancel properly. To better handle shutdown change the following: - each request now takes a reference on NMSecretAgent. That means, as long as there are pending requests, the instance stays alive. The way to get this right during shutdown, is that NMSecretAgent registers itself via nm_shutdown_wait_obj_register() and NetworkManager is supposed to keep running as long as requests are keeping the instance alive. - now, the 3 regular methods are cancellable (which means: we are no longer interested in the result). CancelGetSecrets is not cancellable, but it has a short timeout NM_SHUTDOWN_TIMEOUT_MS to handle this. We anyway don't really care about the result, aside logging and to be sure that the request fully completed. - this means, a request (NMSecretAgentCallId) can now immediately be cancelled and destroyed, both when the request returns and when the caller cancels it. The exception is GetSecrets which keeps the request alive while waiting for CancelGetSecrets. But this is easily handled by unlinking the call-id and pass it on to the CancelGetSecrets callback. Previously, the NMSecretAgentCallId was only destroyed when the D-Bus call returns, even if it was cancelled earlier. That's unnecessary complicated. - previously, D-Bus requests SaveSecrets and DeleteSecrets were not cancellable. That is a problem. We need to be able to cancel them in order to shutdown in time. - use GDBusConnection instead of GDBusProxy. As most of the time, GDBusProxy provides features we don't use. - again, don't log direct pointer values, but obfuscate the indentifiers.
| * secret-agent: use NMCListElem to track permissions in NMSecretAgentThomas Haller2019-08-081-30/+17
| | | | | | | | I don't like GSList.
| * secret-agent/trivial: rename dbus_connection field of NMSecretAgentPrivateThomas Haller2019-08-081-11/+11
| |
| * secret-agent: avoid log plain pointer valuesThomas Haller2019-08-081-9/+23
| | | | | | | | This defeats ASLR. Obfuscate the pointers.
| * dbus-manager: drop unused private-socket functions from "nm-dbus-manager.c"Thomas Haller2019-08-082-90/+0
| | | | | | | | | | | | | | These functions are now unused. Drop them. Also, if we ever reintroduce private unix socket, we sure won't use GDBusProxy. Good riddance.
| * secret-agent: drop unused private-socket code from secret-agentThomas Haller2019-08-081-57/+26
| | | | | | | | | | | | | | | | | | | | In the past, we had a private unix socket. That is long gone. Drop the remains in "nm-secret-agent.c". The request here really always comes from the main D-Bus connection. Maybe the private unix socket makes sense and we might resurrect it one day. But at that point it would be an entire rewrite and the existing code is probably not useful either way. Drop it.
| * secret-agent: enable trace log messagesThomas Haller2019-08-081-5/+5
| | | | | | | | They seem useful for debugging. Don't only enable them --with-more-logging.
| * shared: add nm_c_list_elem_find_first() helper macroThomas Haller2019-08-082-10/+21
|/ | | | | | | | | | | | | - add nm_c_list_elem_find_first() macro that takes a predicate and returns the first match. This macro has a non-function-like behavior, which we often try to avoid because macros should behave like functions. In this case it's however convenient, so let's do it. Also, despite being non-function-like, it should be pretty hard to use wrongly. - rename nm_c_list_elem_find_first() to nm_c_list_elem_find_first_ptr().
* n-dhcp4: allocate memory of right size in n_dhcp4_client_probe_option_new()Thomas Haller2019-08-081-1/+1
| | | | | | Non-critical, as the allocated memory was larger than needed. https://gitlab.freedesktop.org/NetworkManager/NetworkManager/issues/224
* firewall: refactor "nm-firewall-manager.c" to not use GDBusProxyThomas Haller2019-08-074-280/+327
| | | | | | | | | | | | | | | | | | | | | | | | | - Don't use GDBusProxy but plain GDBusConnection. NMFirewallManager is very simple, it doesn't use any of the features that GDBusProxy provides. - make NMFirewallManagerCallId typedef a pointer to the opaque call-id struct, instead of the struct itself. It's confusing to have a variable that does not look like a pointer and assigning %NULL to it. - internally drop the CBInfo typename and name the call-id variable constsistantly as "call_id". - no need to keep the call-id struct alive after cancelling it. That simplifies the lifetime managment of the pending call because the completion callback is always invoked shortly before destroying the call-id. - note that the caller is no longer allowed to cancel a call-id from inside the completion callback. That just complicates the implementation and is not necessary. Assert against that. https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/230
* cli/trivial: align property definitions with spaces and not with tabsThomas Haller2019-08-061-2182/+2182
| | | | | | | | | | | Our coding style is to indent with tabs, but align with spaces. This is not about the coding style though, but about the code looking broken when not using 4 spaces per tab (in fact, some code there is aligned as if using 8 spaces and it's already inconsistent). Realign with spaces. https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/223
* release: bump version to 1.21.1-dev after 1.20.0 release1.21.1-devThomas Haller2019-08-0620-205/+260
|\ | | | | | | | | | | | | | | | | | | After 1.20.0 is released, merge it back into master so that 1.20.0 is part of the history of master. That means, $ git log --first-parent master will also traverse 1.20.0 and 1.20-rc*. Also bump the micro version to 1.21.1-dev to indicate that this is after 1.20.0 is out.
| * libnm/doc: add missing "Since: 1.20" commentsThomas Haller2019-08-063-3/+5
| |
| * ifupdown: fix assertion during logging %NULL storage in load_eni_ifaces()Thomas Haller2019-08-061-1/+1
| |
| * libnm/doc: add Since tag for %NM_SETTING_IP6_CONFIG_METHOD_DISABLEDThomas Haller2019-08-061-0/+2
| |
| * modem: fix memory leakBeniamino Galvani2019-08-061-1/+1
| | | | | | | | Fixes: 9b935fad9b10 ('modem: don't use GAsyncResult pattern for disconnecting modem')
| * build: merge branch 'bg/meson-fixes'Beniamino Galvani2019-08-056-27/+27
| |\ | | | | | | | | | https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/226
| | * build: fix meson warning about 'install' arg in 'configure_file'Beniamino Galvani2019-08-052-3/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | WARNING: Project targetting '>= 0.44.0' but tried to use feature introduced in '0.50.0': install arg in configure_file From the documentation: "install (added 0.50.0) When true, this generated file is installed during the install step, and install_dir must be set and not empty. When false, this generated file is not installed regardless of the value of install_dir. When omitted it defaults to true when install_dir is set and not empty, false otherwise." The parameter can be omitted because install_dir is set. https://gitlab.freedesktop.org/NetworkManager/NetworkManager/issues/216
| | * build: fix meson warning about wrong custom target argumentBeniamino Galvani2019-08-051-2/+1
| | | | | | | | | | | | | | | | | | src/meson.build:294: WARNING: Custom target input 'NetworkManager' can't be converted to File object(s). This will become a hard error in the future.
| | * build: fix meson warning about path separator in targetBeniamino Galvani2019-08-052-21/+25
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Fix the following: WARNING: Target "nm-utils/tests/test-shared-general" has a path separator in its name. This is not supported, it can cause unexpected failures and will become a hard error in the future. https://gitlab.freedesktop.org/NetworkManager/NetworkManager/issues/217
| | * build: fix meson warning about invalid 'depends' keywordBeniamino Galvani2019-08-051-1/+1
| |/ | | | | | | | | | | | | | | Fix this: libnm/meson.build:215: WARNING: Passed invalid keyword argument "depends". WARNING: This will become a hard error in the future.
| * libnm-core: fix ifcfg-rh variable name for DHCPv6 hostnameBeniamino Galvani2019-08-051-1/+1
| | | | | | | | Fixes: 2852b509456d ('ifcfg-rh: add DHCPV6_HOSTNAME and DHCPV6_SEND_HOSTNAME vars')
| * libnm: when stringifying policy routing rule place "not" specifier after ↵Thomas Haller2019-08-052-5/+8
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | "priority" Otherwise, it just looks odd: "not priority 31265 from 0.0.0.0/0 fwmark 0xcb87 table 52103" Better is: "priority 31265 not from 0.0.0.0/0 fwmark 0xcb87 table 52103" The "not" specifier should come after the priority. It makes more sense to read it that way. As far as parsing the string is concerned, the order does not matter. So this change in behavior is no problem. https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/228
| * cli: merge branch 'th/nmcli-add-and-activate-cleanup'Thomas Haller2019-08-051-75/+83
| |\ | | | | | | | | | https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/227
| | * cli: use cleanup macro for freeing AddAndActivateInfoThomas Haller2019-08-051-57/+57
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We should prefer the cleanup macors nm_auto*() because they express ownership in code. Also, they allow to return early without additional cleanup code. That way we can refactor if-else blocks. Also, in cases where we intentionally pass on the reference, we use g_steal_pointer(), which literally spells out what happens in code.
| | * cli: add helper function to create and initialize AddAndActivateInfo structThomas Haller2019-08-051-21/+29
| |/ | | | | | | | | Also use gslice allocator instead of malloc as the size of AddAndActivateInfo is fixed and known beforehand.
| * settings: fix memory leakBeniamino Galvani2019-08-051-2/+2
| | | | | | | | Fixes: d35d3c468a304c3e0e78b4b068d105b1d753876c
| * settings: fix registering AgentManager.RegisterWithCapabilities() twiceThomas Haller2019-08-031-10/+0
| | | | | | | | Fixes: 297d4985abcc7b571b8c090ee90622357fc60e16
| * wireguard: fix use-after free in _peers_remove()Thomas Haller2019-08-031-1/+1
| |
| * libnm: fix leak in NMSettingWireGuard's update_one_secret()Thomas Haller2019-08-031-1/+1
| |
| * merge: branch 'lr/nm-d-wifi-con-update'Lubomir Rintel2019-08-021-29/+69
| |\ | | | | | | | | | https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/219
| | * cli: update the existing connection on "dev wifi connect"lr/nm-d-wifi-con-updateLubomir Rintel2019-08-021-16/+49
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If we find a matching connection, ensure it's exactly as we want it before actually proceeding to activate it. Fixes this problem: # nmcli dev wifi connect "Network of Doom" password santa <-- bad Error: Connection activation failed: (7) Invalid secrets # nmcli dev wifi connect "Network of Doom" password satan <-- correct Error: Connection activation failed: (7) Invalid secrets The password is now correct, but nmcli chose to re-activate the wrong connection it created previously.
| | * cli: take a reference to device in AddAndActivateInfoLubomir Rintel2019-08-021-6/+13
| | | | | | | | | | | | The device could vanish in between.
| | * cli: remove an unnecessary conditionLubomir Rintel2019-08-021-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | Do not check for password when creating a simple connection object for "nmcli dev wifi connect". This makes no difference in practice. The password is checked for existence later on and the connection instance is created anyway. This just makes things look a bit more consistent.
| | * cli: (trivial) rename do_device_wifi_connect_network() to ↵Lubomir Rintel2019-08-021-6/+6
| |/ | | | | | | | | | | | | do_device_wifi_connect() This is consistent with the surrounding code and avoids lines unnecessarily too wide.
| * supplicant: mark static arrays as const and static in ↵Thomas Haller2019-08-021-26/+26
| | | | | | | | | | | | | | | | | | "nm-supplicant-settings-verify.c" They should be "static" and only visible to this source file. Also, they should be "const", that allows the linker to place them into read-only memory.
| * supplicant: don't put binary data in error message for supplicantThomas Haller2019-08-021-4/+10
| | | | | | | | | | | | | | | | For better or worse, the API does not require the value to be a UTF-8 string. We cannot just concatenate binary to a string. Instead, backslash escape it with utf8safe-escape. Also, this will shut up a (wrong) coverity warning at this place.
| * supplicant: fix nm_supplicant_settings_verify_setting() honoring the string ↵Thomas Haller2019-08-021-21/+27
| | | | | | | | | | | | | | length We must not just pretend that the value is a NULL terminated string. That's why we have the length argument.
| * device/bluetooth: explicitly ignore return value of ioctl() in ↵Thomas Haller2019-08-021-1/+1
| | | | | | | | | | | | nm_bluez5_dun_cleanup() Coverity doesn't like us not checking the result.
| * libnm: remove dead code in nm_team_setting_config_get()Thomas Haller2019-08-021-2/+2
| | | | | | | | | | | | | | I was aware that this code is not reachable. But for consistency, it seems better to be explict about it (to avoid future bugs when refactoring). Anyway, Coverity complains about it. So assert instead.
| * device: trigger a connectivity check when device disconnectsThomas Haller2019-08-021-6/+7
| | | | | | | | | | | | https://gitlab.freedesktop.org/NetworkManager/NetworkManager/issues/219 https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/225
| * core: assert for valid arguments in _addresses_sort_cmp()Thomas Haller2019-08-021-0/+3
| | | | | | | | | | Coverity thinks that the arguments could be %NULL. Add an assertion, hoping to silence coverity.
| * tui: newt: remove NULL checks after dereferenceBeniamino Galvani2019-08-021-6/+4
| | | | | | | | | | priv->start_buttons and priv->end_buttons are initialized at construction and never changed and so the checks are not needed.
| * n-dhcp4: remove dead codeBeniamino Galvani2019-08-021-4/+1
| | | | | | | | Reported by coverity.
| * platform/tests: relax assertion for platform signal in test_slave()Thomas Haller2019-08-021-1/+1
| | | | | | | | | | | | | | | | Seen on gitlab-ci. NMPlatformSignalAssert: ../src/platform/tests/test-link.c:260, test_slave(): failure to accept signal [0,2] times: link-changed-changed ifindex 15 (3 times received) ERROR: src/platform/tests/test-link-linux - too few tests run (expected 76, got 6) ERROR: src/platform/tests/test-link-linux - exited with status 133 (terminated by signal 5?)