Hi Frode,

On Wed, Aug 25, 2021 at 01:05:14PM +0200, Frode Nordahl wrote:
> RFC 8996 [0] deprecates the use of TLSv1 and TLSv1.1 for security
> reasons.  Update our default list of protcols to be in compliance.
> 
> Also add TLSv1.3 to the default list of supported protocols.
> 
> 0: https://datatracker.ietf.org/doc/html/rfc8996
> Signed-off-by: Frode Nordahl <frode.nord...@canonical.com>

This patch does two things:
  Deprecate TLSv1 and TLSv1.2
  Add support for TLSv1.3

Can we split them into separate logical patches?

Also, shouldn't we first warn the users about deprecating
TLSv1 and TLSv1.1 for a release period, and then remove from
the default list in the next release?  I mean, this is an user
visible change, so users might need some time to adapt.

What do you think?

fbl

> ---
>  NEWS                  |  7 +++++++
>  lib/ssl-connect.man   |  6 ++++--
>  lib/stream-ssl.c      | 35 +++++++++++++++++++++++++++++------
>  m4/openvswitch.m4     | 16 ++++++++++++++++
>  tests/ovsdb-server.at |  6 ++----
>  5 files changed, 58 insertions(+), 12 deletions(-)
> 
> diff --git a/NEWS b/NEWS
> index 1f2adf718..502e6693a 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -8,6 +8,13 @@ Post-v2.16.0
>         by default.  'other_config:dpdk-socket-limit' can be set equal to
>         the 'other_config:dpdk-socket-mem' to preserve the legacy memory
>         limiting behavior.
> +   - Update default configuration for enabled TLS protocols:
> +     * RFC 8996 deprecates the use of TLSv1 and TLSv1.1 for security reasons.
> +     * Add TLSv1.3 to the default list of enabled protocols when the built 
> with
> +       OpenSSL v1.1.1 and newer.
> +     * The new default is as such: TLSv1.2,TLSv1.3
> +     * As a consequence we no longer support building Open vSwitch with 
> OpenSSL
> +       versions without TLSv1.2 support.
>  
>  
>  v2.16.0 - 16 Aug 2021
> diff --git a/lib/ssl-connect.man b/lib/ssl-connect.man
> index 6e54f77ef..0dd5a29be 100644
> --- a/lib/ssl-connect.man
> +++ b/lib/ssl-connect.man
> @@ -1,10 +1,12 @@
>  .IP "\fB\-\-ssl\-protocols=\fIprotocols\fR"
>  Specifies, in a comma- or space-delimited list, the SSL protocols
>  \fB\*(PN\fR will enable for SSL connections.  Supported
> -\fIprotocols\fR include \fBTLSv1\fR, \fBTLSv1.1\fR, and \fBTLSv1.2\fR.
> +\fIprotocols\fR include \fBTLSv1.2\fR and \fBTLSv1.3\fR depending on
> +which version of OpenSSL Open vSwitch is built with.
>  Regardless of order, the highest protocol supported by both sides will
>  be chosen when making the connection.  The default when this option is
> -omitted is \fBTLSv1,TLSv1.1,TLSv1.2\fR.
> +omitted is \fBTLSv1.2,TLSv1.3\fR if built with a version of OpenSSL that
> +supports \fBTLSv1.3\fR.
>  .
>  .IP "\fB\-\-ssl\-ciphers=\fIciphers\fR"
>  Specifies, in OpenSSL cipher string format, the ciphers \fB\*(PN\fR will 
> diff --git a/lib/stream-ssl.c b/lib/stream-ssl.c
> index 6b4cf6970..954067787 100644
> --- a/lib/stream-ssl.c
> +++ b/lib/stream-ssl.c
> @@ -162,7 +162,13 @@ struct ssl_config_file {
>  static struct ssl_config_file private_key;
>  static struct ssl_config_file certificate;
>  static struct ssl_config_file ca_cert;
> -static char *default_ssl_protocols = "TLSv1,TLSv1.1,TLSv1.2";
> +/* RFC 8996 deprecates the use of TLSv1 and TLSv1.1, users may still specify
> + * them in their configuration, but our defaults are in compliance. */
> +#if OPENSSL_VERSION_NUMBER < 0x10101000L
> +static char *default_ssl_protocols = "TLSv1.2";
> +#else
> +static char *default_ssl_protocols = "TLSv1.2,TLSv1.3";
> +#endif /* OPENSSL_VERSION_NUMBER < 0x10101000L */
>  static char *default_ssl_ciphers = "HIGH:!aNULL:!MD5";
>  static char *ssl_protocols = NULL;
>  static char *ssl_ciphers = NULL;
> @@ -1255,9 +1261,18 @@ stream_ssl_set_protocols(const char *arg)
>          return;
>      }
>  
> +    /* TODO: Using SSL_CTX_set_options to enable individual protocol versions
> +     * is deprecated as of OpenSSL v1.1.0.  Once we can drop support for 
> builds
> +     * with OpenSSL pre v1.1.0 we should use SSL_CTX_set_min_proto_version 
> and
> +     * SSL_CTX_set_max_proto_version instead. */
> +
>      /* Start with all the flags off and turn them on as requested. */
>  #ifndef SSL_OP_NO_SSL_MASK
> -    /* For old OpenSSL without this macro, this is the correct value.  */
> +    /* For old OpenSSL without this macro, this is the correct value.
> +     *
> +     * NOTE: We deliberately did not extend this compatibility macro to
> +     * include SSL_OP_NO_TLSv1_3 because if you do have a version of OpenSSL
> +     * with TLSv1.3 support this macro would be defined by OpenSSL. */
>  #define SSL_OP_NO_SSL_MASK (SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3 | \
>                              SSL_OP_NO_TLSv1 | SSL_OP_NO_TLSv1_1 | \
>                              SSL_OP_NO_TLSv1_2)
> @@ -1272,12 +1287,20 @@ stream_ssl_set_protocols(const char *arg)
>          goto exit;
>      }
>      while (word != NULL) {
> -        long on_flag;
> -        if (!strcasecmp(word, "TLSv1.2")){
> +        long on_flag = 0;
> +        if (!strcasecmp(word, "TLSv1.3")) {
> +#if OPENSSL_VERSION_NUMBER < 0x10101000L
> +            /* OpenSSL versions prior to 1.1.1 do not have TLSv1.3 */
> +            VLOG_ERR("%s: SSL protocol not recognized", word);
> +            goto exit;
> +#else
> +            on_flag = SSL_OP_NO_TLSv1_3;
> +#endif /* OPENSSL_VERSION_NUMBER < 0x10101000 */
> +        } else if (!strcasecmp(word, "TLSv1.2")) {
>              on_flag = SSL_OP_NO_TLSv1_2;
> -        } else if (!strcasecmp(word, "TLSv1.1")){
> +        } else if (!strcasecmp(word, "TLSv1.1")) {
>              on_flag = SSL_OP_NO_TLSv1_1;
> -        } else if (!strcasecmp(word, "TLSv1")){
> +        } else if (!strcasecmp(word, "TLSv1")) {
>              on_flag = SSL_OP_NO_TLSv1;
>          } else {
>              VLOG_ERR("%s: SSL protocol not recognized", word);
> diff --git a/m4/openvswitch.m4 b/m4/openvswitch.m4
> index 772825a71..429aacd11 100644
> --- a/m4/openvswitch.m4
> +++ b/m4/openvswitch.m4
> @@ -257,6 +257,22 @@ OpenFlow connections over SSL will not be supported.
>            else
>              AC_MSG_ERROR([Cannot find openssl (use --disable-ssl to 
> configure without SSL support)])
>            fi])
> +       OPENSSL_SUPPORTS_TLS1_2=no
> +       if test $HAVE_OPENSSL = yes; then
> +          save_CPPFLAGS=$CPPFLAGS
> +          CPPFLAGS="$CPPFLAGS $SSL_INCLUDES"
> +          AC_CHECK_DECL([TLS1_2_VERSION], [OPENSSL_SUPPORTS_TLS1_2=yes],
> +                        [], [#include <openssl/ssl.h>])
> +          if test $OPENSSL_SUPPORTS_TLS1_2 = no; then
> +            if test "$ssl" = check; then
> +              AC_MSG_WARN([Cannot find openssl with TLSv1.2 support. 
> OpenFlow connections over SSL will not be supported. (You may use 
> --disable-ssl to suppress this warning.)])
> +              HAVE_OPENSSL=no
> +            else
> +              AC_MSG_ERROR([Cannot find openssl with TLSv1.2 support. (use 
> --disable-ssl to configure without SSL support)])
> +            fi
> +          fi
> +          CPPFLAGS=$save_CPPFLAGS
> +       fi
>     else
>         HAVE_OPENSSL=no
>     fi
> diff --git a/tests/ovsdb-server.at b/tests/ovsdb-server.at
> index ac243d6a7..b52ca150c 100644
> --- a/tests/ovsdb-server.at
> +++ b/tests/ovsdb-server.at
> @@ -575,7 +575,6 @@ AT_CHECK(
>          "row": {"private_key": "'"$PKIDIR/testpki-privkey2.pem"'",
>                  "certificate": "'"$PKIDIR/testpki-cert2.pem"'",
>                  "ca_cert": "'"$PKIDIR/testpki-cacert.pem"'",
> -                "ssl_protocols": "'"TLSv1.2,TLSv1.1"'",
>                  "ssl_ciphers": 
> "'"HIGH:!aNULL:!MD5:!ECDHE-ECDSA-AES256-GCM-SHA384"'"}}]']],
>    [0], [ignore], [ignore])
>  on_exit 'kill `cat *.pid`'
> @@ -594,7 +593,6 @@ AT_CHECK(
>          --private-key=$PKIDIR/testpki-privkey.pem \
>          --certificate=$PKIDIR/testpki-cert.pem \
>          --ca-cert=$PKIDIR/testpki-cacert.pem \
> -        --ssl-protocols=TLSv1.2,TLSv1.1 \
>          --ssl-ciphers=HIGH:!aNULL:!MD5 \
>          transact ssl:127.0.0.1:$SSL_PORT \
>          '["mydb",
> @@ -608,7 +606,7 @@ AT_CHECK_UNQUOTED(
>    [cat output], [0],
>    [[@<:@{"rows":@<:@{"private_key":"$PKIDIR/testpki-privkey2.pem"}@:>@}@:>@
>  ]], [ignore])
> -# Check that when the server has TLSv1.1+ and the client has
> +# Check that when the server has TLSv1.2+ and the client has
>  # TLSv1 that the connection fails.
>  AT_CHECK(
>    [[ovsdb-client \
> @@ -638,7 +636,7 @@ AT_CHECK(
>          --private-key=$PKIDIR/testpki-privkey.pem \
>          --certificate=$PKIDIR/testpki-cert.pem \
>          --ca-cert=$PKIDIR/testpki-cacert.pem \
> -        --ssl-protocols=TLSv1.2,TLSv1.1 \
> +        --ssl-protocols=TLSv1.2 \
>          --ssl-ciphers=ECDHE-ECDSA-AES256-GCM-SHA384 \
>          transact ssl:127.0.0.1:$SSL_PORT \
>          '["mydb",
> -- 
> 2.32.0
> 
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

-- 
fbl
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to