Thanks!  Applied to master.

On Tue, Mar 26, 2019 at 02:03:53PM -0700, Yifeng Sun wrote:
> Looks good to me, thanks.
> 
> Tested-by: Yifeng Sun <pkusunyif...@gmail.com>
> Reviewed-by: Yifeng Sun <pkusunyif...@gmail.com>
> 
> On Wed, Mar 20, 2019 at 5:39 PM Ben Pfaff <b...@ovn.org> wrote:
> >
> > This TLS extension, introduced in RFC 3546, allows the server to know what
> > host the client believes it is contacting, the TLS equivalent of the Host:
> > header in HTTP.
> >
> > Requested-by: Shivaram Mysore <smys...@servicefractal.com>
> > Signed-off-by: Ben Pfaff <b...@ovn.org>
> > ---
> >  NEWS               |  2 ++
> >  lib/stream-ssl.c   | 63 ++++++++++++++++++++++++++++++++++++++++++----
> >  m4/openvswitch.m4  | 23 ++++++++++++++---
> >  tests/atlocal.in   |  2 ++
> >  tests/ovs-vsctl.at | 20 +++++++++++++++
> >  5 files changed, 102 insertions(+), 8 deletions(-)
> >
> > diff --git a/NEWS b/NEWS
> > index 1e4744dbd244..0f8d02817e61 100644
> > --- a/NEWS
> > +++ b/NEWS
> > @@ -25,6 +25,8 @@ Post-v2.11.0
> >     - OVN:
> >       * Select IPAM mac_prefix in a random manner if not provided by the 
> > user
> >     - New QoS type "linux-netem" on Linux.
> > +   - Added support for TLS Server Name Indication (SNI).
> > +
> >
> >  v2.11.0 - 19 Feb 2019
> >  ---------------------
> > diff --git a/lib/stream-ssl.c b/lib/stream-ssl.c
> > index fed71801b823..81f2409965b2 100644
> > --- a/lib/stream-ssl.c
> > +++ b/lib/stream-ssl.c
> > @@ -1,5 +1,5 @@
> >  /*
> > - * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016 
> > Nicira, Inc.
> > + * Copyright (c) 2008-2016, 2019 Nicira, Inc.
> >   *
> >   * Licensed under the Apache License, Version 2.0 (the "License");
> >   * you may not use this file except in compliance with the License.
> > @@ -220,9 +220,19 @@ want_to_poll_events(int want)
> >      }
> >  }
> >
> > -/* Takes ownership of 'name'. */
> > +/* Creates a new SSL connection based on socket 'fd', as either a client 
> > or a
> > + * server according to 'type', initially in 'state'.  On success, returns 
> > 0 and
> > + * stores the new stream in '*streamp', otherwise returns an errno value 
> > and
> > + * doesn't bother with '*streamp'.
> > + *
> > + * Takes ownership of 'name', which should be the name of the connection 
> > in the
> > + * format that would be used to connect to it, e.g. "ssl:1.2.3.4:5".
> > + *
> > + * For client connections, 'server_name' should be the host name of the 
> > server
> > + * being connected to, for use with SSL SNI (server name indication).  
> > Takes
> > + * ownership of 'server_name'. */
> >  static int
> > -new_ssl_stream(char *name, int fd, enum session_type type,
> > +new_ssl_stream(char *name, char *server_name, int fd, enum session_type 
> > type,
> >                 enum ssl_state state, struct stream **streamp)
> >  {
> >      struct ssl_stream *sslv;
> > @@ -274,6 +284,14 @@ new_ssl_stream(char *name, int fd, enum session_type 
> > type,
> >      if (!verify_peer_cert || (bootstrap_ca_cert && type == CLIENT)) {
> >          SSL_set_verify(ssl, SSL_VERIFY_NONE, NULL);
> >      }
> > +#if OPENSSL_SUPPORTS_SNI
> > +    if (server_name && !SSL_set_tlsext_host_name(ssl, server_name)) {
> > +        VLOG_ERR("%s: failed to set server name indication (%s)",
> > +                 server_name, ERR_error_string(ERR_get_error(), NULL));
> > +        retval = ENOPROTOOPT;
> > +        goto error;
> > +    }
> > +#endif
> >
> >      /* Create and return the ssl_stream. */
> >      sslv = xmalloc(sizeof *sslv);
> > @@ -293,6 +311,7 @@ new_ssl_stream(char *name, int fd, enum session_type 
> > type,
> >      }
> >
> >      *streamp = &sslv->stream;
> > +    free(server_name);
> >      return 0;
> >
> >  error:
> > @@ -301,6 +320,7 @@ error:
> >      }
> >      closesocket(fd);
> >      free(name);
> > +    free(server_name);
> >      return retval;
> >  }
> >
> > @@ -311,6 +331,30 @@ ssl_stream_cast(struct stream *stream)
> >      return CONTAINER_OF(stream, struct ssl_stream, stream);
> >  }
> >
> > +/* Extracts and returns the server name from 'suffix'.  The caller must
> > + * eventually free it.
> > + *
> > + * Returns NULL if there is no server name, and particularly if it is an IP
> > + * address rather than a host name, since RFC 3546 is explicit that IP
> > + * addresses are unsuitable as server name indication (SNI). */
> > +static char *
> > +get_server_name(const char *suffix_)
> > +{
> > +    char *suffix = xstrdup(suffix_);
> > +
> > +    char *host, *port;
> > +    inet_parse_host_port_tokens(suffix, &host, &port);
> > +
> > +    ovs_be32 ipv4;
> > +    struct in6_addr ipv6;
> > +    char *server_name = (ip_parse(host, &ipv4) || ipv6_parse(host, &ipv6)
> > +                         ? NULL : xstrdup(host));
> > +
> > +    free(suffix);
> > +
> > +    return server_name;
> > +}
> > +
> >  static int
> >  ssl_open(const char *name, char *suffix, struct stream **streamp, uint8_t 
> > dscp)
> >  {
> > @@ -325,7 +369,8 @@ ssl_open(const char *name, char *suffix, struct stream 
> > **streamp, uint8_t dscp)
> >                               dscp);
> >      if (fd >= 0) {
> >          int state = error ? STATE_TCP_CONNECTING : STATE_SSL_CONNECTING;
> > -        return new_ssl_stream(xstrdup(name), fd, CLIENT, state, streamp);
> > +        return new_ssl_stream(xstrdup(name), get_server_name(suffix),
> > +                              fd, CLIENT, state, streamp);
> >      } else {
> >          VLOG_ERR("%s: connect: %s", name, ovs_strerror(error));
> >          return error;
> > @@ -514,6 +559,14 @@ ssl_connect(struct stream *stream)
> >              VLOG_INFO("rejecting SSL connection during bootstrap race 
> > window");
> >              return EPROTO;
> >          } else {
> > +#if OPENSSL_SUPPORTS_SNI
> > +            const char *servername = SSL_get_servername(
> > +                sslv->ssl, TLSEXT_NAMETYPE_host_name);
> > +            if (servername) {
> > +                VLOG_DBG("connection indicated server name %s", 
> > servername);
> > +            }
> > +#endif
> > +
> >              char *cn = get_peer_common_name(sslv);
> >
> >              if (cn) {
> > @@ -899,7 +952,7 @@ pssl_accept(struct pstream *pstream, struct stream 
> > **new_streamp)
> >      ds_put_cstr(&name, "ssl:");
> >      ss_format_address(&ss, &name);
> >      ds_put_format(&name, ":%"PRIu16, ss_get_port(&ss));
> > -    return new_ssl_stream(ds_steal_cstr(&name), new_fd, SERVER,
> > +    return new_ssl_stream(ds_steal_cstr(&name), NULL, new_fd, SERVER,
> >                            STATE_SSL_CONNECTING, new_streamp);
> >  }
> >
> > diff --git a/m4/openvswitch.m4 b/m4/openvswitch.m4
> > index 41042c98e286..b599f17d77a1 100644
> > --- a/m4/openvswitch.m4
> > +++ b/m4/openvswitch.m4
> > @@ -1,6 +1,6 @@
> >  # -*- autoconf -*-
> >
> > -# Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016 
> > Nicira, Inc.
> > +# Copyright (c) 2008-2016, 2019 Nicira, Inc.
> >  #
> >  # Licensed under the Apache License, Version 2.0 (the "License");
> >  # you may not use this file except in compliance with the License.
> > @@ -285,7 +285,24 @@ OpenFlow connections over SSL will not be supported.
> >     AM_CONDITIONAL([HAVE_OPENSSL], [test "$HAVE_OPENSSL" = yes])
> >     if test "$HAVE_OPENSSL" = yes; then
> >        AC_DEFINE([HAVE_OPENSSL], [1], [Define to 1 if OpenSSL is 
> > installed.])
> > -   fi])
> > +   fi
> > +
> > +   OPENSSL_SUPPORTS_SNI=no
> > +   if test $HAVE_OPENSSL = yes; then
> > +      save_CPPFLAGS=$CPPFLAGS
> > +      CPPFLAGS="$CPPFLAGS $SSL_INCLUDES"
> > +      AC_CHECK_DECL([SSL_set_tlsext_host_name], [OPENSSL_SUPPORTS_SNI=yes],
> > +                    [], [#include <openssl/ssl.h>
> > +])
> > +      if test $OPENSSL_SUPPORTS_SNI = yes; then
> > +        AC_DEFINE(
> > +          [OPENSSL_SUPPORTS_SNI], [1],
> > +          [Define to 1 if OpenSSL supports Server Name Indication (SNI).])
> > +      fi
> > +      CPPFLAGS=$save_CPPFLAGS
> > +   fi
> > +   AC_SUBST([OPENSSL_SUPPORTS_SNI])
> > +])
> >
> >  dnl Checks for libraries needed by lib/socket-util.c.
> >  AC_DEFUN([OVS_CHECK_SOCKET_LIBS],
> > @@ -691,7 +708,7 @@ AC_DEFUN([OVS_CHECK_CXX],
> >
> >  dnl Checks for unbound library.
> >  AC_DEFUN([OVS_CHECK_UNBOUND],
> > -  [AC_CHECK_LIB(unbound, ub_ctx_create, [HAVE_UNBOUND=yes])
> > +  [AC_CHECK_LIB(unbound, ub_ctx_create, [HAVE_UNBOUND=yes], 
> > [HAVE_UNBOUND=no])
> >     if test "$HAVE_UNBOUND" = yes; then
> >       AC_DEFINE([HAVE_UNBOUND], [1], [Define to 1 if unbound is detected.])
> >       LIBS="$LIBS -lunbound"
> > diff --git a/tests/atlocal.in b/tests/atlocal.in
> > index 5eff0a0aa216..f37f15430ccd 100644
> > --- a/tests/atlocal.in
> > +++ b/tests/atlocal.in
> > @@ -1,8 +1,10 @@
> >  # -*- shell-script -*-
> >  HAVE_OPENSSL='@HAVE_OPENSSL@'
> > +OPENSSL_SUPPORTS_SNI='@OPENSSL_SUPPORTS_SNI@'
> >  HAVE_PYTHON='@HAVE_PYTHON@'
> >  HAVE_PYTHON2='@HAVE_PYTHON2@'
> >  HAVE_PYTHON3='@HAVE_PYTHON3@'
> > +HAVE_UNBOUND='@HAVE_UNBOUND@'
> >  EGREP='@EGREP@'
> >
> >  if test x"$PYTHON" = x; then
> > diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at
> > index 6f37c0da781a..77604c58a2bc 100644
> > --- a/tests/ovs-vsctl.at
> > +++ b/tests/ovs-vsctl.at
> > @@ -1422,3 +1422,23 @@ AT_CHECK([ovs-vsctl -t 5 --no-wait 
> > --db=ssl:127.0.0.1:$SSL_PORT --private-key=$P
> >
> >  OVS_VSCTL_CLEANUP
> >  AT_CLEANUP
> > +
> > +AT_SETUP([TLS server name indication (SNI)])
> > +AT_KEYWORDS([ovsdb server positive ssl tls sni])
> > +AT_SKIP_IF([test "$HAVE_OPENSSL" = no])
> > +AT_SKIP_IF([test "$OPENSSL_SUPPORTS_SNI" = no])
> > +AT_SKIP_IF([test "$HAVE_UNBOUND" = no])
> > +OVSDB_INIT([conf.db])
> > +PKIDIR=$abs_top_builddir/tests
> > +AT_CHECK([ovsdb-server --log-file --detach --no-chdir --pidfile 
> > --private-key=$PKIDIR/testpki-privkey2.pem 
> > --certificate=$PKIDIR/testpki-cert2.pem 
> > --ca-cert=$PKIDIR/testpki-cacert.pem --remote=pssl:0:127.0.0.1 
> > -vPATTERN:file:%m -vstream_ssl conf.db], [0], [ignore], [ignore])
> > +PARSE_LISTENING_PORT([ovsdb-server.log], [SSL_PORT])
> > +
> > +AT_CHECK([ovs-vsctl -t 5 --no-wait --db=ssl:localhost:$SSL_PORT 
> > --private-key=$PKIDIR/testpki-privkey.pem 
> > --certificate=$PKIDIR/testpki-cert.pem 
> > --bootstrap-ca-cert=$PKIDIR/testpki-cacert.pem add-br br0])
> > +
> > +AT_CAPTURE_FILE([ovsdb-server.log])
> > +AT_CHECK([grep "server name" ovsdb-server.log], [0],
> > +         [connection indicated server name localhost
> > +])
> > +
> > +OVS_VSCTL_CLEANUP
> > +AT_CLEANUP
> > --
> > 2.20.1
> >
> > _______________________________________________
> > dev mailing list
> > d...@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to