On 26/10/16 17:47, Steffan Karger wrote: > Following the earlier warning about small block ciphers, now limit the > --reneg-bytes value when using a cipher that susceptible to SWEET32-like > attacks. The 64 MB value has been selected with the researchers who > published the SWEET32 paper. > > Note that this will not change a user-set (non-zero) --reneg-bytes value, > to allow a user to override this behaviour if really needed. > > Signed-off-by: Steffan Karger <steffan.kar...@fox-it.com> > --- > src/openvpn/crypto.c | 5 +++-- > src/openvpn/ssl.c | 24 ++++++++++++++++++++++++ > 2 files changed, 27 insertions(+), 2 deletions(-) > > diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c > index 45689f2..b40fc7a 100644 > --- a/src/openvpn/crypto.c > +++ b/src/openvpn/crypto.c > @@ -836,8 +836,9 @@ init_key_ctx (struct key_ctx *ctx, struct key *key, > cipher_kt_iv_size(kt->cipher)); > if (cipher_kt_block_size(kt->cipher) < 128/8) > { > - msg (M_WARN, "WARNING: this cipher's block size is less than 128 bit " > - "(%d bit). Consider using a --cipher with a larger block size.", > + msg (M_WARN, "WARNING: this cipher's block size is less than 128 " > + "bit (%d bit). This allows attacks like SWEET32. Consider " > + "using a --cipher with a larger block size.", > cipher_kt_block_size(kt->cipher)*8); > } > } > diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c > index dfdc200..2c341a2 100644 > --- a/src/openvpn/ssl.c > +++ b/src/openvpn/ssl.c > @@ -283,6 +283,24 @@ tls_get_cipher_name_pair (const char * cipher_name, > size_t len) { > return NULL; > } > > +/** > + * Limit the reneg_bytes value when using a small-block (<128 bytes) cipher. > + * > + * @param cipher The current cipher (may be NULL). > + * @param reneg_bytes Pointer to the current reneg_bytes, updated if > needed. > + * May *not* be NULL. > + */ > +static void > +tls_limit_reneg_bytes (const cipher_kt_t *cipher, int *reneg_bytes) > +{ > + if (*reneg_bytes == 0 && cipher && (cipher_kt_block_size(cipher) < 128/8))
Should we also consider if *reneg_bytes > 64*1024*1024 and enforce maximum? This will not allow users to override the 64MB limit, though. Perhaps we should allow reneg_bytes in to be overridden in v2.3 but enforce it to max 64MB in v2.4 or v2.5. > + { > + msg (M_WARN, "WARNING: cipher with small block size in use, " > + "reducing reneg-bytes to 64MB to mitigate SWEET32 attacks."); Depending on what we do with the >64MB ... this might need to be slightly reworded. If we allow users to override the 64MB default, it should probably say: "... reneg-bytes is set to 64MB ...". I find "reducing" slightly misleading. Also if we decide not to cap at 64MB in this case, we should print another warning if --reneg-bytes is > 64MB. I know this will be somewhat annoying to users (imaginary #openvpn question: "I get so many warnings now, why!? I want to get rid of them"). My experience is that the more warnings we produce, the easier it is to convince users to fix the issue rather to ignore them. > + *reneg_bytes = 64 * 1024 * 1024; > + } > +} > + > /* > * Max number of bytes we will add > * for data structures common to both > @@ -1742,6 +1760,8 @@ tls_session_update_crypto_params(struct tls_session > *session, > msg (D_TLS_ERRORS, "TLS Error: server generate_key_expansion failed"); > goto cleanup; > } > + tls_limit_reneg_bytes (session->opt->key_type.cipher, > + &session->opt->renegotiate_bytes); > ret = true; > cleanup: > CLEAR (*ks->key_src); > @@ -2126,6 +2146,8 @@ key_method_2_write (struct buffer *buf, struct > tls_session *session) > } > > CLEAR (*ks->key_src); > + tls_limit_reneg_bytes (session->opt->key_type.cipher, > + &session->opt->renegotiate_bytes); > } > > return true; > @@ -2354,6 +2376,8 @@ key_method_2_read (struct buffer *buf, struct tls_multi > *multi, struct tls_sessi > } > > CLEAR (*ks->key_src); > + tls_limit_reneg_bytes (session->opt->key_type.cipher, > + &session->opt->renegotiate_bytes); > } Regarding adding tls_limit_reneg_bytes() in key_method_2_{read,write}() ... How often will those functions be called? Is it once per renegotiation? -- kind regards, David Sommerseth
signature.asc
Description: OpenPGP digital signature
------------------------------------------------------------------------------ The Command Line: Reinvented for Modern Developers Did the resurgence of CLI tooling catch you by surprise? Reconnect with the command line and become more productive. Learn the new .NET and ASP.NET CLI. Get your free copy! http://sdm.link/telerik
_______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel