| Message ID | 20200907161849.20508-1-arne@rfc2549.org |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [Openvpn-devel] Ignore --cipher for cipher negotiation in server client mode | expand |
On 07/09/2020 18:18, Arne Schwabe wrote: > OpenVPN will ignore --cipher in lieu of the replacement data-ciphers > for cipher negioation. > > Signed-off-by: Arne Schwabe <arne@rfc2549.org> > --- > doc/man-sections/protocol-options.rst | 6 ++++-- > src/openvpn/options.c | 26 ++++---------------------- > 2 files changed, 8 insertions(+), 24 deletions(-) I'm not at all convinced about this one. Because this essentially touches the same arguments we had a discussion about on --udp-mtu. Removing an outdated alias was too much hassle for no gain, despite being an alias not been seen in use for ages (it was deprecated before the v1.5 release, and no configs or bug reports turns up in various google/bing/yahoo/ddg searches using --udp-mtu). Dropping this outdated option would with high probability *not* break any configuration in the wild. However, one of the argument raised for keeping it as is was that it *could* break a configuration in the wild. The behaviour between --udp-mtu and --link-mtu are identical. This change on --ciphers adds this warning if --cipher is used in 2.6+: > +········msg(M_WARN,·"Note:·--cipher·set·to·'%s'·but·missing·in" > +············"·--data-ciphers·(%s).·OpenVPN·2.6+·ignores·--cipher·for·" > +············"cipher·negiotiation.", Ignoring --cipher in a future release will have quite a higher probability of breaking existing configurations. Now, this is set in context of --data-ciphers, which is very different code wise. But the code for --ciphers is essentially the same as --data-ciphers-fallback. I am therefore of the opinion, based on the prior --udp-mtu discussion, that --ciphers should be an alias to --data-ciphers-fallback. In addition, since adding a warning about using the deprecated --udp-mtu option and put up a plan for removing it was also considered too much, I don't see why that argument would be much different with --ciphers.
> > Ignoring --cipher in a future release will have quite a higher > probability of breaking existing configurations. Now, this is set in > context of --data-ciphers, which is very different code wise. But the > code for --ciphers is essentially the same as --data-ciphers-fallback. It is not the same. After this change the following will happen: --cipher sets *ONLY* the OCC cipher that we announce during the handshake and give a cipher that it is being ignored. --data-cipher-fallback sets the OCC cipher that we announcer *AND* determines what cipher to use if and only the other side does not announce any cipher. > I am therefore of the opinion, based on the prior --udp-mtu discussion, > that --ciphers should be an alias to --data-ciphers-fallback. > > In addition, since adding a warning about using the deprecated --udp-mtu > option and put up a plan for removing it was also considered too much, I > don't see why that argument would be much different with --ciphers. I think again the difference here is that we want to change what the cipher directives and I think we can agree on: - data-ciphers-fallback behaviour that is only useful for 2.3 and earlier clients with --enable-small does not need to be turned on by default anymore in 2.6. It is already turned off by default in 2.5 if you have no '--cipher' in your config. The --data-ciphers-fallback option is an option that allows you to workaround these situations where the fallback is needed. We should keep it as is. Going forward we need: A way that sets the cipher that will be announced in OCC to keep older (everything 2.4 and below) happy, especially servers/clients that have occ strict enabled. Making --cipher only setting the OCC cipher allows us to be compatible in this as long server/client are 2.4+ as these will negotiate to AES-256-GCM. One thing that we could discuss is if --cipher should add itself to data-ciphers. But if it does we need another switch/option that disables that behaviour again. Otherwise there is no way of migrating (in the sense of not allowing) the cipher used by --cipher. So in summary the options do a combination from: a) set the occ cipher b) set the fallback cipher when the peer is a non-OCC openvpn version c) add themselves to data-ciphers Currently in 2.5: no '--cipher' in config: a) with BF-CBC --cipher xyz in config a), b) and c) if not in --data-ciphers a) but not b) when in --data-ciphers --data-ciphers-fallback a), b) but not c) Basically the idea was that when you ensured that data-ciphers is correct or data-ciphers-fallback was used, you already configured the config with the new NCP in mind. My proposal for 2.6: --cipher just does a) --data-ciphers-fallback does a) and b)
diff --git a/doc/man-sections/protocol-options.rst b/doc/man-sections/protocol-options.rst index e9d5d63d..ca1407b9 100644 --- a/doc/man-sections/protocol-options.rst +++ b/doc/man-sections/protocol-options.rst @@ -57,8 +57,10 @@ configured in a compatible way between both the local and remote side. http://www.cs.ucsd.edu/users/mihir/papers/hmac.html --cipher alg - This option is deprecated for server-client mode. ``--data-ciphers`` - or possibly `--data-ciphers-fallback`` should be used instead. + This option is ignored for server-client mode cipher selection. + ``--data-ciphers`` or possibly ``--data-ciphers-fallback`` must be used + instead. It only determines which cipher is send in the + OCC string (see ``opt-verify``) for compatbility with old peers. Encrypt data channel packets with cipher algorithm ``alg``. diff --git a/src/openvpn/options.c b/src/openvpn/options.c index 01da88ad..7dc3e3eb 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -3074,12 +3074,6 @@ options_postprocess_cipher(struct options *o) "--data-ciphers-fallback config option"); } - msg(M_WARN, "--cipher is not set. Previous OpenVPN version defaulted to " - "BF-CBC as fallback when cipher negotiation failed in this case. " - "If you need this fallback please add '--data-ciphers-fallback " - "BF-CBC' to your configuration and/or add BF-CBC to " - "--data-ciphers."); - /* We still need to set the ciphername to BF-CBC since various other * parts of OpenVPN assert that the ciphername is set */ o->ciphername = "BF-CBC"; @@ -3087,22 +3081,10 @@ options_postprocess_cipher(struct options *o) else if (!o->enable_ncp_fallback && !tls_item_in_cipher_list(o->ciphername, o->ncp_ciphers)) { - msg(M_WARN, "DEPRECATED OPTION: --cipher set to '%s' but missing in" - " --data-ciphers (%s). Future OpenVPN version will " - "ignore --cipher for cipher negotiations. " - "Add '%s' to --data-ciphers or change --cipher '%s' to " - "--data-ciphers-fallback '%s' to silence this warning.", - o->ciphername, o->ncp_ciphers, o->ciphername, - o->ciphername, o->ciphername); - o->enable_ncp_fallback = true; - - /* Append the --cipher to ncp_ciphers to allow it in NCP */ - size_t newlen = strlen(o->ncp_ciphers) + 1 + strlen(o->ciphername) + 1; - char *ncp_ciphers = gc_malloc(newlen, false, &o->gc); - - ASSERT(openvpn_snprintf(ncp_ciphers, newlen, "%s:%s", o->ncp_ciphers, - o->ciphername)); - o->ncp_ciphers = ncp_ciphers; + msg(M_WARN, "Note: --cipher set to '%s' but missing in" + " --data-ciphers (%s). OpenVPN 2.6+ ignores --cipher for " + "cipher negiotiation.", + o->ciphername, o->ncp_ciphers); } }
OpenVPN will ignore --cipher in lieu of the replacement data-ciphers for cipher negioation. Signed-off-by: Arne Schwabe <arne@rfc2549.org> --- doc/man-sections/protocol-options.rst | 6 ++++-- src/openvpn/options.c | 26 ++++---------------------- 2 files changed, 8 insertions(+), 24 deletions(-)