| Message ID | 20210611062452.362848-1-hzhou@ovn.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev,1/4] ovn-northd.at: Fix test "northd ssl file change -- ovn-northd-ddlog". | expand |
Hi Han, I'm fine with fixing the test this way, since like you said it's not meant to test RBAC. Acked-by: Mark Michelson <mmichels@redhat.com> However, based on how this sounds, there is still a bug in ovn-northd-ddlog wrt RBAC, and that should still be fixed since this could cause failures for other tests. On 6/11/21 2:24 AM, Han Zhou wrote: > This test fails for ovn-northd-ddlog because of the RBAC role when using > the SSL connection. RBAC is not the purpose of the test case, so this > patch fixes it without enabling RBAC. > > Signed-off-by: Han Zhou <hzhou@ovn.org> > --- > tests/ovn-northd.at | 19 +++++++++++++++++-- > 1 file changed, 17 insertions(+), 2 deletions(-) > > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > index 4692775ad..ad1732da3 100644 > --- a/tests/ovn-northd.at > +++ b/tests/ovn-northd.at > @@ -3618,9 +3618,23 @@ ovn_start --backup-northd=none > as northd > OVS_APP_EXIT_AND_WAIT([NORTHD_TYPE]) > > +as ovn-sb > +OVS_APP_EXIT_AND_WAIT([ovsdb-server]) > + > +key_server=testpki-test-privkey.pem > +cert_server=testpki-test-cert.pem > +cacert=testpki-cacert.pem > + > +cd ovn-sb > +rm ovsdb-server.log > +ssl_options="--remote=pssl:0:127.0.0.1 ovn-sb.db -p $PKIDIR/$key_server -c $PKIDIR/$cert_server -C $PKIDIR/$cacert" > +AT_CHECK([ovsdb-server --detach --no-chdir --pidfile --log-file $ssl_options], [0], [], [stderr]) > +on_exit "kill `cat ovsdb-server.pid`" > +PARSE_LISTENING_PORT([ovsdb-server.log], [TCP_PORT]) > +cd .. > + > key=testpki-hv1-privkey.pem > cert=testpki-hv1-cert.pem > -cacert=testpki-cacert.pem > > key2=testpki-hv2-privkey.pem > cert3=testpki-hv3-cert.pem > @@ -3629,8 +3643,9 @@ cert3=testpki-hv3-cert.pem > cp $PKIDIR/$key2 $key > cp $PKIDIR/$cert3 $cert > cp $PKIDIR/$cacert $cacert > +as northd > start_daemon ovn$NORTHD_TYPE -vjsonrpc \ > - --ovnnb-db=$OVN_NB_DB --ovnsb-db=$SSL_OVN_SB_DB \ > + --ovnnb-db=$OVN_NB_DB --ovnsb-db=ssl:127.0.0.1:$TCP_PORT \ > -p $key -c $cert -C $cacert > > # SSL should not connect because of key and cert mismatch >
On Fri, Jun 11, 2021 at 11:24 AM Mark Michelson <mmichels@redhat.com> wrote: > > Hi Han, > > I'm fine with fixing the test this way, since like you said it's not > meant to test RBAC. > > Acked-by: Mark Michelson <mmichels@redhat.com> > Thanks Mark! I applied the patch 1-3 of this series to master and branch-2.16. I will address your comment for patch 4. > However, based on how this sounds, there is still a bug in > ovn-northd-ddlog wrt RBAC, and that should still be fixed since this > could cause failures for other tests. > In fact this is not a fault of ovn-northd-ddlog. It may be a general problem of north RBAC - the only role we defined is for ovn-controller, which is set in the Connection table's "role" column for the SSL connect method. It means there is no role for northd to apply RBAC. So if northd wants to use SSL, it needs a different SSL connection (probably a different port) in the connection table without setting the "role" (leave it empty). It has been discussed somewhere (probably in github) for a workaround to the problem, is to use iptables rules or other FW mechanism to allow access to the port dedicated for northd (without RBAC) only from northd IPs. For this test, it fails because the SSL connection created by the ovn_start() already sets the "role" to ovn-controller, so through this connection it is not allowed to update tables other than the ones allowed for ovn-controller role. For the regular ovn-northd this test case always succeeds because in fact the regular ovn-northd handles sb_cfg update incorrectly when the update to SB fails. It updates the expected sb_cfg back to NB even if it is not written to SB successfully. It can be fixed as a separate patch (not something urgent though). Thanks, Han > On 6/11/21 2:24 AM, Han Zhou wrote: > > This test fails for ovn-northd-ddlog because of the RBAC role when using > > the SSL connection. RBAC is not the purpose of the test case, so this > > patch fixes it without enabling RBAC. > > > > Signed-off-by: Han Zhou <hzhou@ovn.org> > > --- > > tests/ovn-northd.at | 19 +++++++++++++++++-- > > 1 file changed, 17 insertions(+), 2 deletions(-) > > > > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > > index 4692775ad..ad1732da3 100644 > > --- a/tests/ovn-northd.at > > +++ b/tests/ovn-northd.at > > @@ -3618,9 +3618,23 @@ ovn_start --backup-northd=none > > as northd > > OVS_APP_EXIT_AND_WAIT([NORTHD_TYPE]) > > > > +as ovn-sb > > +OVS_APP_EXIT_AND_WAIT([ovsdb-server]) > > + > > +key_server=testpki-test-privkey.pem > > +cert_server=testpki-test-cert.pem > > +cacert=testpki-cacert.pem > > + > > +cd ovn-sb > > +rm ovsdb-server.log > > +ssl_options="--remote=pssl:0:127.0.0.1 ovn-sb.db -p $PKIDIR/$key_server -c $PKIDIR/$cert_server -C $PKIDIR/$cacert" > > +AT_CHECK([ovsdb-server --detach --no-chdir --pidfile --log-file $ssl_options], [0], [], [stderr]) > > +on_exit "kill `cat ovsdb-server.pid`" > > +PARSE_LISTENING_PORT([ovsdb-server.log], [TCP_PORT]) > > +cd .. > > + > > key=testpki-hv1-privkey.pem > > cert=testpki-hv1-cert.pem > > -cacert=testpki-cacert.pem > > > > key2=testpki-hv2-privkey.pem > > cert3=testpki-hv3-cert.pem > > @@ -3629,8 +3643,9 @@ cert3=testpki-hv3-cert.pem > > cp $PKIDIR/$key2 $key > > cp $PKIDIR/$cert3 $cert > > cp $PKIDIR/$cacert $cacert > > +as northd > > start_daemon ovn$NORTHD_TYPE -vjsonrpc \ > > - --ovnnb-db=$OVN_NB_DB --ovnsb-db=$SSL_OVN_SB_DB \ > > + --ovnnb-db=$OVN_NB_DB --ovnsb-db=ssl:127.0.0.1:$TCP_PORT \ > > -p $key -c $cert -C $cacert > > > > # SSL should not connect because of key and cert mismatch > > >
On 6/11/21 6:30 PM, Han Zhou wrote: > > > On Fri, Jun 11, 2021 at 11:24 AM Mark Michelson <mmichels@redhat.com > <mailto:mmichels@redhat.com>> wrote: > > > > Hi Han, > > > > I'm fine with fixing the test this way, since like you said it's not > > meant to test RBAC. > > > > Acked-by: Mark Michelson <mmichels@redhat.com > <mailto:mmichels@redhat.com>> > > > Thanks Mark! I applied the patch 1-3 of this series to master and > branch-2.16. I will address your comment for patch 4. > > > However, based on how this sounds, there is still a bug in > > ovn-northd-ddlog wrt RBAC, and that should still be fixed since this > > could cause failures for other tests. > > > > In fact this is not a fault of ovn-northd-ddlog. It may be a general > problem of north RBAC - the only role we defined is for ovn-controller, > which is set in the Connection table's "role" column for the SSL connect > method. It means there is no role for northd to apply RBAC. So if northd > wants to use SSL, it needs a different SSL connection (probably a > different port) in the connection table without setting the "role" > (leave it empty). It has been discussed somewhere (probably in github) > for a workaround to the problem, is to use iptables rules or other FW > mechanism to allow access to the port dedicated for northd (without > RBAC) only from northd IPs. Unless a test specifically is testing SSL between northd and the southbound database, it's probably OK for northd to connect to the southbound database using a unix socket by default. Then if SSL is required for a specific test, then create the second connection using a separate port. > > For this test, it fails because the SSL connection created by the > ovn_start() already sets the "role" to ovn-controller, so through this > connection it is not allowed to update tables other than the ones > allowed for ovn-controller role. For the regular ovn-northd this test > case always succeeds because in fact the regular ovn-northd handles > sb_cfg update incorrectly when the update to SB fails. It updates the > expected sb_cfg back to NB even if it is not written to SB successfully. > It can be fixed as a separate patch (not something urgent though). > I agree. This isn't something super urgent. > Thanks, > Han > > > On 6/11/21 2:24 AM, Han Zhou wrote: > > > This test fails for ovn-northd-ddlog because of the RBAC role when > using > > > the SSL connection. RBAC is not the purpose of the test case, so this > > > patch fixes it without enabling RBAC. > > > > > > Signed-off-by: Han Zhou <hzhou@ovn.org <mailto:hzhou@ovn.org>> > > > --- > > > tests/ovn-northd.at <http://ovn-northd.at> | 19 +++++++++++++++++-- > > > 1 file changed, 17 insertions(+), 2 deletions(-) > > > > > > diff --git a/tests/ovn-northd.at <http://ovn-northd.at> > b/tests/ovn-northd.at <http://ovn-northd.at> > > > index 4692775ad..ad1732da3 100644 > > > --- a/tests/ovn-northd.at <http://ovn-northd.at> > > > +++ b/tests/ovn-northd.at <http://ovn-northd.at> > > > @@ -3618,9 +3618,23 @@ ovn_start --backup-northd=none > > > as northd > > > OVS_APP_EXIT_AND_WAIT([NORTHD_TYPE]) > > > > > > +as ovn-sb > > > +OVS_APP_EXIT_AND_WAIT([ovsdb-server]) > > > + > > > +key_server=testpki-test-privkey.pem > > > +cert_server=testpki-test-cert.pem > > > +cacert=testpki-cacert.pem > > > + > > > +cd ovn-sb > > > +rm ovsdb-server.log > > > +ssl_options="--remote=pssl:0:127.0.0.1 ovn-sb.db -p > $PKIDIR/$key_server -c $PKIDIR/$cert_server -C $PKIDIR/$cacert" > > > +AT_CHECK([ovsdb-server --detach --no-chdir --pidfile --log-file > $ssl_options], [0], [], [stderr]) > > > +on_exit "kill `cat ovsdb-server.pid`" > > > +PARSE_LISTENING_PORT([ovsdb-server.log], [TCP_PORT]) > > > +cd .. > > > + > > > key=testpki-hv1-privkey.pem > > > cert=testpki-hv1-cert.pem > > > -cacert=testpki-cacert.pem > > > > > > key2=testpki-hv2-privkey.pem > > > cert3=testpki-hv3-cert.pem > > > @@ -3629,8 +3643,9 @@ cert3=testpki-hv3-cert.pem > > > cp $PKIDIR/$key2 $key > > > cp $PKIDIR/$cert3 $cert > > > cp $PKIDIR/$cacert $cacert > > > +as northd > > > start_daemon ovn$NORTHD_TYPE -vjsonrpc \ > > > - --ovnnb-db=$OVN_NB_DB --ovnsb-db=$SSL_OVN_SB_DB \ > > > + --ovnnb-db=$OVN_NB_DB --ovnsb-db=ssl:127.0.0.1:$TCP_PORT \ > > > -p $key -c $cert -C $cacert > > > > > > # SSL should not connect because of key and cert mismatch > > > > >
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index 4692775ad..ad1732da3 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -3618,9 +3618,23 @@ ovn_start --backup-northd=none as northd OVS_APP_EXIT_AND_WAIT([NORTHD_TYPE]) +as ovn-sb +OVS_APP_EXIT_AND_WAIT([ovsdb-server]) + +key_server=testpki-test-privkey.pem +cert_server=testpki-test-cert.pem +cacert=testpki-cacert.pem + +cd ovn-sb +rm ovsdb-server.log +ssl_options="--remote=pssl:0:127.0.0.1 ovn-sb.db -p $PKIDIR/$key_server -c $PKIDIR/$cert_server -C $PKIDIR/$cacert" +AT_CHECK([ovsdb-server --detach --no-chdir --pidfile --log-file $ssl_options], [0], [], [stderr]) +on_exit "kill `cat ovsdb-server.pid`" +PARSE_LISTENING_PORT([ovsdb-server.log], [TCP_PORT]) +cd .. + key=testpki-hv1-privkey.pem cert=testpki-hv1-cert.pem -cacert=testpki-cacert.pem key2=testpki-hv2-privkey.pem cert3=testpki-hv3-cert.pem @@ -3629,8 +3643,9 @@ cert3=testpki-hv3-cert.pem cp $PKIDIR/$key2 $key cp $PKIDIR/$cert3 $cert cp $PKIDIR/$cacert $cacert +as northd start_daemon ovn$NORTHD_TYPE -vjsonrpc \ - --ovnnb-db=$OVN_NB_DB --ovnsb-db=$SSL_OVN_SB_DB \ + --ovnnb-db=$OVN_NB_DB --ovnsb-db=ssl:127.0.0.1:$TCP_PORT \ -p $key -c $cert -C $cacert # SSL should not connect because of key and cert mismatch
This test fails for ovn-northd-ddlog because of the RBAC role when using the SSL connection. RBAC is not the purpose of the test case, so this patch fixes it without enabling RBAC. Signed-off-by: Han Zhou <hzhou@ovn.org> --- tests/ovn-northd.at | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-)