| Message ID | 20200219112153.13013-1-arne@rfc2549.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [Openvpn-devel,v2] Warn about insecure ciphers also in init_key_type | expand |
On 19-02-2020 12:21, Arne Schwabe wrote: > With modern Clients and server initialising the crypto cipher later > and not when reading in the config, most users never the warning when > having selected BF-CBC in the configuration. > > This patch adds the logic to print out warning to init_key_type. > > Main reason for this patch is a personal experience with someone who was > strictly against putting 'cipher' into a config file because he did not > like hardcoding a cipher and "OpenVPN will do AES-GCM anyway" and thinks > that it is better to not have it in configuration even after told by me > that 15 year defaults might not be good anymore. > > Patch V2: rebase on master, fix minor style issues > > Signed-off-by: Arne Schwabe <arne@rfc2549.org> > --- > src/openvpn/crypto.c | 26 ++++++++++++++++++-------- > 1 file changed, 18 insertions(+), 8 deletions(-) > > diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c > index 65e789ed..453cb20a 100644 > --- a/src/openvpn/crypto.c > +++ b/src/openvpn/crypto.c > @@ -736,6 +736,17 @@ crypto_max_overhead(void) > +max_int(OPENVPN_MAX_HMAC_SIZE, OPENVPN_AEAD_TAG_LENGTH); > } > > +static void warn_insecure_key_type(const char* ciphername, const cipher_kt_t *cipher) > +{ > + if (cipher_kt_insecure(cipher)) > + { > + msg(M_WARN, "WARNING: INSECURE cipher (%s) with block size less than 128" > + " bit (%d bit). This allows attacks like SWEET32. Mitigate by " > + "using a --cipher with a larger block size (e.g. AES-256-CBC).", > + ciphername, cipher_kt_block_size(cipher)*8); > + } > +} > + > /* > * Build a struct key_type. > */ > @@ -779,6 +790,10 @@ init_key_type(struct key_type *kt, const char *ciphername, > { > msg(M_FATAL, "Cipher '%s' not allowed: block size too big.", ciphername); > } > + if (warn) > + { > + warn_insecure_key_type(ciphername, kt->cipher); > + } > } > else > { > @@ -831,9 +846,10 @@ init_key_ctx(struct key_ctx *ctx, const struct key *key, > cipher_ctx_init(ctx->cipher, key->cipher, kt->cipher_length, > kt->cipher, enc); > > + const char* ciphername = translate_cipher_name_to_openvpn(cipher_kt_name(kt->cipher)); > msg(D_HANDSHAKE, "%s: Cipher '%s' initialized with %d bit key", > prefix, > - translate_cipher_name_to_openvpn(cipher_kt_name(kt->cipher)), > + ciphername, > kt->cipher_length *8); > > dmsg(D_SHOW_KEYS, "%s: CIPHER KEY: %s", prefix, > @@ -841,13 +857,7 @@ init_key_ctx(struct key_ctx *ctx, const struct key *key, > dmsg(D_CRYPTO_DEBUG, "%s: CIPHER block_size=%d iv_size=%d", > prefix, cipher_kt_block_size(kt->cipher), > cipher_kt_iv_size(kt->cipher)); > - if (cipher_kt_insecure(kt->cipher)) > - { > - msg(M_WARN, "WARNING: INSECURE cipher with block size less than 128" > - " bit (%d bit). This allows attacks like SWEET32. Mitigate by " > - "using a --cipher with a larger block size (e.g. AES-256-CBC).", > - cipher_kt_block_size(kt->cipher)*8); > - } > + warn_insecure_key_type(ciphername, kt->cipher); > } > if (kt->digest && kt->hmac_length > 0) > { > Tested, works as advertised. Acked-by: Steffan Karger <steffan.karger@foxcrypto.com> -Steffan
diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c index 65e789ed..453cb20a 100644 --- a/src/openvpn/crypto.c +++ b/src/openvpn/crypto.c @@ -736,6 +736,17 @@ crypto_max_overhead(void) +max_int(OPENVPN_MAX_HMAC_SIZE, OPENVPN_AEAD_TAG_LENGTH); } +static void warn_insecure_key_type(const char* ciphername, const cipher_kt_t *cipher) +{ + if (cipher_kt_insecure(cipher)) + { + msg(M_WARN, "WARNING: INSECURE cipher (%s) with block size less than 128" + " bit (%d bit). This allows attacks like SWEET32. Mitigate by " + "using a --cipher with a larger block size (e.g. AES-256-CBC).", + ciphername, cipher_kt_block_size(cipher)*8); + } +} + /* * Build a struct key_type. */ @@ -779,6 +790,10 @@ init_key_type(struct key_type *kt, const char *ciphername, { msg(M_FATAL, "Cipher '%s' not allowed: block size too big.", ciphername); } + if (warn) + { + warn_insecure_key_type(ciphername, kt->cipher); + } } else { @@ -831,9 +846,10 @@ init_key_ctx(struct key_ctx *ctx, const struct key *key, cipher_ctx_init(ctx->cipher, key->cipher, kt->cipher_length, kt->cipher, enc); + const char* ciphername = translate_cipher_name_to_openvpn(cipher_kt_name(kt->cipher)); msg(D_HANDSHAKE, "%s: Cipher '%s' initialized with %d bit key", prefix, - translate_cipher_name_to_openvpn(cipher_kt_name(kt->cipher)), + ciphername, kt->cipher_length *8); dmsg(D_SHOW_KEYS, "%s: CIPHER KEY: %s", prefix, @@ -841,13 +857,7 @@ init_key_ctx(struct key_ctx *ctx, const struct key *key, dmsg(D_CRYPTO_DEBUG, "%s: CIPHER block_size=%d iv_size=%d", prefix, cipher_kt_block_size(kt->cipher), cipher_kt_iv_size(kt->cipher)); - if (cipher_kt_insecure(kt->cipher)) - { - msg(M_WARN, "WARNING: INSECURE cipher with block size less than 128" - " bit (%d bit). This allows attacks like SWEET32. Mitigate by " - "using a --cipher with a larger block size (e.g. AES-256-CBC).", - cipher_kt_block_size(kt->cipher)*8); - } + warn_insecure_key_type(ciphername, kt->cipher); } if (kt->digest && kt->hmac_length > 0) {
With modern Clients and server initialising the crypto cipher later and not when reading in the config, most users never the warning when having selected BF-CBC in the configuration. This patch adds the logic to print out warning to init_key_type. Main reason for this patch is a personal experience with someone who was strictly against putting 'cipher' into a config file because he did not like hardcoding a cipher and "OpenVPN will do AES-GCM anyway" and thinks that it is better to not have it in configuration even after told by me that 15 year defaults might not be good anymore. Patch V2: rebase on master, fix minor style issues Signed-off-by: Arne Schwabe <arne@rfc2549.org> --- src/openvpn/crypto.c | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-)