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

Han Zhou zhouhan at gmail.com
Mon Oct 8 17:46:57 UTC 2018


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?

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

>> >
>> > -    if test X"$create_insecure_remote" = Xyes; then
>> > -        set "$@" --remote=ptcp:$port:$addr
>> > +    if test X"$ovn_db_ssl_key" != X; then
>> > +        set "$@" --private-key=$ovn_db_ssl_key
>> > +    else
>> > +        set "$@" --private-key=db:$schema_name,SSL,private_key
>> > +    fi
>> > +    if test X"$ovn_db_ssl_cert" != X; then
>> > +        set "$@" --certificate=$ovn_db_ssl_cert
>> > +    else
>> > +        set "$@" --certificate=db:$schema_name,SSL,certificate
>> > +    fi
>> > +    if test X"$ovn_db_ssl_cacert" != X; then
>> > +        set "$@" --ca-cert=$ovn_db_ssl_cacert
>> > +    else
>> > +        set "$@" --ca-cert=db:$schema_name,SSL,ca_cert
>> >      fi
>> >
>> > +    set "$@" --ssl-protocols=db:$schema_name,SSL,ssl_protocols
>> > +    set "$@" --ssl-ciphers=db:$schema_name,SSL,ssl_ciphers
>> > +
>> >      if test $mode = active_passive; then
>> >          set "$@" --sync-from=`cat $active_conf_file`
>> >      fi
>> > @@ -481,6 +502,15 @@ set_defaults () {
>> >      OVN_NORTHD_SB_DB="unix:$DB_SB_SOCK"
>> >      DB_NB_USE_REMOTE_IN_DB="yes"
>> >      DB_SB_USE_REMOTE_IN_DB="yes"
>> > +
>> > +    OVN_NB_DB_SSL_KEY=""
>> > +    OVN_NB_DB_SSL_CERT=""
>> > +    OVN_NB_DB_SSL_CA_CERT=""
>> > +
>> > +    OVN_SB_DB_SSL_KEY=""
>> > +    OVN_SB_DB_SSL_CERT=""
>> > +    OVN_SB_DB_SSL_CA_CERT=""
>> > +
>> >  }
>> >
>> >  set_option () {
>> > @@ -536,6 +566,12 @@ Options:
>> >    --ovn-controller-ssl-cert=CERT OVN Southbound SSL certificate file
>> >    --ovn-controller-ssl-ca-cert=CERT OVN Southbound SSL CA certificate
file
>> >    --ovn-controller-ssl-bootstrap-ca-cert=CERT Bootstrapped OVN
Southbound SSL CA certificate file
>> > +  --ovn-nb-db-ssl-key=KEY OVN Northbound DB SSL private key file
>> > +  --ovn-nb-db-ssl-cert=CERT OVN Northbound DB SSL certificate file
>> > +  --ovn-nb-db-ssl-ca-cert=CERT OVN Northbound DB SSL CA certificate
file
>> > +  --ovn-sb-db-ssl-key=KEY OVN Southbound DB SSL private key file
>> > +  --ovn-sb-db-ssl-cert=CERT OVN Southbound DB SSL certificate file
>> > +  --ovn-sb-db-ssl-ca-cert=CERT OVN Southbound DB SSL CA certificate
file
>> >    --ovn-manage-ovsdb=yes|no        Whether or not the OVN databases
should be
>> >                                     automatically started and stopped
along
>> >                                     with ovn-northd. The default is
"yes". If
>> > --
>> > 1.9.1
>> >
>> > _______________________________________________
>> > dev mailing list
>> > dev at openvswitch.org
>> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev


More information about the dev mailing list