[ovs-dev] [PATCH 1/2] ovn-ctl: Allow passing ssl certs when starting OVN DBs in ssl mode.

aginwala aginwala amginwal at gmail.com
Tue Oct 9 01:49:47 UTC 2018


Sure. I will add ssl usage example with some brief in the ovn-ctl.8.xml and
send v3 for this patch . Does that sound good?

On Mon, Oct 8, 2018 at 5:32 PM Han Zhou <zhouhan at gmail.com> wrote:

>
>
> On Mon, Oct 8, 2018 at 4:55 PM aginwala aginwala <amginwal at gmail.com>
> wrote:
> >
> >
> >
> > On Mon, Oct 8, 2018 at 10:47 AM Han Zhou <zhouhan at gmail.com> wrote:
> >>
> >>
> >>
> >> On Fri, Oct 5, 2018 at 6:48 PM aginwala aginwala <amginwal at gmail.com>
> wrote:
> >> >
> >> > Thanks for the review Han. Please find the comments inline below:
> >> > On Thu, Oct 4, 2018 at 9:58 AM Han Zhou <zhouhan at gmail.com> wrote:
> >> >>
> >> >> Thanks Ali, please see my comments below
> >> >>
> >> >> On Fri, Sep 21, 2018 at 5:34 PM <amginwal at gmail.com> wrote:
> >> >> >
> >> >> >  For OVN DBs to work with SSL in HA, we need to have capability to
> >> >> >  pass ssl certs when starting OVN DBs. Say when starting OVN DBs
> in active
> >> >> >  passive mode, in order for the standby DBs to sync from master
> node, it
> >> >> >  cannot sync because the required ssl certs are not passed when
> standby DBs
> >> >> >  are initialized. Hence, we need to have this option.
> >> >> >
> >> >> > e.g. start nb db with ssl certs as below:
> >> >> > /usr/share/openvswitch/scripts/ovn-ctl
> --ovn-nb-db-ssl-key=/etc/openvswitch/ovnnb-privkey.pem \
> >> >> > --ovn-nb-db-ssl-cert=/etc/openvswitch/ovnnb-cert.pem \
> >> >> > --ovn-nb-db-ssl-ca-cert=/etc/openvswitch/cacert.pem \
> >> >> > --db-nb-create-insecure-remote=no start_nb_ovsdb
> >> >> >
> >> >> > Certs can be generated based on ovs ssl docs:
> >> >> > http://docs.openvswitch.org/en/latest/howto/ssl/
> >> >> >
> >> >> > Signed-off-by: aginwala <aginwala at ebay.com>
> >> >> > ---
> >> >> >  ovn/utilities/ovn-ctl | 50
> +++++++++++++++++++++++++++++++++++++++++++-------
> >> >> >  1 file changed, 43 insertions(+), 7 deletions(-)
> >> >> >
> >> >> > diff --git a/ovn/utilities/ovn-ctl b/ovn/utilities/ovn-ctl
> >> >> > index 3ff0df6..4f45f3d 100755
> >> >> > --- a/ovn/utilities/ovn-ctl
> >> >> > +++ b/ovn/utilities/ovn-ctl
> >> >> > @@ -116,6 +116,9 @@ start_ovsdb__() {
> >> >> >      local addr
> >> >> >      local active_conf_file
> >> >> >      local use_remote_in_db
> >> >> > +    local ovn_db_ssl_key
> >> >> > +    local ovn_db_ssl_cert
> >> >> > +    local ovn_db_ssl_cacert
> >> >> >      eval pid=\$DB_${DB}_PID
> >> >> >      eval cluster_local_addr=\$DB_${DB}_CLUSTER_LOCAL_ADDR
> >> >> >      eval cluster_local_port=\$DB_${DB}_CLUSTER_LOCAL_PORT
> >> >> > @@ -137,6 +140,9 @@ start_ovsdb__() {
> >> >> >      eval addr=\$DB_${DB}_ADDR
> >> >> >      eval active_conf_file=\$ovn${db}_active_conf_file
> >> >> >      eval use_remote_in_db=\$DB_${DB}_USE_REMOTE_IN_DB
> >> >> > +    eval ovn_db_ssl_key=\$OVN_${DB}_DB_SSL_KEY
> >> >> > +    eval ovn_db_ssl_cert=\$OVN_${DB}_DB_SSL_CERT
> >> >> > +    eval ovn_db_ssl_cacert=\$OVN_${DB}_DB_SSL_CA_CERT
> >> >> >
> >> >> >      # Check and eventually start ovsdb-server for DB
> >> >> >      if pidfile_is_running $pid; then
> >> >> > @@ -182,17 +188,32 @@ $cluster_remote_port
> >> >> >
> >> >> >      if test X"$use_remote_in_db" != Xno; then
> >> >> >          set "$@" --remote=db:$schema_name,$table_name,connections
> >> >> > +        if test X"$create_insecure_remote" = Xno; then
> >> >> > +            set "$@" --remote=pssl:$port:$addr
> >> >> > +        elif test X"$create_insecure_remote" = Xyes; then
> >> >> > +            set "$@" --remote=ptcp:$port:$addr
> >> >> > +        fi
> >> >> Why moving the logic here? This if block only says if the connection
> settings in DB table should be used. Whether insecure mode is allowed was
> supposed to be independent with this condition. Could you explain the
> reason behind the change?
> >> >> >> I moved it because remote=db is needed if ovsdb is running as a
> standalone node or an active ovsdb server node. For standby nodes in case
> of active_passive mode, remote=db will not be there because it uses
> --sync-from. Hope its clear.
> >>
> >> As discussed, $use_remote_in_db and $create_insecure_remote were
> independent. Moving $create_insecure_remote logic here make it useful only
> when $use_remote_in_db is "yes", which is not how it was supposed to be.
> >>
> >> I understand that for standby node, we will set $use_remote_in_db as
> "no". Is this a problem?
> >>
> > >> Just verified and have kept the logic intact as even for ssl, we have
> connection table set for 0.0.0.0 for LB use case and it works fine. Hence,
> have updated it in v2. Please take a look and let me know.
> >>
> >> --sync-from has nothing to do with "remote".
> >> >
> >> >
> >> >>
> >> >> >      fi
> >> >> > -    set "$@" --private-key=db:$schema_name,SSL,private_key
> >> >> > -    set "$@" --certificate=db:$schema_name,SSL,certificate
> >> >> > -    set "$@" --ca-cert=db:$schema_name,SSL,ca_cert
> >> >> > -    set "$@" --ssl-protocols=db:$schema_name,SSL,ssl_protocols
> >> >> > -    set "$@" --ssl-ciphers=db:$schema_name,SSL,ssl_ciphers
> >> >>
> >> >> So it will not use the settings in DB any more? It seems this change
> removed support for "--use-remote-in-db=yes", which is the default behavior
> that should be kept. The DB settings should not be used only if
> "--use-remote-in-db=no"
> >> >
> >> > >>  As discussed, this is similar support that we have for
> ovn-controller with ssl. If the key is passed from cli, it will use the key
> else fall back to default setters for ssl.
> >> >>
> >> I missed the "else" below. So it does fallback to use DB config when
> command line option is not specified. It looks good for me, but it would be
> better to clarify the behavior in help message of this tool that command
> line options for these SSL related parameters overrides any DB setting.
> >
> > >>> Cool. So it does shows ssl key in  options section when you type
> help similar to ovn-controller ssl support and hope that suffices.  Please
> ack if all ok in v2 patch that I sent.
> >>
> I meant documenting the behavior that command line option always override
> DB settings if both exist.
>


More information about the dev mailing list