Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Settings validation for LZO looks incorrect #500

Open
stephane-rochoy-stormshield opened this issue Feb 16, 2024 · 5 comments
Open

Settings validation for LZO looks incorrect #500

stephane-rochoy-stormshield opened this issue Feb 16, 2024 · 5 comments
Assignees
Labels
bug patch-submitted There is a patch on the list waiting for merge

Comments

@stephane-rochoy-stormshield

openvpn/src/openvpn/comp.c

Lines 189 to 204 in 5447571

#ifndef ENABLE_LZ4
if (info->alg == COMP_ALGV2_LZ4 || info->alg == COMP_ALG_LZ4)
{
msg(msglevel, "OpenVPN is compiled without LZ4 support. Requested "
"compression cannot be enabled.");
return false;
}
#endif
#ifndef ENABLE_LZO
if (info->alg == COMP_ALG_LZO || info->alg == COMP_ALG_LZ4)
{
msg(msglevel, "OpenVPN is compiled without LZO support. Requested "
"compression cannot be enabled.");
return false;
}
#endif

LZ4 is wrongly mixed into the condition at line 198.

@flichtenheld
Copy link
Member

Indeed. Introduced by e950ca1 in 2.6.2.
Probably not a lot of people are hitting this since OpenVPN is usually compiled with LZO support and hopefully compression usage is going down due to security recommendations. But should definitely be fixed.

@flichtenheld flichtenheld self-assigned this Feb 16, 2024
@flichtenheld
Copy link
Member

@stephane-rochoy-stormshield
Copy link
Author

Was hit after moving from LZO to LZ4 to get better performances. But if I understand correctly, the recommendation is to fully disable compression?

@flichtenheld
Copy link
Member

Was hit after moving from LZO to LZ4 to get better performances. But if I understand correctly, the recommendation is to fully disable compression?

Yes, disabling compression is generally advised. New DCO kernel drivers for OpenVPN don't even support it. Quoting the man page for --compress:

Security Considerations

Compression and encryption is a tricky combination. If an attacker knows or is able to control (parts of) the plain-text of packets that contain secrets, the attacker might be able to extract the secret if compression is enabled. See e.g. the CRIME and BREACH attacks on TLS and VORACLE on VPNs which also leverage to break encryption. If you are not entirely sure that the above does not apply to your traffic, you are advised to not enable compression.

@stephane-rochoy-stormshield
Copy link
Author

Ok, got it. Thanks for your time!

@flichtenheld flichtenheld added the patch-submitted There is a patch on the list waiting for merge label Mar 6, 2024
cron2 pushed a commit that referenced this issue Mar 8, 2024
Probably introduced by copy & paste since there is no
COMP_ALGV2_LZO.

Github: #500
Change-Id: Id6b038c1c0095b2f22033e9dc7090e2507a373ab
Signed-off-by: Frank Lichtenheld <[email protected]>
Acked-by: Arne Schwabe <[email protected]>
Message-Id: <[email protected]>
URL: https://www.mail-archive.com/[email protected]/msg28251.html
Signed-off-by: Gert Doering <[email protected]>
(cherry picked from commit 4076d24)
cron2 pushed a commit that referenced this issue Mar 8, 2024
Probably introduced by copy & paste since there is no
COMP_ALGV2_LZO.

Github: #500
Change-Id: Id6b038c1c0095b2f22033e9dc7090e2507a373ab
Signed-off-by: Frank Lichtenheld <[email protected]>
Acked-by: Arne Schwabe <[email protected]>
Message-Id: <[email protected]>
URL: https://www.mail-archive.com/[email protected]/msg28251.html
Signed-off-by: Gert Doering <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug patch-submitted There is a patch on the list waiting for merge
Projects
None yet
Development

No branches or pull requests

2 participants