man: add security considerations to --compress section
PATCH 1/3 man: add security considerations to --compress section
As Ahamed Nafeez reported to the OpenVPN security team, we did not sufficiently inform our users about the risks of combining encryption and compression. This patch adds a "Security Considerations" paragraph to the --compress section of the manpage to point the risks out to our users.
--- doc/openvpn.8 | 10 ++++++++++ 1 file changed, 10 insertions(+)
doc/openvpn.8 +10 -0
@@ -2516,6 +2516,16 @@ If the 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. + +.B Security Considerations + +Compression and encryption is a tricky combination. If an attacker knows or is +able to control (parts of) the plaintext 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 which also leverage compression 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. + .\"********************************************************* .TP .B \-\-comp\-lzo [mode]
Signature · 1 line
2.7.4
PATCH 2/3 Reject unadvertised compression algorithms
A server should not push us compression algorithms we didn't specify. If the server does so anyway, reject the compression algorithm.
This will result in a warning being printed, and a non-working connection to be set up. This is currently our way to "handle push/pull errors", which should probably be improved. But I didn't want refactor that in this patch.
--- doc/openvpn.8 | 16 +++++++--- src/openvpn/options.c | 85 ++++++++++++++++++++++++++++++++------------------- 2 files changed, 65 insertions(+), 36 deletions(-)
doc/openvpn.8 +11 -5
@@ -2505,11 +2505,12 @@ Enable a compression algorithm. The .B 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 v2.4, use "lzo" -(which is identical to the older option "\-\-comp\-lzo yes"). +parameter may be empty, "stub", "stub-v2", "lzo", "lz4", or "lz4-v2". + +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 v2.4, use "lzo" (which is identical to the older option +"\-\-comp\-lzo yes"). If the .B algorithm
@@ -2517,6 +2518,11 @@ 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. +If the +.B algorithm +parameter is "stub" or "stub-v2", compression framing is enabled, but no +compression will be used (even if pushed by the server). + .B Security Considerations Compression and encryption is a tricky combination. If an attacker knows or is
src/openvpn/options.c +54 -31
104 lines not on this page · raw message
Signature · 1 line
2.7.4
Re: [Openvpn-devel] [PATCH 2/3] Reject unadvertised compression algorithms
Hi,
Quoted 2 lines · Steffan Karger
A server should not push us compression algorithms we didn't specify. If the server does so anyway, reject the compression algorithm.
I can see why you do this, but if I understand this right, this will break lots of currently-working OpenVPN setups - where we use the IV_ variables to parse out "what can the client do?" and send matching "compress <foo>" PUSH_REPLYs.
So, feature-NAK.
Adding warnings (1/3) and a warning-override (3/3) is good measure, but interfering with server-to-client pushing of options needs more consideration.
gert
Signature · 6 lines
"If was one thing all people took for granted, was conviction that if you
feed honest figures into a computer, honest figures come out. Never doubted
it myself till I met a computer with a sense of humor."
Robert A. Heinlein, The Moon is a Harsh Mistress
Gert Doering - Munich, Germany [email protected]...Re: [Openvpn-devel] [PATCH 2/3] Reject unadvertised compression algorithms
Hi
Quoted 10 lines · Gert Doering
Quoted 2 lines · Steffan Karger
A server should not push us compression algorithms we didn't specify. If the server does so anyway, reject the compression algorithm.
I can see why you do this, but if I understand this right, this will break lots of currently-working OpenVPN setups - where we use the
IV_variables to parse out "what can the client do?" and send matching "compress <foo>"PUSH_REPLYs.So, feature-NAK.
As clarified on IRC, I misread the text. So I withdraw the feature-NAK and need to read this more closely and better think through the problem space.
gert
Signature · 6 lines
"If was one thing all people took for granted, was conviction that if you
feed honest figures into a computer, honest figures come out. Never doubted
it myself till I met a computer with a sense of humor."
Robert A. Heinlein, The Moon is a Harsh Mistress
Gert Doering - Munich, Germany [email protected]...PATCH 3/3 Print a --verb 1 warning when a connection uses compression
In reply to Steffan KargerCan be suppressed by adding a "nowarn" flag to the compress options, for those that are really sure that compression is fine for their use case.
---
This patch is also meant to discuss how far we want to go in warning users about using compression. I think this approach is reasonable, but I'm not sure everyone agrees.
doc/openvpn.8 | 11 +++++++++-- src/openvpn/comp.c | 14 ++++++++++++++ src/openvpn/comp.h | 1 + 3 files changed, 24 insertions(+), 2 deletions(-)
doc/openvpn.8 +9 -2
@@ -2500,12 +2500,13 @@ consecutive messages in the same category. This is useful to limit repetitive logging of similar message types. .\"********************************************************* .TP -.B \-\-compress [algorithm] +.B \-\-compress [algorithm] ["nowarn"] Enable a compression algorithm. The .B algorithm -parameter may be empty, "stub", "stub-v2", "lzo", "lz4", or "lz4-v2". +parameter may be empty, "any", "stub", "stub-v2", "lzo", "lz4", or "lz4-v2". +If left empty, OpenVPN defaults to "any". LZO and LZ4 are different compression algorithms, with LZ4 generally offering the best performance with least CPU usage. For backwards compatibility with
@@ -2532,6 +2533,12 @@ e.g. the CRIME and BREACH attacks on TLS which also leverage compression 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. +If you have carefully considered the above, and are sure that using compression +is safe for your use case, you can add +.B "nowarn" +as the second parameter to suppress warnings about the risk of enabling +compression. + .\"********************************************************* .TP .B \-\-comp\-lzo [mode]
src/openvpn/comp.c +14 -0
@@ -40,6 +40,20 @@struct compress_context *comp_init (const struct compress_options *opt) { +switch (opt->alg) + { +case COMP_ALG_UNDEF : +case COMP_ALG_STUB : +case COMP_ALGV2_UNCOMPRESSED : +break ; +default : +if (!(opt->flags &COMP_F_NOWARN )) + { +msg (M_INFO ,"WARNING: Compression enabled, might be insure. " +"See --compress in the man page." ); + } + } +struct compress_context *compctx =NULL ;switch (opt->alg) {
src/openvpn/comp.h +1 -0
@@ -56,6 +56,7 @@#define COMP_F_ASYM (1<<1)/* only downlink is compressed, not uplink */ #define COMP_F_SWAP (1<<2)/* initial command byte is swapped with last byte in buffer to preserve payload alignment */ #define COMP_F_ADVERTISE_STUBS_ONLY (1<<3)/* tell server that we only support compression stubs */ +#define COMP_F_NOWARN (1<<4)/* Suppress warning about insure compression */ /*
Signature · 1 line
2.7.4
Re: [Openvpn-devel] [PATCH 3/3] Print a --verb 1 warning when a connection uses compression
Hi,
Quoted 55 lines · Steffan Karger
Can be suppressed by adding a "nowarn" flag to the compress options, for those that are really sure that compression is fine for their use case.
Signed-off-by Steffan Karger <[email protected]...>---
This patch is also meant to discuss how far we want to go in warning users about using compression. I think this approach is reasonable, but I'm not sure everyone agrees.doc/openvpn.8 | 11 +++++++++-- src/openvpn/comp.c | 14 ++++++++++++++ src/openvpn/comp.h | 1 + 3 files changed, 24 insertions(+), 2 deletions(-)doc/openvpn.8 +9
-2@@ -2500,12 +2500,13 @@ consecutive messages in the same category. This is useful to limit repetitive logging of similar message types. .\"********************************************************* .TP -.B \-\-compress [algorithm] +.B \-\-compress [algorithm] ["nowarn"] Enable a compression algorithm. The .B algorithm -parameter may be empty, "stub", "stub-v2", "lzo", "lz4", or "lz4-v2". +parameter may be empty, "any", "stub", "stub-v2", "lzo", "lz4", or "lz4-v2". +If left empty, OpenVPN defaults to "any". LZO and LZ4 are different compression algorithms, with LZ4 generally offering the best performance with least CPU usage. For backwards compatibility with@@ -2532,6 +2533,12 @@ e.g. the CRIME and BREACH attacks on TLS which also leverage compression 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. +If you have carefully considered the above, and are sure that using compression +is safe for your use case, you can add +.B "nowarn" +as the second parameter to suppress warnings about the risk of enabling +compression. + .\"********************************************************* .TP .B \-\-comp\-lzo [mode]src/openvpn/comp.c +0
-0@@ -40,6 +40,20 @@struct compress_context *comp_init (const struct compress_options *opt) {
src/openvpn/comp.c line 52
Quoted 10 lines
src/openvpn/comp.c +10
-0+switch (opt->alg) + { +case COMP_ALG_UNDEF : +case COMP_ALG_STUB : +case COMP_ALGV2_UNCOMPRESSED : +break ; +default : +if (!(opt->flags &COMP_F_NOWARN )) + { + msg(M_INFO, "WARNING: Compression enabled, might be insure. "
May I suggest to change that to M_INFO|M_WARN so that it gets the warning tag "W" that could be parsed by UI's. OpenVPN Windows GUI we like to highlight (in colour) warnings and errors and this will slip through as a info message.
Ideally we should redefine M_WARN as M_WARN + verb1 or make a M_WARN_GENERIC that prints only in verb 1 and use it everywhere, but that is a different topic.
Selva
Re: [Openvpn-devel] [PATCH 3/3] Print a --verb 1 warning when a connection uses compression
In reply to Steffan KargerAm 03.06.18 um 12:11 schrieb Steffan Karger:
src/openvpn/comp.c line 53
Quoted 2 lines
src/openvpn/comp.c +2
-0+msg (M_INFO ,"WARNING: Compression enabled, might be insure. " +"See --compress in the man page." );
With my client maintainer hat on, this message is too vague. People will ask why compression is insure, because the warning message does not really tell much. I think the warning message should be more verbose something like
WARNING: Compression enabled, Compression has been used in the past to break encryption and for strengthening security, it is recommended to disable it. For more details see --compress in the man page
Arne
Re: [Openvpn-devel] [PATCH 3/3] Print a --verb 1 warning when a connection uses compression
In reply to Steffan KargerQuoted 24 lines · Steffan Karger
Can be suppressed by adding a "nowarn" flag to the compress options, for those that are really sure that compression is fine for their use case.
Signed-off-by Steffan Karger <[email protected]...>---
This patch is also meant to discuss how far we want to go in warning users about using compression. I think this approach is reasonable, but I'm not sure everyone agrees.doc/openvpn.8 | 11 +++++++++-- src/openvpn/comp.c | 14 ++++++++++++++ src/openvpn/comp.h | 1 + 3 files changed, 24 insertions(+), 2 deletions(-)doc/openvpn.8 +0
-0src/openvpn/comp.c +0
-0@@ -40,6 +40,20 @@struct compress_context *comp_init (const struct compress_options *opt) {
src/openvpn/comp.c line 52
Quoted 10 lines
src/openvpn/comp.c +10
-0+switch (opt->alg) + { +case COMP_ALG_UNDEF : +case COMP_ALG_STUB : +case COMP_ALGV2_UNCOMPRESSED : +break ; +default : +if (!(opt->flags &COMP_F_NOWARN )) + { + msg(M_INFO, "WARNING: Compression enabled, might be insure. "
Did you mean /insecure/?
Signature · 4 lines
kind regards, David Sommerseth OpenVPN Inc
This makes sense. Whatever else we do, explaining the *why* parts is helping users make an educated choice.
Your patch has been applied to the master and release/2.4 branch.
commit a59fd1475089eda4c89942d345070bb942180223 (master) commit 6795a5f3d55f658fc1a28eb9f3b11d1217e3329c (release/2.4)
Author: Steffan Karger Date: Sun Jun 3 12:11:56 2018 +0200
man: add security considerations to --compress section
Signed-off-by: Steffan Karger <[email protected]...>
Acked-by: Gert Doering <[email protected]...>
Message-Id: <[email protected]...>
URL: https://www.mail-archive.com/[email protected]/msg16919.html
Signed-off-by: Gert Doering <[email protected]...>--
kind regards,
Gert Doering