After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
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
 
 
Reported: 2018-02-09 07:52 UTC by Märt Bakhoff
Modified: 2018-02-15 09:22 UTC
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!