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

Wed Oct 10 00:52:04 UTC 2018

Hi Han:

Have added the man section for ssl in v3. PTAL.

Thanks,

On Mon, Oct 8, 2018 at 6:49 PM aginwala aginwala <amginwal at gmail.com> wrote:

> 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.
>>
>