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 760746 - Import openvpn config
Import openvpn config
Status: RESOLVED FIXED
Product: NetworkManager
Classification: Platform
Component: nm-applet
unspecified
Other Linux
: Normal major
: ---
Assigned To: NetworkManager maintainer(s)
NetworkManager maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2016-01-17 16:55 UTC by ktatar156
Modified: 2016-01-20 09:57 UTC
See Also:
GNOME target: ---
GNOME version: ---


Description ktatar156 2016-01-17 16:55:42 UTC 
Hello,
I'm on Arch Linux, Gnome 3, stable channel.
step to reproduce bug:
download some .ovpn file from http://www.vpngate.net/en/ and add it by network part of Gnome. Expected result (like in KDE) - everything is imported, all keys/certificates, credential to login, etc. and we need only to click 'connect'. In Gnome it imports some basic information from file, but in example no certificates - we need to cut them manually from .ovpn file to some other files and add them manually in gnome vpn network options part.
Comment 1 Beniamino Galvani 2016-01-17 21:16:31 UTC 
Are you using nm-connection-editor? If so, what version of
NetworkManager-openvpn gnome plugin do you have?

Importing inline certificates is supported by the plugin since 1.0.8;
however the plugin fails with configuration file from
http://www.vpngate.net/en/ since it's quite picky about the file
syntax.

In particular, the plugin should be modified to deal with the
following:

 - CRLF line terminators
 - new line between "-----END CERTIFICATE-----" and tag end
 - "BEGIN RSA PRIVATE KEY" instead of "BEGIN PRIVATE KEY"

I'll post a patch.
Comment 2 Beniamino Galvani 2016-01-18 13:00:13 UTC 
Pushed branch bg/inline-cert-fixes-bgo760746.
Comment 3 Thomas Haller 2016-01-18 14:24:03 UTC 
(In reply to Beniamino Galvani from comment #2)
> Pushed branch bg/inline-cert-fixes-bgo760746.

This removes empty lines ~before~ do_import() without having any context about what these lines mean. Note that do_import already skips empty lines, only handle_blob_item() does not. Better handle the parsing exception closer to handle_blob_item() where you have the context about what a particular line means.

For the same reason, I'd move

  if (!g_utf8_validate (contents, -1, NULL))
     ...
  lines = g_strsplit_set (contents, "\r\n", 0);

inside do_import() and pass the entire blob to do_import(). Understanding line-breaks (and splitting lines) should be done by the parsing code in one place.


patch 2 lgtm.
Comment 4 Beniamino Galvani 2016-01-18 22:51:35 UTC 
(In reply to Thomas Haller from comment #3)
> (In reply to Beniamino Galvani from comment #2)
> > Pushed branch bg/inline-cert-fixes-bgo760746.
> 
> This removes empty lines ~before~ do_import() without having any context
> about what these lines mean. Note that do_import already skips empty lines,
> only handle_blob_item() does not. Better handle the parsing exception closer
> to handle_blob_item() where you have the context about what a particular
> line means.

Fair enough, changed.

> For the same reason, I'd move
> 
>   if (!g_utf8_validate (contents, -1, NULL))
>      ...
>   lines = g_strsplit_set (contents, "\r\n", 0);
> 
> inside do_import() and pass the entire blob to do_import(). Understanding
> line-breaks (and splitting lines) should be done by the parsing code in one
> place.

Are you suggesting only to move those lines in do_import(), keeping the parsing as it is now, without real understanding of different line endings (because it just splits at \r and \n so in case of \r\n an empty line is created)? Or to add specific understanding of different terminators?

Repushed branch using the first approach.
Comment 5 Thomas Haller 2016-01-20 08:54:07 UTC 
v2 looks exactly how I thought it should be.

lgtm.