On Sun, 2016-01-24 at 23:02 +0100, Matthias Berndt wrote:
Hi, I've been thinking about the code that I recently modified. The handle_blob_item function checks if the blob markers (----BEGIN CERTIFICATE---- etc.) are present and returns false without consuming any lines if they're missing. I fail to see the point, why not just copy everything between the begin and end tags? This is simpler and more consistent, because for non-inline certificates/keys/... this is also not checked, not to mention that pkcs12 blobs (which currently also don't work in nm-openvpn) don't have those markers at all. It also makes error detection harder. If you import an OpenVPN configuration with broken blob markers, nm-openvpn will silently ignore the blobs and proceed with the import, leaving people unable to figure out what went wrong. Otoh OpenVPN *will* tell you want went wrong if you try to use a certificate with broken blob markers: "Cannot load CA certificate file /home/mberndt/.cert/client-ca.pem (no entries were read) (OpenSSL)".
Sounds good. Patches welcome!!
Oh, and there's another thing: afaics, if you don't use inline blobs but files for the certificate/key/ca, nm-openvpn will not copy them somewhere safe (~/.cert, say) – bad idea. Jane User will plug in her USB stick, import her OpenVPN configuration from it and then start cursing the next day when she can't connect any longer after unplugging it.
Certificate files anyway have many problems. This isn't restricted to (open-) VPN. Your USB-stick example (https://bugzilla.gnome.org/show_bug.cgi?id=6898 18). Or for example: the VPN service runs in a different context then the user-provided file, so possibly SELinux doesn't allow accessing the file. On the other hand, copying the file somewhere also has problems. For example it's hard to know that a certificate is no longer used and effectively it doesn't get garbage collected. And it might be surprising to the user that the file gets copied in the first place. Lubomir did a patch for nm-applet, where the UI would always store the certificates under ~/.certs, but that didn't get merged. Seems long term the solution will be https://wiki.gnome.org/Projects/Ne tworkManager/PKCS11 Thomas
Attachment:
signature.asc
Description: This is a digitally signed message part