"ipsec-psk" is located in "vpn.data" field

[dvornikov-aa]

I think it would be more secure to place it into "vpn.secrets" field.

[dkosovic]

I definitely agree and have been wanting to do that, problem is the GNOME GUI from this repository isn't the only front end (e.g. KDE, deepin) and the change would break the other front ends.

I think NetworkManager-openvpn had to maintain both vpn.data and vpn.secrets for backwards compatibility for one of its fields. I guess I could do something similar for NetworkManager-l2tp-1.8.0 that I'm working on. Eventually after a few years vpn.data could be dropped. It's not an ideal solution, but not sure how else to deal with it.

[dkosovic]

I won't be modifying NetworkManager-l2tp to use vpn.secrets for the PSK for the following reasons:

• NetworkManager-l2tp on the master branch now supports user and machine certificates. The PKCS#12 certificates and private keys use vpn.secrets for their passwords (if they require a password). Using a machine certificate is much more secure than PSK.
• The added certificate support makes maintaining backwards compatibility with other GUI frontends much more difficult if PSK uses both vpn.data and vpn.secrets (as suggested in previous message).
• The PSK was being stored in clear text in the generated ipsec.secrets file (although with file permissions only readable by root), so using vpn.secrets doesn't buy much.

[joanbm]

This prevents versioning the VPN configuration file with something like etckeeper since the PSK can not be separated from the rest of the configuration file by storing it per-user so it goes in the keyring.

Certificates are not always an option since the VPN setup is often outside the user's control.

Not breaking compatibility with older versions and frontends looks like the hard part here. Other than coordinating the changes with everyone else, a possibility could be to have two options like "PSK (legacy, plaintext)" and "PSK (new)" so that users could opt-in and the change be more gradual. But that's may be confusing from a UX point of view...

[joanbm]

This is an early attempt at a patch to make the IPSec PSK a NetworkManager secret, at the moment this disregards compatibility with older versions and other frontends. Also I couldn't test it much yet. It applies over 1.8.6.

0001-Store-PSK-as-a-VPN-secret.patch.zip

[dkosovic]

Looks like I was wrong about NetworkManager-openvpn using secrets while providing backwards compatibility for legacy data passwords, turns out it was actually NetworkManager-openconnect that was providing backwards compatibility.

Extract from nm-openconnect-editor.c :

value = nm_setting_vpn_get_secret (s_vpn, NM_OPENCONNECT_KEY_TOKEN_SECRET);
if (!value)
    value = nm_setting_vpn_get_data_item (s_vpn, NM_OPENCONNECT_KEY_TOKEN_SECRET);
if (value)
    gtk_text_buffer_set_text (buffer, value, -1);

so I guess something like that (along with any other workarounds) could be done here for the PSK.

Like you said the patch disregards compatibility with older versions and other frontends at the moment, but I think it is a good start and it isn't too hard to add backwards compatibility. e.g.

• legacy workarounds similar to NetworkManager-openconnect previously mentioned.
• instead of testing for NM_L2TP_AUTHTYPE_PSK, test for NM_L2TP_AUTHTYPE_TLS and fallback to PSK otherwise.

I'm not able to test the code as I don't have access to a Linux desktop at the moment.

[joanbm]

Thanks for the feedback, I was not aware of this code solving a similar problem in another VPN implementation. I tried now tried to use the same strategy here in L2TP to keep compatibility with old connections, while using secrets for new connections, and transparently migrating old PSKs to secrets when the user updates them, here are the changes (I also fixed some bugs that were on the first patch):

https://github.com/joanbm/NetworkManager-l2tp/commits/73ac698f8af2b129418c5e13be7fb101e0c29d40

This is enough for the usual case where you are only using one frontend to edit connections. However, I don't think a nice solution is possible for the (rather convoluted) case where the user is editing the same connection with multiple frontends, some of which are aware the PSK is now a secret, and some of which are unaware. For example, if the user creates a connection from the secret-aware frontend, the secret-unaware frontend will be unable to read the secret, and even if the user re-configures it from there so it's stored in vpn.data as a non-secret, we will still use the secret when connecting.

And as far as I can tell, coordinating a "big bang" update of all frontends is impossible, since each Linux distribution has the last word and may still distribute an incompatible mix. And making the decision explicit to the user by adding some obscure option to the UI to "store the PSK in legacy format" will bother 99% of the users that don't have this problem. So I think the best we can do is try to find all the other active frontends and file an issue, to try to minimize the time window where this can happen.

[dkosovic]

I'm happy to do a pull request, but I can't really test the code, when I get home from holiday leave in the middle of January, I can test multiple Linux distros and frontends running on VMs.

I agree with what you are saying in regards to synchronizing the frontends and to add to it, some Linux distos only use the KDE Plasma LTS releases which can be a long time between releases and it is not only the GUI front ends, lots of users use the NetworkManager command-line (nmcli) and manually create the VPN connection profile, e.g. nmcli usage :

• list all NM connections: nmcli con
• bring up VPN connection : nmcli con up id VPN-Connection-Name
• bring down VPN connection: nmcli con down id VPN-Connection-Name

Where VPN-Connection-Name is the actual name of the VPN connection as listed in the nmcli con output.

Use nmcli con reload whenever a new VPN connection profile file (usually located under /etc/NetworkManager/system-connections/) has been created or has been edited, so Networkmanager will notice the change.

it'll be interesting to see if nmcli con up id VPN-Connection-Name will be happy with the PSK with either secret or legacy data. I'm not able to check myself at the moment as I don't have access to Linux at the moment. I suspect it will be okay.

[joanbm]

No hurry at all. I want to do some further experimentation with the other frontends and test the changes some more as well, but it's a bit tedious at the moment since I don't have my usual computer and internet speed those weeks.

Happy new year by the way!

[dkosovic]

@joanbm I can confirm your modifications are backwards compatible with KDE plasma-nm and it seems to be working with the nmcli command-line and successfully prompts for the PSK :

$ nmcli con --ask up id "Test VPN"
You need to authenticate to access the Virtual Private Network “Test VPN”.
Password (vpn.secrets.password): •••••••••••••••
You need to authenticate to access the Virtual Private Network “Test VPN”.
Pre-shared key (PSK) (vpn.secrets.ipsec-psk): •••••••••••

Did you do any further experimentation? I'm happy to merge it with a pull request.

[joanbm]

@dkosovic Sorry for the silence, as I got busy with other stuff. I didn't experiment further with other frontends, but the patch is complete and I've been using my patch myself without any problem so far (from nm-connection-editor and I also tested with nmcli while developing).

If I remember correctly, the only "overhanging" problem is that once you create a connection where the PSK is stored as a secret such as from nm-conection-editor after applying the patch, and then try to edit it again from another frontend such as plasma-nm, as it will not be able to read the PSK since it doesn't understand that the PSK is stored as a secret now. I don't think there's much of a reasonable way to do much about it though, and it's a pretty obscure case.

I believe just simply connecting from plasma-nm should not be a problem, and the other way around (creating from plasma-nm and then editing from nm-connection-editor) should also work. I'll see if I can do some tests with plasma-nm or the Deepin frontend on the weekend and I can submit a PR if it works as I expect.

[dkosovic]

I completely agree with your assessment of the "overhanging" problem.

Annoyingly plasma-nm strips away any VPN configuration options it doesn't know about at connection time (and not just edit time). There was one time soon after I became the maintainer, I added a "Disable PFS" checkbox (which was previously hard-coded as pfs=no in the generated ipsec config file and decided to implement it exactly the same way as NetworkManager-libreswan does). For plasma-nm users there was no way to disable PFS even if they used nm-connection-editor as it would get stripped, so there was no trivial way to connect to VPN servers not using PFS (apart from switching to strongswan). Anyway I just bring this topic up because with your obscure case, at least users have the ability to add the PSK back by using the plasma-nm editor and they are not stranded like with my disable PFS case.

It has been a while since I last used the Deepin frontend, at the time I didn't know how to contact the developers, so didn't contribute any GUI updates like I did with plasma-nm.

[dkosovic]

I'll be releasing a new NetworkManager-l2tp 1.16.0 release soon which is going to have a significant bit of redundant/backwards-compatibility preprocessor code removed as well as clang-format formatted code (i.e. lots of white-spaces changes amongst others) that would make merging of your commits difficult. So I have merged now while it is still simple to do so. I also figured having a new release that needs NetworkManager >= 1.16 is a good opportunely to introduce anything that is significant to the GUI.

[joanbm]

Thanks, glad a good opportunity came for this to be merged anyway despite the fact I got into other stuff and left it aside.