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 <a...@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.kar...@foxcrypto.com>

-Steffan

Attachment: signature.asc
Description: OpenPGP digital signature

_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Reply via email to