| Message ID | 1528020718-12721-3-git-send-email-steffan@karger.me |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [Openvpn-devel,1/3] man: add security considerations to --compress section | expand |
Hi, On Sun, Jun 3, 2018 at 6:11 AM, Steffan Karger <steffan@karger.me> wrote: > 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 <steffan@karger.me> > --- > 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(-) > > diff --git a/doc/openvpn.8 b/doc/openvpn.8 > index 9e988b3..21a3c42 100644 > --- a/doc/openvpn.8 > +++ b/doc/openvpn.8 > @@ -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] > diff --git a/src/openvpn/comp.c b/src/openvpn/comp.c > index a945913..a34e64a 100644 > --- a/src/openvpn/comp.c > +++ b/src/openvpn/comp.c > @@ -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. " 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 ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Am 03.06.18 um 12:11 schrieb Steffan Karger: > + 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 ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
On 03/06/18 12:11, Steffan Karger wrote: > 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 <steffan@karger.me> > --- > 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(-) > > diff --git a/doc/openvpn.8 b/doc/openvpn.8 > index 9e988b3..21a3c42 100644 > diff --git a/src/openvpn/comp.c b/src/openvpn/comp.c > index a945913..a34e64a 100644 > --- a/src/openvpn/comp.c > +++ b/src/openvpn/comp.c > @@ -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. " Did you mean /insecure/?
diff --git a/doc/openvpn.8 b/doc/openvpn.8 index 9e988b3..21a3c42 100644 --- a/doc/openvpn.8 +++ b/doc/openvpn.8 @@ -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] diff --git a/src/openvpn/comp.c b/src/openvpn/comp.c index a945913..a34e64a 100644 --- a/src/openvpn/comp.c +++ b/src/openvpn/comp.c @@ -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) { diff --git a/src/openvpn/comp.h b/src/openvpn/comp.h index 0dadd1e..0fa9b10 100644 --- a/src/openvpn/comp.h +++ b/src/openvpn/comp.h @@ -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 */ /*
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 <steffan@karger.me> --- 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(-)