| Message ID | 20210904095629.6273-2-a@unstable.cc |
|---|---|
| State | Accepted |
| Headers | show |
| Series | change defaults and introduce compat-mode | expand |
Am 04.09.21 um 11:56 schrieb Antonio Quartulli: > The new condition is equivalent to the old one, but easier to grasp. > > Also add message to inform uset that cipher negotiation, in this case, > it indeed disabled. > > Signed-off-by: Arne Schwabe <arne@rfc2549.org> > Signed-off-by: Antonio Quartulli <a@unstable.cc> > --- > src/openvpn/options.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/src/openvpn/options.c b/src/openvpn/options.c > index 00ba6044..0d6b85cf 100644 > --- a/src/openvpn/options.c > +++ b/src/openvpn/options.c > @@ -3076,8 +3076,12 @@ options_postprocess_verify(const struct options *o) > static void > options_postprocess_cipher(struct options *o) > { > - if (!o->pull && !(o->mode == MODE_SERVER)) > + if (!o->tls_server && !o->tls_client) > { > + /* we are in the classic P2P mode */ > + msg(M_WARN, "Cipher negotiation is disabled since TLS " > + "mode is not enabled"); > + > /* If the cipher is not set, use the old default of BF-CBC. We will > * warn that this is deprecated on cipher initialisation, no need > * to warn here as well */ > Yes. Makes sense. The change makes it is a lot more clear. I think it is actually not equivalent but the new one is definitively the correct one. Acked-By: Arne Schwabe <arne@rfc2549.org> Arne
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256 Sent with ProtonMail Secure Email. ‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐ On Monday, September 6th, 2021 at 14:19, Arne Schwabe <arne@rfc2549.org> wrote: > Am 04.09.21 um 11:56 schrieb Antonio Quartulli: > > > The new condition is equivalent to the old one, but easier to grasp. > > > > Also add message to inform uset that cipher negotiation, in this case, > > uset -> user > > it indeed disabled. > > it -> is > > Signed-off-by: Arne Schwabe arne@rfc2549.org > > > > Signed-off-by: Antonio Quartulli a@unstable.cc > > -------------------------------------------------------------------------------------------- > > > > src/openvpn/options.c | 6 +++++- > > > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/src/openvpn/options.c b/src/openvpn/options.c > > > > index 00ba6044..0d6b85cf 100644 > > > > --- a/src/openvpn/options.c > > > > +++ b/src/openvpn/options.c > > > > @@ -3076,8 +3076,12 @@ options_postprocess_verify(const struct options *o) > > > > static void > > > > options_postprocess_cipher(struct options *o) > > > > { > > > > - if (!o->pull && !(o->mode == MODE_SERVER)) > > > > - if (!o->tls_server && !o->tls_client) > > > > { > > - /* we are in the classic P2P mode */ > > > > > > - msg(M_WARN, "Cipher negotiation is disabled since TLS " > > > > > > - "mode is not enabled"); > > > > > > - /* If the cipher is not set, use the old default of BF-CBC. We will > > * warn that this is deprecated on cipher initialisation, no need > > * to warn here as well */ > > > > > > Yes. Makes sense. The change makes it is a lot more clear. I think it is > > actually not equivalent but the new one is definitively the correct one. > > Acked-By: Arne Schwabe arne@rfc2549.org > > Arne > > Openvpn-devel mailing list > > Openvpn-devel@lists.sourceforge.net > > https://lists.sourceforge.net/lists/listinfo/openvpn-devel -----BEGIN PGP SIGNATURE----- Version: ProtonMail wsBzBAEBCAAGBQJhNhflACEJEE+XnPZrkLidFiEECbw9RGejjXJ5xVVVT5ec 9muQuJ2H0wgAup4i4ArQzzTGnSsFtHYAjzeKiwIPemMiOw76gv64TqI7dRST 6SsSkpS4Xs10HQysdYzyoCW4Pju2u1zEn654UeonDa43HPYtrL0V3NDmtRYT 3+aJOtqkE74MAZXpMxh1zo1KVkP+UMtpJmFVngK9IvMnSMjBnEj/1np2aZeE KJSRKpskDgBXC5ISzo6JB3T48zbbl1+4zq8TOKrr9uvyXluw5Cme+YGL1yC5 l0ApubT29ANQMOIwBMwGaHm49sRZtkFod/hO630NNcjD/veksS2eyT0fo7oY P38ZVT0rbQWO06D1/0D4PGlXeukYHa0aVRqCR+nWm01dqa22gedDzg== =S+qj -----END PGP SIGNATURE-----
> > Yes. Makes sense. The change makes it is a lot more clear. I think it is > actually not equivalent but the new one is definitively the correct one. > I looked at the code again and I have to actually retract my ACK. The previous code means P2P mode with static key or P2P mode without --pull while the new condition means only P2P mode with static key. The code that follows for that section should be better commented: if (!o->ciphername) { o->ciphername = "BF-CBC"; } else { o->enable_ncp_fallback = true; } return; It basically condeses to having always a valid string in o->ciphername, which is then used in static key code (which ignores data-ciphers) or allowing falling back to the cipher explicitly set via --cipher if we are in TLS mode (with NCP) but not to the implicit BF-CBC. Arne
diff --git a/src/openvpn/options.c b/src/openvpn/options.c index 00ba6044..0d6b85cf 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -3076,8 +3076,12 @@ options_postprocess_verify(const struct options *o) static void options_postprocess_cipher(struct options *o) { - if (!o->pull && !(o->mode == MODE_SERVER)) + if (!o->tls_server && !o->tls_client) { + /* we are in the classic P2P mode */ + msg(M_WARN, "Cipher negotiation is disabled since TLS " + "mode is not enabled"); + /* If the cipher is not set, use the old default of BF-CBC. We will * warn that this is deprecated on cipher initialisation, no need * to warn here as well */