| Message ID | 1537460151-22894-1-git-send-email-aginwala@ebay.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [ovs-dev,1/2] ovn-ctl: Allow passing ssl certs when starting OVN DBs in ssl mode. | expand |
Thanks Ali, please see my comments below On Fri, Sep 21, 2018 at 5:34 PM <amginwal@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@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? > 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" > > - 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@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Thanks for the review Han. Please find the comments inline below: On Thu, Oct 4, 2018 at 9:58 AM Han Zhou <zhouhan@gmail.com> wrote: > Thanks Ali, please see my comments below > > On Fri, Sep 21, 2018 at 5:34 PM <amginwal@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@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. > > > 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. > > > > - 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@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
On Fri, Oct 5, 2018 at 6:48 PM aginwala aginwala <amginwal@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@gmail.com> wrote: >> >> Thanks Ali, please see my comments below >> >> On Fri, Sep 21, 2018 at 5:34 PM <amginwal@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@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@openvswitch.org >> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
On Mon, Oct 8, 2018 at 10:47 AM Han Zhou <zhouhan@gmail.com> wrote: > > > On Fri, Oct 5, 2018 at 6:48 PM aginwala aginwala <amginwal@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@gmail.com> wrote: > >> > >> Thanks Ali, please see my comments below > >> > >> On Fri, Sep 21, 2018 at 5:34 PM <amginwal@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@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. > >> > > >> > - 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@openvswitch.org > >> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
On Mon, Oct 8, 2018 at 4:55 PM aginwala aginwala <amginwal@gmail.com> wrote: > > > > On Mon, Oct 8, 2018 at 10:47 AM Han Zhou <zhouhan@gmail.com> wrote: >> >> >> >> On Fri, Oct 5, 2018 at 6:48 PM aginwala aginwala <amginwal@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@gmail.com> wrote: >> >> >> >> Thanks Ali, please see my comments below >> >> >> >> On Fri, Sep 21, 2018 at 5:34 PM <amginwal@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@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.
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@gmail.com> wrote: > > > On Mon, Oct 8, 2018 at 4:55 PM aginwala aginwala <amginwal@gmail.com> > wrote: > > > > > > > > On Mon, Oct 8, 2018 at 10:47 AM Han Zhou <zhouhan@gmail.com> wrote: > >> > >> > >> > >> On Fri, Oct 5, 2018 at 6:48 PM aginwala aginwala <amginwal@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@gmail.com> wrote: > >> >> > >> >> Thanks Ali, please see my comments below > >> >> > >> >> On Fri, Sep 21, 2018 at 5:34 PM <amginwal@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@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. >
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@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@gmail.com> wrote: > >> >> >> On Mon, Oct 8, 2018 at 4:55 PM aginwala aginwala <amginwal@gmail.com> >> wrote: >> > >> > >> > >> > On Mon, Oct 8, 2018 at 10:47 AM Han Zhou <zhouhan@gmail.com> wrote: >> >> >> >> >> >> >> >> On Fri, Oct 5, 2018 at 6:48 PM aginwala aginwala <amginwal@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@gmail.com> wrote: >> >> >> >> >> >> Thanks Ali, please see my comments below >> >> >> >> >> >> On Fri, Sep 21, 2018 at 5:34 PM <amginwal@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@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. >> >
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 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 - 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
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@ebay.com> --- ovn/utilities/ovn-ctl | 50 +++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 43 insertions(+), 7 deletions(-)