Added "remote-override" config directive.
PATCH 1/4 Added "remote-override" config directive.
remote-override <alt-remote> -- replace the hostname in all remote directives with alt-remote.
Merged from OpenVPN 2.1
--- src/openvpn/options.c | 7 ++++++- src/openvpn/options.h | 2 ++ 2 files changed, 8 insertions(+), 1 deletion(-)
src/openvpn/options.c +6 -1
@@ -4512,6 +4512,11 @@ add_option (struct options *options, goto err; }#endif + else if (streq (p[0], "remote-override") && p[1]) + { +VERIFY_PERMISSION (OPT_P_GENERAL ); + options->remote_override = p[1 ]; + } else if (streq (p[0], "remote") && p[1]) {struct remote_entry re;
@@ -4520,7 +4525,7 @@ add_option (struct options *options, re.af=0 ;VERIFY_PERMISSION (OPT_P_GENERAL |OPT_P_CONNECTION ); - re.remote = p[1 ]; + re.remote = options->remote_override ? options->remote_override : p[1 ];if (p[2 ]) { re.remote_port = p[2 ];
src/openvpn/options.h +2 -0
@@ -222,6 +222,8 @@ struct optionsstruct remote_host_store *rh_store; +const char *remote_override; +bool remote_random;const char *ipchange;const char *dev;
Signature · 1 line
1.8.5.5
PATCH 2/4 Added PIP_OPT_MASK for process_ip_header fast exit path.
Define PIP_OPT_MASK to represent all flags of interest to process_ip_header, so that it can have a fast exit path if no flags are set.
Merged from OpenVPN 2.1
--- src/openvpn/forward.c | 6 +----- src/openvpn/forward.h | 4 +++- 2 files changed, 4 insertions(+), 6 deletions(-)
src/openvpn/forward.c +1 -5
@@ -1026,11 +1026,7 @@ process_ip_header (struct context *c, unsigned int flags, struct buffer *buf)* The --passtos and --mssfix options require * us to examine the IPv4 header. */ -#if PASSTOS_CAPABILITY -if (flags & (PIPV4_PASSTOS |PIP_MSSFIX )) -#else - if (flags & PIP_MSSFIX) -#endif +if (flags &PIP_OPT_MASK ) {struct buffer ipbuf = *buf;if (is_ipv4 (TUNNEL_TYPE (c->c1.tuntap), &ipbuf))
src/openvpn/forward.h +3 -1
@@ -229,9 +229,11 @@ bool send_control_channel_string (struct context *c, const char *str, int msglev#define PIPV4_PASSTOS (1<<0)#define PIP_MSSFIX (1<<1)/* v4 and v6 */ -#define PIPV4_OUTGOING (1<<2)#define PIPV4_EXTRACT_DHCP_ROUTER (1<<3)#define PIPV4_CLIENT_NAT (1<<4) +#define PIP_OPT_MASK 0xFFFF/* all possible options for */ +/* process_ip_header() */ +#define PIPV4_OUTGOING (1<<16)void process_ip_header (struct context *c,unsigned int flags,struct buffer *buf);
Signature · 1 line
1.8.5.5
PATCH 3/4 Use native strtoull() with MSVC 2013.
In reply to James YonanMSVC 2013 C library now defines strtoull() function, so use the native implementation when available.
--- config-msvc.h | 3 +++ 1 file changed, 3 insertions(+)
config-msvc.h +3 -0
@@ -89,7 +89,10 @@#define strncasecmp strnicmp#define strcasecmp _stricmp#define snprintf _snprintf + +#if _MSC_VER <1800 #define strtoull strtoul +#endif #define in_addr_t uint32_t#define ssize_t SSIZE_T
Signature · 1 line
1.8.5.5
PATCH 4/4 When tls-version-min is unspecified, revert to original versioning approach.
In reply to James YonanFor OpenSSL, this means to use TLSv1_(client|server)_method rather than SSLv23_(client|server)_method combined with SSL_OP_NO_x flags for specific TLS versions to disable.
For PolarSSL, this means to avoid calling ssl_set_min_version and instead implicitly control the TLS version via allowed ciphersuites.
--- src/openvpn/ssl.c | 4 ++-- src/openvpn/ssl_backend.h | 15 +++++++++------ src/openvpn/ssl_openssl.c | 31 ++++++++++++++++++++++--------- src/openvpn/ssl_polarssl.c | 43 +++++++++++++++++++++++-------------------- 4 files changed, 56 insertions(+), 37 deletions(-)
src/openvpn/ssl.c +2 -2
@@ -486,12 +486,12 @@ init_ssl (const struct options *options, struct tls_root_ctx *new_ctx)if (options->tls_server) { -tls_ctx_server_new (new_ctx); +tls_ctx_server_new (new_ctx, options->ssl_flags);tls_ctx_load_dh_params (new_ctx, options->dh_file, options->dh_file_inline); }else /* if client */ { -tls_ctx_client_new (new_ctx); +tls_ctx_client_new (new_ctx, options->ssl_flags); }tls_ctx_set_options (new_ctx, options->ssl_flags);
src/openvpn/ssl_backend.h +9 -6
@@ -109,10 +109,11 @@ void tls_clear_error();* @return One of the TLS_VER_x constants or TLS_VER_BAD * if a parse error should be flagged. */ -#define TLS_VER_BAD -1 -#define TLS_VER_1_0 0/* default */ -#define TLS_VER_1_1 1 -#define TLS_VER_1_2 2 +#define TLS_VER_BAD -1 +#define TLS_VER_UNSPEC 0 +#define TLS_VER_1_0 1/* default */ +#define TLS_VER_1_1 2 +#define TLS_VER_1_2 3int tls_version_min_parse (const char *vstr,const char *extra);/**
@@ -127,15 +128,17 @@ int tls_version_max(void);* Initialise a library-specific TLS context for a server. * * @param ctx TLS context to initialise +* @param ssl_flags SSLF_x flags from ssl_common.h */ -void tls_ctx_server_new (struct tls_root_ctx *ctx); +void tls_ctx_server_new (struct tls_root_ctx *ctx,unsigned int ssl_flags);/** * Initialises a library-specific TLS context for a client. * * @param ctx TLS context to initialise +* @param ssl_flags SSLF_x flags from ssl_common.h */ -void tls_ctx_client_new (struct tls_root_ctx *ctx); +void tls_ctx_client_new (struct tls_root_ctx *ctx,unsigned int ssl_flags);/** * Frees the library-specific TLSv1 context
src/openvpn/ssl_openssl.c +22 -9
@@ -94,22 +94,32 @@ tls_clear_error() }void -tls_ctx_server_new (struct tls_root_ctx *ctx) +tls_ctx_server_new (struct tls_root_ctx *ctx,unsigned int ssl_flags) { +const int tls_version_min = (ssl_flags >>SSLF_TLS_VERSION_SHIFT ) &SSLF_TLS_VERSION_MASK ; +ASSERT (NULL != ctx); - ctx->ctx =SSL_CTX_new (SSLv23_server_method ()); +if (tls_version_min >TLS_VER_UNSPEC ) + ctx->ctx =SSL_CTX_new (SSLv23_server_method ()); +else + ctx->ctx =SSL_CTX_new (TLSv1_server_method ());if (ctx->ctx ==NULL )msg (M_SSLERR ,"SSL_CTX_new SSLv23_server_method" ); }void -tls_ctx_client_new (struct tls_root_ctx *ctx) +tls_ctx_client_new (struct tls_root_ctx *ctx,unsigned int ssl_flags) { +const int tls_version_min = (ssl_flags >>SSLF_TLS_VERSION_SHIFT ) &SSLF_TLS_VERSION_MASK ; +ASSERT (NULL != ctx); - ctx->ctx =SSL_CTX_new (SSLv23_client_method ()); +if (tls_version_min >TLS_VER_UNSPEC ) + ctx->ctx =SSL_CTX_new (SSLv23_client_method ()); +else + ctx->ctx =SSL_CTX_new (TLSv1_client_method ());if (ctx->ctx ==NULL )msg (M_SSLERR ,"SSL_CTX_new SSLv23_client_method" );
@@ -182,16 +192,19 @@ tls_ctx_set_options (struct tls_root_ctx *ctx, unsigned int ssl_flags) {long sslopt =SSL_OP_SINGLE_DH_USE |SSL_OP_NO_TICKET | SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3;const int tls_version_min = (ssl_flags >>SSLF_TLS_VERSION_SHIFT ) &SSLF_TLS_VERSION_MASK ; -if (tls_version_min >TLS_VER_1_0 ) - sslopt |= SSL_OP_NO_TLSv1; +if (tls_version_min >TLS_VER_UNSPEC ) + { +if (tls_version_min >TLS_VER_1_0 ) + sslopt |= SSL_OP_NO_TLSv1;#ifdef SSL_OP_NO_TLSv1_1 -if (tls_version_min >TLS_VER_1_1 ) - sslopt |= SSL_OP_NO_TLSv1_1; +if (tls_version_min >TLS_VER_1_1 ) + sslopt |= SSL_OP_NO_TLSv1_1;#endif #ifdef SSL_OP_NO_TLSv1_2if (tls_version_min >TLS_VER_1_2 ) - sslopt |= SSL_OP_NO_TLSv1_2; + sslopt |= SSL_OP_NO_TLSv1_2;#endif + }SSL_CTX_set_options (ctx->ctx, sslopt); }
src/openvpn/ssl_polarssl.c +23 -20
73 lines not on this page · raw message
Signature · 1 line
1.8.5.5
Re: [Openvpn-devel] [PATCH 4/4] When tls-version-min is unspecified, revert to original versioning approach.
Hi,
Quoted 3 lines · James Yonan
For OpenSSL, this means to use TLSv1_(client|server)_method rather than SSLv23_(client|server)_method combined with
SSL_OP_NO_xflags for specific TLS versions to disable.
I'm not sure I understand the rationale behind this. If I don't specify a minimum version, my maximum version changes to TLS 1.0? Could you maybe explain the "why" for this patch?
-Steffan
Re: [Openvpn-devel] [PATCH 4/4] When tls-version-min is unspecified, revert to original versioning approach.
Hi,
Quoted 8 lines · Steffan Karger
Quoted 3 lines · James Yonan
For OpenSSL, this means to use TLSv1_(client|server)_method rather than SSLv23_(client|server)_method combined with
SSL_OP_NO_xflags for specific TLS versions to disable.I'm not sure I understand the rationale behind this. If I don't specify a minimum version, my maximum version changes to TLS 1.0? Could you maybe explain the "why" for this patch?
"turn it off!", without introducing yet another config directive (or, more precisely, "do not turn it on by default" if I read the patch right).
There seem to be some not-yet-fully-understood combinations of OpenSSL library versions/library builds that will break 2.3.3 clients <-> git-master servers if the TLS version negotiation patch is active, so having a way to turn it off at run-time (if only to see if that is the problem) is certainly useful.
gert
Signature · 4 lines
USENET is *not* the non-clickable part of WWW!
//www.muc.de/~gert/
Gert Doering - Munich, Germany [email protected]...
fax: +49-89-35655025 [email protected]...Re: [Openvpn-devel] [PATCH 4/4] When tls-version-min is unspecified, revert to original versioning approach.
Quoted 10 lines · Gert Doering
Hi,
Quoted 7 lines · Steffan Karger
Quoted 3 lines · James Yonan
For OpenSSL, this means to use TLSv1_(client|server)_method rather than SSLv23_(client|server)_method combined with
SSL_OP_NO_xflags for specific TLS versions to disable.I'm not sure I understand the rationale behind this. If I don't specify a minimum version, my maximum version changes to TLS 1.0? Could you maybe explain the "why" for this patch?
Quoted 9 lines
"turn it off!", without introducing yet another config directive (or, more precisely, "do not turn it on by default" if I read the patch right).
There seem to be some not-yet-fully-understood combinations of OpenSSL library versions/library builds that will break 2.3.3 clients <-> git-master servers if the TLS version negotiation patch is active, so having a way to turn it off at run-time (if only to see if that is the problem) is certainly useful.
Yes. But with this patch it is always turned off, keeping OpenVPN in 99% of installations in TLS 1.0. Is there any other known case where it breaks aside from the Tomato OpenVPN client?
Arne
[Openvpn-devel] [PATCH applied] Re: Use native strtoull() with MSVC 2013.
In reply to James YonanACK (you're the one who builds most with MSVC anyway, so you know).
Your patch has been applied to the master and release/23 branches.
commit 6b8e2f4a8143a7260a06b6999dcb21c4c72fc620 (master) commit 040b306f58fa0cc175c66ed67f390c0a289ddd4e (release/2.3)
Author: James Yonan Date: Mon Apr 21 01:10:03 2014 -0600
Use native strtoull() with MSVC 2013.
Signed-off-by: James Yonan <[email protected]...>
Acked-by: Gert Doering <[email protected]...>
Message-Id: <[email protected]...>
URL: http://article.gmane.org/gmane.network.openvpn.devel/8561
Signed-off-by: Gert Doering <[email protected]...>--
kind regards,
Gert Doering
Re: [Openvpn-devel] [PATCH 2/4] Added PIP_OPT_MASK for process_ip_header fast exit path.
In reply to James YonanQuoted 3 lines · James Yonan
Define
PIP_OPT_MASKto represent all flags of interest toprocess_ip_header, so that it can have a fast exit path if no flags are set.
I haven't look at the code but if remember correctly, this method does not get passed the actual flags but the flags should *potientially* be checked.
See also my never reviewed patch: http://thread.gmane.org/gmane.network.openvpn.devel/7532
Arne
Quoted 3 lines · James Yonan
remote-override <alt-remote> -- replace the hostname in all remote directives with alt-remote.
Just a question. How does remote-override differ from ip-remote-hint? Both options seem to have the same effect aside from very subtle differences. Perhaps we should add this as an alias for the already existing option.
Arne
Re: [Openvpn-devel] [PATCH 4/4] When tls-version-min is unspecified, revert to original versioning approach.
In reply to Arne SchwabeHi,
Quoted 3 lines · Arne Schwabe
Yes. But with this patch it is always turned off, keeping OpenVPN in 99% of installations in TLS 1.0. Is there any other known case where it breaks aside from the Tomato OpenVPN client?
http://community.openvpn.net/openvpn/ticket/385
this is the only case I know - and I blaim the openssl library on the server side (ARM). So for me, "default-on with a way to turn it off" would be sufficient. But I assume James has much more visibility...
gert
Signature · 4 lines
USENET is *not* the non-clickable part of WWW!
//www.muc.de/~gert/
Gert Doering - Munich, Germany [email protected]...
fax: +49-89-35655025 [email protected]...Re: [Openvpn-devel] [PATCH 4/4] When tls-version-min is unspecified, revert to original versioning approach.
Quoted 7 lines · Gert Doering
Hi,
Quoted 3 lines · Arne Schwabe
Yes. But with this patch it is always turned off, keeping OpenVPN in 99% of installations in TLS 1.0. Is there any other known case where it breaks aside from the Tomato OpenVPN client?
Quoted 5 lines
http://community.openvpn.net/openvpn/ticket/385
this is the only case I know - and I blaim the openssl library on the server side (ARM). So for me, "default-on with a way to turn it off" would be sufficient. But I assume James has much more visibility...
The problem with tls-version-min is that it defaults to 1.0 even if not used. We've received many reports of breakage with this approach, probably because setting the min to 1.0 is actually subtly different from the approach used before tls-version-min was implemented.
So this patch turns off tls-version-min unless it's explicit in the config.
James
Re: [Openvpn-devel] [PATCH 2/4] Added PIP_OPT_MASK for process_ip_header fast exit path.
In reply to Arne SchwabeQuoted 8 lines · Arne Schwabe
Quoted 3 lines · James Yonan
Define
PIP_OPT_MASKto represent all flags of interest toprocess_ip_header, so that it can have a fast exit path if no flags are set.I haven't look at the code but if remember correctly, this method does not get passed the actual flags but the flags should *potientially* be checked.
Yes, the point is to aggregate the potential flags of interest to process_ip_header under a single mask, i.e. PIP_OPT_MASK.
So instead of having "if (flags & (PIPV4_PASSTOS|PIP_MSSFIX|...|...))" you just have "if (flags & PIP_OPT_MASK)".
The main issue for us is that we have an internal patch for one of our services that adds an additional flag, and the previous usage is a merge conflict attractor because all the flags of interest are explicitly given on a single line.
Quoted 2 lines
See also my never reviewed patch:
http://thread.gmane.org/gmane.network.openvpn.devel/7532
Your patch would be simplified with this approach because you don't need to add the test for PIPV4_CLIENT_NAT to "if (flags & ...)".
James
Quoted 8 lines · Arne Schwabe
Quoted 3 lines · James Yonan
remote-override <alt-remote> -- replace the hostname in all remote directives with alt-remote.
Just a question. How does remote-override differ from ip-remote-hint? Both options seem to have the same effect aside from very subtle differences. Perhaps we should add this as an alias for the already existing option.
They are different options.
ip-remote-hint is used when the app driving the client via management interface has already done a DNS lookup on the remote address, and wants to save the OpenVPN core from having to do a redundant lookup.
remote-override is used when the client UI has an option that allows the end user to override the default server address. For example a UI might give the user a drop-down list of regions to connect to. Since it is common in OpenVPN configs to have a list of many "remote" directives for failover, we needed a directive that would do a "search/replace" on the hostname component of each "remote" item.
That's different from the approach of ip-remote-hint which is designed to only provide a DNS resolution hint for the first "remote" item.
James
Re: [Openvpn-devel] [PATCH 2/4] Added PIP_OPT_MASK for process_ip_header fast exit path.
In reply to James YonanAm 21.04.14 21:26, schrieb James Yonan:
Quoted 13 lines · Arne Schwabe
Quoted 8 lines · James Yonan
Quoted 3 lines
Define
PIP_OPT_MASKto represent all flags of interest toprocess_ip_header, so that it can have a fast exit path if no flags are set.I haven't look at the code but if remember correctly, this method does not get passed the actual flags but the flags should *potientially* be checked.
Yes, the point is to aggregate the potential flags of interest to
process_ip_headerunder a single mask, i.e.PIP_OPT_MASK.
Quoted 8 lines
So instead of having "if (flags & (PIPV4_PASSTOS|PIP_MSSFIX|...|...))" you just have "if (flags &
PIP_OPT_MASK)".The main issue for us is that we have an internal patch for one of our services that adds an additional flag, and the previous usage is a merge conflict attractor because all the flags of interest are explicitly given on a single line.
I only wanted to point out my patch also exits. I looked at the code again.
I think to really get rid of the #ifdef here we also need to unconditionally enable the other &= ~ statements:
Change
#if PASSTOS_CAPABILITY
if (!c->options.passtos)
flags &= ~PIPV4_PASSTOS;
#endif
to
#if PASSTOS_CAPABILITY
if (!c->options.passtos)
#endif
flags &= ~PIPV4_PASSTOS;Otherwise if the function is called with PIPV4_PASSTOS but not compiled with PASSTOS_CAPABILITY the
flags & PIP_OPT_MASK will be always true.
Arne
Re: [Openvpn-devel] [PATCH 2/4] Added PIP_OPT_MASK for process_ip_header fast exit path.
Quoted 42 lines · Arne Schwabe
Am 21.04.14 21:26, schrieb James Yonan:
Quoted 13 lines · Arne Schwabe
Quoted 8 lines · James Yonan
Quoted 3 lines
Define
PIP_OPT_MASKto represent all flags of interest toprocess_ip_header, so that it can have a fast exit path if no flags are set.I haven't look at the code but if remember correctly, this method does not get passed the actual flags but the flags should *potientially* be checked.
Yes, the point is to aggregate the potential flags of interest to
process_ip_headerunder a single mask, i.e.PIP_OPT_MASK.Quoted 8 lines
So instead of having "if (flags & (PIPV4_PASSTOS|PIP_MSSFIX|...|...))" you just have "if (flags &
PIP_OPT_MASK)".The main issue for us is that we have an internal patch for one of our services that adds an additional flag, and the previous usage is a merge conflict attractor because all the flags of interest are explicitly given on a single line.
I only wanted to point out my patch also exits. I looked at the code again.
I think to really get rid of the #ifdef here we also need to unconditionally enable the other &= ~ statements:
Change
#if PASSTOS_CAPABILITY if (!c->options.passtos) flags &= ~PIPV4_PASSTOS;#endif
to
#if PASSTOS_CAPABILITY if (!c->options.passtos) #endif flags &= ~PIPV4_PASSTOS;
Quoted 6 lines
Otherwise if the function is called with
PIPV4_PASSTOSbut not compiled withPASSTOS_CAPABILITYtheflags &
PIP_OPT_MASKwill be always true.Arne
Sure, that makes sense.
James
Re: [Openvpn-devel] [PATCH 4/4] When tls-version-min is unspecified, revert to original versioning approach.
In reply to James YonanHi,
Quoted 6 lines · James Yonan
For OpenSSL, this means to use TLSv1_(client|server)_method rather than SSLv23_(client|server)_method combined with
SSL_OP_NO_xflags for specific TLS versions to disable.For PolarSSL, this means to avoid calling
ssl_set_min_versionand instead implicitly control the TLS version via allowed ciphersuites.
As per the discussion on IRC, I've adapted the patch to 2.3 only (some small incompatibilities in ssl_polarssl.c), and added a section in the openvpn.8 man page.
I have tested OpenSSL and PolarSSL builds, talking to a git master server.
With OpenSSL:
no --tls-version-min in config --> TLSv1 is used
--tls-version-min 1.0 or 1.2 --> TLSv1.2 is used
(clearly visible in both server and client logs)
With PolarSSL, I always get TLSv1.2, and can't really see whether or not it makes a difference. But given James' comments, this seems to be expected (I didn't play with --tls-cipher settings).
The attached patch is what I intend to commit to release/2.3 *only*, not to master - as agreed at the IRC meeting. "Please ACK" :-)
gert
Signature · 4 lines
USENET is *not* the non-clickable part of WWW!
//www.muc.de/~gert/
Gert Doering - Munich, Germany [email protected]...
fax: +49-89-35655025 [email protected]...From 986a3382de536a3ca530429f985f9989783d5996 Mon Sep 17 00:00:00 2001
From: James Yonan <[email protected]...>
Date: Mon, 21 Apr 2014 01:10:04 -0600
Subject: [PATCH] When tls-version-min is unspecified, revert to original
versioning approach.
For OpenSSL, this means to use TLSv1_(client|server)_method rather than SSLv23_(client|server)_method combined with SSL_OP_NO_x flags for specific TLS versions to disable.
For PolarSSL, this means to avoid calling ssl_set_min_version and instead implicitly control the TLS version via allowed ciphersuites.
Point out off-by-default-now setting in the openvpn(8) man page.
This patch is only included in the release/2.3 branch, because it's a stopgap measure. 2.4 will have it on-by-default, when the remaining handshake problems are fully debugged and solved.
--- doc/openvpn.8 | 8 +++++++- src/openvpn/ssl.c | 4 ++-- src/openvpn/ssl_backend.h | 15 +++++++++------ src/openvpn/ssl_openssl.c | 33 +++++++++++++++++++++++---------- src/openvpn/ssl_polarssl.c | 41 ++++++++++++++++++++++------------------- 5 files changed, 63 insertions(+), 38 deletions(-)
doc/openvpn.8 +7 -1
@@ -4275,12 +4275,18 @@ above). .\"********************************************************* .TP .B \-\-tls-version-min version ['or-highest'] -Sets the minimum +Enable TLS version negotiation, and set the minimum TLS version we will accept from the peer (default is "1.0"). Examples for version include "1.0", "1.1", or "1.2". If 'or-highest' is specified and version is not recognized, we will only accept the highest TLS version supported by the local SSL implementation. + +If this options is not set, the code in OpenVPN 2.3.4 will default +to using TLS 1.0 only, without any version negotiation. This reverts +the beaviour to what OpenVPN versions up to 2.3.2 did, as it turned +out that TLS version negotiation can lead to handshake problems due +to new signature algorithms in TLS 1.2. .\"********************************************************* .TP .B \-\-pkcs12 file
src/openvpn/ssl.c +2 -2
@@ -486,12 +486,12 @@ init_ssl (const struct options *options, struct tls_root_ctx *new_ctx)if (options->tls_server) { -tls_ctx_server_new (new_ctx); +tls_ctx_server_new (new_ctx, options->ssl_flags);tls_ctx_load_dh_params (new_ctx, options->dh_file, options->dh_file_inline); }else /* if client */ { -tls_ctx_client_new (new_ctx); +tls_ctx_client_new (new_ctx, options->ssl_flags); }tls_ctx_set_options (new_ctx, options->ssl_flags);
src/openvpn/ssl_backend.h +9 -6
@@ -109,10 +109,11 @@ void tls_clear_error();* @return One of the TLS_VER_x constants or TLS_VER_BAD * if a parse error should be flagged. */ -#define TLS_VER_BAD -1 -#define TLS_VER_1_0 0/* default */ -#define TLS_VER_1_1 1 -#define TLS_VER_1_2 2 +#define TLS_VER_BAD -1 +#define TLS_VER_UNSPEC 0/* default */ +#define TLS_VER_1_0 1 +#define TLS_VER_1_1 2 +#define TLS_VER_1_2 3int tls_version_min_parse (const char *vstr,const char *extra);/**
@@ -127,15 +128,17 @@ int tls_version_max(void);* Initialise a library-specific TLS context for a server. * * @param ctx TLS context to initialise +* @param ssl_flags SSLF_x flags from ssl_common.h */ -void tls_ctx_server_new (struct tls_root_ctx *ctx); +void tls_ctx_server_new (struct tls_root_ctx *ctx,unsigned int ssl_flags);/** * Initialises a library-specific TLS context for a client. * * @param ctx TLS context to initialise +* @param ssl_flags SSLF_x flags from ssl_common.h */ -void tls_ctx_client_new (struct tls_root_ctx *ctx); +void tls_ctx_client_new (struct tls_root_ctx *ctx,unsigned int ssl_flags);/** * Frees the library-specific TLSv1 context
src/openvpn/ssl_openssl.c +23 -10
@@ -119,11 +119,16 @@ tmp_rsa_cb (SSL * s, int is_export, int keylength) }void -tls_ctx_server_new (struct tls_root_ctx *ctx) +tls_ctx_server_new (struct tls_root_ctx *ctx,unsigned int ssl_flags) { +const int tls_version_min = (ssl_flags >>SSLF_TLS_VERSION_SHIFT ) &SSLF_TLS_VERSION_MASK ; +ASSERT (NULL != ctx); - ctx->ctx =SSL_CTX_new (SSLv23_server_method ()); +if (tls_version_min >TLS_VER_UNSPEC ) + ctx->ctx =SSL_CTX_new (SSLv23_server_method ()); +else + ctx->ctx =SSL_CTX_new (TLSv1_server_method ());if (ctx->ctx ==NULL )msg (M_SSLERR ,"SSL_CTX_new SSLv23_server_method" );
@@ -132,11 +137,16 @@ tls_ctx_server_new(struct tls_root_ctx *ctx) }void -tls_ctx_client_new (struct tls_root_ctx *ctx) +tls_ctx_client_new (struct tls_root_ctx *ctx,unsigned int ssl_flags) { +const int tls_version_min = (ssl_flags >>SSLF_TLS_VERSION_SHIFT ) &SSLF_TLS_VERSION_MASK ; +ASSERT (NULL != ctx); - ctx->ctx =SSL_CTX_new (SSLv23_client_method ()); +if (tls_version_min >TLS_VER_UNSPEC ) + ctx->ctx =SSL_CTX_new (SSLv23_client_method ()); +else + ctx->ctx =SSL_CTX_new (TLSv1_client_method ());if (ctx->ctx ==NULL )msg (M_SSLERR ,"SSL_CTX_new SSLv23_client_method" );
@@ -209,16 +219,19 @@ tls_ctx_set_options (struct tls_root_ctx *ctx, unsigned int ssl_flags) {long sslopt =SSL_OP_SINGLE_DH_USE |SSL_OP_NO_TICKET | SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3;const int tls_version_min = (ssl_flags >>SSLF_TLS_VERSION_SHIFT ) &SSLF_TLS_VERSION_MASK ; -if (tls_version_min >TLS_VER_1_0 ) - sslopt |= SSL_OP_NO_TLSv1; +if (tls_version_min >TLS_VER_UNSPEC ) + { +if (tls_version_min >TLS_VER_1_0 ) + sslopt |= SSL_OP_NO_TLSv1;#ifdef SSL_OP_NO_TLSv1_1 -if (tls_version_min >TLS_VER_1_1 ) - sslopt |= SSL_OP_NO_TLSv1_1; +if (tls_version_min >TLS_VER_1_1 ) + sslopt |= SSL_OP_NO_TLSv1_1;#endif #ifdef SSL_OP_NO_TLSv1_2 -if (tls_version_min >TLS_VER_1_2 ) - sslopt |= SSL_OP_NO_TLSv1_2; +if (tls_version_min >TLS_VER_1_2 ) + sslopt |= SSL_OP_NO_TLSv1_2;#endif + }SSL_CTX_set_options (ctx->ctx, sslopt); }
src/openvpn/ssl_polarssl.c +22 -19
65 lines not on this page · raw message
Signature · 1 line
1.8.3.2
Re: [Openvpn-devel] [PATCH 4/4] When tls-version-min is unspecified, revert to original versioning approach.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
Hi,
Quoted 3 lines · Gert Doering
On Mon, Apr 21, 2014 at 01:10:04AM -0600, James Yonan wrote: The attached patch is what I intend to commit to release/2.3 *only*, not to master - as agreed at the IRC meeting. "Please ACK" :-)
Sorry, but NAK. The OpenSSL bits look fine, but the PolarSSL bits would also allow for SSL_MINOR_VERSION_0, which is SSLv3 and thus a reduction in security (and actually increases the handshake complexity).
I think the ssl_polarssl.c can stay the way it is, one has to specify tls cipher suites anyway to restrict the handshake. Or am I missing something here?
- -Steffan
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.14 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
iQEcBAEBCAAGBQJTXWQgAAoJEJgCyj0AftKILQoH+wa0ojSrip0vhrPKq/AOa6Bw cpVaDzTo6v7KiOoPf+xWfaEw9aNybd5a8GCVZlCGgSB+vOn53bLtJ7hSPL5fVzb1 8UQw5hWhWyjiZRksyCJNyYEHgzE7ZiRK/LhSd/RhYHlwUTPJfJQ6nYHlM/oMMthz mvj0juA6jCYGCznUD/2fioy5JtpGUqpwkJzQ2hIMtqV8sxyHgJ90R6DpV6cP2nRd AAQndNYVRhC5dQfaQtX+4TStMQK65Q7ZlHZDYb3h6TpKk93y/nWdm5tsew1oLbZ+ E27hMtswug0CSWqnSFtT+bW1shYJgrveDnHu7K1Tgh9KJ9DD2SbvXlgADCaao1U= =uc7+
-----END PGP SIGNATURE-----
Re: [Openvpn-devel] [PATCH 4/4] When tls-version-min is unspecified, revert to original versioning approach.
Hi,
Quoted 6 lines · Steffan Karger
Quoted 3 lines · Gert Doering
On Mon, Apr 21, 2014 at 01:10:04AM -0600, James Yonan wrote: The attached patch is what I intend to commit to release/2.3 *only*, not to master - as agreed at the IRC meeting. "Please ACK" :-)
Sorry, but NAK.
On a more constructive note: attached a new proposal for this patch.
Quoted 1 line
The OpenSSL bits look fine
On a closer look, the wrapping "if (tls_version_min > TLS_VER_UNSPEC)" in tls_ctx_set_options() seems redundant, because TLS_VER_UNSPEC < TLS_VER_1_0 < TLS_VER_1_1 < TLS_VER_1_2 and the ifs are checking for "tls_version_min > TLS_VER_1_x". I've removed these changes in the attached patch proposal.
Quoted 3 lines
the PolarSSL bits
would also allow forSSL_MINOR_VERSION_0, which is SSLv3 and thus a reduction in security (and actually increases the handshake complexity).
To elaborate a bit: The naming is a bit confusing, but SSL_MAJOR_VERSION_3 + SSL_MINOR_VERSION_O means SSLv3, ... + SSL_MINOR_VERSION_1 means TLSv1.0, ... + SSL_MINOR_VERSION_2 means TLSv1.1, etc. If none are given (what would happen in the previous version of the patch), PolarSSL defaults the minimum version to SSLv3 and maximum to TLSv1.2. The attached patch thus removes the wrapping "if (tls_version_min > TLS_VER_UNSPEC)" and sets the default to TLSv1.0 to TLSv1.2 again. That is equal to the behaviour prior to the TLS versioning patch.
-Steffan
From 558fdb6142f4d5b0c039437a001b13110a910e5b Mon Sep 17 00:00:00 2001
From: James Yonan <[email protected]...>
Date: Mon, 28 Apr 2014 22:52:11 +0200
Subject: [PATCH] When tls-version-min is unspecified, revert to original
versioning approach.
For OpenSSL, this means to use TLSv1_(client|server)_method rather than SSLv23_(client|server)_method combined with SSL_OP_NO_x flags for specific TLS versions to disable.
For PolarSSL, this means to implicitly control the TLS version via allowed ciphersuites.
Point out off-by-default-now setting in the openvpn(8) man page.
This patch is only included in the release/2.3 branch, because it's a stopgap measure. 2.4 will have it on-by-default, when the remaining handshake problems are fully debugged and solved.
--- doc/openvpn.8 | 8 +++++++- src/openvpn/ssl.c | 4 ++-- src/openvpn/ssl_backend.h | 15 +++++++++------ src/openvpn/ssl_openssl.c | 18 ++++++++++++++---- src/openvpn/ssl_polarssl.c | 4 ++-- 5 files changed, 34 insertions(+), 15 deletions(-)
doc/openvpn.8 +7 -1
@@ -4275,12 +4275,18 @@ above). .\"********************************************************* .TP .B \-\-tls-version-min version ['or-highest'] -Sets the minimum +Enable TLS version negotiation, and set the minimum TLS version we will accept from the peer (default is "1.0"). Examples for version include "1.0", "1.1", or "1.2". If 'or-highest' is specified and version is not recognized, we will only accept the highest TLS version supported by the local SSL implementation. + +If this options is not set, the code in OpenVPN 2.3.4 will default +to using TLS 1.0 only, without any version negotiation. This reverts +the beaviour to what OpenVPN versions up to 2.3.2 did, as it turned +out that TLS version negotiation can lead to handshake problems due +to new signature algorithms in TLS 1.2. .\"********************************************************* .TP .B \-\-pkcs12 file
src/openvpn/ssl.c +2 -2
@@ -486,12 +486,12 @@ init_ssl (const struct options *options, struct tls_root_ctx *new_ctx)if (options->tls_server) { -tls_ctx_server_new (new_ctx); +tls_ctx_server_new (new_ctx, options->ssl_flags);tls_ctx_load_dh_params (new_ctx, options->dh_file, options->dh_file_inline); }else /* if client */ { -tls_ctx_client_new (new_ctx); +tls_ctx_client_new (new_ctx, options->ssl_flags); }tls_ctx_set_options (new_ctx, options->ssl_flags);
src/openvpn/ssl_backend.h +9 -6
@@ -109,10 +109,11 @@ void tls_clear_error();* @return One of the TLS_VER_x constants or TLS_VER_BAD * if a parse error should be flagged. */ -#define TLS_VER_BAD -1 -#define TLS_VER_1_0 0/* default */ -#define TLS_VER_1_1 1 -#define TLS_VER_1_2 2 +#define TLS_VER_BAD -1 +#define TLS_VER_UNSPEC 0/* default */ +#define TLS_VER_1_0 1 +#define TLS_VER_1_1 2 +#define TLS_VER_1_2 3int tls_version_min_parse (const char *vstr,const char *extra);/**
@@ -127,15 +128,17 @@ int tls_version_max(void);* Initialise a library-specific TLS context for a server. * * @param ctx TLS context to initialise +* @param ssl_flags SSLF_x flags from ssl_common.h */ -void tls_ctx_server_new (struct tls_root_ctx *ctx); +void tls_ctx_server_new (struct tls_root_ctx *ctx,unsigned int ssl_flags);/** * Initialises a library-specific TLS context for a client. * * @param ctx TLS context to initialise +* @param ssl_flags SSLF_x flags from ssl_common.h */ -void tls_ctx_client_new (struct tls_root_ctx *ctx); +void tls_ctx_client_new (struct tls_root_ctx *ctx,unsigned int ssl_flags);/** * Frees the library-specific TLSv1 context
src/openvpn/ssl_openssl.c +14 -4
@@ -119,11 +119,16 @@ tmp_rsa_cb (SSL * s, int is_export, int keylength) }void -tls_ctx_server_new (struct tls_root_ctx *ctx) +tls_ctx_server_new (struct tls_root_ctx *ctx,unsigned int ssl_flags) { +const int tls_version_min = (ssl_flags >>SSLF_TLS_VERSION_SHIFT ) &SSLF_TLS_VERSION_MASK ; +ASSERT (NULL != ctx); - ctx->ctx =SSL_CTX_new (SSLv23_server_method ()); +if (tls_version_min >TLS_VER_UNSPEC ) + ctx->ctx =SSL_CTX_new (SSLv23_server_method ()); +else + ctx->ctx =SSL_CTX_new (TLSv1_server_method ());if (ctx->ctx ==NULL )msg (M_SSLERR ,"SSL_CTX_new SSLv23_server_method" );
@@ -132,11 +137,16 @@ tls_ctx_server_new(struct tls_root_ctx *ctx) }void -tls_ctx_client_new (struct tls_root_ctx *ctx) +tls_ctx_client_new (struct tls_root_ctx *ctx,unsigned int ssl_flags) { +const int tls_version_min = (ssl_flags >>SSLF_TLS_VERSION_SHIFT ) &SSLF_TLS_VERSION_MASK ; +ASSERT (NULL != ctx); - ctx->ctx =SSL_CTX_new (SSLv23_client_method ()); +if (tls_version_min >TLS_VER_UNSPEC ) + ctx->ctx =SSL_CTX_new (SSLv23_client_method ()); +else + ctx->ctx =SSL_CTX_new (TLSv1_client_method ());if (ctx->ctx ==NULL )msg (M_SSLERR ,"SSL_CTX_new SSLv23_client_method" );
src/openvpn/ssl_polarssl.c +2 -2
@@ -67,7 +67,7 @@ tls_clear_error() }void -tls_ctx_server_new (struct tls_root_ctx *ctx) +tls_ctx_server_new (struct tls_root_ctx *ctx,unsigned int ssl_flags) {ASSERT (NULL != ctx);CLEAR (*ctx);
@@ -84,7 +84,7 @@ tls_ctx_server_new(struct tls_root_ctx *ctx) }void -tls_ctx_client_new (struct tls_root_ctx *ctx) +tls_ctx_client_new (struct tls_root_ctx *ctx,unsigned int ssl_flags) {ASSERT (NULL != ctx);CLEAR (*ctx);
Signature · 1 line
1.9.1
Re: [Openvpn-devel] [PATCH 4/4] When tls-version-min is unspecified, revert to original versioning approach.
Quoted 34 lines · Steffan Karger
Hi,
Quoted 6 lines · Steffan Karger
Quoted 3 lines · Gert Doering
On Mon, Apr 21, 2014 at 01:10:04AM -0600, James Yonan wrote: The attached patch is what I intend to commit to release/2.3 *only*, not to master - as agreed at the IRC meeting. "Please ACK" :-)
Sorry, but NAK.
On a more constructive note: attached a new proposal for this patch.
Quoted 1 line
The OpenSSL bits look fine
On a closer look, the wrapping "if (
tls_version_min>TLS_VER_UNSPEC)" intls_ctx_set_options()seems redundant, becauseTLS_VER_UNSPEC<TLS_VER_1_0<TLS_VER_1_1<TLS_VER_1_2and the ifs are checking for "tls_version_min>TLS_VER_1_x". I've removed these changes in the attached patch proposal.Quoted 3 lines
the PolarSSL bits
would also allow forSSL_MINOR_VERSION_0, which is SSLv3 and thus a reduction in security (and actually increases the handshake complexity).To elaborate a bit: The naming is a bit confusing, but
SSL_MAJOR_VERSION_3+SSL_MINOR_VERSION_Omeans SSLv3, ... +SSL_MINOR_VERSION_1means TLSv1.0, ... +SSL_MINOR_VERSION_2means TLSv1.1, etc. If none are given (what would happen in the previous version of the patch), PolarSSL defaults the minimum version to SSLv3 and maximum to TLSv1.2. The attached patch thus removes the wrapping "if (tls_version_min>TLS_VER_UNSPEC)" and sets the default to TLSv1.0 to TLSv1.2 again. That is equal to the behaviour prior to the TLS versioning patch.
Quoted 1 line
-Steffan
This is fine. I can ACK this.
Thanks,
James
[Openvpn-devel] [PATCH applied] Re: When tls-version-min is unspecified, revert to original versioning approach.
In reply to Steffan KargerLatest iteration of this patch has been applied to the release/2.3 branch, thanks to all who helped.
commit a291825f7145679e6d1806029290402d0430b465 (release/2.3)
Author: James Yonan Date: Mon Apr 28 22:52:11 2014 +0200
When tls-version-min is unspecified, revert to original versioning approach.
Signed-off-by: James Yonan <[email protected]...>
Signed-off-by: Gert Doering <[email protected]...>
Signed-off-by: Steffan Karger <[email protected]...>
Acked-by: James Yonan <[email protected]...>
Message-Id: <[email protected]...>
URL: http://article.gmane.org/gmane.network.openvpn.devel/8665
Signed-off-by: Gert Doering <[email protected]...>--
kind regards,
Gert Doering