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 777768 - [RFE] add "compress" option (lz4)
[RFE] add "compress" option (lz4)
Status: RESOLVED OBSOLETE
Product: NetworkManager
Classification: Platform
Component: VPN: openvpn
git master
Other Linux
: Normal major
: ---
Assigned To: NetworkManager maintainer(s)
NetworkManager maintainer(s)
Depends on:
Blocks: nm-review nm-openvpn-options
 
 
Reported: 2017-01-25 22:47 UTC by trexs
Modified: 2020-11-12 14:29 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch: Add support for tls-crypt (4.48 KB, patch)
2017-01-29 12:37 UTC, Pau Espin Pedrol
none Details | Review
Introduce partial support for import of "compress lzo" as alias for "comp-lzo yes". (3.60 KB, patch)
2018-04-24 10:14 UTC, mads
none Details | Review

Description trexs 2017-01-25 22:47:44 UTC
Openvpn 2.4 introduced two new features : 

---
compress [algorithm]
    Enable a compression algorithm.

    The algorithm parameter may be "lzo", "lz4", or empty. LZO and LZ4 are different compression algorithms, with LZ4 generally offering the best performance with least CPU usage. For backwards compatibility with OpenVPN versions before 2.4, use "lzo" (which is identical to the older option "--comp-lzo yes").

    If the algorithm parameter is empty, compression will be turned off, but the packet framing for compression will still be enabled, allowing a different setting to be pushed later.
---

---
tls-crypt keyfile

    Encrypt and authenticate all control channel packets with the key from keyfile. (See --tls-auth for more background.)

    Encrypting (and authenticating) control channel packets:

        •
            provides more privacy by hiding the certificate used for the TLS connection, 
        •
            makes it harder to identify OpenVPN traffic as such, 
        •
            provides "poor-man's" post-quantum security, against attackers who will never know the pre-shared key (i.e. no forward secrecy). 
---

Note that tls-crypt and tls-auth cannot be used together.
Comment 1 Pau Espin Pedrol 2017-01-29 12:36:16 UTC
Hi,

Yesterday I run into an issue trying to set up a VPN conn which was using tls-crypt. I created a patch and I'm now able to use it.

I sent the patch to the NM mailing-list but it seems it needs to be accepted by the moderator as I'm not subscribed to to it.

I attach the patch here as it solves one of the 2 required options to be implemented for the ticket.
Comment 2 Pau Espin Pedrol 2017-01-29 12:37:11 UTC
Created attachment 344482 [details] [review]
patch: Add support for tls-crypt
Comment 3 Thomas Haller 2017-01-29 14:41:10 UTC
(In reply to Pau Espin Pedrol from comment #2)
> Created attachment 344482 [details] [review] [review]
> patch: Add support for tls-crypt

the patch stores the tls-crypt path as NM_OPENVPN_KEY_TA, which is also used by tls-auth.

It aims to differenciate, based on the presence of NM_OPENVPN_KEY_TA_DIR.


} else if (NM_IN_STRSET (params[0], NMV_OVPN_TAG_TLS_AUTH, 
                                    NMV_OVPN_TAG_TLS_CRYPT)) {
»···setting_vpn_add_data_item_path (s_vpn, NM_OPENVPN_KEY_TA, file);
»···if (s_direction)
»···»···setting_vpn_add_data_item (s_vpn, NM_OPENVPN_KEY_TA_DIR, s_direction);

this will encode

--tls-auth PATH
--tls-crypt PATH

the same way.


I think, there should instead be a new "NM_OPENVPN_KEY_TLS_CRYPT" key.



Also, tls-auth and tls-crypt options conflict. Similarly, import should reject such connections
Maybe src/nm-openvpn-service.c should also reject such connections, or silently proceed and only do tls-crypt.
Comment 4 Pau Espin Pedrol 2017-01-30 10:38:39 UTC
Hi,

I based my patch on the idea that NM_OPENVPN_KEY_TA_DIR must be set in --tls-auth, but it seems it can actually be omitted, then obviously my patch should not work fine in that scenario.

From https://community.openvpn.net/openvpn/wiki/Hardening :

"And reference it in the configs as such. The 0/1 value is arbitrary and must be the opposite between peers (or omitted entirely.)"
Comment 5 Beniamino Galvani 2017-05-12 15:44:28 UTC
I pushed your commit to branch:

https://git.gnome.org/browse/network-manager-openvpn/log/?h=bg/options

and added two more commits for review.
Comment 6 Thomas Haller 2017-05-12 16:14:38 UTC
pushed a fixup. Branch lgtm.
Comment 7 Beniamino Galvani 2017-05-12 20:57:31 UTC
tls-crypt support merged:

https://git.gnome.org/browse/network-manager-openvpn/commit/?id=d90cf9105e33fd4f646adf4f1ef8b9f95afc737b

'compress' still to do.
Comment 8 mads 2018-04-20 00:17:47 UTC
I added partial support for "compress lzo" - good enough to import my .ovpn .

See https://paste.fedoraproject.org/paste/gdBXcXRuJiQt9jOv0jZAyQ
Comment 9 Thomas Haller 2018-04-24 07:34:15 UTC
(In reply to mads from comment #8)
> I added partial support for "compress lzo" - good enough to import my .ovpn .
> 
> See https://paste.fedoraproject.org/paste/gdBXcXRuJiQt9jOv0jZAyQ

would you be willing to cleanup the patch and attach it here? Note that fpaste expires rather soon.



The manual page says: "For backwards compatibility with OpenVPN versions before v2.4, use "lzo" (which is identical to the older option "--comp-lzo yes").". Hence, the patch is wrong in this regard, because it sets "adaptive".

Also, the handling of --comp-lzo is a mess. Note that effectively there are 4 modes, it must be carefully checked how --compress relates to these. https://git.gnome.org/browse/network-manager-openvpn/tree/src/nm-openvpn-service.c?id=26773ab2e94a025a24146c196eb1dad4bab13996#n1503



Currently the patch does 
+			setting_vpn_add_data_item (s_vpn, NM_OPENVPN_KEY_COMP_LZO, "adaptive");
so, when importing a "--compress lzo" setting, it will always result in "--comp-lzo adaptive". That's not right.
There are two possibilities:
(1) The profile clearly distinuishes between whether to use "--comp-lzo" and "--compress". That can be either done, by introducing new values to NM_OPENVPN_KEY_COMP_LZO, like "compress-lzo", or by introducing a new key NM_OPENVPN_KEY_COMRPRESS.
(2) openvpn probably will eventually drop the the depreacted option. That sucks, because it will make all exiting connections unusable. Maybe the plugin should try to be helpful, and automatically choose the preferred option. Like it's done for --tls-remote: https://git.gnome.org/browse/network-manager-openvpn/tree/src/nm-openvpn-service.c?id=26773ab2e94a025a24146c196eb1dad4bab13996#n1681

Probably, we should do (1) *and* (2).
Comment 10 mads 2018-04-24 09:56:58 UTC
I did not see the option for attaching files here. What did I miss? fpaste was just an easy way to try to make a drive-by contribution ;-)

Agreed, I realize "yes" would be more correct than "adaptive". (It seems odd that adaptive *only* can be set with the deprecated --comp-lzo method. I guess adaptive is deprecated too.)

Agreed, compress has other options that also could be supported. But I guess that can come later.

Agreed, it would be nice to have full support for the compress option and be ready for the day comp-lzo is removed. That seems like a bigger change. For now, I think this patch (when changed to "yes") provide a "correct" downgrade from the otherwise unsupported "compress" option to the supported "comp-lzo" option. It trades some tech debt and increase short term revenue ;-)
Comment 11 mads 2018-04-24 10:14:49 UTC
Created attachment 371317 [details] [review]
Introduce partial support for import of "compress lzo" as alias for "comp-lzo yes".
Comment 12 mads 2018-04-24 10:16:26 UTC
(Found the attachment option. I'm sure it wasn't there the other day. Spam protection?)
Comment 13 André Klapper 2020-11-12 14:29:29 UTC
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).