Hi,

On Wed, Dec 01, 2021 at 07:07:19PM +0100, Arne Schwabe wrote:
> This allows to use the same configuration multiple platforms/ssl libraries
> and include optional algorithms that are not available on all platforms
> 
> For example "AES-256-GCM:AES-128-GCM:?CHACHA20-POLY1305" can be used to
> emulate the default behaviour of OpenVPN 2.6.

NAK on this, for two small and one big reason...

> diff --git a/Changes.rst b/Changes.rst
> index 7cceffcdb..c1a04deed 100644
> --- a/Changes.rst
> +++ b/Changes.rst
> @@ -58,6 +58,10 @@ OpenSSL 3.0 support
>      (and other deprecated) algorithm by default and the new option 
> ``--providers``
>      allows loading the legacy provider to renable these algorithms.
>  
> +Optional ciphers in ``--data-ciphers``
> +    Ciphers in ``--data-ciphers`` can now be prefixes with a ``?`` to mark
> +    those as optional and only use them if the SSL library supports them.

Typo, "prefixed"

> +
>  Deprecated features
>  -------------------
>  ``inetd`` has been removed
> diff --git a/doc/man-sections/protocol-options.rst 
> b/doc/man-sections/protocol-options.rst
> index c7aa6b0e3..7095b6f4d 100644
> diff --git a/src/openvpn/ssl_ncp.c b/src/openvpn/ssl_ncp.c
> index 022a9dc3b..b0b248aae 100644
> --- a/src/openvpn/ssl_ncp.c
> +++ b/src/openvpn/ssl_ncp.c
> @@ -109,7 +109,18 @@ mutate_ncp_cipher_list(const char *list, struct gc_arena 
> *gc)
>           * (and translate_cipher_name_from_openvpn/
>           * translate_cipher_name_to_openvpn) also normalises the cipher name,
>           * e.g. replacing AeS-128-gCm with AES-128-GCM
> +         *
> +         * ciphers that have ? in front of them are considered optional and
> +         * OpenVPN will only warn if they are not found (and remove them from
> +         * the list)
>           */
> +
> +        bool optional = false;
> +        if (token[0] == '?')
> +        {
> +            token= token + 1;

The whitespace dragon says this should be "token++;"

(These are small, and I could fix that on the fly).


The problem is here:

> @@ -121,8 +132,9 @@ mutate_ncp_cipher_list(const char *list, struct gc_arena 
> *gc)
>          }
>          if (!ktc && strcmp(token, "none") != 0)
>          {
> -            msg(M_WARN, "Unsupported cipher in --data-ciphers: %s", token);
> -            error_found = true;
> +            const char* optstr = optional ? "optional ": "";
> +            msg(M_WARN, "Unsupported %scipher in --data-ciphers: %s", 
> optstr, token);
> +            error_found = !optional;
>          }

This only works if the last cipher in the list is "borken or optional".

If you do

  openvpn --data-ciphers Vollbit:Littlebit:AES-256-CBC:BF-CBC:?nixbit

it will log

2021-12-05 18:09:27 Unsupported cipher in --data-ciphers: Vollbit
2021-12-05 18:09:27 Unsupported cipher in --data-ciphers: Littlebit
2021-12-05 18:09:27 Unsupported optional cipher in --data-ciphers: nixbit

... and go ahead...

Dec  5 18:09:28 gentoo tap-tcp-p2p[26765]: peer info: 
IV_CIPHERS=AES-256-CBC:BF-CBC

without flagging "Vollbit" and "Littlebit" as hard errors.

If the "nixbit" cipher is removed

  openvpn --data-ciphers Vollbit:Littlebit:AES-256-CBC:BF-CBC

will lead to

2021-12-05 18:10:29 Unsupported cipher in --data-ciphers: Vollbit
2021-12-05 18:10:29 Unsupported cipher in --data-ciphers: Littlebit
Options error: --data-ciphers list contains unsupported ciphers or is too long.


Sorry for excercising my special ability of breaking everything :-)

gert

-- 
"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                             g...@greenie.muc.de

Attachment: signature.asc
Description: PGP signature

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

Reply via email to