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 789383 - NM doesn't save gsm password/pin to secret agent
NM doesn't save gsm password/pin to secret agent
Status: RESOLVED FIXED
Product: NetworkManager
Classification: Platform
Component: Mobile broadband
1.8.x
Other Linux
: Normal normal
: ---
Assigned To: NetworkManager maintainer(s)
NetworkManager maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2017-10-24 06:43 UTC by Jan Grulich
Modified: 2017-11-21 12:58 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
[PATCH] settings: preserve agent-owned secrets on connection add (3.64 KB, patch)
2017-11-20 20:29 UTC, Beniamino Galvani
none Details | Review

Description Jan Grulich 2017-10-24 06:43:43 UTC
When saving a GSM connection containing password or pin which are marked to be saved to secret agent (password-flags or pin-flags are set to agent-owned), then when NM asks a secret agent to save them, the passed map doesn't contain secrets and the agent has nothing to save. I've been comparing SaveSecrets() calls for e.g. wireless connection and there I can see secrets, while for gsm connections I don't see them.

NM version: NetworkManager-1.8.4-4.fc27.x86_64
Comment 1 Beniamino Galvani 2017-11-20 18:27:04 UTC
It looks like since 1.8 NM doesn't save agent-owned secrets for connections written by the keyfile plugin. I'm checking when this broke...
Comment 2 Beniamino Galvani 2017-11-20 20:29:35 UTC
Created attachment 364077 [details] [review]
[PATCH] settings: preserve agent-owned secrets on connection add
Comment 3 Thomas Haller 2017-11-21 09:21:35 UTC
(In reply to Beniamino Galvani from comment #2)
> Created attachment 364077 [details] [review] [review]
> [PATCH] settings: preserve agent-owned secrets on connection add

It would be nicer if nm_settings_plugin_add_connection() would handle this. However, nm_settings_plugin_add_connection() currently just calls the virtual add_connection(), and this shouldn't be re-implemented in each virtual add_connection() function. Instead, add_connection() should be split, so that nm_settings_plugin_add_connection() could do the steps write-connection and update-connection separately.
If the plugin would expose a virtual write-connection() method, then it could also be used to implement NMSettingsConnection:commit_changes() in the parent class -- instead of duplicating that too.

But for the moment, the patch is fine as is.
Comment 4 Beniamino Galvani 2017-11-21 12:58:03 UTC
Applied to master, nm-1-10 and nm-1-8 branches.

https://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?id=62141d59cb3f41081ecc2af1f716f78c3d2a89b5