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

Ben Pfaff blp at ovn.org
Tue Oct 4 22:11:51 UTC 2016


Hi, this is still word-wrapped.

On Mon, Oct 03, 2016 at 02:47:54PM -0700, Ethan Rahn wrote:
> From ae9961aa2521ebd5cfeef28812d8a089b7b5e55b Mon Sep 17 00:00:00 2001
> From: Ethan Rahn <erahn at arista.com>
> Date: Mon, 22 Aug 2016 11:26:54 -0700
> Subject: [PATCH] Add support for specifying SSL connection parameters to ovsdb
> 
> 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.
> 
> Signed-off-by: Ethan Rahn <erahn at arista.com>
> ---
>  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 at vmware.com
>  Eohyung Lee             liquidnuker at gmail.com
>  Eric Sesterhenn         eric.sesterhenn at lsexperts.de
>  Ethan J. Jackson        ejj at eecs.berkeley.edu
> +Ethan Rahn              erahn at arista.com
>  Eziz Durdyyev           ezizdurdy at gmail.com
>  Flavio Fernandes        flavio at flaviof.com
>  Flavio Leitner          fbl at 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
> 
> On Mon, Oct 3, 2016 at 2:44 PM, Ethan Rahn <erahn at arista.com> wrote:
> > Ben,
> >
> > Thanks for your follow up email about the patch and feature freeze.
> > I'll resend the patch in the next few minutes.
> >
> > Cheers,
> >
> > Ethan
> >
> > On Mon, Oct 3, 2016 at 1:55 PM, Ben Pfaff <blp at ovn.org> wrote:
> >> On Mon, Aug 22, 2016 at 12:17:07PM -0700, Ethan Rahn wrote:
> >>> From ae9961aa2521ebd5cfeef28812d8a089b7b5e55b Mon Sep 17 00:00:00 2001
> >>> From: Ethan Rahn <erahn at arista.com>
> >>> Date: Mon, 22 Aug 2016 11:26:54 -0700
> >>> Subject: [PATCH] Add support for specifying SSL connection parameters to
> >>> ovsdb
> >>>
> >>> 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.
> >>
> >> Hi, this patch is word-wrapped.  Can you resubmit it without
> >> word-wrapping?  "git send-email" does the right thing, if you can use
> >> it.  As an alternative, you can submit the patch as a github pull
> >> request, though we prefer email.
> >>
> >> Thanks,
> >>
> >> Ben.



More information about the dev mailing list