secret-agent: rework secret-agent to better handle service shutdown

System Internals / NetworkManager - Thomas Haller [redhat.com] - 8 August 2019 08:10 EDT

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.

f66246594 secret-agent: rework secret-agent to better handle service shutdown
src/settings/nm-agent-manager.c | 33 +--
src/settings/nm-secret-agent.c | 625 +++++++++++++++++++++-------------------
src/settings/nm-secret-agent.h | 6 +-
3 files changed, 351 insertions(+), 313 deletions(-)

Upstream: cgit.freedesktop.org


  • Share