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

Han Zhou zhouhan at gmail.com
Tue Oct 9 00:32:34 UTC 2018


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