Bug 276486

Summary:	WPA-EAP MSCHAPv2 password stored in KWallet cannot be changed once set (20110616 nm09 snapshot)
Product:	[Unmaintained] Network Management	Reporter:	Kevin Kofler <kevin.kofler>
Component:	Wireless	Assignee:	Will Stephenson <wstephenson>
Status:	RESOLVED UPSTREAM
Severity:	normal	CC:	collura, lamarque, laurent.rineau, rajeeshknambiar
Priority:	NOR
Version:	0.9
Target Milestone:	---
Platform:	Fedora RPMs
OS:	Linux
Latest Commit:		Version Fixed In:
Sentry Crash Report:
Attachments:	Do not insert wep key incorrectly, instead replace on Edit Patch against NetworkManager

Description Kevin Kofler 2011-06-25 22:53:18 UTC

Version: 0.9 (using KDE 4.6.3)
OS: Linux

In Fedora, we are currently testing
kde-plasma-networkmanagement-0.9-0.53.20110616git.nm09.fc15, a build from the
nm09 branch.

While working around bug #276485, I found another annoying bug in secrets handling: once I have set the WPA-EAP MSCHAPv2 password (stored in KWallet), I cannot change it through the connection editing UI anymore!

I tried to set a password of "test" first (because I wasn't sure whether it'd be stored in KWallet or written to the systemwide config file in cleartext, it did the former, which is what I expected it to do). Then, I wanted to change it to the real password, it didn't do it. It just didn't accept the password change. I had to manually find and edit the entry in kwalletmanager. (The average user would probably have had to delete and recreate the connection from scratch.)

Reproducible: Always

Steps to Reproduce:
1. Use a kde-plasma-networkmanagement snapshot from the nm09 branch.
2. Create a WPA-EAP connection. Set the password to something incorrect, e.g. "test". OK the connection.
3. Edit your connection. Change the password to the correct password.

Actual Results:
It keeps remembering the incorrect password from step 2 and does not perform the password change. (Both the dialog and kwalletmanager show the old password still being stored.)

Expected Results:
The password is changed in step 3.

Workaround: Find the password in kwalletmanager (under an entry named after the connection's UUID) and edit it. (Not very user-friendly.)

Comment 1 Lamarque V. Souza 2011-06-26 01:39:14 UTC

You are using NM version above 0.8.9997, right? O guess NM newer than that is not saving secrets when Plasma NM calls the update on the connection. If it true it explains both this bug and bug 276485.

Comment 2 Kevin Kofler 2011-06-26 02:18:15 UTC

I'm using NetworkManager-0.8.9997-4.git20110620.fc15.

Comment 3 Lamarque V. Souza 2011-06-26 04:54:02 UTC

Well, nothing is updated when Plasma NM uses NM > 0.8.999. Plasma NM calls NM's update method passing the correct map with the new settings but NM does not update anything. I will have to investigate more what is happening.

Comment 4 Lamarque V. Souza 2011-06-26 05:44:15 UTC

(In reply to comment #3)
> Well, nothing is updated when Plasma NM uses NM > 0.8.999. Plasma NM calls NM's
> update method passing the correct map with the new settings but NM does not
> update anything. I will have to investigate more what is happening.

Hmmm latest NM git version saves settings correctly. When I call the NM's DBus method to update the connection some time later NM's calls the secret agent to save the old secret instead of the new one. This seems to be a bug in NM, can you reproduce it with nm-applet? nm-applet is not saving secrets here, so I cannot test it.

Comment 5 Lamarque V. Souza 2011-06-26 05:57:57 UTC

(In reply to comment #4)
> (In reply to comment #3)
> > Well, nothing is updated when Plasma NM uses NM > 0.8.999. Plasma NM calls NM's
> > update method passing the correct map with the new settings but NM does not
> > update anything. I will have to investigate more what is happening.
> 
> Hmmm latest NM git version saves settings correctly. When I call the NM's DBus
> method to update the connection some time later NM's calls the secret agent to
> save the old secret instead of the new one. This seems to be a bug in NM, can
> you reproduce it with nm-applet? nm-applet is not saving secrets here, so I
> cannot test it.

Ok, that is getting wierd, something NM saves the connection's settings sometimes not :-/

Comment 6 Rajeesh K V 2011-06-26 13:32:26 UTC

I can also reproduce the issue with kde-plasma-networkmanagement-0.9-0.53.20110616git.nm09.fc15 from Fedora (which doesn't use nm-compat patches), but no issue with nm-applet (NetworkManager-gnome-0.8.9997-4.git20110620).

Comment 7 Rajeesh K V 2011-06-26 14:02:39 UTC

I guess I spoke too soon.
If I "OK" from KCM module, WEP secrets are getting saved correctly. So, I can't reproduce this.

Comment 8 Rajeesh K V 2011-06-27 19:20:58 UTC

Created attachment 61382 [details]
Do not insert wep key incorrectly, instead replace on Edit

Comment 9 Rajeesh K V 2011-06-27 19:21:38 UTC

I can reproduce this issue for WEP as well - just tried adding a new Adhoc connection with WEP Hex keys, saving connection secrets into kwallet. Next time changing the WEP key is not affected.

Debugged and found couple of issues:
1. WepWidget::writeConfig() is doing d->keys.insert, which causes new key to be pushed into currentIndex, instead of replacing it. Attached patch hopefully fix it.
2. OrgFreedesktopNetworkManagerSettingsConnectionInterface::Update() is probably not saving the new secrets correctly, OR
3. OrgFreedesktopNetworkManagerSettingsConnectionInterface::GetSecrets() called in NMDBusSettingsConnectionProvider::getConnectionSecrets() is not getting correct values, but old unchanged value.

Not sure how to debug 2 and 3.

Comment 10 Lamarque V. Souza 2011-06-27 19:59:11 UTC

Git commit 473fadaa9c86a481f479945f230b5cb7741b0a1c by Lamarque V. Souza.
Committed on 27/06/2011 at 21:58.
Pushed by lvsouza into branch 'nm09'.

Do not insert wep key incorrectly, instead replace on Edit.
Thanks Rajeesh K Nambiar for this patch.

CCBUG: 276486

M  +1    -1    libs/ui/security/wepwidget.cpp     

http://commits.kde.org/networkmanagement/473fadaa9c86a481f479945f230b5cb7741b0a1c

Comment 11 Lamarque V. Souza 2011-06-27 20:01:27 UTC

Git commit d07f01faa8d0236683bf9aae894d28db257188f3 by Lamarque V. Souza.
Committed on 27/06/2011 at 22:00.
Pushed by lvsouza into branch 'master'.

Do not insert wep key incorrectly, instead replace on Edit.
Thanks Rajeesh K Nambiar for this patch.

CCBUG: 276486

M  +1    -1    libs/ui/security/wepwidget.cpp     

http://commits.kde.org/networkmanagement/d07f01faa8d0236683bf9aae894d28db257188f3

Comment 12 Lamarque V. Souza 2011-06-27 20:07:34 UTC

(In reply to comment #9)
> I can reproduce this issue for WEP as well - just tried adding a new Adhoc
> connection with WEP Hex keys, saving connection secrets into kwallet. Next time
> changing the WEP key is not affected.
> 
> Debugged and found couple of issues:
> 1. WepWidget::writeConfig() is doing d->keys.insert, which causes new key to be
> pushed into currentIndex, instead of replacing it. Attached patch hopefully fix
> it.
> 2. OrgFreedesktopNetworkManagerSettingsConnectionInterface::Update() is
> probably not saving the new secrets correctly, OR

I already checked, Plasma NM is passing the correct secrets to Update but NetworkManager is callling SaveSecrets with the old values instead of the new ones. I cannot do anything until NM stops doing that. I have asked on NM mailing list how are NM's clients supposed to update secrets, nobody has answered so far. So until I get some answers you have better not changing secrets.

> 3. OrgFreedesktopNetworkManagerSettingsConnectionInterface::GetSecrets() called
> in NMDBusSettingsConnectionProvider::getConnectionSecrets() is not getting
> correct values, but old unchanged value.
>
> Not sure how to debug 2 and 3.

I use kDebug for that.

Comment 13 Lamarque V. Souza 2011-07-04 03:34:04 UTC

Created attachment 61591 [details]
Patch against NetworkManager

The attached patch fixed the problem for me. The patch is against Networkmanager, not Plasma NM, so this is indeed an upstream bug. I already sent the patch the NM's developers, let's see what they have to say about it. The patch is incomplete because it only works with wireless secrets. Probably NM's developers can improve it to work with other settings. Let's hope so.

Comment 14 Lamarque V. Souza 2011-07-18 20:41:47 UTC

Well, the NetworkManager people have not given me any answer in despite of my e-mails. If anyone here knows any NM developer please try to contact him/her to help test my patch against NM.

Comment 15 Lamarque V. Souza 2011-07-31 00:27:04 UTC

Git commit 8fc5d8547eb76a503c23fdbfd6a072eb4393ca0b by Lamarque V. Souza.
Committed on 31/07/2011 at 01:45.
Pushed by lvsouza into branch 'nm09'.

Since NM guys are being too slow to fix this bug I am adding this hack until
they fix it for good. Please try and see if it does not bring new bugs.

CCBUG: 276486

M  +9    -4    backends/NetworkManager/nmdbussettingsconnectionprovider.cpp

http://commits.kde.org/networkmanagement/8fc5d8547eb76a503c23fdbfd6a072eb4393ca0b

Comment 16 Lamarque V. Souza 2011-08-16 23:43:40 UTC

Ok, this bugs has been fixed in NetworkManager git repository, commit e2d88f59e6b95edfbd0b5ae7e7e396e8ab462664 . I will close this bug since it is a upstream issue. More details about the problem in http://mail.gnome.org/archives/networkmanager-list/2011-August/msg00102.html