| Message ID | 20191122143315.8564-1-arne@rfc2549.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [Openvpn-devel,v7,1/2] Make tls_version_max return the actual maximum version | expand |
Hi, On Fri, Nov 22, 2019 at 9:34 AM Arne Schwabe <arne@rfc2549.org> wrote: > Before OpenSSL 1.1.1 there could be no mismatch between > compiled and actual OpenSSL version. With OpenSSL 1.1.1 we need > runtime detection to detect the actual best TLS version supported. > > Allowing this runtime detection also allows removing some of the > TLS 1.3/OpenSSL 1.1.1 #ifdefs > > Without this patch tls-min-version 1.3 or-highest will actually > downgrade to TLS 1.3 I think that should read "TLS 1.2" not "TLS 1.3" > in the "compiled with 1.1.0 and linked against > 1.1.1" scenario. > > Signed-off-by: Arne Schwabe <arne@rfc2549.org> > --- > src/openvpn/ssl.c | 11 +++++------ > src/openvpn/ssl_openssl.c | 39 ++++++++++++++++++++++++++++++++++++--- > 2 files changed, 41 insertions(+), 9 deletions(-) > > diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c > index 4455ebb8..e708fc93 100644 > --- a/src/openvpn/ssl.c > +++ b/src/openvpn/ssl.c > @@ -4194,12 +4194,11 @@ show_available_tls_ciphers(const char *cipher_list, > { > printf("Available TLS Ciphers, listed in order of preference:\n"); > > -#if (ENABLE_CRYPTO_OPENSSL && OPENSSL_VERSION_NUMBER >= 0x1010100fL) > - printf("\nFor TLS 1.3 and newer (--tls-ciphersuites):\n\n"); > - show_available_tls_ciphers_list(cipher_list_tls13, tls_cert_profile, > true); > -#else > - (void) cipher_list_tls13; /* Avoid unused warning */ > -#endif > + if (tls_version_max() >= TLS_VER_1_3) > + { > + printf("\nFor TLS 1.3 and newer (--tls-ciphersuites):\n\n"); > + show_available_tls_ciphers_list(cipher_list_tls13, > tls_cert_profile, true); > + } > > printf("\nFor TLS 1.2 and older (--tls-cipher):\n\n"); > show_available_tls_ciphers_list(cipher_list, tls_cert_profile, false); > diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c > index 07916c3c..a080338e 100644 > --- a/src/openvpn/ssl_openssl.c > +++ b/src/openvpn/ssl_openssl.c > @@ -215,7 +215,26 @@ int > tls_version_max(void) > { > #if defined(TLS1_3_VERSION) > + /* If this is defined we can safely assume TLS 1.3 support */ > return TLS_VER_1_3; > +#elif OPENSSL_VERSION_NUMBER >= 0x10100000L > + /* > + * If TLS_VER_1_3 is not defined, we were compiled against a version > that > + * did not support TLS 1.3. > + * > + * However, the library we are *linked* against might be OpenSSL 1.1.1 > + * and therefore supports TLS 1.3. This needs to be checked at runtime > + * since we can be compiled against 1.1.0 and then the library can be > + * upgraded to 1.1.1 > + */ > + if (OpenSSL_version_num() >= 0x1010100fL) > + { > + return TLS_VER_1_3; > + } > + else > + { > + return TLS_VER_1_2; > + } > #elif defined(TLS1_2_VERSION) || defined(SSL_OP_NO_TLSv1_2) > return TLS_VER_1_2; > #elif defined(TLS1_1_VERSION) || defined(SSL_OP_NO_TLSv1_1) > @@ -241,12 +260,25 @@ openssl_tls_version(int ver) > { > return TLS1_2_VERSION; > } > -#if defined(TLS1_3_VERSION) > else if (ver == TLS_VER_1_3) > { > + /* > + * Supporting the library upgraded to TLS1.3 without recompile > + * is enough to support here with a simple constant that the same > + * as in the TLS 1.3, so spec it is very unlikely that OpenSSL > + * will change this constant > I have no idea what that comment means, but I get the idea and will pass :) > + */ > +#ifndef TLS1_3_VERSION > + /* > + * We do not want to define TLS_VER_1_3 if not defined > + * since other parts of the code use the existance of this macro > + * as proxy for TLS 1.3 support > + */ > + return 0x0304; > +#else > return TLS1_3_VERSION; > - } > #endif > + } > return 0; > } > > @@ -2015,7 +2047,8 @@ show_available_tls_ciphers_list(const char > *cipher_list, > #if defined(TLS1_3_VERSION) > if (tls13) > { > - SSL_CTX_set_min_proto_version(tls_ctx.ctx, TLS1_3_VERSION); > + SSL_CTX_set_min_proto_version(tls_ctx.ctx, > + openssl_tls_version(TLS_VER_1_3)); > tls_ctx_restrict_ciphers_tls13(&tls_ctx, cipher_list); > } > else > -- > Tested using OpenSSL 1.1.0 based build with OpenSSL 1.1.1 at runtime. Acked-by: selva.nair@gmail.com <div dir="ltr"><div>Hi,</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Nov 22, 2019 at 9:34 AM Arne Schwabe <<a href="mailto:arne@rfc2549.org">arne@rfc2549.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Before OpenSSL 1.1.1 there could be no mismatch between<br> compiled and actual OpenSSL version. With OpenSSL 1.1.1 we need<br> runtime detection to detect the actual best TLS version supported.<br> <br> Allowing this runtime detection also allows removing some of the<br> TLS 1.3/OpenSSL 1.1.1 #ifdefs<br> <br> Without this patch tls-min-version 1.3 or-highest will actually<br> downgrade to TLS 1.3 </blockquote><div><br></div><div>I think that should read "TLS 1.2" not "TLS 1.3"</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">in the "compiled with 1.1.0 and linked against<br> 1.1.1" scenario.<br> <br> Signed-off-by: Arne Schwabe <<a href="mailto:arne@rfc2549.org" target="_blank">arne@rfc2549.org</a>><br> ---<br> src/openvpn/ssl.c | 11 +++++------<br> src/openvpn/ssl_openssl.c | 39 ++++++++++++++++++++++++++++++++++++---<br> 2 files changed, 41 insertions(+), 9 deletions(-)<br> <br> diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c<br> index 4455ebb8..e708fc93 100644<br> --- a/src/openvpn/ssl.c<br> +++ b/src/openvpn/ssl.c<br> @@ -4194,12 +4194,11 @@ show_available_tls_ciphers(const char *cipher_list,<br> {<br> printf("Available TLS Ciphers, listed in order of preference:\n");<br> <br> -#if (ENABLE_CRYPTO_OPENSSL && OPENSSL_VERSION_NUMBER >= 0x1010100fL)<br> - printf("\nFor TLS 1.3 and newer (--tls-ciphersuites):\n\n");<br> - show_available_tls_ciphers_list(cipher_list_tls13, tls_cert_profile, true);<br> -#else<br> - (void) cipher_list_tls13; /* Avoid unused warning */<br> -#endif<br> + if (tls_version_max() >= TLS_VER_1_3)<br> + {<br> + printf("\nFor TLS 1.3 and newer (--tls-ciphersuites):\n\n");<br> + show_available_tls_ciphers_list(cipher_list_tls13, tls_cert_profile, true);<br> + }<br> <br> printf("\nFor TLS 1.2 and older (--tls-cipher):\n\n");<br> show_available_tls_ciphers_list(cipher_list, tls_cert_profile, false);<br> diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c<br> index 07916c3c..a080338e 100644<br> --- a/src/openvpn/ssl_openssl.c<br> +++ b/src/openvpn/ssl_openssl.c<br> @@ -215,7 +215,26 @@ int<br> tls_version_max(void)<br> {<br> #if defined(TLS1_3_VERSION)<br> + /* If this is defined we can safely assume TLS 1.3 support */<br> return TLS_VER_1_3;<br> +#elif OPENSSL_VERSION_NUMBER >= 0x10100000L<br> + /*<br> + * If TLS_VER_1_3 is not defined, we were compiled against a version that<br> + * did not support TLS 1.3.<br> + *<br> + * However, the library we are *linked* against might be OpenSSL 1.1.1<br> + * and therefore supports TLS 1.3. This needs to be checked at runtime<br> + * since we can be compiled against 1.1.0 and then the library can be<br> + * upgraded to 1.1.1<br> + */<br> + if (OpenSSL_version_num() >= 0x1010100fL)<br> + {<br> + return TLS_VER_1_3;<br> + }<br> + else<br> + {<br> + return TLS_VER_1_2;<br> + }<br> #elif defined(TLS1_2_VERSION) || defined(SSL_OP_NO_TLSv1_2)<br> return TLS_VER_1_2;<br> #elif defined(TLS1_1_VERSION) || defined(SSL_OP_NO_TLSv1_1)<br> @@ -241,12 +260,25 @@ openssl_tls_version(int ver)<br> {<br> return TLS1_2_VERSION;<br> }<br> -#if defined(TLS1_3_VERSION)<br> else if (ver == TLS_VER_1_3)<br> {<br> + /*<br> + * Supporting the library upgraded to TLS1.3 without recompile<br> + * is enough to support here with a simple constant that the same<br> + * as in the TLS 1.3, so spec it is very unlikely that OpenSSL<br> + * will change this constant<br></blockquote><div><br></div><div>I have no idea what that comment means, but I get the idea and will pass :)</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> + */<br> +#ifndef TLS1_3_VERSION<br> + /*<br> + * We do not want to define TLS_VER_1_3 if not defined<br> + * since other parts of the code use the existance of this macro<br> + * as proxy for TLS 1.3 support<br> + */<br> + return 0x0304;<br> +#else<br> return TLS1_3_VERSION;<br> - }<br> #endif<br> + }<br> return 0;<br> }<br> <br> @@ -2015,7 +2047,8 @@ show_available_tls_ciphers_list(const char *cipher_list,<br> #if defined(TLS1_3_VERSION)<br> if (tls13)<br> {<br> - SSL_CTX_set_min_proto_version(tls_ctx.ctx, TLS1_3_VERSION);<br> + SSL_CTX_set_min_proto_version(tls_ctx.ctx,<br> + openssl_tls_version(TLS_VER_1_3));<br> tls_ctx_restrict_ciphers_tls13(&tls_ctx, cipher_list);<br> }<br> else<br> -- <br></blockquote><div><br></div><div>Tested using OpenSSL 1.1.0 based build with OpenSSL 1.1.1 at</div><div>runtime.</div><div><br></div><div>Acked-by: <a href="mailto:selva.nair@gmail.com">selva.nair@gmail.com</a></div></div></div>
Am 23.11.19 um 03:04 schrieb Selva Nair: > Hi, > > On Fri, Nov 22, 2019 at 9:34 AM Arne Schwabe <arne@rfc2549.org > <mailto:arne@rfc2549.org>> wrote: > > Before OpenSSL 1.1.1 there could be no mismatch between > compiled and actual OpenSSL version. With OpenSSL 1.1.1 we need > runtime detection to detect the actual best TLS version supported. > > Allowing this runtime detection also allows removing some of the > TLS 1.3/OpenSSL 1.1.1 #ifdefs > > Without this patch tls-min-version 1.3 or-highest will actually > downgrade to TLS 1.3 > > > I think that should read "TLS 1.2" not "TLS 1.3" Yes, you are correct. @David, Gert, can you fix this one on the commit message or do you want a new version for that? Arne
Your patch has been applied to the master branch.
Thanks for the confirmation about that commit message change - it seemed
to make sense, but to hear from the author is better :-) - changed on
the fly, as requested.
Lightly tested with OpenSSL 1.0.2o, OpenSSL 1.0.1l, OpenSSL 1.1.1d.
*Not* tested with "compiled with 1.1.0, dynlinked to 1.1.1, does TLS 1.3"
because too lazy right now to set up things to properly verify that.
commit 6328aef94a748bd9859ae5cd264b7e50fbb8a325
Author: Arne Schwabe
Date: Fri Nov 22 15:33:14 2019 +0100
Make tls_version_max return the actual maximum version
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Selva Nair <selva.nair@gmail.com>
Message-Id: <20191122143315.8564-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19186.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
--
kind regards,
Gert Doering
diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index 4455ebb8..e708fc93 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -4194,12 +4194,11 @@ show_available_tls_ciphers(const char *cipher_list, { printf("Available TLS Ciphers, listed in order of preference:\n"); -#if (ENABLE_CRYPTO_OPENSSL && OPENSSL_VERSION_NUMBER >= 0x1010100fL) - printf("\nFor TLS 1.3 and newer (--tls-ciphersuites):\n\n"); - show_available_tls_ciphers_list(cipher_list_tls13, tls_cert_profile, true); -#else - (void) cipher_list_tls13; /* Avoid unused warning */ -#endif + if (tls_version_max() >= TLS_VER_1_3) + { + printf("\nFor TLS 1.3 and newer (--tls-ciphersuites):\n\n"); + show_available_tls_ciphers_list(cipher_list_tls13, tls_cert_profile, true); + } printf("\nFor TLS 1.2 and older (--tls-cipher):\n\n"); show_available_tls_ciphers_list(cipher_list, tls_cert_profile, false); diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c index 07916c3c..a080338e 100644 --- a/src/openvpn/ssl_openssl.c +++ b/src/openvpn/ssl_openssl.c @@ -215,7 +215,26 @@ int tls_version_max(void) { #if defined(TLS1_3_VERSION) + /* If this is defined we can safely assume TLS 1.3 support */ return TLS_VER_1_3; +#elif OPENSSL_VERSION_NUMBER >= 0x10100000L + /* + * If TLS_VER_1_3 is not defined, we were compiled against a version that + * did not support TLS 1.3. + * + * However, the library we are *linked* against might be OpenSSL 1.1.1 + * and therefore supports TLS 1.3. This needs to be checked at runtime + * since we can be compiled against 1.1.0 and then the library can be + * upgraded to 1.1.1 + */ + if (OpenSSL_version_num() >= 0x1010100fL) + { + return TLS_VER_1_3; + } + else + { + return TLS_VER_1_2; + } #elif defined(TLS1_2_VERSION) || defined(SSL_OP_NO_TLSv1_2) return TLS_VER_1_2; #elif defined(TLS1_1_VERSION) || defined(SSL_OP_NO_TLSv1_1) @@ -241,12 +260,25 @@ openssl_tls_version(int ver) { return TLS1_2_VERSION; } -#if defined(TLS1_3_VERSION) else if (ver == TLS_VER_1_3) { + /* + * Supporting the library upgraded to TLS1.3 without recompile + * is enough to support here with a simple constant that the same + * as in the TLS 1.3, so spec it is very unlikely that OpenSSL + * will change this constant + */ +#ifndef TLS1_3_VERSION + /* + * We do not want to define TLS_VER_1_3 if not defined + * since other parts of the code use the existance of this macro + * as proxy for TLS 1.3 support + */ + return 0x0304; +#else return TLS1_3_VERSION; - } #endif + } return 0; } @@ -2015,7 +2047,8 @@ show_available_tls_ciphers_list(const char *cipher_list, #if defined(TLS1_3_VERSION) if (tls13) { - SSL_CTX_set_min_proto_version(tls_ctx.ctx, TLS1_3_VERSION); + SSL_CTX_set_min_proto_version(tls_ctx.ctx, + openssl_tls_version(TLS_VER_1_3)); tls_ctx_restrict_ciphers_tls13(&tls_ctx, cipher_list); } else
Before OpenSSL 1.1.1 there could be no mismatch between compiled and actual OpenSSL version. With OpenSSL 1.1.1 we need runtime detection to detect the actual best TLS version supported. Allowing this runtime detection also allows removing some of the TLS 1.3/OpenSSL 1.1.1 #ifdefs Without this patch tls-min-version 1.3 or-highest will actually downgrade to TLS 1.3 in the "compiled with 1.1.0 and linked against 1.1.1" scenario. Signed-off-by: Arne Schwabe <arne@rfc2549.org> --- src/openvpn/ssl.c | 11 +++++------ src/openvpn/ssl_openssl.c | 39 ++++++++++++++++++++++++++++++++++++--- 2 files changed, 41 insertions(+), 9 deletions(-)