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

2016-11-02 Thread Ethan Rahn
Added a 'Signed-off-by' line. Thanks for getting back to me and this being
patient while I learned how to use 'git send-email'. I'm glad I could
contribute upstream to the project.

Cheers,

Ethan

On Wed, Nov 2, 2016 at 4:19 PM, Ethan Rahn  wrote:

>
> Signed-off-by: Ethan Rahn 
> ---
>  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 Eliahuelia...@vmware.com
>  Eohyung Lee liquidnu...@gmail.com
>  Eric Sesterhenn eric.sesterh...@lsexperts.de
>  Ethan J. Jacksone...@eecs.berkeley.edu
> +Ethan Rahn  er...@arista.com
>  Eziz Durdyyev   ezizdu...@gmail.com
>  Flavio Fernandesfla...@flaviof.com
>  Flavio Leitner  f...@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 000..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 000..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 || 

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

2016-11-02 Thread Ben Pfaff
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  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 Eliahuelia...@vmware.com
> >  Eohyung Lee liquidnu...@gmail.com
> >  Eric Sesterhenn eric.sesterh...@lsexperts.de
> >  Ethan J. Jacksone...@eecs.berkeley.edu
> > +Ethan Rahn  er...@arista.com
> >  Eziz Durdyyev   ezizdu...@gmail.com
> >  Flavio Fernandesfla...@flaviof.com
> >  Flavio Leitner  f...@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 000..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 000..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;
> 

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

2016-10-06 Thread Ethan Rahn
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  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 Eliahuelia...@vmware.com
>  Eohyung Lee liquidnu...@gmail.com
>  Eric Sesterhenn eric.sesterh...@lsexperts.de
>  Ethan J. Jacksone...@eecs.berkeley.edu
> +Ethan Rahn  er...@arista.com
>  Eziz Durdyyev   ezizdu...@gmail.com
>  Flavio Fernandesfla...@flaviof.com
>  Flavio Leitner  f...@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 000..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 000..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)
> +

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

2016-10-04 Thread Ben Pfaff
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 
> 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 
> ---
>  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 Eliahuelia...@vmware.com
>  Eohyung Lee liquidnu...@gmail.com
>  Eric Sesterhenn eric.sesterh...@lsexperts.de
>  Ethan J. Jacksone...@eecs.berkeley.edu
> +Ethan Rahn  er...@arista.com
>  Eziz Durdyyev   ezizdu...@gmail.com
>  Flavio Fernandesfla...@flaviof.com
>  Flavio Leitner  f...@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 000..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 000..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 

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

2016-10-03 Thread Ethan Rahn
From ae9961aa2521ebd5cfeef28812d8a089b7b5e55b Mon Sep 17 00:00:00 2001
From: Ethan Rahn 
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 
---
 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 Eliahuelia...@vmware.com
 Eohyung Lee liquidnu...@gmail.com
 Eric Sesterhenn eric.sesterh...@lsexperts.de
 Ethan J. Jacksone...@eecs.berkeley.edu
+Ethan Rahn  er...@arista.com
 Eziz Durdyyev   ezizdu...@gmail.com
 Flavio Fernandesfla...@flaviof.com
 Flavio Leitner  f...@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 000..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 000..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 

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

2016-10-03 Thread Ethan Rahn
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  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 
>> 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.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


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

2016-10-03 Thread Ben Pfaff
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 
> 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.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


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

2016-10-03 Thread Ben Pfaff
It hasn't been lost.  We've been frozen for new features since Aug. 1,
so no one has looked at it yet.  We just released OVS 2.6, so I'm going
through the feature proposals from the last two months now.

On Wed, Sep 21, 2016 at 01:02:06PM -0700, Ethan Rahn wrote:
> Hello,
> 
> I submitted this about a month ago and haven't heard a response, so I
> wanted to follow up on this.
> 
> Cheers,
> 
> Ethan
> 
> On Mon, Aug 22, 2016 at 12:17 PM, Ethan Rahn  wrote:
> 
> > From ae9961aa2521ebd5cfeef28812d8a089b7b5e55b Mon Sep 17 00:00:00 2001
> > From: Ethan Rahn 
> > 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 
> > ---
> >  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 Eliahuelia...@vmware.com
> >  Eohyung Lee liquidnu...@gmail.com
> >  Eric Sesterhenn eric.sesterh...@lsexperts.de
> >  Ethan J. Jacksone...@eecs.berkeley.edu
> > +Ethan Rahn  er...@arista.com
> >  Eziz Durdyyev   ezizdu...@gmail.com
> >  Flavio Fernandesfla...@flaviof.com
> >  Flavio Leitner  f...@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 000..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 000..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 

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

2016-09-21 Thread Ethan Rahn
Hello,

I submitted this about a month ago and haven't heard a response, so I
wanted to follow up on this.

Cheers,

Ethan

On Mon, Aug 22, 2016 at 12:17 PM, Ethan Rahn  wrote:

> From ae9961aa2521ebd5cfeef28812d8a089b7b5e55b Mon Sep 17 00:00:00 2001
> From: Ethan Rahn 
> 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 
> ---
>  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 Eliahuelia...@vmware.com
>  Eohyung Lee liquidnu...@gmail.com
>  Eric Sesterhenn eric.sesterh...@lsexperts.de
>  Ethan J. Jacksone...@eecs.berkeley.edu
> +Ethan Rahn  er...@arista.com
>  Eziz Durdyyev   ezizdu...@gmail.com
>  Flavio Fernandesfla...@flaviof.com
>  Flavio Leitner  f...@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 000..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 000..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
> @@