[ovs-dev] Add support for specifying SSL connection parameters to ovsdb

---
 AUTHORS                                   |  1 +
 lib/automake.mk                           |  2 +
 lib/ssl-connect-syn.man                   |  5 +++
 lib/ssl-connect.man                       | 16 +++++++
 lib/stream-ssl.c                          | 70 +++++++++++++++++++++++++++++++
 lib/stream-ssl.h                          | 20 ++++++++-
 manpages.mk                               |  8 ++++
 ovn/controller-vtep/ovn-controller-vtep.c |  3 +-
 ovn/controller/ovn-controller.c           |  3 +-
 ovn/northd/ovn-northd.c                   |  1 +
 ovn/utilities/ovn-nbctl.c                 |  3 +-
 ovn/utilities/ovn-sbctl.c                 |  3 +-
 ovn/utilities/ovn-trace.c                 |  1 +
 ovsdb/ovsdb-client.1.in                   |  3 ++
 ovsdb/ovsdb-client.c                      |  3 +-
 ovsdb/ovsdb-server.1.in                   |  3 ++
 ovsdb/ovsdb-server.c                      | 23 ++++++++--
 tests/ovsdb-server.at                     | 68 +++++++++++++++++++++++++++++-
 tests/test-jsonrpc.c                      |  3 +-
 utilities/ovs-ofctl.c                     |  3 +-
 utilities/ovs-testcontroller.c            |  3 +-
 utilities/ovs-vsctl.c                     |  3 +-
 vswitchd/ovs-vswitchd.c                   |  1 +
 vtep/vtep-ctl.c                           |  3 +-
 24 files changed, 234 insertions(+), 18 deletions(-)
 create mode 100644 lib/ssl-connect-syn.man
 create mode 100644 lib/ssl-connect.man

Third time is the charm :) Sent via git send-email this time around.
Thanks for your patience and sorry for the trouble.

Cheers,

Ethan

On Thu, Oct 6, 2016 at 4:21 PM, Ethan Rahn <erahn@arista.com> wrote:
> ---
>  AUTHORS                                   |  1 +
>  lib/automake.mk                           |  2 +
>  lib/ssl-connect-syn.man                   |  5 +++
>  lib/ssl-connect.man                       | 16 +++++++
>  lib/stream-ssl.c                          | 70 +++++++++++++++++++++++++++++++
>  lib/stream-ssl.h                          | 20 ++++++++-
>  manpages.mk                               |  8 ++++
>  ovn/controller-vtep/ovn-controller-vtep.c |  3 +-
>  ovn/controller/ovn-controller.c           |  3 +-
>  ovn/northd/ovn-northd.c                   |  1 +
>  ovn/utilities/ovn-nbctl.c                 |  3 +-
>  ovn/utilities/ovn-sbctl.c                 |  3 +-
>  ovn/utilities/ovn-trace.c                 |  1 +
>  ovsdb/ovsdb-client.1.in                   |  3 ++
>  ovsdb/ovsdb-client.c                      |  3 +-
>  ovsdb/ovsdb-server.1.in                   |  3 ++
>  ovsdb/ovsdb-server.c                      | 23 ++++++++--
>  tests/ovsdb-server.at                     | 68 +++++++++++++++++++++++++++++-
>  tests/test-jsonrpc.c                      |  3 +-
>  utilities/ovs-ofctl.c                     |  3 +-
>  utilities/ovs-testcontroller.c            |  3 +-
>  utilities/ovs-vsctl.c                     |  3 +-
>  vswitchd/ovs-vswitchd.c                   |  1 +
>  vtep/vtep-ctl.c                           |  3 +-
>  24 files changed, 234 insertions(+), 18 deletions(-)
>  create mode 100644 lib/ssl-connect-syn.man
>  create mode 100644 lib/ssl-connect.man
>
> diff --git a/AUTHORS b/AUTHORS
> index c089d59..197142f 100644
> --- a/AUTHORS
> +++ b/AUTHORS
> @@ -80,6 +80,7 @@ Eitan Eliahu            eliahue@vmware.com
>  Eohyung Lee             liquidnuker@gmail.com
>  Eric Sesterhenn         eric.sesterhenn@lsexperts.de
>  Ethan J. Jackson        ejj@eecs.berkeley.edu
> +Ethan Rahn              erahn@arista.com
>  Eziz Durdyyev           ezizdurdy@gmail.com
>  Flavio Fernandes        flavio@flaviof.com
>  Flavio Leitner          fbl@redhat.com
> diff --git a/lib/automake.mk b/lib/automake.mk
> index 165e6a8..62bb17b 100644
> --- a/lib/automake.mk
> +++ b/lib/automake.mk
> @@ -462,6 +462,8 @@ MAN_FRAGMENTS += \
>         lib/ssl-peer-ca-cert-syn.man \
>         lib/ssl.man \
>         lib/ssl-syn.man \
> +       lib/ssl-connect.man \
> +       lib/ssl-connect-syn.man \
>         lib/table.man \
>         lib/unixctl.man \
>         lib/unixctl-syn.man \
> diff --git a/lib/ssl-connect-syn.man b/lib/ssl-connect-syn.man
> new file mode 100644
> index 0000000..0510a59
> --- /dev/null
> +++ b/lib/ssl-connect-syn.man
> @@ -0,0 +1,5 @@
> +.IP "SSL connection options:"
> +[\fB\-\-ssl\-protocols=\fITLSv1,TLSv1.1,TLSv1.2\fR]
> +.br
> +[\fB\-\-ssl\-ciphers=\fIHIGH:!aNULL:!MD5\fR]
> +.br
> diff --git a/lib/ssl-connect.man b/lib/ssl-connect.man
> new file mode 100644
> index 0000000..dcc6a79
> --- /dev/null
> +++ b/lib/ssl-connect.man
> @@ -0,0 +1,16 @@
> +.de IQ
> +.  br
> +.  ns
> +.  IP "\\$1"
> +..
> +.IQ "\fB\-\-ssl\-protocols=\fITLSv1,TLSv1.1,TLSv1.2\fR"
> +Specifies, in a comma or white-list delimited, list the SSL protocols \fB\*(PN\fR
> +will support for SSL connections. Supported protocols are: TLSv1, TLSv1.1,
> +TLSv1.2. Order does not matter, the highest protocol supported by both sides
> +will be choosen when making the connection.
> +.
> +.IQ "\fB\-\-ssl\-ciphers=\fIHIGH:!aNULL:!MD5\fR"
> +Specifies, in OpenSSL cipher string format, the ciphers \fB\*(PN\fR will
> +support for SSL connections.
> +
> +
> diff --git a/lib/stream-ssl.c b/lib/stream-ssl.c
> index a5c32a1..87b8de9 100644
> --- a/lib/stream-ssl.c
> +++ b/lib/stream-ssl.c
> @@ -162,6 +162,8 @@ 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 *ssl_protocols = "TLSv1,TLSv1.1,TLSv1.2";
> +static char *ssl_ciphers = "HIGH:!aNULL:!MD5";
>
>  /* Ordinarily, the SSL client and server verify each other's certificates using
>   * a CA certificate.  Setting this to false disables this behavior.  (This is a
> @@ -966,6 +968,7 @@ do_ssl_init(void)
>      SSL_CTX_set_verify(ctx, SSL_VERIFY_PEER | SSL_VERIFY_FAIL_IF_NO_PEER_CERT,
>                         NULL);
>      SSL_CTX_set_session_cache_mode(ctx, SSL_SESS_CACHE_OFF);
> +    SSL_CTX_set_cipher_list(ctx, "HIGH:!aNULL:!MD5");
>
>      return 0;
>  }
> @@ -1114,6 +1117,73 @@ stream_ssl_set_key_and_cert(const char *private_key_file,
>      }
>  }
>
> +/* Sets SSL ciphers based on string input. Aborts with an error message
> + * if 'arg' is invalid. */
> +void
> +stream_ssl_set_ciphers(const char *arg)
> +{
> +    if (ssl_init() || !arg || !strcmp(ssl_ciphers, arg)){
> +       return;
> +    }
> +    if (SSL_CTX_set_cipher_list(ctx,arg) == 0)
> +    {
> +     VLOG_ERR("SSL_CTX_set_cipher_list: %s",
> +              ERR_error_string(ERR_get_error(), NULL));
> +    }
> +    ssl_ciphers = xstrdup(arg);
> +}
> +
> +/* Set SSL protocols based on the string input. Aborts with an error message
> + * if 'arg' is invalid. */
> +void
> +stream_ssl_set_protocols(const char *arg)
> +{
> +    char *s;
> +    char *save_ptr = NULL;
> +    char *word;
> +    long protocolFlags;
> +
> +    if (ssl_init() || !arg || !strcmp(arg, ssl_protocols)){
> +       return;
> +    }
> +
> +    s = xstrdup(arg);
> +    /* Start with all the flags off and turn them on as requested. */
> +    protocolFlags |= SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3 | SSL_OP_NO_TLSv1;
> +    protocolFlags |= SSL_OP_NO_TLSv1_1 | SSL_OP_NO_TLSv1_2;
> +
> +    word = strtok_r(s, " ,\t", &save_ptr);
> +    if (word == NULL) {
> +       VLOG_ERR( "SSL protocol settings invalid" );
> +       goto exit;
> +    }
> +    while (word != NULL) {
> +       long onFlag;
> +       if (!strcasecmp(word, "TLSv1.2")){
> +          onFlag = SSL_OP_NO_TLSv1_2;
> +       } else if (!strcasecmp(word, "TLSv1.1")){
> +          onFlag = SSL_OP_NO_TLSv1_1;
> +       } else if (!strcasecmp(word, "TLSv1")){
> +          onFlag = SSL_OP_NO_TLSv1;
> +       } else {
> +          VLOG_ERR( "SSL Protocol not recognized" );
> +          goto exit;
> +       }
> +       /* Reverse the no flag and mask it out in the flags
> +        * to turn on that protocol. */
> +       protocolFlags &= ~onFlag;
> +       word = strtok_r(NULL, " ,\t", &save_ptr);
> +    };
> +
> +    // Finally, set the actual options
> +    SSL_CTX_set_options(ctx, protocolFlags);
> +
> +    ssl_protocols = xstrdup(arg);
> +
> +exit:
> +    free(s);
> +}
> +
>  /* Reads the X509 certificate or certificates in file 'file_name'.  On success,
>   * stores the address of the first element in an array of pointers to
>   * certificates in '*certs' and the number of certificates in the array in
> diff --git a/lib/stream-ssl.h b/lib/stream-ssl.h
> index 030f662..4bfe09b 100644
> --- a/lib/stream-ssl.h
> +++ b/lib/stream-ssl.h
> @@ -25,11 +25,19 @@ void stream_ssl_set_ca_cert_file(const char *file_name, bool bootstrap);
>  void stream_ssl_set_peer_ca_cert_file(const char *file_name);
>  void stream_ssl_set_key_and_cert(const char *private_key_file,
>                                   const char *certificate_file);
> +void stream_ssl_set_protocols(const char *arg);
> +void stream_ssl_set_ciphers(const char *arg);
> +
> +#define SSL_OPTION_ENUMS \
> +        OPT_SSL_PROTOCOLS, \
> +        OPT_SSL_CIPHERS
>
>  #define STREAM_SSL_LONG_OPTIONS                     \
>          {"private-key", required_argument, NULL, 'p'}, \
>          {"certificate", required_argument, NULL, 'c'}, \
> -        {"ca-cert",     required_argument, NULL, 'C'}
> +        {"ca-cert",     required_argument, NULL, 'C'}, \
> +        {"ssl-protocols", required_argument, NULL, OPT_SSL_PROTOCOLS}, \
> +        {"ssl-ciphers", required_argument, NULL, OPT_SSL_CIPHERS}
>
>  #define STREAM_SSL_OPTION_HANDLERS                      \
>          case 'p':                                       \
> @@ -42,6 +50,14 @@ void stream_ssl_set_key_and_cert(const char *private_key_file,
>                                                          \
>          case 'C':                                       \
>              stream_ssl_set_ca_cert_file(optarg, false); \
> -            break;
> +            break;                                      \
> +                                                        \
> +        case OPT_SSL_PROTOCOLS:                         \
> +            stream_ssl_set_protocols(optarg);           \
> +            break;                                      \
> +                                                        \
> +        case OPT_SSL_CIPHERS:                           \
> +            stream_ssl_set_ciphers(optarg);             \
> +            break;
>
>  #endif /* stream-ssl.h */
> diff --git a/manpages.mk b/manpages.mk
> index fa9e59b..2ff7658 100644
> --- a/manpages.mk
> +++ b/manpages.mk
> @@ -20,6 +20,8 @@ ovsdb/ovsdb-client.1: \
>         lib/daemon.man \
>         lib/ssl-bootstrap-syn.man \
>         lib/ssl-bootstrap.man \
> +       lib/ssl-connect-syn.man \
> +       lib/ssl-connect.man \
>         lib/ssl-syn.man \
>         lib/ssl.man \
>         lib/table.man \
> @@ -34,6 +36,8 @@ lib/daemon-syn.man:
>  lib/daemon.man:
>  lib/ssl-bootstrap-syn.man:
>  lib/ssl-bootstrap.man:
> +lib/ssl-connect-syn.man:
> +lib/ssl-connect.man:
>  lib/ssl-syn.man:
>  lib/ssl.man:
>  lib/table.man:
> @@ -54,6 +58,8 @@ ovsdb/ovsdb-server.1: \
>         lib/service.man \
>         lib/ssl-bootstrap-syn.man \
>         lib/ssl-bootstrap.man \
> +       lib/ssl-connect-syn.man \
> +       lib/ssl-connect.man \
>         lib/ssl-peer-ca-cert-syn.man \
>         lib/ssl-peer-ca-cert.man \
>         lib/ssl-syn.man \
> @@ -78,6 +84,8 @@ lib/service-syn.man:
>  lib/service.man:
>  lib/ssl-bootstrap-syn.man:
>  lib/ssl-bootstrap.man:
> +lib/ssl-connect-syn.man:
> +lib/ssl-connect.man:
>  lib/ssl-peer-ca-cert-syn.man:
>  lib/ssl-peer-ca-cert.man:
>  lib/ssl-syn.man:
> diff --git a/ovn/controller-vtep/ovn-controller-vtep.c b/ovn/controller-vtep/ovn-controller-vtep.c
> index baee789..f70d9bc 100644
> --- a/ovn/controller-vtep/ovn-controller-vtep.c
> +++ b/ovn/controller-vtep/ovn-controller-vtep.c
> @@ -169,7 +169,8 @@ parse_options(int argc, char *argv[])
>          OPT_PEER_CA_CERT = UCHAR_MAX + 1,
>          OPT_BOOTSTRAP_CA_CERT,
>          VLOG_OPTION_ENUMS,
> -        DAEMON_OPTION_ENUMS
> +        DAEMON_OPTION_ENUMS,
> +        SSL_OPTION_ENUMS,
>      };
>
>      static struct option long_options[] = {
> diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
> index 66a364f..0e57cb9 100644
> --- a/ovn/controller/ovn-controller.c
> +++ b/ovn/controller/ovn-controller.c
> @@ -552,7 +552,8 @@ parse_options(int argc, char *argv[])
>          OPT_PEER_CA_CERT = UCHAR_MAX + 1,
>          OPT_BOOTSTRAP_CA_CERT,
>          VLOG_OPTION_ENUMS,
> -        DAEMON_OPTION_ENUMS
> +        DAEMON_OPTION_ENUMS,
> +        SSL_OPTION_ENUMS,
>      };
>
>      static struct option long_options[] = {
> diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
> index 0874a9c..c0e8e98 100644
> --- a/ovn/northd/ovn-northd.c
> +++ b/ovn/northd/ovn-northd.c
> @@ -4201,6 +4201,7 @@ parse_options(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
>      enum {
>          DAEMON_OPTION_ENUMS,
>          VLOG_OPTION_ENUMS,
> +        SSL_OPTION_ENUMS,
>      };
>      static const struct option long_options[] = {
>          {"ovnsb-db", required_argument, NULL, 'd'},
> diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
> index d6d64ea..3785073 100644
> --- a/ovn/utilities/ovn-nbctl.c
> +++ b/ovn/utilities/ovn-nbctl.c
> @@ -167,7 +167,8 @@ parse_options(int argc, char *argv[], struct shash *local_options)
>          OPT_COMMANDS,
>          OPT_OPTIONS,
>          VLOG_OPTION_ENUMS,
> -        TABLE_OPTION_ENUMS
> +        TABLE_OPTION_ENUMS,
> +        SSL_OPTION_ENUMS,
>      };
>      static const struct option global_long_options[] = {
>          {"db", required_argument, NULL, OPT_DB},
> diff --git a/ovn/utilities/ovn-sbctl.c b/ovn/utilities/ovn-sbctl.c
> index b894b8b..a067de6 100644
> --- a/ovn/utilities/ovn-sbctl.c
> +++ b/ovn/utilities/ovn-sbctl.c
> @@ -162,7 +162,8 @@ parse_options(int argc, char *argv[], struct shash *local_options)
>          OPT_COMMANDS,
>          OPT_OPTIONS,
>          VLOG_OPTION_ENUMS,
> -        TABLE_OPTION_ENUMS
> +        TABLE_OPTION_ENUMS,
> +        SSL_OPTION_ENUMS,
>      };
>      static const struct option global_long_options[] = {
>          {"db", required_argument, NULL, OPT_DB},
> diff --git a/ovn/utilities/ovn-trace.c b/ovn/utilities/ovn-trace.c
> index df2ff21..4d8abe9 100644
> --- a/ovn/utilities/ovn-trace.c
> +++ b/ovn/utilities/ovn-trace.c
> @@ -153,6 +153,7 @@ parse_options(int argc, char *argv[])
>          OPT_MINIMAL,
>          OPT_ALL,
>          DAEMON_OPTION_ENUMS,
> +        SSL_OPTION_ENUMS,
>          VLOG_OPTION_ENUMS
>      };
>      static const struct option long_options[] = {
> diff --git a/ovsdb/ovsdb-client.1.in b/ovsdb/ovsdb-client.1.in
> index 1bb7419..9658291 100644
> --- a/ovsdb/ovsdb-client.1.in
> +++ b/ovsdb/ovsdb-client.1.in
> @@ -55,6 +55,7 @@ ovsdb\-client \- command-line interface to \fBovsdb-server\fR(1)
>  .so lib/vlog-syn.man
>  .so lib/ssl-syn.man
>  .so lib/ssl-bootstrap-syn.man
> +.so lib/ssl-connect-syn.man
>  .so lib/common-syn.man
>  .
>  .SH DESCRIPTION
> @@ -205,6 +206,8 @@ With any other command, they have no effect.
>  .SS "Public Key Infrastructure Options"
>  .so lib/ssl.man
>  .so lib/ssl-bootstrap.man
> +.SS "SSL Connection Options"
> +.so lib/ssl-connect.man
>  .SS "Other Options"
>  .so lib/common.man
>  .SH "SEE ALSO"
> diff --git a/ovsdb/ovsdb-client.c b/ovsdb/ovsdb-client.c
> index 1f83f3b..452c416 100644
> --- a/ovsdb/ovsdb-client.c
> +++ b/ovsdb/ovsdb-client.c
> @@ -174,7 +174,8 @@ parse_options(int argc, char *argv[])
>          OPT_TIMESTAMP,
>          VLOG_OPTION_ENUMS,
>          DAEMON_OPTION_ENUMS,
> -        TABLE_OPTION_ENUMS
> +        TABLE_OPTION_ENUMS,
> +        SSL_OPTION_ENUMS,
>      };
>      static const struct option long_options[] = {
>          {"help", no_argument, NULL, 'h'},
> diff --git a/ovsdb/ovsdb-server.1.in b/ovsdb/ovsdb-server.1.in
> index d1ba83b..470645b 100644
> --- a/ovsdb/ovsdb-server.1.in
> +++ b/ovsdb/ovsdb-server.1.in
> @@ -23,6 +23,7 @@ ovsdb\-server \- Open vSwitch database server
>  .so lib/ssl-syn.man
>  .so lib/ssl-bootstrap-syn.man
>  .so lib/ssl-peer-ca-cert-syn.man
> +.so lib/ssl-connect-syn.man
>  .so lib/unixctl-syn.man
>  .so lib/common-syn.man
>  .
> @@ -133,6 +134,8 @@ one row in \fItable\fR.)
>  .so lib/ssl.man
>  .so lib/ssl-bootstrap.man
>  .so lib/ssl-peer-ca-cert.man
> +.SS "SSL Connection Options"
> +.so lib/ssl-connect.man
>  .SS "Other Options"
>  .so lib/unixctl.man
>  .so lib/common.man
> diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c
> index e08c341..f34d0d8 100644
> --- a/ovsdb/ovsdb-server.c
> +++ b/ovsdb/ovsdb-server.c
> @@ -66,6 +66,8 @@ struct db;
>  static char *private_key_file;
>  static char *certificate_file;
>  static char *ca_cert_file;
> +static char *ssl_protocols;
> +static char *ssl_ciphers;
>  static bool bootstrap_ca_cert;
>
>  /* Replication configuration. */
> @@ -1024,13 +1026,19 @@ reconfigure_ssl(const struct shash *all_dbs)
>      const char *resolved_private_key;
>      const char *resolved_certificate;
>      const char *resolved_ca_cert;
> +    const char *resolved_ssl_protocols;
> +    const char *resolved_ssl_ciphers;
>
>      resolved_private_key = query_db_string(all_dbs, private_key_file, &errors);
>      resolved_certificate = query_db_string(all_dbs, certificate_file, &errors);
>      resolved_ca_cert = query_db_string(all_dbs, ca_cert_file, &errors);
> +    resolved_ssl_protocols = query_db_string(all_dbs, ssl_protocols, &errors);
> +    resolved_ssl_ciphers = query_db_string(all_dbs, ssl_ciphers, &errors);
>
>      stream_ssl_set_key_and_cert(resolved_private_key, resolved_certificate);
>      stream_ssl_set_ca_cert_file(resolved_ca_cert, bootstrap_ca_cert);
> +    stream_ssl_set_protocols(resolved_ssl_protocols);
> +    stream_ssl_set_ciphers(resolved_ssl_ciphers);
>
>      return errors.string;
>  }
> @@ -1384,7 +1392,8 @@ parse_options(int *argcp, char **argvp[],
>          OPT_SYNC_FROM,
>          OPT_SYNC_EXCLUDE,
>          VLOG_OPTION_ENUMS,
> -        DAEMON_OPTION_ENUMS
> +        DAEMON_OPTION_ENUMS,
> +        SSL_OPTION_ENUMS,
>      };
>      static const struct option long_options[] = {
>          {"remote",      required_argument, NULL, OPT_REMOTE},
> @@ -1398,9 +1407,7 @@ parse_options(int *argcp, char **argvp[],
>          VLOG_LONG_OPTIONS,
>          {"bootstrap-ca-cert", required_argument, NULL, OPT_BOOTSTRAP_CA_CERT},
>          {"peer-ca-cert", required_argument, NULL, OPT_PEER_CA_CERT},
> -        {"private-key", required_argument, NULL, 'p'},
> -        {"certificate", required_argument, NULL, 'c'},
> -        {"ca-cert",     required_argument, NULL, 'C'},
> +        STREAM_SSL_LONG_OPTIONS,
>          {"sync-from",   required_argument, NULL, OPT_SYNC_FROM},
>          {"sync-exclude-tables", required_argument, NULL, OPT_SYNC_EXCLUDE},
>          {NULL, 0, NULL, 0},
> @@ -1454,6 +1461,14 @@ parse_options(int *argcp, char **argvp[],
>              bootstrap_ca_cert = false;
>              break;
>
> +        case OPT_SSL_PROTOCOLS:
> +            ssl_protocols = optarg;
> +            break;
> +
> +        case OPT_SSL_CIPHERS:
> +            ssl_ciphers = optarg;
> +            break;
> +
>          case OPT_BOOTSTRAP_CA_CERT:
>              ca_cert_file = optarg;
>              bootstrap_ca_cert = true;
> diff --git a/tests/ovsdb-server.at b/tests/ovsdb-server.at
> index 0436de8..6347829 100644
> --- a/tests/ovsdb-server.at
> +++ b/tests/ovsdb-server.at
> @@ -513,9 +513,13 @@ AT_DATA([schema],
>           "columns": {
>             "private_key": {"type": "string"},
>             "certificate": {"type": "string"},
> -           "ca_cert": {"type": "string"}}}}}
> +           "ca_cert": {"type": "string"},
> +           "ssl_protocols" : {"type": "string"},
> +           "ssl_ciphers" : {"type" : "string"}}}}}
>  ]])
>  AT_CHECK([ovsdb-tool create db schema], [0], [stdout], [ignore])
> +# The !ECDHE-ECDSA-AES256-GCM-SHA384 in the ssl_ciphers is so that
> +# a cipher negotiation failure can be tested for later.
>  AT_CHECK(
>    [[ovsdb-tool transact db \
>       '["mydb",
> @@ -523,13 +527,17 @@ AT_CHECK(
>          "table": "SSL",
>          "row": {"private_key": "'"$PKIDIR/testpki-privkey2.pem"'",
>                  "certificate": "'"$PKIDIR/testpki-cert2.pem"'",
> -                "ca_cert": "'"$PKIDIR/testpki-cacert.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])
>  AT_CHECK(
>    [ovsdb-server --log-file --detach --no-chdir --pidfile="`pwd`"/pid \
>          --private-key=db:mydb,SSL,private_key \
>          --certificate=db:mydb,SSL,certificate \
>          --ca-cert=db:mydb,SSL,ca_cert \
> +        --ssl-protocols=db:mydb,SSL,ssl_protocols \
> +        --ssl-ciphers=db:mydb,SSL,ssl_ciphers \
>          --remote=pssl:0:127.0.0.1 --unixctl="`pwd`"/unixctl db],
>    [0], [ignore], [ignore])
>  PARSE_LISTENING_PORT([ovsdb-server.log], [SSL_PORT])
> @@ -538,6 +546,8 @@ 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",
>            {"op": "select",
> @@ -550,6 +560,60 @@ AT_CHECK_UNQUOTED(
>    [cat output], [0],
>    [[@<:@{"rows":@<:@{"private_key":"$PKIDIR/testpki-privkey2.pem"}@:>@}@:>@
>  ]], [ignore], [test ! -e pid || kill `cat pid`])
> +# Check that when the server has TLSv1.1+ and the client has
> +# TLSv1 that the connection fails.
> +AT_CHECK(
> +  [[ovsdb-client \
> +        --private-key=$PKIDIR/testpki-privkey.pem \
> +        --certificate=$PKIDIR/testpki-cert.pem \
> +        --ca-cert=$PKIDIR/testpki-cacert.pem \
> +        --ssl-protocols=TLSv1 \
> +        --ssl-ciphers=HIGH:!aNULL:!MD5 \
> +        transact ssl:127.0.0.1:$SSL_PORT \
> +        '["mydb",
> +          {"op": "select",
> +           "table": "SSL",
> +           "where": [],
> +           "columns": ["private_key"]}]']],
> +  [1], [stdout],
> +  [stderr],
> +  [test ! -e pid || kill `cat pid`])
> +cat stderr > output
> +AT_CHECK_UNQUOTED(
> +  [grep "failed to connect" output], [0],
> +  [ovsdb-client: failed to connect to "ssl:127.0.0.1:$SSL_PORT" (Protocol error)
> +],
> +  [ignore], [test ! -e pid || kill `cat pid`])
> +# Check that when ciphers are not compatible, that a negotiation
> +# failure occurs.
> +AT_CHECK(
> +  [[ovsdb-client \
> +        --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=ECDHE-ECDSA-AES256-GCM-SHA384 \
> +        transact ssl:127.0.0.1:$SSL_PORT \
> +        '["mydb",
> +          {"op": "select",
> +           "table": "SSL",
> +           "where": [],
> +           "columns": ["private_key"]}]']],
> +  [1], [stdout],
> +  [stderr],
> +  [test ! -e pid || kill `cat pid`])
> +cat stderr > output
> +AT_CHECK_UNQUOTED(
> +  [grep "failed to connect" output], [0],
> +  [ovsdb-client: failed to connect to "ssl:127.0.0.1:$SSL_PORT" (Protocol error)
> +],
> +  [ignore], [test ! -e pid || kill `cat pid`])
> +# The error message for being unable to negotiate a shared ciphersuite
> +# is 'sslv3 alert handshake failure'. This is not the clearest message.
> +AT_CHECK_UNQUOTED(
> +  [grep "sslv3 alert handshake failure" output], [0],
> +  [stdout],
> +  [ignore], [test ! -e pid || kill `cat pid`])
>  OVSDB_SERVER_SHUTDOWN
>  AT_CLEANUP
>
> diff --git a/tests/test-jsonrpc.c b/tests/test-jsonrpc.c
> index 684601a..a7ca691 100644
> --- a/tests/test-jsonrpc.c
> +++ b/tests/test-jsonrpc.c
> @@ -55,7 +55,8 @@ parse_options(int argc, char *argv[])
>  {
>      enum {
>          OPT_BOOTSTRAP_CA_CERT = UCHAR_MAX + 1,
> -        DAEMON_OPTION_ENUMS
> +        DAEMON_OPTION_ENUMS,
> +        SSL_OPTION_ENUMS,
>      };
>      static const struct option long_options[] = {
>          {"verbose", optional_argument, NULL, 'v'},
> diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
> index 6fd3818..fa2466e 100644
> --- a/utilities/ovs-ofctl.c
> +++ b/utilities/ovs-ofctl.c
> @@ -190,7 +190,8 @@ parse_options(int argc, char *argv[])
>          OPT_READ_ONLY,
>          DAEMON_OPTION_ENUMS,
>          OFP_VERSION_OPTION_ENUMS,
> -        VLOG_OPTION_ENUMS
> +        VLOG_OPTION_ENUMS,
> +        SSL_OPTION_ENUMS,
>      };
>      static const struct option long_options[] = {
>          {"timeout", required_argument, NULL, 't'},
> diff --git a/utilities/ovs-testcontroller.c b/utilities/ovs-testcontroller.c
> index 1db3bbe..99f5bf8 100644
> --- a/utilities/ovs-testcontroller.c
> +++ b/utilities/ovs-testcontroller.c
> @@ -258,7 +258,8 @@ parse_options(int argc, char *argv[])
>          OPT_UNIXCTL,
>          VLOG_OPTION_ENUMS,
>          DAEMON_OPTION_ENUMS,
> -        OFP_VERSION_OPTION_ENUMS
> +        OFP_VERSION_OPTION_ENUMS,
> +        SSL_OPTION_ENUMS,
>      };
>      static const struct option long_options[] = {
>          {"hub",         no_argument, NULL, 'H'},
> diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c
> index e710095..25f0e8a 100644
> --- a/utilities/ovs-vsctl.c
> +++ b/utilities/ovs-vsctl.c
> @@ -207,7 +207,8 @@ parse_options(int argc, char *argv[], struct shash *local_options)
>          OPT_COMMANDS,
>          OPT_OPTIONS,
>          VLOG_OPTION_ENUMS,
> -        TABLE_OPTION_ENUMS
> +        TABLE_OPTION_ENUMS,
> +        SSL_OPTION_ENUMS,
>      };
>      static const struct option global_long_options[] = {
>          {"db", required_argument, NULL, OPT_DB},
> diff --git a/vswitchd/ovs-vswitchd.c b/vswitchd/ovs-vswitchd.c
> index 72448bb..0e3b6d0 100644
> --- a/vswitchd/ovs-vswitchd.c
> +++ b/vswitchd/ovs-vswitchd.c
> @@ -145,6 +145,7 @@ parse_options(int argc, char *argv[], char **unixctl_pathp)
>          OPT_DISABLE_SYSTEM,
>          DAEMON_OPTION_ENUMS,
>          OPT_DPDK,
> +        SSL_OPTION_ENUMS,
>          OPT_DUMMY_NUMA,
>      };
>      static const struct option long_options[] = {
> diff --git a/vtep/vtep-ctl.c b/vtep/vtep-ctl.c
> index 245ba0d..af20f83 100644
> --- a/vtep/vtep-ctl.c
> +++ b/vtep/vtep-ctl.c
> @@ -166,7 +166,8 @@ parse_options(int argc, char *argv[], struct shash *local_options)
>          OPT_PEER_CA_CERT,
>          OPT_LOCAL,
>          VLOG_OPTION_ENUMS,
> -        TABLE_OPTION_ENUMS
> +        TABLE_OPTION_ENUMS,
> +        SSL_OPTION_ENUMS,
>      };
>      static const struct option global_long_options[] = {
>          {"db", required_argument, NULL, OPT_DB},
> --
> 1.8.1.4
>

Thanks a lot.  I have only a few style fixes for this.  May I have a
Signed-off-by?

Thanks,

Ben.

On Thu, Oct 06, 2016 at 04:22:19PM -0700, Ethan Rahn wrote:
> Third time is the charm :) Sent via git send-email this time around.
> Thanks for your patience and sorry for the trouble.
> 
> Cheers,
> 
> Ethan
> 
> On Thu, Oct 6, 2016 at 4:21 PM, Ethan Rahn <erahn@arista.com> wrote:
> > ---
> >  AUTHORS                                   |  1 +
> >  lib/automake.mk                           |  2 +
> >  lib/ssl-connect-syn.man                   |  5 +++
> >  lib/ssl-connect.man                       | 16 +++++++
> >  lib/stream-ssl.c                          | 70 +++++++++++++++++++++++++++++++
> >  lib/stream-ssl.h                          | 20 ++++++++-
> >  manpages.mk                               |  8 ++++
> >  ovn/controller-vtep/ovn-controller-vtep.c |  3 +-
> >  ovn/controller/ovn-controller.c           |  3 +-
> >  ovn/northd/ovn-northd.c                   |  1 +
> >  ovn/utilities/ovn-nbctl.c                 |  3 +-
> >  ovn/utilities/ovn-sbctl.c                 |  3 +-
> >  ovn/utilities/ovn-trace.c                 |  1 +
> >  ovsdb/ovsdb-client.1.in                   |  3 ++
> >  ovsdb/ovsdb-client.c                      |  3 +-
> >  ovsdb/ovsdb-server.1.in                   |  3 ++
> >  ovsdb/ovsdb-server.c                      | 23 ++++++++--
> >  tests/ovsdb-server.at                     | 68 +++++++++++++++++++++++++++++-
> >  tests/test-jsonrpc.c                      |  3 +-
> >  utilities/ovs-ofctl.c                     |  3 +-
> >  utilities/ovs-testcontroller.c            |  3 +-
> >  utilities/ovs-vsctl.c                     |  3 +-
> >  vswitchd/ovs-vswitchd.c                   |  1 +
> >  vtep/vtep-ctl.c                           |  3 +-
> >  24 files changed, 234 insertions(+), 18 deletions(-)
> >  create mode 100644 lib/ssl-connect-syn.man
> >  create mode 100644 lib/ssl-connect.man
> >
> > diff --git a/AUTHORS b/AUTHORS
> > index c089d59..197142f 100644
> > --- a/AUTHORS
> > +++ b/AUTHORS
> > @@ -80,6 +80,7 @@ Eitan Eliahu            eliahue@vmware.com
> >  Eohyung Lee             liquidnuker@gmail.com
> >  Eric Sesterhenn         eric.sesterhenn@lsexperts.de
> >  Ethan J. Jackson        ejj@eecs.berkeley.edu
> > +Ethan Rahn              erahn@arista.com
> >  Eziz Durdyyev           ezizdurdy@gmail.com
> >  Flavio Fernandes        flavio@flaviof.com
> >  Flavio Leitner          fbl@redhat.com
> > diff --git a/lib/automake.mk b/lib/automake.mk
> > index 165e6a8..62bb17b 100644
> > --- a/lib/automake.mk
> > +++ b/lib/automake.mk
> > @@ -462,6 +462,8 @@ MAN_FRAGMENTS += \
> >         lib/ssl-peer-ca-cert-syn.man \
> >         lib/ssl.man \
> >         lib/ssl-syn.man \
> > +       lib/ssl-connect.man \
> > +       lib/ssl-connect-syn.man \
> >         lib/table.man \
> >         lib/unixctl.man \
> >         lib/unixctl-syn.man \
> > diff --git a/lib/ssl-connect-syn.man b/lib/ssl-connect-syn.man
> > new file mode 100644
> > index 0000000..0510a59
> > --- /dev/null
> > +++ b/lib/ssl-connect-syn.man
> > @@ -0,0 +1,5 @@
> > +.IP "SSL connection options:"
> > +[\fB\-\-ssl\-protocols=\fITLSv1,TLSv1.1,TLSv1.2\fR]
> > +.br
> > +[\fB\-\-ssl\-ciphers=\fIHIGH:!aNULL:!MD5\fR]
> > +.br
> > diff --git a/lib/ssl-connect.man b/lib/ssl-connect.man
> > new file mode 100644
> > index 0000000..dcc6a79
> > --- /dev/null
> > +++ b/lib/ssl-connect.man
> > @@ -0,0 +1,16 @@
> > +.de IQ
> > +.  br
> > +.  ns
> > +.  IP "\\$1"
> > +..
> > +.IQ "\fB\-\-ssl\-protocols=\fITLSv1,TLSv1.1,TLSv1.2\fR"
> > +Specifies, in a comma or white-list delimited, list the SSL protocols \fB\*(PN\fR
> > +will support for SSL connections. Supported protocols are: TLSv1, TLSv1.1,
> > +TLSv1.2. Order does not matter, the highest protocol supported by both sides
> > +will be choosen when making the connection.
> > +.
> > +.IQ "\fB\-\-ssl\-ciphers=\fIHIGH:!aNULL:!MD5\fR"
> > +Specifies, in OpenSSL cipher string format, the ciphers \fB\*(PN\fR will
> > +support for SSL connections.
> > +
> > +
> > diff --git a/lib/stream-ssl.c b/lib/stream-ssl.c
> > index a5c32a1..87b8de9 100644
> > --- a/lib/stream-ssl.c
> > +++ b/lib/stream-ssl.c
> > @@ -162,6 +162,8 @@ 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 *ssl_protocols = "TLSv1,TLSv1.1,TLSv1.2";
> > +static char *ssl_ciphers = "HIGH:!aNULL:!MD5";
> >
> >  /* Ordinarily, the SSL client and server verify each other's certificates using
> >   * a CA certificate.  Setting this to false disables this behavior.  (This is a
> > @@ -966,6 +968,7 @@ do_ssl_init(void)
> >      SSL_CTX_set_verify(ctx, SSL_VERIFY_PEER | SSL_VERIFY_FAIL_IF_NO_PEER_CERT,
> >                         NULL);
> >      SSL_CTX_set_session_cache_mode(ctx, SSL_SESS_CACHE_OFF);
> > +    SSL_CTX_set_cipher_list(ctx, "HIGH:!aNULL:!MD5");
> >
> >      return 0;
> >  }
> > @@ -1114,6 +1117,73 @@ stream_ssl_set_key_and_cert(const char *private_key_file,
> >      }
> >  }
> >
> > +/* Sets SSL ciphers based on string input. Aborts with an error message
> > + * if 'arg' is invalid. */
> > +void
> > +stream_ssl_set_ciphers(const char *arg)
> > +{
> > +    if (ssl_init() || !arg || !strcmp(ssl_ciphers, arg)){
> > +       return;
> > +    }
> > +    if (SSL_CTX_set_cipher_list(ctx,arg) == 0)
> > +    {
> > +     VLOG_ERR("SSL_CTX_set_cipher_list: %s",
> > +              ERR_error_string(ERR_get_error(), NULL));
> > +    }
> > +    ssl_ciphers = xstrdup(arg);
> > +}
> > +
> > +/* Set SSL protocols based on the string input. Aborts with an error message
> > + * if 'arg' is invalid. */
> > +void
> > +stream_ssl_set_protocols(const char *arg)
> > +{
> > +    char *s;
> > +    char *save_ptr = NULL;
> > +    char *word;
> > +    long protocolFlags;
> > +
> > +    if (ssl_init() || !arg || !strcmp(arg, ssl_protocols)){
> > +       return;
> > +    }
> > +
> > +    s = xstrdup(arg);
> > +    /* Start with all the flags off and turn them on as requested. */
> > +    protocolFlags |= SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3 | SSL_OP_NO_TLSv1;
> > +    protocolFlags |= SSL_OP_NO_TLSv1_1 | SSL_OP_NO_TLSv1_2;
> > +
> > +    word = strtok_r(s, " ,\t", &save_ptr);
> > +    if (word == NULL) {
> > +       VLOG_ERR( "SSL protocol settings invalid" );
> > +       goto exit;
> > +    }
> > +    while (word != NULL) {
> > +       long onFlag;
> > +       if (!strcasecmp(word, "TLSv1.2")){
> > +          onFlag = SSL_OP_NO_TLSv1_2;
> > +       } else if (!strcasecmp(word, "TLSv1.1")){
> > +          onFlag = SSL_OP_NO_TLSv1_1;
> > +       } else if (!strcasecmp(word, "TLSv1")){
> > +          onFlag = SSL_OP_NO_TLSv1;
> > +       } else {
> > +          VLOG_ERR( "SSL Protocol not recognized" );
> > +          goto exit;
> > +       }
> > +       /* Reverse the no flag and mask it out in the flags
> > +        * to turn on that protocol. */
> > +       protocolFlags &= ~onFlag;
> > +       word = strtok_r(NULL, " ,\t", &save_ptr);
> > +    };
> > +
> > +    // Finally, set the actual options
> > +    SSL_CTX_set_options(ctx, protocolFlags);
> > +
> > +    ssl_protocols = xstrdup(arg);
> > +
> > +exit:
> > +    free(s);
> > +}
> > +
> >  /* Reads the X509 certificate or certificates in file 'file_name'.  On success,
> >   * stores the address of the first element in an array of pointers to
> >   * certificates in '*certs' and the number of certificates in the array in
> > diff --git a/lib/stream-ssl.h b/lib/stream-ssl.h
> > index 030f662..4bfe09b 100644
> > --- a/lib/stream-ssl.h
> > +++ b/lib/stream-ssl.h
> > @@ -25,11 +25,19 @@ void stream_ssl_set_ca_cert_file(const char *file_name, bool bootstrap);
> >  void stream_ssl_set_peer_ca_cert_file(const char *file_name);
> >  void stream_ssl_set_key_and_cert(const char *private_key_file,
> >                                   const char *certificate_file);
> > +void stream_ssl_set_protocols(const char *arg);
> > +void stream_ssl_set_ciphers(const char *arg);
> > +
> > +#define SSL_OPTION_ENUMS \
> > +        OPT_SSL_PROTOCOLS, \
> > +        OPT_SSL_CIPHERS
> >
> >  #define STREAM_SSL_LONG_OPTIONS                     \
> >          {"private-key", required_argument, NULL, 'p'}, \
> >          {"certificate", required_argument, NULL, 'c'}, \
> > -        {"ca-cert",     required_argument, NULL, 'C'}
> > +        {"ca-cert",     required_argument, NULL, 'C'}, \
> > +        {"ssl-protocols", required_argument, NULL, OPT_SSL_PROTOCOLS}, \
> > +        {"ssl-ciphers", required_argument, NULL, OPT_SSL_CIPHERS}
> >
> >  #define STREAM_SSL_OPTION_HANDLERS                      \
> >          case 'p':                                       \
> > @@ -42,6 +50,14 @@ void stream_ssl_set_key_and_cert(const char *private_key_file,
> >                                                          \
> >          case 'C':                                       \
> >              stream_ssl_set_ca_cert_file(optarg, false); \
> > -            break;
> > +            break;                                      \
> > +                                                        \
> > +        case OPT_SSL_PROTOCOLS:                         \
> > +            stream_ssl_set_protocols(optarg);           \
> > +            break;                                      \
> > +                                                        \
> > +        case OPT_SSL_CIPHERS:                           \
> > +            stream_ssl_set_ciphers(optarg);             \
> > +            break;
> >
> >  #endif /* stream-ssl.h */
> > diff --git a/manpages.mk b/manpages.mk
> > index fa9e59b..2ff7658 100644
> > --- a/manpages.mk
> > +++ b/manpages.mk
> > @@ -20,6 +20,8 @@ ovsdb/ovsdb-client.1: \
> >         lib/daemon.man \
> >         lib/ssl-bootstrap-syn.man \
> >         lib/ssl-bootstrap.man \
> > +       lib/ssl-connect-syn.man \
> > +       lib/ssl-connect.man \
> >         lib/ssl-syn.man \
> >         lib/ssl.man \
> >         lib/table.man \
> > @@ -34,6 +36,8 @@ lib/daemon-syn.man:
> >  lib/daemon.man:
> >  lib/ssl-bootstrap-syn.man:
> >  lib/ssl-bootstrap.man:
> > +lib/ssl-connect-syn.man:
> > +lib/ssl-connect.man:
> >  lib/ssl-syn.man:
> >  lib/ssl.man:
> >  lib/table.man:
> > @@ -54,6 +58,8 @@ ovsdb/ovsdb-server.1: \
> >         lib/service.man \
> >         lib/ssl-bootstrap-syn.man \
> >         lib/ssl-bootstrap.man \
> > +       lib/ssl-connect-syn.man \
> > +       lib/ssl-connect.man \
> >         lib/ssl-peer-ca-cert-syn.man \
> >         lib/ssl-peer-ca-cert.man \
> >         lib/ssl-syn.man \
> > @@ -78,6 +84,8 @@ lib/service-syn.man:
> >  lib/service.man:
> >  lib/ssl-bootstrap-syn.man:
> >  lib/ssl-bootstrap.man:
> > +lib/ssl-connect-syn.man:
> > +lib/ssl-connect.man:
> >  lib/ssl-peer-ca-cert-syn.man:
> >  lib/ssl-peer-ca-cert.man:
> >  lib/ssl-syn.man:
> > diff --git a/ovn/controller-vtep/ovn-controller-vtep.c b/ovn/controller-vtep/ovn-controller-vtep.c
> > index baee789..f70d9bc 100644
> > --- a/ovn/controller-vtep/ovn-controller-vtep.c
> > +++ b/ovn/controller-vtep/ovn-controller-vtep.c
> > @@ -169,7 +169,8 @@ parse_options(int argc, char *argv[])
> >          OPT_PEER_CA_CERT = UCHAR_MAX + 1,
> >          OPT_BOOTSTRAP_CA_CERT,
> >          VLOG_OPTION_ENUMS,
> > -        DAEMON_OPTION_ENUMS
> > +        DAEMON_OPTION_ENUMS,
> > +        SSL_OPTION_ENUMS,
> >      };
> >
> >      static struct option long_options[] = {
> > diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
> > index 66a364f..0e57cb9 100644
> > --- a/ovn/controller/ovn-controller.c
> > +++ b/ovn/controller/ovn-controller.c
> > @@ -552,7 +552,8 @@ parse_options(int argc, char *argv[])
> >          OPT_PEER_CA_CERT = UCHAR_MAX + 1,
> >          OPT_BOOTSTRAP_CA_CERT,
> >          VLOG_OPTION_ENUMS,
> > -        DAEMON_OPTION_ENUMS
> > +        DAEMON_OPTION_ENUMS,
> > +        SSL_OPTION_ENUMS,
> >      };
> >
> >      static struct option long_options[] = {
> > diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
> > index 0874a9c..c0e8e98 100644
> > --- a/ovn/northd/ovn-northd.c
> > +++ b/ovn/northd/ovn-northd.c
> > @@ -4201,6 +4201,7 @@ parse_options(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
> >      enum {
> >          DAEMON_OPTION_ENUMS,
> >          VLOG_OPTION_ENUMS,
> > +        SSL_OPTION_ENUMS,
> >      };
> >      static const struct option long_options[] = {
> >          {"ovnsb-db", required_argument, NULL, 'd'},
> > diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
> > index d6d64ea..3785073 100644
> > --- a/ovn/utilities/ovn-nbctl.c
> > +++ b/ovn/utilities/ovn-nbctl.c
> > @@ -167,7 +167,8 @@ parse_options(int argc, char *argv[], struct shash *local_options)
> >          OPT_COMMANDS,
> >          OPT_OPTIONS,
> >          VLOG_OPTION_ENUMS,
> > -        TABLE_OPTION_ENUMS
> > +        TABLE_OPTION_ENUMS,
> > +        SSL_OPTION_ENUMS,
> >      };
> >      static const struct option global_long_options[] = {
> >          {"db", required_argument, NULL, OPT_DB},
> > diff --git a/ovn/utilities/ovn-sbctl.c b/ovn/utilities/ovn-sbctl.c
> > index b894b8b..a067de6 100644
> > --- a/ovn/utilities/ovn-sbctl.c
> > +++ b/ovn/utilities/ovn-sbctl.c
> > @@ -162,7 +162,8 @@ parse_options(int argc, char *argv[], struct shash *local_options)
> >          OPT_COMMANDS,
> >          OPT_OPTIONS,
> >          VLOG_OPTION_ENUMS,
> > -        TABLE_OPTION_ENUMS
> > +        TABLE_OPTION_ENUMS,
> > +        SSL_OPTION_ENUMS,
> >      };
> >      static const struct option global_long_options[] = {
> >          {"db", required_argument, NULL, OPT_DB},
> > diff --git a/ovn/utilities/ovn-trace.c b/ovn/utilities/ovn-trace.c
> > index df2ff21..4d8abe9 100644
> > --- a/ovn/utilities/ovn-trace.c
> > +++ b/ovn/utilities/ovn-trace.c
> > @@ -153,6 +153,7 @@ parse_options(int argc, char *argv[])
> >          OPT_MINIMAL,
> >          OPT_ALL,
> >          DAEMON_OPTION_ENUMS,
> > +        SSL_OPTION_ENUMS,
> >          VLOG_OPTION_ENUMS
> >      };
> >      static const struct option long_options[] = {
> > diff --git a/ovsdb/ovsdb-client.1.in b/ovsdb/ovsdb-client.1.in
> > index 1bb7419..9658291 100644
> > --- a/ovsdb/ovsdb-client.1.in
> > +++ b/ovsdb/ovsdb-client.1.in
> > @@ -55,6 +55,7 @@ ovsdb\-client \- command-line interface to \fBovsdb-server\fR(1)
> >  .so lib/vlog-syn.man
> >  .so lib/ssl-syn.man
> >  .so lib/ssl-bootstrap-syn.man
> > +.so lib/ssl-connect-syn.man
> >  .so lib/common-syn.man
> >  .
> >  .SH DESCRIPTION
> > @@ -205,6 +206,8 @@ With any other command, they have no effect.
> >  .SS "Public Key Infrastructure Options"
> >  .so lib/ssl.man
> >  .so lib/ssl-bootstrap.man
> > +.SS "SSL Connection Options"
> > +.so lib/ssl-connect.man
> >  .SS "Other Options"
> >  .so lib/common.man
> >  .SH "SEE ALSO"
> > diff --git a/ovsdb/ovsdb-client.c b/ovsdb/ovsdb-client.c
> > index 1f83f3b..452c416 100644
> > --- a/ovsdb/ovsdb-client.c
> > +++ b/ovsdb/ovsdb-client.c
> > @@ -174,7 +174,8 @@ parse_options(int argc, char *argv[])
> >          OPT_TIMESTAMP,
> >          VLOG_OPTION_ENUMS,
> >          DAEMON_OPTION_ENUMS,
> > -        TABLE_OPTION_ENUMS
> > +        TABLE_OPTION_ENUMS,
> > +        SSL_OPTION_ENUMS,
> >      };
> >      static const struct option long_options[] = {
> >          {"help", no_argument, NULL, 'h'},
> > diff --git a/ovsdb/ovsdb-server.1.in b/ovsdb/ovsdb-server.1.in
> > index d1ba83b..470645b 100644
> > --- a/ovsdb/ovsdb-server.1.in
> > +++ b/ovsdb/ovsdb-server.1.in
> > @@ -23,6 +23,7 @@ ovsdb\-server \- Open vSwitch database server
> >  .so lib/ssl-syn.man
> >  .so lib/ssl-bootstrap-syn.man
> >  .so lib/ssl-peer-ca-cert-syn.man
> > +.so lib/ssl-connect-syn.man
> >  .so lib/unixctl-syn.man
> >  .so lib/common-syn.man
> >  .
> > @@ -133,6 +134,8 @@ one row in \fItable\fR.)
> >  .so lib/ssl.man
> >  .so lib/ssl-bootstrap.man
> >  .so lib/ssl-peer-ca-cert.man
> > +.SS "SSL Connection Options"
> > +.so lib/ssl-connect.man
> >  .SS "Other Options"
> >  .so lib/unixctl.man
> >  .so lib/common.man
> > diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c
> > index e08c341..f34d0d8 100644
> > --- a/ovsdb/ovsdb-server.c
> > +++ b/ovsdb/ovsdb-server.c
> > @@ -66,6 +66,8 @@ struct db;
> >  static char *private_key_file;
> >  static char *certificate_file;
> >  static char *ca_cert_file;
> > +static char *ssl_protocols;
> > +static char *ssl_ciphers;
> >  static bool bootstrap_ca_cert;
> >
> >  /* Replication configuration. */
> > @@ -1024,13 +1026,19 @@ reconfigure_ssl(const struct shash *all_dbs)
> >      const char *resolved_private_key;
> >      const char *resolved_certificate;
> >      const char *resolved_ca_cert;
> > +    const char *resolved_ssl_protocols;
> > +    const char *resolved_ssl_ciphers;
> >
> >      resolved_private_key = query_db_string(all_dbs, private_key_file, &errors);
> >      resolved_certificate = query_db_string(all_dbs, certificate_file, &errors);
> >      resolved_ca_cert = query_db_string(all_dbs, ca_cert_file, &errors);
> > +    resolved_ssl_protocols = query_db_string(all_dbs, ssl_protocols, &errors);
> > +    resolved_ssl_ciphers = query_db_string(all_dbs, ssl_ciphers, &errors);
> >
> >      stream_ssl_set_key_and_cert(resolved_private_key, resolved_certificate);
> >      stream_ssl_set_ca_cert_file(resolved_ca_cert, bootstrap_ca_cert);
> > +    stream_ssl_set_protocols(resolved_ssl_protocols);
> > +    stream_ssl_set_ciphers(resolved_ssl_ciphers);
> >
> >      return errors.string;
> >  }
> > @@ -1384,7 +1392,8 @@ parse_options(int *argcp, char **argvp[],
> >          OPT_SYNC_FROM,
> >          OPT_SYNC_EXCLUDE,
> >          VLOG_OPTION_ENUMS,
> > -        DAEMON_OPTION_ENUMS
> > +        DAEMON_OPTION_ENUMS,
> > +        SSL_OPTION_ENUMS,
> >      };
> >      static const struct option long_options[] = {
> >          {"remote",      required_argument, NULL, OPT_REMOTE},
> > @@ -1398,9 +1407,7 @@ parse_options(int *argcp, char **argvp[],
> >          VLOG_LONG_OPTIONS,
> >          {"bootstrap-ca-cert", required_argument, NULL, OPT_BOOTSTRAP_CA_CERT},
> >          {"peer-ca-cert", required_argument, NULL, OPT_PEER_CA_CERT},
> > -        {"private-key", required_argument, NULL, 'p'},
> > -        {"certificate", required_argument, NULL, 'c'},
> > -        {"ca-cert",     required_argument, NULL, 'C'},
> > +        STREAM_SSL_LONG_OPTIONS,
> >          {"sync-from",   required_argument, NULL, OPT_SYNC_FROM},
> >          {"sync-exclude-tables", required_argument, NULL, OPT_SYNC_EXCLUDE},
> >          {NULL, 0, NULL, 0},
> > @@ -1454,6 +1461,14 @@ parse_options(int *argcp, char **argvp[],
> >              bootstrap_ca_cert = false;
> >              break;
> >
> > +        case OPT_SSL_PROTOCOLS:
> > +            ssl_protocols = optarg;
> > +            break;
> > +
> > +        case OPT_SSL_CIPHERS:
> > +            ssl_ciphers = optarg;
> > +            break;
> > +
> >          case OPT_BOOTSTRAP_CA_CERT:
> >              ca_cert_file = optarg;
> >              bootstrap_ca_cert = true;
> > diff --git a/tests/ovsdb-server.at b/tests/ovsdb-server.at
> > index 0436de8..6347829 100644
> > --- a/tests/ovsdb-server.at
> > +++ b/tests/ovsdb-server.at
> > @@ -513,9 +513,13 @@ AT_DATA([schema],
> >           "columns": {
> >             "private_key": {"type": "string"},
> >             "certificate": {"type": "string"},
> > -           "ca_cert": {"type": "string"}}}}}
> > +           "ca_cert": {"type": "string"},
> > +           "ssl_protocols" : {"type": "string"},
> > +           "ssl_ciphers" : {"type" : "string"}}}}}
> >  ]])
> >  AT_CHECK([ovsdb-tool create db schema], [0], [stdout], [ignore])
> > +# The !ECDHE-ECDSA-AES256-GCM-SHA384 in the ssl_ciphers is so that
> > +# a cipher negotiation failure can be tested for later.
> >  AT_CHECK(
> >    [[ovsdb-tool transact db \
> >       '["mydb",
> > @@ -523,13 +527,17 @@ AT_CHECK(
> >          "table": "SSL",
> >          "row": {"private_key": "'"$PKIDIR/testpki-privkey2.pem"'",
> >                  "certificate": "'"$PKIDIR/testpki-cert2.pem"'",
> > -                "ca_cert": "'"$PKIDIR/testpki-cacert.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])
> >  AT_CHECK(
> >    [ovsdb-server --log-file --detach --no-chdir --pidfile="`pwd`"/pid \
> >          --private-key=db:mydb,SSL,private_key \
> >          --certificate=db:mydb,SSL,certificate \
> >          --ca-cert=db:mydb,SSL,ca_cert \
> > +        --ssl-protocols=db:mydb,SSL,ssl_protocols \
> > +        --ssl-ciphers=db:mydb,SSL,ssl_ciphers \
> >          --remote=pssl:0:127.0.0.1 --unixctl="`pwd`"/unixctl db],
> >    [0], [ignore], [ignore])
> >  PARSE_LISTENING_PORT([ovsdb-server.log], [SSL_PORT])
> > @@ -538,6 +546,8 @@ 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",
> >            {"op": "select",
> > @@ -550,6 +560,60 @@ AT_CHECK_UNQUOTED(
> >    [cat output], [0],
> >    [[@<:@{"rows":@<:@{"private_key":"$PKIDIR/testpki-privkey2.pem"}@:>@}@:>@
> >  ]], [ignore], [test ! -e pid || kill `cat pid`])
> > +# Check that when the server has TLSv1.1+ and the client has
> > +# TLSv1 that the connection fails.
> > +AT_CHECK(
> > +  [[ovsdb-client \
> > +        --private-key=$PKIDIR/testpki-privkey.pem \
> > +        --certificate=$PKIDIR/testpki-cert.pem \
> > +        --ca-cert=$PKIDIR/testpki-cacert.pem \
> > +        --ssl-protocols=TLSv1 \
> > +        --ssl-ciphers=HIGH:!aNULL:!MD5 \
> > +        transact ssl:127.0.0.1:$SSL_PORT \
> > +        '["mydb",
> > +          {"op": "select",
> > +           "table": "SSL",
> > +           "where": [],
> > +           "columns": ["private_key"]}]']],
> > +  [1], [stdout],
> > +  [stderr],
> > +  [test ! -e pid || kill `cat pid`])
> > +cat stderr > output
> > +AT_CHECK_UNQUOTED(
> > +  [grep "failed to connect" output], [0],
> > +  [ovsdb-client: failed to connect to "ssl:127.0.0.1:$SSL_PORT" (Protocol error)
> > +],
> > +  [ignore], [test ! -e pid || kill `cat pid`])
> > +# Check that when ciphers are not compatible, that a negotiation
> > +# failure occurs.
> > +AT_CHECK(
> > +  [[ovsdb-client \
> > +        --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=ECDHE-ECDSA-AES256-GCM-SHA384 \
> > +        transact ssl:127.0.0.1:$SSL_PORT \
> > +        '["mydb",
> > +          {"op": "select",
> > +           "table": "SSL",
> > +           "where": [],
> > +           "columns": ["private_key"]}]']],
> > +  [1], [stdout],
> > +  [stderr],
> > +  [test ! -e pid || kill `cat pid`])
> > +cat stderr > output
> > +AT_CHECK_UNQUOTED(
> > +  [grep "failed to connect" output], [0],
> > +  [ovsdb-client: failed to connect to "ssl:127.0.0.1:$SSL_PORT" (Protocol error)
> > +],
> > +  [ignore], [test ! -e pid || kill `cat pid`])
> > +# The error message for being unable to negotiate a shared ciphersuite
> > +# is 'sslv3 alert handshake failure'. This is not the clearest message.
> > +AT_CHECK_UNQUOTED(
> > +  [grep "sslv3 alert handshake failure" output], [0],
> > +  [stdout],
> > +  [ignore], [test ! -e pid || kill `cat pid`])
> >  OVSDB_SERVER_SHUTDOWN
> >  AT_CLEANUP
> >
> > diff --git a/tests/test-jsonrpc.c b/tests/test-jsonrpc.c
> > index 684601a..a7ca691 100644
> > --- a/tests/test-jsonrpc.c
> > +++ b/tests/test-jsonrpc.c
> > @@ -55,7 +55,8 @@ parse_options(int argc, char *argv[])
> >  {
> >      enum {
> >          OPT_BOOTSTRAP_CA_CERT = UCHAR_MAX + 1,
> > -        DAEMON_OPTION_ENUMS
> > +        DAEMON_OPTION_ENUMS,
> > +        SSL_OPTION_ENUMS,
> >      };
> >      static const struct option long_options[] = {
> >          {"verbose", optional_argument, NULL, 'v'},
> > diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
> > index 6fd3818..fa2466e 100644
> > --- a/utilities/ovs-ofctl.c
> > +++ b/utilities/ovs-ofctl.c
> > @@ -190,7 +190,8 @@ parse_options(int argc, char *argv[])
> >          OPT_READ_ONLY,
> >          DAEMON_OPTION_ENUMS,
> >          OFP_VERSION_OPTION_ENUMS,
> > -        VLOG_OPTION_ENUMS
> > +        VLOG_OPTION_ENUMS,
> > +        SSL_OPTION_ENUMS,
> >      };
> >      static const struct option long_options[] = {
> >          {"timeout", required_argument, NULL, 't'},
> > diff --git a/utilities/ovs-testcontroller.c b/utilities/ovs-testcontroller.c
> > index 1db3bbe..99f5bf8 100644
> > --- a/utilities/ovs-testcontroller.c
> > +++ b/utilities/ovs-testcontroller.c
> > @@ -258,7 +258,8 @@ parse_options(int argc, char *argv[])
> >          OPT_UNIXCTL,
> >          VLOG_OPTION_ENUMS,
> >          DAEMON_OPTION_ENUMS,
> > -        OFP_VERSION_OPTION_ENUMS
> > +        OFP_VERSION_OPTION_ENUMS,
> > +        SSL_OPTION_ENUMS,
> >      };
> >      static const struct option long_options[] = {
> >          {"hub",         no_argument, NULL, 'H'},
> > diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c
> > index e710095..25f0e8a 100644
> > --- a/utilities/ovs-vsctl.c
> > +++ b/utilities/ovs-vsctl.c
> > @@ -207,7 +207,8 @@ parse_options(int argc, char *argv[], struct shash *local_options)
> >          OPT_COMMANDS,
> >          OPT_OPTIONS,
> >          VLOG_OPTION_ENUMS,
> > -        TABLE_OPTION_ENUMS
> > +        TABLE_OPTION_ENUMS,
> > +        SSL_OPTION_ENUMS,
> >      };
> >      static const struct option global_long_options[] = {
> >          {"db", required_argument, NULL, OPT_DB},
> > diff --git a/vswitchd/ovs-vswitchd.c b/vswitchd/ovs-vswitchd.c
> > index 72448bb..0e3b6d0 100644
> > --- a/vswitchd/ovs-vswitchd.c
> > +++ b/vswitchd/ovs-vswitchd.c
> > @@ -145,6 +145,7 @@ parse_options(int argc, char *argv[], char **unixctl_pathp)
> >          OPT_DISABLE_SYSTEM,
> >          DAEMON_OPTION_ENUMS,
> >          OPT_DPDK,
> > +        SSL_OPTION_ENUMS,
> >          OPT_DUMMY_NUMA,
> >      };
> >      static const struct option long_options[] = {
> > diff --git a/vtep/vtep-ctl.c b/vtep/vtep-ctl.c
> > index 245ba0d..af20f83 100644
> > --- a/vtep/vtep-ctl.c
> > +++ b/vtep/vtep-ctl.c
> > @@ -166,7 +166,8 @@ parse_options(int argc, char *argv[], struct shash *local_options)
> >          OPT_PEER_CA_CERT,
> >          OPT_LOCAL,
> >          VLOG_OPTION_ENUMS,
> > -        TABLE_OPTION_ENUMS
> > +        TABLE_OPTION_ENUMS,
> > +        SSL_OPTION_ENUMS,
> >      };
> >      static const struct option global_long_options[] = {
> >          {"db", required_argument, NULL, OPT_DB},
> > --
> > 1.8.1.4
> >

OVSDB currently does not support fine-tuning the SSL parameters used for connections. This means that users are unable to specify not using ciphers widely considered to be unsafe or to avoid using TLS protocols that do not meet their organizational standards.

This adds two new commands "--ssl-protocols" and "--ssl-ciphers" to the ovsdb programs to specify which SSL protocols and ciphers to use. In addition, the default cipher string is set to "HIGH:!aNULL:!MD5". This is the current default for nginx and removes weak ciphers while allowing most services from the last several years to still connect.

The patch was tested by adding new test cases that check that the options can be set and that when incompatible SSL parameters are used that it results in a failure to communicate. Additionally, since this adds 2 new files, "make distcheck" was used to verify that this works correctly.

Ethan Rahn (1):
  Add support for specifying SSL connection parameters to ovsdb

 AUTHORS                                   |  1 +
 lib/automake.mk                           |  2 +
 lib/ssl-connect-syn.man                   |  5 +++
 lib/ssl-connect.man                       | 16 +++++++
 lib/stream-ssl.c                          | 70 +++++++++++++++++++++++++++++++
 lib/stream-ssl.h                          | 20 ++++++++-
 manpages.mk                               |  8 ++++
 ovn/controller-vtep/ovn-controller-vtep.c |  3 +-
 ovn/controller/ovn-controller.c           |  3 +-
 ovn/northd/ovn-northd.c                   |  1 +
 ovn/utilities/ovn-nbctl.c                 |  3 +-
 ovn/utilities/ovn-sbctl.c                 |  3 +-
 ovn/utilities/ovn-trace.c                 |  1 +
 ovsdb/ovsdb-client.1.in                   |  3 ++
 ovsdb/ovsdb-client.c                      |  3 +-
 ovsdb/ovsdb-server.1.in                   |  3 ++
 ovsdb/ovsdb-server.c                      | 23 ++++++++--
 tests/ovsdb-server.at                     | 68 +++++++++++++++++++++++++++++-
 tests/test-jsonrpc.c                      |  3 +-
 utilities/ovs-ofctl.c                     |  3 +-
 utilities/ovs-testcontroller.c            |  3 +-
 utilities/ovs-vsctl.c                     |  3 +-
 vswitchd/ovs-vswitchd.c                   |  1 +
 vtep/vtep-ctl.c                           |  3 +-
 24 files changed, 234 insertions(+), 18 deletions(-)
 create mode 100644 lib/ssl-connect-syn.man
 create mode 100644 lib/ssl-connect.man