After evaluation, there is a proposed plan to migrate from Bugzilla to GitLab (more info on GitLab).
Your feedback as a project maintainer and developer is welcome in the mailing list thread.
Bug 793324 - AddAndActivateConnection loses agent secrets
AddAndActivateConnection loses agent secrets
Status: RESOLVED FIXED
Product: NetworkManager
Classification: Platform
Component: general
1.10.x
Other Linux
: Normal normal
: ---
Assigned To: NetworkManager maintainer(s)
NetworkManager maintainer(s)
:
Depends on:
Blocks: nm-review
  Show dependency tree
 
Reported: 2018-02-09 07:52 UTC by Märt Bakhoff
Modified: 2018-02-15 09:22 UTC (History)
2 users (show)

See Also:
GNOME target: ---
GNOME version: ---


Attachments
backtraces and patch (10.55 KB, text/plain)
2018-02-09 07:52 UTC, Märt Bakhoff
  Details
[PATCH] settings: preserve agent-owned secrets on connection update (2.61 KB, patch)
2018-02-14 10:57 UTC, Beniamino Galvani
none Details | Review

Description Märt Bakhoff 2018-02-09 07:52:18 UTC
Created attachment 368173 [details]
backtraces and patch

Repro steps:
1) Start KUbuntu 16.04
2) Open plasma-nm network gui on the tray
3) Click "Connect" for some wpa-psk network, enter passphrase in the textbox, press enter
4) Popup appears, asking for the passphrase again

This also reproduces with my gentoo setup:
kde-plasma/plasma-nm-5.12.0
kde-frameworks/networkmanager-qt-5.42.0
net-misc/networkmanager-1.10.2

Some additional details follow. I've had no prior knowledge of networkmanager internals before debugging this, so some things may be wrong. 

This is what afaik should happen:
1) plasma-nm sends connection details to NetworkManager over dbus.
2) NetworkManager sends the passphrase to the secret agent. This should be kded5 in KDE. At least it seems like kded5 is what is showing the popup from step 4.
3) NetworkManager connects to the network

This is what is actually happening according to gdb:
1) plasma-nm (Handler::addAndActivateConnection) calls AddAndActivateConnection over dbus. the request reaches impl_manager_add_and_activate_connection in NetworkManager and the passphrase is present: '802-11-wireless-security': {'key-mgmt': <'wpa-psk'>, 'psk': <'111111111'>, 'psk-flags': <1>}
2) NetworkManager reaches nm_secret_agent_save_secrets. the connection info contains the right network name, but the passphrase is already lost. the SaveSecrets dbus call reaches kded5 successfully, but doesn't contain any agent secrets.
3) in between, the nm_settings_connection_update method is called that writes some stuff to disk and in the process fails to restore the agent secrets. here is the backtrace of where that happens:

(gdb) bt
#0 nm_settings_connection_update
#1 activation_add_done
#2 pk_add_cb
#3 auth_chain_finish
#4 g_idle_dispatch

My speculation:
The nm_settings_connection_update method saves system secrets to disk and then reads the file back in, replacing the connection secrets. In the process the agent secrets are lost. If update_agent_secrets_cache was called before calling nm_settings_connection_update, then the agent secrets are automatically restored from the cache. 

Most callers of nm_settings_connection_update either call update_agent_secrets_cache or pass a copy of the connection info (see https://bugzilla.gnome.org/show_bug.cgi?id=789383). activation_add_done seems to do neither, causing only the system secrets to be preserved. 

I tried to patch activation_add_done to pass a copy of the connection info to nm_settings_connection_update and this seems to fix the issue. See attached info.txt for full backtraces and the patch I tried.

This bug was also reported long ago in https://bugs.kde.org/show_bug.cgi?id=365719
Comment 1 Beniamino Galvani 2018-02-14 10:57:00 UTC
Created attachment 368337 [details] [review]
[PATCH] settings: preserve agent-owned secrets on connection update

Hi,

thanks for the detailed analysis! Your patch didn't compile so I have prepared a new one using a different approach.
Comment 2 Thomas Haller 2018-02-14 11:14:56 UTC
(In reply to Beniamino Galvani from comment #1)
> Created attachment 368337 [details] [review] [review]
> [PATCH] settings: preserve agent-owned secrets on connection update
> 
> Hi,
> 
> thanks for the detailed analysis! Your patch didn't compile so I have
> prepared a new one using a different approach.

lgtm
Comment 3 Märt Bakhoff 2018-02-14 17:12:01 UTC
patch from comment #1 works for me. thank you!

Note You need to log in before you can comment on or make changes to this bug.