(In reply to comment #0)
> OpenVPN, openconnect, openswan, and vpnc all support passing a config file in
> addition to / instead of command-line options, and we should just do that, and
> allow the config to include options that we don't know about. And we should

I disagree with this though, if by this you mean just having the VPN service pass a config file to openswan/vpnc/etc when starting the connection.  There's a number of good reasons it's done the way it's done, mainly:

1) starting/stopping VPNs is a root-level operation (except for openconnect) and NM is opening a root hole to the user, which means we'd better be very careful about the options we pass and how we get those options.  Second, security contexts like selinux and apparmor may not allow the VPN service to access the configuration file the user specifies, so NM would have to copy that somewhere anyway and deal with name conflict management.

2) just passing a config file means we can't store the other stuff NM cares about, which includes secret flags, where those secrets are stored, how they are stored, etc; storing them in some kind of lookaside format would be pretty ugly.

3) tempfile management is icky and it's quite nice not to have to clean up after yourself when the VPN service quits or something crashes; you also don't have to deal with permissions or names or conflicts or whatever.

4) Config files just don't provide a good user experience when you need to change something in the VPN configuration; yes, *often* you get the config from an administrator, but you also often construct it yourself.  Editing a config file is not the experience we want to provide...

Also, we can allow the options to be specified in the configuration, but we don't necessarily need to provide a UI for all of them.

So, some of your comments assume things that don't match what I was thinking, but it may not really matter anyway because of this:

> 1) starting/stopping VPNs is a root-level operation (except for openconnect)
> and NM is opening a root hole to the user, which means we'd better be very
> careful about the options we pass and how we get those options.

This basically kills *any* feature that would let the user set arbitrary unknown-to-NM options, right?

(In reply to comment #1)
> 2) just passing a config file means we can't store the other stuff NM cares
> about
> 3) tempfile management is icky
> 4) Config files just don't provide a good user experience

OK, so I guess there are several parts to my initial proposal:

  A. NM should support VPN options that it doesn't provide UI to configure
     (or alternatively, it should provide at least a basic UI for every
     known option for every VPN).

  B. We should make config-file import/export more visible and more
     recommended (as a way for admins to provide VPN settings to users)

  C. We should actually use native config files internally rather than
     keyfiles

  D. We should provide a config-file-editing UI for VPN config (in addition
     to the current UI).

So let's drop C and D and worry just about A and B.

(Though as a counter-argument to "config files just don't provide a good user experience", note that the config-file-based approach does at least make it easy to see all in one place which properties have been set to non-default values; something that is impossible to figure out from the current UI.)

> 1) starting/stopping VPNs is a root-level operation (except for openconnect)
> and NM is opening a root hole to the user, which means we'd better be very
> careful about the options we pass and how we get those options.

Right. OK, so we still need to parse out each option individually. We just want to be able to parse *more* options than we can now. Ideally, all of them.

I think the big thing that keeps us from accepting more VPN options now (even when people provide patches) is that the properties dialogs are already Lovecraftian horrors, and we don't even want to think about where to add more options to them. The answer may be to just give up on trying to have the VPN config dialogs be pretty, and instead let them be comprehensive.

There are only so many types of data supported by each VPN (hostname, username, password, certificate, boolean, integer in the range X to Y, "anything matching this regex: ...", etc). So given a list of properties and their types (and a little bit of categorizing and "property X requires property Y" information), it wouldn't be too hard to have nm-connection-editor and nmtui autogenerate their UIs. (In nm-connection-editor we could still keep most of the properties in an "Advanced" window like we do now. There would just be more tabs in that window.)

And that information would also make it easy for the plugins to validate imported config files that used arbitrary options, even when some of the options aren't explicitly supported by the plugin. (Eg, if a property has the "certificate" type, then as long as its value is a path to a file that is readable by the user, then the plugin knows that it's safe to pass that property to the VPN binary, even if it doesn't know what that certificate is being used for.)

I really think the solution for this must be broader. I opened bug 732292 for that. Sorry, for the long text, but I feel strongly about this :)

Something to keep in mind while you folks are working out how to solve this problem:

Some of the options in an ovpn file (e.g. ns-cert-type, verify-x509-name) are security-critical.  When NetworkManager's OpenVPN file importer silently ignores them, it is tricking the user into thinking his VPN is working, while secretly stripping away the protections that were carefully chosen by the provider, thereby exposing him to attack.  The most charitable way I can think of to describe this behavior is "irresponsible".

https://openvpn.net/index.php/open-source/documentation/howto.html#mitm

If you insist on tampering with the settings in a VPN config file instead of using it as it was provided, you really ought to warn the user what changes you are making (including ignored options) and call his attention to the fact that those changes might very well have destroyed all of the privacy and security assurances that a VPN is supposed to provide.

I'm sorry if this comment seems abrasive.  Please keep in mind that it is not in any way meant as a personal attack.  It's just that this is a very important issue, particularly to people in parts of the world where exposed conversations can endanger someone's personal safety.

Ignoring unknown options happens here:
https://git.gnome.org/browse/network-manager-openvpn/tree/properties/import-export.c?id=7f17349ed949b8c434c1a91f6f828b13822278ac#n1379

removing the "continue" would cause the import to fail.

Note that openvpn understands a gazillion of options, so that would mean that you easily fail to import settings. As it is, you can create a partial import that might work and modify it according to your needs later.

Warning is a good idea and doable, but requires a significant implementation effort. For example,
- if you use the import code from nm-connection-editor, printing to stderr isn't useful because the user usually doesn't read that.
- if you use the import code from nmcli, you probably still don't want the import function to spam the terminal. Instead the warnings must also be propagated and printed by nmcli.

(In reply to Forest from comment #5)
>
> Some of the options in an ovpn file (e.g. ns-cert-type, verify-x509-name)
> are security-critical.  When NetworkManager's OpenVPN file importer silently
> ignores them, it is tricking the user into thinking his VPN is working,
> while secretly stripping away the protections that were carefully chosen by
> the provider, thereby exposing him to attack.  The most charitable way I can
> think of to describe this behavior is "irresponsible".

The main problem is that the plugin doesn't handle ns-cert-type, not that the import doesn't handle it (yes, a warning would be definitely useful).

Note that import merely takes a ovpn file and create a NM connection out of it. The same result can be achieved manually as well (e.g. via the nm-connection-editor UI). But as the plugin doesn't support the option...

Hi,
i wanted to file a bug to import openconnect to the networkmanager.
I work for the City of Munich and we use K/Ubuntu for our Workstationsystem for the city administration. Perhaps the word "limux" does ring a bell :)

I'll try to explain, why this is important to us.
We use OpenConnect clientside for our RemoteAccess-System; but we use the MachineCertificate for Client-Authentication and pin+token for userlogin. The User shall not have access to the certificate, otherwise the user would copy it to other devices.

At the moment we use a hack where we provide the certificate to openconnect in memory to the process, but this code is not upstream and will probably never be, since it is an ugly hack.

If openconnect would be part of networkmanager, it would enable the possibility that the user can use the networkmanager to login into our Network, but he/she does not need access to the certificate, since the networkmanager can provide that itself.

Please do NOT refrain from taking up plugins into the networkmanager. The Networkmanager is really cool, but without integrations like that we are really handicapped in integrating the desktop into enterprise structure.

Thanks for your time, Dennis

(In reply to Dennis knorr from comment #8).
> The User shall not have access to the certificate, otherwise the user would
> copy it to other devices.

This is what PKCS#11 is for. It allows us to use certs from crypto tokens which don't allow us to copy the private key away; only to use it.

We have use hardware tokens or a TPM and we can also use the p11-kit "remote" option which allows the actual crypto to happen elsewhere... on another machine or in a different account. 

Assuming you have a software key which is accessible to root but not to the user  it should be fairly easy to make that work is a supportable fashion without hacks.

Perhaps mail me offline...

Simple test case, key accessible by root, but it could be any unprivileged user if you prefer it that way. And although I'm typing the root password for my test, SSH can of course be configured to use key auth, and permit only the *one* command (p11-tool remote ...). You could use sudo for the same purpose.

And I'm using softhsm but you could do fairly much anything you like. But still, simple test case for "only root can see the key and user can't"...

$ cat ~/.config/pkcs11/modules/remote.module 
remote:|ssh root@localhost p11-kit remote /usr/lib64/pkcs11/libsofthsm2.so
$ sudo softhsm2-util --init-token --slot 0 --label foo --pin 1234 --so-pin 1234
$ p11tool --login --write 'pkcs11:token=bar' --load-certificate certificate.pem \
  --load-privkey ~/.cert/priv-key.pem --label vpnkey
$ openconnect -c 'pkcs11:token=bar;object=vpnkey' $SERVER

Sorry, the "--label foo" in the softhsm2-util commsnd, and the later "pkcs11:token=bar" should obviously be made to use the *same* name.

bugzilla.gnome.org is being shut down in favor of a GitLab instance. 
We are closing all old bug reports and feature requests in GNOME Bugzilla which have not seen updates for a long time.

If you still use NetworkManager and if you still see this bug / want this feature in a recent and supported version of NetworkManager, then please feel free to report it at https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/

Thank you for creating this report and we are sorry it could not be implemented (workforce and time is unfortunately limited).