diff mbox series

[ovs-dev,2/2] add tests for ssl ciphers

Message ID 20240111080557.54577-2-amginwal@gmail.com
State Superseded
Headers show
Series [ovs-dev,1/2] fix segfault due to ssl-ciphers | expand

Commit Message

aginwala aginwala Jan. 11, 2024, 8:05 a.m. UTC
From: Aliasgar Ginwala <aginwala@ebay.com>

Signed-off-by: Aliasgar Ginwala <aginwala@ebay.com>
---
 tests/ovn-controller.at |  26 ++++++
 tests/ovn.at            | 182 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 208 insertions(+)

Comments

Ales Musil Jan. 12, 2024, 10:08 a.m. UTC | #1
On Thu, Jan 11, 2024 at 2:30 PM <amginwal@gmail.com> wrote:

> From: Aliasgar Ginwala <aginwala@ebay.com>
>
> Signed-off-by: Aliasgar Ginwala <aginwala@ebay.com>
> ---
>

Hi Aliasgar,

thank you for the series. Those two patches should be squashed to one and I
have a couple of small comments down below.


>  tests/ovn-controller.at |  26 ++++++
>  tests/ovn.at            | 182 ++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 208 insertions(+)
>
> diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at
> index 9d2a37c72..df5662527 100644
> --- a/tests/ovn-controller.at
> +++ b/tests/ovn-controller.at
> @@ -2712,3 +2712,29 @@ AT_CHECK([as hv1 ovs-ofctl dump-flows br-int
> table=40 | grep -q controller], [1]
>  OVN_CLEANUP([hv1])
>  AT_CLEANUP
>  ])
> +
> +
> +AT_SETUP([ovn-controller - ssl ciphers using command line options])
> +AT_KEYWORDS([ovn])
> +AT_SKIP_IF([test "$HAVE_OPENSSL" = no])
> +PKIDIR="$(cd $abs_top_builddir/tests && pwd)"
> +AT_SKIP_IF([expr "$PKIDIR" : ".*[[     '\"
> +\\]]"])
>

This check is not needed, as we don't operate with the PKI files directly.


> +ovn_start
> +
> +net_add n1
> +sim_add hv1
> +ovs-vsctl add-br br-phys
> +ovn_attach n1 br-phys 192.168.0.20
> +
> +# Set cipher and and it should connect
> +OVS_APP_EXIT_AND_WAIT([ovn-controller])
> +start_daemon ovn-controller --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1'
> --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2'
> +
> +OVS_WAIT_FOR_OUTPUT([ovn-appctl -t ovn-controller connection-status],
> [0], [connected
> +])
> +
> +cat hv1/ovn-controller.log
> +
> +OVN_CLEANUP([hv1])
> +AT_CLEANUP
> diff --git a/tests/ovn.at b/tests/ovn.at
> index c3644ac78..34f277ef9 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -37588,3 +37588,185 @@ OVN_CLEANUP([hv1])
>
>  AT_CLEANUP
>  ])
> +
> +AT_SETUP([read-only sb db:pssl access with ssl-ciphers and ssl-protocols])
> +AT_SKIP_IF([test "$HAVE_OPENSSL" = no])
> +PKIDIR="$(cd $abs_top_builddir/tests && pwd)"
> +AT_SKIP_IF([expr "$PKIDIR" : ".*[[     '\"
> +\\]]"])
> +
> +: > .$1.db.~lock~
> +ovsdb-tool create ovn-sb.db "$abs_top_srcdir"/ovn-sb.ovsschema
> +
> +# Add read-only remote to sb ovsdb-server
> +AT_CHECK(
> +  [ovsdb-tool transact ovn-sb.db \
> +     ['["OVN_Southbound",
> +       {"op": "insert",
> +        "table": "SB_Global",
> +        "row": {
> +          "connections": ["set", [["named-uuid", "xyz"]]]}},
> +       {"op": "insert",
> +        "table": "Connection",
> +        "uuid-name": "xyz",
> +        "row": {"target": "pssl:0:127.0.0.1",
> +               "read_only": true}}]']], [0], [ignore], [ignore])
> +
> +start_daemon ovsdb-server --remote=punix:ovn-sb.sock \
> +
> --remote=db:OVN_Southbound,SB_Global,connections \
> +
> --private-key="$PKIDIR/testpki-test2-privkey.pem" \
> +                          --certificate="$PKIDIR/testpki-test2-cert.pem" \
> +                          --ca-cert="$PKIDIR/testpki-cacert.pem" \
> +                          --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1' \
> +                          --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
> +                          ovn-sb.db
> +
> +PARSE_LISTENING_PORT([ovsdb-server.log], [TCP_PORT])
> +
> +# read-only accesses should succeed
> +AT_CHECK([ovn-sbctl --db=ssl:127.0.0.1:$TCP_PORT \
> +                    --private-key=$PKIDIR/testpki-test-privkey.pem \
> +                    --certificate=$PKIDIR/testpki-test-cert.pem \
> +                    --ca-cert=$PKIDIR/testpki-cacert.pem \
> +                    --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1' \
> +                    --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
> +                    list SB_Global], [0], [stdout], [ignore])
> +AT_CHECK([ovn-sbctl --db=ssl:127.0.0.1:$TCP_PORT \
> +                    --private-key=$PKIDIR/testpki-test-privkey.pem \
> +                    --certificate=$PKIDIR/testpki-test-cert.pem \
> +                    --ca-cert=$PKIDIR/testpki-cacert.pem \
> +                    --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1' \
> +                    --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
> +                    list Connection], [0], [stdout], [ignore])
> +
> +# write access should fail
> +AT_CHECK([ovn-sbctl --db=ssl:127.0.0.1:$TCP_PORT \
> +                    --private-key=$PKIDIR/testpki-test-privkey.pem \
> +                    --certificate=$PKIDIR/testpki-test-cert.pem \
> +                    --ca-cert=$PKIDIR/testpki-cacert.pem \
> +                    --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1' \
> +                    --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
> +                    chassis-add ch vxlan 1.2.4.8], [1], [ignore],
> +[ovn-sbctl: transaction error: {"details":"insert operation not allowed
> when database server is in read only mode","error":"not allowed"}
> +])
> +
> +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
> +AT_CLEANUP
>

I don't think that this test has anything to do with allowed
ciphers/protocols. We actually have a read only test so we can most likely
drop this one.


> +
> +AT_SETUP([nb connection/ssl commands with ssl-ciphers and ssl-protocols])
> +AT_SKIP_IF([test "$HAVE_OPENSSL" = no])
> +PKIDIR="$(cd $abs_top_builddir/tests && pwd)"
> +AT_SKIP_IF([expr "$PKIDIR" : ".*[[     '\"
> +\\]]"])
> +
> +: > .$1.db.~lock~
> +ovsdb-tool create ovn-nb.db "$abs_top_srcdir"/ovn-nb.ovsschema
>

nit: To slightly simplify the test you could use something like the below:

m4_define([SSL_PARAMS], [--ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1' \
                         --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2'])
m4_define([PKI], [--private-key=$PKIDIR/testpki-test-privkey.pem \
                  --certificate=$PKIDIR/testpki-test-cert.pem \
                  --ca-cert=$PKIDIR/testpki-cacert.pem])

+
> +# Start nb db server using db connection/ssl entries (unpopulated
> initially)
> +start_daemon ovsdb-server --remote=punix:ovnnb_db.sock \
> +
> --remote=db:OVN_Northbound,NB_Global,connections \
> +                          --private-key=db:OVN_Northbound,SSL,private_key
> \
> +                          --certificate=db:OVN_Northbound,SSL,certificate
> \
> +                          --ca-cert=db:OVN_Northbound,SSL,ca_cert \
> +                          --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1' \
> +                          --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
> +                          ovn-nb.db
> +
> +# Populate SSL configuration entries in nb db
> +AT_CHECK(
> +    [ovn-nbctl set-ssl $PKIDIR/testpki-test-privkey.pem \
> +                       $PKIDIR/testpki-test-cert.pem \
> +                       $PKIDIR/testpki-cacert.pem], [0], [stdout],
> [ignore])
> +
> +# Populate a passive SSL connection in nb db
> +AT_CHECK([ovn-nbctl set-connection pssl:0:127.0.0.1], [0], [stdout],
> [ignore])
> +
> +PARSE_LISTENING_PORT([ovsdb-server.log], [TCP_PORT])
> +
> +# Verify SSL connetivity to nb db server
> +AT_CHECK([ovn-nbctl --db=ssl:127.0.0.1:$TCP_PORT \
> +                    --private-key=$PKIDIR/testpki-test-privkey.pem \
> +                    --certificate=$PKIDIR/testpki-test-cert.pem \
> +                    --ca-cert=$PKIDIR/testpki-cacert.pem \
> +                    --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1' \
> +                    --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
> +          list NB_Global],
>

With the m4_define above this could be reduced to "ovn-nbctl
--db=ssl:127.0.0.1:$TCP_PORT PKI SSL_PARAMS list NB_Global".
This is applicable to all the ovn-nbctl and ovn-sbctl commands.


> +         [0], [stdout], [ignore])
> +AT_CHECK([ovn-nbctl --db=ssl:127.0.0.1:$TCP_PORT \
> +                    --private-key=$PKIDIR/testpki-test-privkey.pem \
> +                    --certificate=$PKIDIR/testpki-test-cert.pem \
> +                    --ca-cert=$PKIDIR/testpki-cacert.pem \
> +                    --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1' \
> +                    --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
> +          list Connection],
> +         [0], [stdout], [ignore])
> +AT_CHECK([ovn-nbctl --db=ssl:127.0.0.1:$TCP_PORT \
> +                    --private-key=$PKIDIR/testpki-test-privkey.pem \
> +                    --certificate=$PKIDIR/testpki-test-cert.pem \
> +                    --ca-cert=$PKIDIR/testpki-cacert.pem \
> +                    --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1' \
> +                    --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
> +          get-connection],
> +         [0], [stdout], [ignore])
> +
> +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
> +AT_CLEANUP
> +
> +AT_SETUP([sb connection/ssl commands with ssl-ciphers and ssl-protocols])
> +AT_SKIP_IF([test "$HAVE_OPENSSL" = no])
> +PKIDIR="$(cd $abs_top_builddir/tests && pwd)"
> +AT_SKIP_IF([expr "$PKIDIR" : ".*[[     '\"
> +\\]]"])
> +
> +: > .$1.db.~lock~
> +ovsdb-tool create ovn-sb.db "$abs_top_srcdir"/ovn-sb.ovsschema
> +
> +# Start sb db server using db connection/ssl entries (unpopulated
> initially)
> +start_daemon ovsdb-server --remote=punix:ovnsb_db.sock \
> +
> --remote=db:OVN_Southbound,SB_Global,connections \
> +                          --private-key=db:OVN_Southbound,SSL,private_key
> \
> +                          --certificate=db:OVN_Southbound,SSL,certificate
> \
> +                          --ca-cert=db:OVN_Southbound,SSL,ca_cert \
> +                          --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1' \
> +                          --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
> +                          ovn-sb.db
> +
> +# Populate SSL configuration entries in sb db
> +AT_CHECK(
> +    [ovn-sbctl set-ssl $PKIDIR/testpki-test-privkey.pem \
> +                       $PKIDIR/testpki-test-cert.pem \
> +                       $PKIDIR/testpki-cacert.pem], [0], [stdout],
> [ignore])
> +
> +# Populate a passive SSL connection in sb db
> +AT_CHECK([ovn-sbctl set-connection pssl:0:127.0.0.1], [0], [stdout],
> [ignore])
> +
> +PARSE_LISTENING_PORT([ovsdb-server.log], [TCP_PORT])
> +
> +# Verify SSL connetivity to sb db server
> +AT_CHECK([ovn-sbctl --db=ssl:127.0.0.1:$TCP_PORT \
> +                    --private-key=$PKIDIR/testpki-test-privkey.pem \
> +                    --certificate=$PKIDIR/testpki-test-cert.pem \
> +                    --ca-cert=$PKIDIR/testpki-cacert.pem \
> +                    --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1' \
> +                    --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
> +          list SB_Global],
> +         [0], [stdout], [ignore])
> +AT_CHECK([ovn-sbctl --db=ssl:127.0.0.1:$TCP_PORT \
> +                    --private-key=$PKIDIR/testpki-test-privkey.pem \
> +                    --certificate=$PKIDIR/testpki-test-cert.pem \
> +                    --ca-cert=$PKIDIR/testpki-cacert.pem \
> +                    --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1' \
> +                    --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
> +          list Connection],
> +         [0], [stdout], [ignore])
> +AT_CHECK([ovn-sbctl --db=ssl:127.0.0.1:$TCP_PORT \
> +                    --private-key=$PKIDIR/testpki-test-privkey.pem \
> +                    --certificate=$PKIDIR/testpki-test-cert.pem \
> +                    --ca-cert=$PKIDIR/testpki-cacert.pem \
> +                    --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1' \
> +                    --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
> +          get-connection],
> +         [0], [stdout], [ignore])
> +
> +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
> +AT_CLEANUP
> --
> 2.39.3 (Apple Git-145)
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
Thanks,
Ales
aginwala Jan. 12, 2024, 4:11 p.m. UTC | #2
Thanks for review.

On Fri, Jan 12, 2024 at 2:08 AM Ales Musil <amusil@redhat.com> wrote:

> On Thu, Jan 11, 2024 at 2:30 PM <amginwal@gmail.com> wrote:
>
> > From: Aliasgar Ginwala <aginwala@ebay.com>
> >
> > Signed-off-by: Aliasgar Ginwala <aginwala@ebay.com>
> > ---
> >
>
> Hi Aliasgar,
>
> thank you for the series. Those two patches should be squashed to one and I
> have a couple of small comments down below.
>
Can it be done while applying. I realized I didnt add tests so added an add
on commit for the same. Would be neat to keep it that way too. Let me know
I can still proceed to squash in one commit if not acceptable.

>
>
> >  tests/ovn-controller.at |  26 ++++++
> >  tests/ovn.at            | 182 ++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 208 insertions(+)
> >
> > diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at
> > index 9d2a37c72..df5662527 100644
> > --- a/tests/ovn-controller.at
> > +++ b/tests/ovn-controller.at
> > @@ -2712,3 +2712,29 @@ AT_CHECK([as hv1 ovs-ofctl dump-flows br-int
> > table=40 | grep -q controller], [1]
> >  OVN_CLEANUP([hv1])
> >  AT_CLEANUP
> >  ])
> > +
> > +
> > +AT_SETUP([ovn-controller - ssl ciphers using command line options])
> > +AT_KEYWORDS([ovn])
> > +AT_SKIP_IF([test "$HAVE_OPENSSL" = no])
> > +PKIDIR="$(cd $abs_top_builddir/tests && pwd)"
> > +AT_SKIP_IF([expr "$PKIDIR" : ".*[[     '\"
> > +\\]]"])
> >
>
> This check is not needed, as we don't operate with the PKI files directly.
>
> I would like to keep it to cover at-least one case for ssl-cipher arg for
ovn-controller. Its needed as it didnt execute ovs-vsctl del-ssl command so
ovn will use the pki bits from ovsdb and -ssl-ciphers will be passed
explicitly

>
> > +ovn_start
> > +
> > +net_add n1
> > +sim_add hv1
> > +ovs-vsctl add-br br-phys
> > +ovn_attach n1 br-phys 192.168.0.20
> > +
> > +# Set cipher and and it should connect
> > +OVS_APP_EXIT_AND_WAIT([ovn-controller])
> > +start_daemon ovn-controller --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL
> =1'
> > --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2'
> > +
> > +OVS_WAIT_FOR_OUTPUT([ovn-appctl -t ovn-controller connection-status],
> > [0], [connected
> > +])
> > +
> > +cat hv1/ovn-controller.log
> > +
> > +OVN_CLEANUP([hv1])
> > +AT_CLEANUP
> > diff --git a/tests/ovn.at b/tests/ovn.at
> > index c3644ac78..34f277ef9 100644
> > --- a/tests/ovn.at
> > +++ b/tests/ovn.at
> > @@ -37588,3 +37588,185 @@ OVN_CLEANUP([hv1])
> >
> >  AT_CLEANUP
> >  ])
> > +
> > +AT_SETUP([read-only sb db:pssl access with ssl-ciphers and
> ssl-protocols])
> > +AT_SKIP_IF([test "$HAVE_OPENSSL" = no])
> > +PKIDIR="$(cd $abs_top_builddir/tests && pwd)"
> > +AT_SKIP_IF([expr "$PKIDIR" : ".*[[     '\"
> > +\\]]"])
> > +
> > +: > .$1.db.~lock~
> > +ovsdb-tool create ovn-sb.db "$abs_top_srcdir"/ovn-sb.ovsschema
> > +
> > +# Add read-only remote to sb ovsdb-server
> > +AT_CHECK(
> > +  [ovsdb-tool transact ovn-sb.db \
> > +     ['["OVN_Southbound",
> > +       {"op": "insert",
> > +        "table": "SB_Global",
> > +        "row": {
> > +          "connections": ["set", [["named-uuid", "xyz"]]]}},
> > +       {"op": "insert",
> > +        "table": "Connection",
> > +        "uuid-name": "xyz",
> > +        "row": {"target": "pssl:0:127.0.0.1",
> > +               "read_only": true}}]']], [0], [ignore], [ignore])
> > +
> > +start_daemon ovsdb-server --remote=punix:ovn-sb.sock \
> > +
> > --remote=db:OVN_Southbound,SB_Global,connections \
> > +
> > --private-key="$PKIDIR/testpki-test2-privkey.pem" \
> > +
> --certificate="$PKIDIR/testpki-test2-cert.pem" \
> > +                          --ca-cert="$PKIDIR/testpki-cacert.pem" \
> > +                          --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1'
> \
> > +                          --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
> > +                          ovn-sb.db
> > +
> > +PARSE_LISTENING_PORT([ovsdb-server.log], [TCP_PORT])
> > +
> > +# read-only accesses should succeed
> > +AT_CHECK([ovn-sbctl --db=ssl:127.0.0.1:$TCP_PORT \
> > +                    --private-key=$PKIDIR/testpki-test-privkey.pem \
> > +                    --certificate=$PKIDIR/testpki-test-cert.pem \
> > +                    --ca-cert=$PKIDIR/testpki-cacert.pem \
> > +                    --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1' \
> > +                    --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
> > +                    list SB_Global], [0], [stdout], [ignore])
> > +AT_CHECK([ovn-sbctl --db=ssl:127.0.0.1:$TCP_PORT \
> > +                    --private-key=$PKIDIR/testpki-test-privkey.pem \
> > +                    --certificate=$PKIDIR/testpki-test-cert.pem \
> > +                    --ca-cert=$PKIDIR/testpki-cacert.pem \
> > +                    --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1' \
> > +                    --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
> > +                    list Connection], [0], [stdout], [ignore])
> > +
> > +# write access should fail
> > +AT_CHECK([ovn-sbctl --db=ssl:127.0.0.1:$TCP_PORT \
> > +                    --private-key=$PKIDIR/testpki-test-privkey.pem \
> > +                    --certificate=$PKIDIR/testpki-test-cert.pem \
> > +                    --ca-cert=$PKIDIR/testpki-cacert.pem \
> > +                    --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1' \
> > +                    --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
> > +                    chassis-add ch vxlan 1.2.4.8], [1], [ignore],
> > +[ovn-sbctl: transaction error: {"details":"insert operation not allowed
> > when database server is in read only mode","error":"not allowed"}
> > +])
> > +
> > +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
> > +AT_CLEANUP
> >
>
> I don't think that this test has anything to do with allowed
> ciphers/protocols. We actually have a read only test so we can most likely
> drop this one.
>
> I kept it to cover both read and write part of ssl as we dont have much
coverage in the repo so kept it. Doesnt harm.

>
> > +
> > +AT_SETUP([nb connection/ssl commands with ssl-ciphers and
> ssl-protocols])
> > +AT_SKIP_IF([test "$HAVE_OPENSSL" = no])
> > +PKIDIR="$(cd $abs_top_builddir/tests && pwd)"
> > +AT_SKIP_IF([expr "$PKIDIR" : ".*[[     '\"
> > +\\]]"])
> > +
> > +: > .$1.db.~lock~
> > +ovsdb-tool create ovn-nb.db "$abs_top_srcdir"/ovn-nb.ovsschema
> >
>
> nit: To slightly simplify the test you could use something like the below:
>
> m4_define([SSL_PARAMS], [--ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1' \
>                          --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2'])
> m4_define([PKI], [--private-key=$PKIDIR/testpki-test-privkey.pem \
>                   --certificate=$PKIDIR/testpki-test-cert.pem \
>                   --ca-cert=$PKIDIR/testpki-cacert.pem])
>
> +
> > +# Start nb db server using db connection/ssl entries (unpopulated
> > initially)
> > +start_daemon ovsdb-server --remote=punix:ovnnb_db.sock \
> > +
> > --remote=db:OVN_Northbound,NB_Global,connections \
> > +
> --private-key=db:OVN_Northbound,SSL,private_key
> > \
> > +
> --certificate=db:OVN_Northbound,SSL,certificate
> > \
> > +                          --ca-cert=db:OVN_Northbound,SSL,ca_cert \
> > +                          --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1'
> \
> > +                          --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
> > +                          ovn-nb.db
> > +
> > +# Populate SSL configuration entries in nb db
> > +AT_CHECK(
> > +    [ovn-nbctl set-ssl $PKIDIR/testpki-test-privkey.pem \
> > +                       $PKIDIR/testpki-test-cert.pem \
> > +                       $PKIDIR/testpki-cacert.pem], [0], [stdout],
> > [ignore])
> > +
> > +# Populate a passive SSL connection in nb db
> > +AT_CHECK([ovn-nbctl set-connection pssl:0:127.0.0.1], [0], [stdout],
> > [ignore])
> > +
> > +PARSE_LISTENING_PORT([ovsdb-server.log], [TCP_PORT])
> > +
> > +# Verify SSL connetivity to nb db server
> > +AT_CHECK([ovn-nbctl --db=ssl:127.0.0.1:$TCP_PORT \
> > +                    --private-key=$PKIDIR/testpki-test-privkey.pem \
> > +                    --certificate=$PKIDIR/testpki-test-cert.pem \
> > +                    --ca-cert=$PKIDIR/testpki-cacert.pem \
> > +                    --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1' \
> > +                    --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
> > +          list NB_Global],
> >
>
> With the m4_define above this could be reduced to "ovn-nbctl
> --db=ssl:127.0.0.1:$TCP_PORT PKI SSL_PARAMS list NB_Global".
> This is applicable to all the ovn-nbctl and ovn-sbctl commands.
>
>
> I think its better to optimize with the follow up patch as its already in
multiple places e.g
https://github.com/ovn-org/ovn/blob/main/tests/ovn.at#L10342 .That is why I
retained original way.

> > +         [0], [stdout], [ignore])
> > +AT_CHECK([ovn-nbctl --db=ssl:127.0.0.1:$TCP_PORT \
> > +                    --private-key=$PKIDIR/testpki-test-privkey.pem \
> > +                    --certificate=$PKIDIR/testpki-test-cert.pem \
> > +                    --ca-cert=$PKIDIR/testpki-cacert.pem \
> > +                    --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1' \
> > +                    --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
> > +          list Connection],
> > +         [0], [stdout], [ignore])
> > +AT_CHECK([ovn-nbctl --db=ssl:127.0.0.1:$TCP_PORT \
> > +                    --private-key=$PKIDIR/testpki-test-privkey.pem \
> > +                    --certificate=$PKIDIR/testpki-test-cert.pem \
> > +                    --ca-cert=$PKIDIR/testpki-cacert.pem \
> > +                    --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1' \
> > +                    --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
> > +          get-connection],
> > +         [0], [stdout], [ignore])
> > +
> > +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
> > +AT_CLEANUP
> > +
> > +AT_SETUP([sb connection/ssl commands with ssl-ciphers and
> ssl-protocols])
> > +AT_SKIP_IF([test "$HAVE_OPENSSL" = no])
> > +PKIDIR="$(cd $abs_top_builddir/tests && pwd)"
> > +AT_SKIP_IF([expr "$PKIDIR" : ".*[[     '\"
> > +\\]]"])
> > +
> > +: > .$1.db.~lock~
> > +ovsdb-tool create ovn-sb.db "$abs_top_srcdir"/ovn-sb.ovsschema
> > +
> > +# Start sb db server using db connection/ssl entries (unpopulated
> > initially)
> > +start_daemon ovsdb-server --remote=punix:ovnsb_db.sock \
> > +
> > --remote=db:OVN_Southbound,SB_Global,connections \
> > +
> --private-key=db:OVN_Southbound,SSL,private_key
> > \
> > +
> --certificate=db:OVN_Southbound,SSL,certificate
> > \
> > +                          --ca-cert=db:OVN_Southbound,SSL,ca_cert \
> > +                          --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1'
> \
> > +                          --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
> > +                          ovn-sb.db
> > +
> > +# Populate SSL configuration entries in sb db
> > +AT_CHECK(
> > +    [ovn-sbctl set-ssl $PKIDIR/testpki-test-privkey.pem \
> > +                       $PKIDIR/testpki-test-cert.pem \
> > +                       $PKIDIR/testpki-cacert.pem], [0], [stdout],
> > [ignore])
> > +
> > +# Populate a passive SSL connection in sb db
> > +AT_CHECK([ovn-sbctl set-connection pssl:0:127.0.0.1], [0], [stdout],
> > [ignore])
> > +
> > +PARSE_LISTENING_PORT([ovsdb-server.log], [TCP_PORT])
> > +
> > +# Verify SSL connetivity to sb db server
> > +AT_CHECK([ovn-sbctl --db=ssl:127.0.0.1:$TCP_PORT \
> > +                    --private-key=$PKIDIR/testpki-test-privkey.pem \
> > +                    --certificate=$PKIDIR/testpki-test-cert.pem \
> > +                    --ca-cert=$PKIDIR/testpki-cacert.pem \
> > +                    --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1' \
> > +                    --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
> > +          list SB_Global],
> > +         [0], [stdout], [ignore])
> > +AT_CHECK([ovn-sbctl --db=ssl:127.0.0.1:$TCP_PORT \
> > +                    --private-key=$PKIDIR/testpki-test-privkey.pem \
> > +                    --certificate=$PKIDIR/testpki-test-cert.pem \
> > +                    --ca-cert=$PKIDIR/testpki-cacert.pem \
> > +                    --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1' \
> > +                    --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
> > +          list Connection],
> > +         [0], [stdout], [ignore])
> > +AT_CHECK([ovn-sbctl --db=ssl:127.0.0.1:$TCP_PORT \
> > +                    --private-key=$PKIDIR/testpki-test-privkey.pem \
> > +                    --certificate=$PKIDIR/testpki-test-cert.pem \
> > +                    --ca-cert=$PKIDIR/testpki-cacert.pem \
> > +                    --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1' \
> > +                    --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
> > +          get-connection],
> > +         [0], [stdout], [ignore])
> > +
> > +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
> > +AT_CLEANUP
> > --
> > 2.39.3 (Apple Git-145)
> >
> > _______________________________________________
> > dev mailing list
> > dev@openvswitch.org
> >
> https://urldefense.com/v3/__https://mail.openvswitch.org/mailman/listinfo/ovs-dev__;!!IKRxdwAv5BmarQ!b9pUg4X974A51WZFKZfkdEpZfonrVQK6Py8Vn8OwxGmUPEMOPQt5bxmdT-lSbLg4SRGxmwmpDKmTpQ$
> >
> >
> Thanks,
> Ales
>
> --
>
> Ales Musil
>
> Senior Software Engineer - OVN Core
>
> Red Hat EMEA <
> https://urldefense.com/v3/__https://www.redhat.com__;!!IKRxdwAv5BmarQ!b9pUg4X974A51WZFKZfkdEpZfonrVQK6Py8Vn8OwxGmUPEMOPQt5bxmdT-lSbLg4SRGxmwncGrVSrg$
> >
>
> amusil@redhat.com
> <
> https://urldefense.com/v3/__https://red.ht/sig__;!!IKRxdwAv5BmarQ!b9pUg4X974A51WZFKZfkdEpZfonrVQK6Py8Vn8OwxGmUPEMOPQt5bxmdT-lSbLg4SRGxmwlOrm0WvQ$
> >
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
>
> https://urldefense.com/v3/__https://mail.openvswitch.org/mailman/listinfo/ovs-dev__;!!IKRxdwAv5BmarQ!b9pUg4X974A51WZFKZfkdEpZfonrVQK6Py8Vn8OwxGmUPEMOPQt5bxmdT-lSbLg4SRGxmwmpDKmTpQ$
>
aginwala Jan. 17, 2024, 5:22 a.m. UTC | #3
Hi Ales:

Ping. Following up for the test coverage bits.  I would like to keep it
slick for the bug fix/feature gap first. Please confirm so I can amend
accordingly.


Ali

On Fri, Jan 12, 2024 at 8:11 AM aginwala <aginwala@asu.edu> wrote:

> Thanks for review.
>
> On Fri, Jan 12, 2024 at 2:08 AM Ales Musil <amusil@redhat.com> wrote:
>
>> On Thu, Jan 11, 2024 at 2:30 PM <amginwal@gmail.com> wrote:
>>
>> > From: Aliasgar Ginwala <aginwala@ebay.com>
>> >
>> > Signed-off-by: Aliasgar Ginwala <aginwala@ebay.com>
>> > ---
>> >
>>
>> Hi Aliasgar,
>>
>> thank you for the series. Those two patches should be squashed to one and
>> I
>> have a couple of small comments down below.
>>
> Can it be done while applying. I realized I didnt add tests so added an
> add on commit for the same. Would be neat to keep it that way too. Let me
> know I can still proceed to squash in one commit if not acceptable.
>
>>
>>
>> >  tests/ovn-controller.at |  26 ++++++
>> >  tests/ovn.at            | 182 ++++++++++++++++++++++++++++++++++++++++
>> >  2 files changed, 208 insertions(+)
>> >
>> > diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at
>> > index 9d2a37c72..df5662527 100644
>> > --- a/tests/ovn-controller.at
>> > +++ b/tests/ovn-controller.at
>> > @@ -2712,3 +2712,29 @@ AT_CHECK([as hv1 ovs-ofctl dump-flows br-int
>> > table=40 | grep -q controller], [1]
>> >  OVN_CLEANUP([hv1])
>> >  AT_CLEANUP
>> >  ])
>> > +
>> > +
>> > +AT_SETUP([ovn-controller - ssl ciphers using command line options])
>> > +AT_KEYWORDS([ovn])
>> > +AT_SKIP_IF([test "$HAVE_OPENSSL" = no])
>> > +PKIDIR="$(cd $abs_top_builddir/tests && pwd)"
>> > +AT_SKIP_IF([expr "$PKIDIR" : ".*[[     '\"
>> > +\\]]"])
>> >
>>
>> This check is not needed, as we don't operate with the PKI files directly.
>>
>> I would like to keep it to cover at-least one case for ssl-cipher arg for
> ovn-controller. Its needed as it didnt execute ovs-vsctl del-ssl command so
> ovn will use the pki bits from ovsdb and -ssl-ciphers will be passed
> explicitly
>
>>
>> > +ovn_start
>> > +
>> > +net_add n1
>> > +sim_add hv1
>> > +ovs-vsctl add-br br-phys
>> > +ovn_attach n1 br-phys 192.168.0.20
>> > +
>> > +# Set cipher and and it should connect
>> > +OVS_APP_EXIT_AND_WAIT([ovn-controller])
>> > +start_daemon ovn-controller --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL
>> =1'
>> > --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2'
>> > +
>> > +OVS_WAIT_FOR_OUTPUT([ovn-appctl -t ovn-controller connection-status],
>> > [0], [connected
>> > +])
>> > +
>> > +cat hv1/ovn-controller.log
>> > +
>> > +OVN_CLEANUP([hv1])
>> > +AT_CLEANUP
>> > diff --git a/tests/ovn.at b/tests/ovn.at
>> > index c3644ac78..34f277ef9 100644
>> > --- a/tests/ovn.at
>> > +++ b/tests/ovn.at
>> > @@ -37588,3 +37588,185 @@ OVN_CLEANUP([hv1])
>> >
>> >  AT_CLEANUP
>> >  ])
>> > +
>> > +AT_SETUP([read-only sb db:pssl access with ssl-ciphers and
>> ssl-protocols])
>> > +AT_SKIP_IF([test "$HAVE_OPENSSL" = no])
>> > +PKIDIR="$(cd $abs_top_builddir/tests && pwd)"
>> > +AT_SKIP_IF([expr "$PKIDIR" : ".*[[     '\"
>> > +\\]]"])
>> > +
>> > +: > .$1.db.~lock~
>> > +ovsdb-tool create ovn-sb.db "$abs_top_srcdir"/ovn-sb.ovsschema
>> > +
>> > +# Add read-only remote to sb ovsdb-server
>> > +AT_CHECK(
>> > +  [ovsdb-tool transact ovn-sb.db \
>> > +     ['["OVN_Southbound",
>> > +       {"op": "insert",
>> > +        "table": "SB_Global",
>> > +        "row": {
>> > +          "connections": ["set", [["named-uuid", "xyz"]]]}},
>> > +       {"op": "insert",
>> > +        "table": "Connection",
>> > +        "uuid-name": "xyz",
>> > +        "row": {"target": "pssl:0:127.0.0.1",
>> > +               "read_only": true}}]']], [0], [ignore], [ignore])
>> > +
>> > +start_daemon ovsdb-server --remote=punix:ovn-sb.sock \
>> > +
>> > --remote=db:OVN_Southbound,SB_Global,connections \
>> > +
>> > --private-key="$PKIDIR/testpki-test2-privkey.pem" \
>> > +
>> --certificate="$PKIDIR/testpki-test2-cert.pem" \
>> > +                          --ca-cert="$PKIDIR/testpki-cacert.pem" \
>> > +                          --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1'
>> \
>> > +                          --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
>> > +                          ovn-sb.db
>> > +
>> > +PARSE_LISTENING_PORT([ovsdb-server.log], [TCP_PORT])
>> > +
>> > +# read-only accesses should succeed
>> > +AT_CHECK([ovn-sbctl --db=ssl:127.0.0.1:$TCP_PORT \
>> > +                    --private-key=$PKIDIR/testpki-test-privkey.pem \
>> > +                    --certificate=$PKIDIR/testpki-test-cert.pem \
>> > +                    --ca-cert=$PKIDIR/testpki-cacert.pem \
>> > +                    --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1' \
>> > +                    --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
>> > +                    list SB_Global], [0], [stdout], [ignore])
>> > +AT_CHECK([ovn-sbctl --db=ssl:127.0.0.1:$TCP_PORT \
>> > +                    --private-key=$PKIDIR/testpki-test-privkey.pem \
>> > +                    --certificate=$PKIDIR/testpki-test-cert.pem \
>> > +                    --ca-cert=$PKIDIR/testpki-cacert.pem \
>> > +                    --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1' \
>> > +                    --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
>> > +                    list Connection], [0], [stdout], [ignore])
>> > +
>> > +# write access should fail
>> > +AT_CHECK([ovn-sbctl --db=ssl:127.0.0.1:$TCP_PORT \
>> > +                    --private-key=$PKIDIR/testpki-test-privkey.pem \
>> > +                    --certificate=$PKIDIR/testpki-test-cert.pem \
>> > +                    --ca-cert=$PKIDIR/testpki-cacert.pem \
>> > +                    --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1' \
>> > +                    --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
>> > +                    chassis-add ch vxlan 1.2.4.8], [1], [ignore],
>> > +[ovn-sbctl: transaction error: {"details":"insert operation not allowed
>> > when database server is in read only mode","error":"not allowed"}
>> > +])
>> > +
>> > +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
>> > +AT_CLEANUP
>> >
>>
>> I don't think that this test has anything to do with allowed
>> ciphers/protocols. We actually have a read only test so we can most likely
>> drop this one.
>>
>> I kept it to cover both read and write part of ssl as we dont have much
> coverage in the repo so kept it. Doesnt harm.
>
>>
>> > +
>> > +AT_SETUP([nb connection/ssl commands with ssl-ciphers and
>> ssl-protocols])
>> > +AT_SKIP_IF([test "$HAVE_OPENSSL" = no])
>> > +PKIDIR="$(cd $abs_top_builddir/tests && pwd)"
>> > +AT_SKIP_IF([expr "$PKIDIR" : ".*[[     '\"
>> > +\\]]"])
>> > +
>> > +: > .$1.db.~lock~
>> > +ovsdb-tool create ovn-nb.db "$abs_top_srcdir"/ovn-nb.ovsschema
>> >
>>
>> nit: To slightly simplify the test you could use something like the below:
>>
>> m4_define([SSL_PARAMS], [--ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1' \
>>                          --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2'])
>> m4_define([PKI], [--private-key=$PKIDIR/testpki-test-privkey.pem \
>>                   --certificate=$PKIDIR/testpki-test-cert.pem \
>>                   --ca-cert=$PKIDIR/testpki-cacert.pem])
>>
>> +
>> > +# Start nb db server using db connection/ssl entries (unpopulated
>> > initially)
>> > +start_daemon ovsdb-server --remote=punix:ovnnb_db.sock \
>> > +
>> > --remote=db:OVN_Northbound,NB_Global,connections \
>> > +
>> --private-key=db:OVN_Northbound,SSL,private_key
>> > \
>> > +
>> --certificate=db:OVN_Northbound,SSL,certificate
>> > \
>> > +                          --ca-cert=db:OVN_Northbound,SSL,ca_cert \
>> > +                          --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1'
>> \
>> > +                          --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
>> > +                          ovn-nb.db
>> > +
>> > +# Populate SSL configuration entries in nb db
>> > +AT_CHECK(
>> > +    [ovn-nbctl set-ssl $PKIDIR/testpki-test-privkey.pem \
>> > +                       $PKIDIR/testpki-test-cert.pem \
>> > +                       $PKIDIR/testpki-cacert.pem], [0], [stdout],
>> > [ignore])
>> > +
>> > +# Populate a passive SSL connection in nb db
>> > +AT_CHECK([ovn-nbctl set-connection pssl:0:127.0.0.1], [0], [stdout],
>> > [ignore])
>> > +
>> > +PARSE_LISTENING_PORT([ovsdb-server.log], [TCP_PORT])
>> > +
>> > +# Verify SSL connetivity to nb db server
>> > +AT_CHECK([ovn-nbctl --db=ssl:127.0.0.1:$TCP_PORT \
>> > +                    --private-key=$PKIDIR/testpki-test-privkey.pem \
>> > +                    --certificate=$PKIDIR/testpki-test-cert.pem \
>> > +                    --ca-cert=$PKIDIR/testpki-cacert.pem \
>> > +                    --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1' \
>> > +                    --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
>> > +          list NB_Global],
>> >
>>
>> With the m4_define above this could be reduced to "ovn-nbctl
>> --db=ssl:127.0.0.1:$TCP_PORT PKI SSL_PARAMS list NB_Global".
>> This is applicable to all the ovn-nbctl and ovn-sbctl commands.
>>
>>
>> I think its better to optimize with the follow up patch as its already in
> multiple places e.g
> https://github.com/ovn-org/ovn/blob/main/tests/ovn.at#L10342 .That is why
> I retained original way.
>
>> > +         [0], [stdout], [ignore])
>> > +AT_CHECK([ovn-nbctl --db=ssl:127.0.0.1:$TCP_PORT \
>> > +                    --private-key=$PKIDIR/testpki-test-privkey.pem \
>> > +                    --certificate=$PKIDIR/testpki-test-cert.pem \
>> > +                    --ca-cert=$PKIDIR/testpki-cacert.pem \
>> > +                    --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1' \
>> > +                    --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
>> > +          list Connection],
>> > +         [0], [stdout], [ignore])
>> > +AT_CHECK([ovn-nbctl --db=ssl:127.0.0.1:$TCP_PORT \
>> > +                    --private-key=$PKIDIR/testpki-test-privkey.pem \
>> > +                    --certificate=$PKIDIR/testpki-test-cert.pem \
>> > +                    --ca-cert=$PKIDIR/testpki-cacert.pem \
>> > +                    --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1' \
>> > +                    --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
>> > +          get-connection],
>> > +         [0], [stdout], [ignore])
>> > +
>> > +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
>> > +AT_CLEANUP
>> > +
>> > +AT_SETUP([sb connection/ssl commands with ssl-ciphers and
>> ssl-protocols])
>> > +AT_SKIP_IF([test "$HAVE_OPENSSL" = no])
>> > +PKIDIR="$(cd $abs_top_builddir/tests && pwd)"
>> > +AT_SKIP_IF([expr "$PKIDIR" : ".*[[     '\"
>> > +\\]]"])
>> > +
>> > +: > .$1.db.~lock~
>> > +ovsdb-tool create ovn-sb.db "$abs_top_srcdir"/ovn-sb.ovsschema
>> > +
>> > +# Start sb db server using db connection/ssl entries (unpopulated
>> > initially)
>> > +start_daemon ovsdb-server --remote=punix:ovnsb_db.sock \
>> > +
>> > --remote=db:OVN_Southbound,SB_Global,connections \
>> > +
>> --private-key=db:OVN_Southbound,SSL,private_key
>> > \
>> > +
>> --certificate=db:OVN_Southbound,SSL,certificate
>> > \
>> > +                          --ca-cert=db:OVN_Southbound,SSL,ca_cert \
>> > +                          --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1'
>> \
>> > +                          --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
>> > +                          ovn-sb.db
>> > +
>> > +# Populate SSL configuration entries in sb db
>> > +AT_CHECK(
>> > +    [ovn-sbctl set-ssl $PKIDIR/testpki-test-privkey.pem \
>> > +                       $PKIDIR/testpki-test-cert.pem \
>> > +                       $PKIDIR/testpki-cacert.pem], [0], [stdout],
>> > [ignore])
>> > +
>> > +# Populate a passive SSL connection in sb db
>> > +AT_CHECK([ovn-sbctl set-connection pssl:0:127.0.0.1], [0], [stdout],
>> > [ignore])
>> > +
>> > +PARSE_LISTENING_PORT([ovsdb-server.log], [TCP_PORT])
>> > +
>> > +# Verify SSL connetivity to sb db server
>> > +AT_CHECK([ovn-sbctl --db=ssl:127.0.0.1:$TCP_PORT \
>> > +                    --private-key=$PKIDIR/testpki-test-privkey.pem \
>> > +                    --certificate=$PKIDIR/testpki-test-cert.pem \
>> > +                    --ca-cert=$PKIDIR/testpki-cacert.pem \
>> > +                    --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1' \
>> > +                    --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
>> > +          list SB_Global],
>> > +         [0], [stdout], [ignore])
>> > +AT_CHECK([ovn-sbctl --db=ssl:127.0.0.1:$TCP_PORT \
>> > +                    --private-key=$PKIDIR/testpki-test-privkey.pem \
>> > +                    --certificate=$PKIDIR/testpki-test-cert.pem \
>> > +                    --ca-cert=$PKIDIR/testpki-cacert.pem \
>> > +                    --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1' \
>> > +                    --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
>> > +          list Connection],
>> > +         [0], [stdout], [ignore])
>> > +AT_CHECK([ovn-sbctl --db=ssl:127.0.0.1:$TCP_PORT \
>> > +                    --private-key=$PKIDIR/testpki-test-privkey.pem \
>> > +                    --certificate=$PKIDIR/testpki-test-cert.pem \
>> > +                    --ca-cert=$PKIDIR/testpki-cacert.pem \
>> > +                    --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1' \
>> > +                    --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
>> > +          get-connection],
>> > +         [0], [stdout], [ignore])
>> > +
>> > +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
>> > +AT_CLEANUP
>> > --
>> > 2.39.3 (Apple Git-145)
>> >
>> > _______________________________________________
>> > dev mailing list
>> > dev@openvswitch.org
>> >
>> https://urldefense.com/v3/__https://mail.openvswitch.org/mailman/listinfo/ovs-dev__;!!IKRxdwAv5BmarQ!b9pUg4X974A51WZFKZfkdEpZfonrVQK6Py8Vn8OwxGmUPEMOPQt5bxmdT-lSbLg4SRGxmwmpDKmTpQ$
>> >
>> >
>> Thanks,
>> Ales
>>
>> --
>>
>> Ales Musil
>>
>> Senior Software Engineer - OVN Core
>>
>> Red Hat EMEA <
>> https://urldefense.com/v3/__https://www.redhat.com__;!!IKRxdwAv5BmarQ!b9pUg4X974A51WZFKZfkdEpZfonrVQK6Py8Vn8OwxGmUPEMOPQt5bxmdT-lSbLg4SRGxmwncGrVSrg$
>> >
>>
>> amusil@redhat.com
>> <
>> https://urldefense.com/v3/__https://red.ht/sig__;!!IKRxdwAv5BmarQ!b9pUg4X974A51WZFKZfkdEpZfonrVQK6Py8Vn8OwxGmUPEMOPQt5bxmdT-lSbLg4SRGxmwlOrm0WvQ$
>> >
>> _______________________________________________
>> dev mailing list
>> dev@openvswitch.org
>>
>> https://urldefense.com/v3/__https://mail.openvswitch.org/mailman/listinfo/ovs-dev__;!!IKRxdwAv5BmarQ!b9pUg4X974A51WZFKZfkdEpZfonrVQK6Py8Vn8OwxGmUPEMOPQt5bxmdT-lSbLg4SRGxmwmpDKmTpQ$
>>
>
Ales Musil Jan. 17, 2024, 6:20 a.m. UTC | #4
On Wed, Jan 17, 2024 at 6:22 AM aginwala <aginwala@asu.edu> wrote:

> Hi Ales:
>
> Ping. Following up for the test coverage bits.  I would like to keep it
> slick for the bug fix/feature gap first. Please confirm so I can amend
> accordingly.
>

Sorry for the late response, please see reply inline.


>
>
> Ali
>
> On Fri, Jan 12, 2024 at 8:11 AM aginwala <aginwala@asu.edu> wrote:
>
>> Thanks for review.
>>
>> On Fri, Jan 12, 2024 at 2:08 AM Ales Musil <amusil@redhat.com> wrote:
>>
>>> On Thu, Jan 11, 2024 at 2:30 PM <amginwal@gmail.com> wrote:
>>>
>>> > From: Aliasgar Ginwala <aginwala@ebay.com>
>>> >
>>> > Signed-off-by: Aliasgar Ginwala <aginwala@ebay.com>
>>> > ---
>>> >
>>>
>>> Hi Aliasgar,
>>>
>>> thank you for the series. Those two patches should be squashed to one
>>> and I
>>> have a couple of small comments down below.
>>>
>> Can it be done while applying. I realized I didnt add tests so added an
>> add on commit for the same. Would be neat to keep it that way too. Let me
>> know I can still proceed to squash in one commit if not acceptable.
>>
>>>
>>>
>>> >  tests/ovn-controller.at |  26 ++++++
>>> >  tests/ovn.at            | 182
>>> ++++++++++++++++++++++++++++++++++++++++
>>> >  2 files changed, 208 insertions(+)
>>> >
>>> > diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at
>>> > index 9d2a37c72..df5662527 100644
>>> > --- a/tests/ovn-controller.at
>>> > +++ b/tests/ovn-controller.at
>>> > @@ -2712,3 +2712,29 @@ AT_CHECK([as hv1 ovs-ofctl dump-flows br-int
>>> > table=40 | grep -q controller], [1]
>>> >  OVN_CLEANUP([hv1])
>>> >  AT_CLEANUP
>>> >  ])
>>> > +
>>> > +
>>> > +AT_SETUP([ovn-controller - ssl ciphers using command line options])
>>> > +AT_KEYWORDS([ovn])
>>> > +AT_SKIP_IF([test "$HAVE_OPENSSL" = no])
>>> > +PKIDIR="$(cd $abs_top_builddir/tests && pwd)"
>>> > +AT_SKIP_IF([expr "$PKIDIR" : ".*[[     '\"
>>> > +\\]]"])
>>> >
>>>
>>> This check is not needed, as we don't operate with the PKI files
>>> directly.
>>>
>>> I would like to keep it to cover at-least one case for ssl-cipher arg
>> for ovn-controller. Its needed as it didnt execute ovs-vsctl del-ssl
>> command so ovn will use the pki bits from ovsdb and -ssl-ciphers will be
>> passed explicitly
>>
>
There is probably some misunderstanding, the test case is fine and should
stay. By not needed I meant only the following part:

PKIDIR="$(cd $abs_top_builddir/tests && pwd)"
AT_SKIP_IF([expr "$PKIDIR" : ".*[[     '\"
\\]]"])


>
>>> > +ovn_start
>>> > +
>>> > +net_add n1
>>> > +sim_add hv1
>>> > +ovs-vsctl add-br br-phys
>>> > +ovn_attach n1 br-phys 192.168.0.20
>>> > +
>>> > +# Set cipher and and it should connect
>>> > +OVS_APP_EXIT_AND_WAIT([ovn-controller])
>>> > +start_daemon ovn-controller --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL
>>> =1'
>>> > --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2'
>>> > +
>>> > +OVS_WAIT_FOR_OUTPUT([ovn-appctl -t ovn-controller connection-status],
>>> > [0], [connected
>>> > +])
>>> > +
>>> > +cat hv1/ovn-controller.log
>>> > +
>>> > +OVN_CLEANUP([hv1])
>>> > +AT_CLEANUP
>>> > diff --git a/tests/ovn.at b/tests/ovn.at
>>> > index c3644ac78..34f277ef9 100644
>>> > --- a/tests/ovn.at
>>> > +++ b/tests/ovn.at
>>> > @@ -37588,3 +37588,185 @@ OVN_CLEANUP([hv1])
>>> >
>>> >  AT_CLEANUP
>>> >  ])
>>> > +
>>> > +AT_SETUP([read-only sb db:pssl access with ssl-ciphers and
>>> ssl-protocols])
>>> > +AT_SKIP_IF([test "$HAVE_OPENSSL" = no])
>>> > +PKIDIR="$(cd $abs_top_builddir/tests && pwd)"
>>> > +AT_SKIP_IF([expr "$PKIDIR" : ".*[[     '\"
>>> > +\\]]"])
>>> > +
>>> > +: > .$1.db.~lock~
>>> > +ovsdb-tool create ovn-sb.db "$abs_top_srcdir"/ovn-sb.ovsschema
>>> > +
>>> > +# Add read-only remote to sb ovsdb-server
>>> > +AT_CHECK(
>>> > +  [ovsdb-tool transact ovn-sb.db \
>>> > +     ['["OVN_Southbound",
>>> > +       {"op": "insert",
>>> > +        "table": "SB_Global",
>>> > +        "row": {
>>> > +          "connections": ["set", [["named-uuid", "xyz"]]]}},
>>> > +       {"op": "insert",
>>> > +        "table": "Connection",
>>> > +        "uuid-name": "xyz",
>>> > +        "row": {"target": "pssl:0:127.0.0.1",
>>> > +               "read_only": true}}]']], [0], [ignore], [ignore])
>>> > +
>>> > +start_daemon ovsdb-server --remote=punix:ovn-sb.sock \
>>> > +
>>> > --remote=db:OVN_Southbound,SB_Global,connections \
>>> > +
>>> > --private-key="$PKIDIR/testpki-test2-privkey.pem" \
>>> > +
>>> --certificate="$PKIDIR/testpki-test2-cert.pem" \
>>> > +                          --ca-cert="$PKIDIR/testpki-cacert.pem" \
>>> > +                          --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1'
>>> \
>>> > +                          --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
>>> > +                          ovn-sb.db
>>> > +
>>> > +PARSE_LISTENING_PORT([ovsdb-server.log], [TCP_PORT])
>>> > +
>>> > +# read-only accesses should succeed
>>> > +AT_CHECK([ovn-sbctl --db=ssl:127.0.0.1:$TCP_PORT \
>>> > +                    --private-key=$PKIDIR/testpki-test-privkey.pem \
>>> > +                    --certificate=$PKIDIR/testpki-test-cert.pem \
>>> > +                    --ca-cert=$PKIDIR/testpki-cacert.pem \
>>> > +                    --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1' \
>>> > +                    --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
>>> > +                    list SB_Global], [0], [stdout], [ignore])
>>> > +AT_CHECK([ovn-sbctl --db=ssl:127.0.0.1:$TCP_PORT \
>>> > +                    --private-key=$PKIDIR/testpki-test-privkey.pem \
>>> > +                    --certificate=$PKIDIR/testpki-test-cert.pem \
>>> > +                    --ca-cert=$PKIDIR/testpki-cacert.pem \
>>> > +                    --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1' \
>>> > +                    --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
>>> > +                    list Connection], [0], [stdout], [ignore])
>>> > +
>>> > +# write access should fail
>>> > +AT_CHECK([ovn-sbctl --db=ssl:127.0.0.1:$TCP_PORT \
>>> > +                    --private-key=$PKIDIR/testpki-test-privkey.pem \
>>> > +                    --certificate=$PKIDIR/testpki-test-cert.pem \
>>> > +                    --ca-cert=$PKIDIR/testpki-cacert.pem \
>>> > +                    --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1' \
>>> > +                    --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
>>> > +                    chassis-add ch vxlan 1.2.4.8], [1], [ignore],
>>> > +[ovn-sbctl: transaction error: {"details":"insert operation not
>>> allowed
>>> > when database server is in read only mode","error":"not allowed"}
>>> > +])
>>> > +
>>> > +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
>>> > +AT_CLEANUP
>>> >
>>>
>>> I don't think that this test has anything to do with allowed
>>> ciphers/protocols. We actually have a read only test so we can most
>>> likely
>>> drop this one.
>>>
>>> I kept it to cover both read and write part of ssl as we dont have much
>> coverage in the repo so kept it. Doesnt harm.
>>
>
It doesn't harm, but arguably it has still nothing to do with the
ciphers/protocols change. However I won't block the patch on this.


>
>>> > +
>>> > +AT_SETUP([nb connection/ssl commands with ssl-ciphers and
>>> ssl-protocols])
>>> > +AT_SKIP_IF([test "$HAVE_OPENSSL" = no])
>>> > +PKIDIR="$(cd $abs_top_builddir/tests && pwd)"
>>> > +AT_SKIP_IF([expr "$PKIDIR" : ".*[[     '\"
>>> > +\\]]"])
>>> > +
>>> > +: > .$1.db.~lock~
>>> > +ovsdb-tool create ovn-nb.db "$abs_top_srcdir"/ovn-nb.ovsschema
>>> >
>>>
>>> nit: To slightly simplify the test you could use something like the
>>> below:
>>>
>>> m4_define([SSL_PARAMS], [--ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1' \
>>>                          --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2'])
>>> m4_define([PKI], [--private-key=$PKIDIR/testpki-test-privkey.pem \
>>>                   --certificate=$PKIDIR/testpki-test-cert.pem \
>>>                   --ca-cert=$PKIDIR/testpki-cacert.pem])
>>>
>>> +
>>> > +# Start nb db server using db connection/ssl entries (unpopulated
>>> > initially)
>>> > +start_daemon ovsdb-server --remote=punix:ovnnb_db.sock \
>>> > +
>>> > --remote=db:OVN_Northbound,NB_Global,connections \
>>> > +
>>> --private-key=db:OVN_Northbound,SSL,private_key
>>> > \
>>> > +
>>> --certificate=db:OVN_Northbound,SSL,certificate
>>> > \
>>> > +                          --ca-cert=db:OVN_Northbound,SSL,ca_cert \
>>> > +                          --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1'
>>> \
>>> > +                          --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
>>> > +                          ovn-nb.db
>>> > +
>>> > +# Populate SSL configuration entries in nb db
>>> > +AT_CHECK(
>>> > +    [ovn-nbctl set-ssl $PKIDIR/testpki-test-privkey.pem \
>>> > +                       $PKIDIR/testpki-test-cert.pem \
>>> > +                       $PKIDIR/testpki-cacert.pem], [0], [stdout],
>>> > [ignore])
>>> > +
>>> > +# Populate a passive SSL connection in nb db
>>> > +AT_CHECK([ovn-nbctl set-connection pssl:0:127.0.0.1], [0], [stdout],
>>> > [ignore])
>>> > +
>>> > +PARSE_LISTENING_PORT([ovsdb-server.log], [TCP_PORT])
>>> > +
>>> > +# Verify SSL connetivity to nb db server
>>> > +AT_CHECK([ovn-nbctl --db=ssl:127.0.0.1:$TCP_PORT \
>>> > +                    --private-key=$PKIDIR/testpki-test-privkey.pem \
>>> > +                    --certificate=$PKIDIR/testpki-test-cert.pem \
>>> > +                    --ca-cert=$PKIDIR/testpki-cacert.pem \
>>> > +                    --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1' \
>>> > +                    --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
>>> > +          list NB_Global],
>>> >
>>>
>>> With the m4_define above this could be reduced to "ovn-nbctl
>>> --db=ssl:127.0.0.1:$TCP_PORT PKI SSL_PARAMS list NB_Global".
>>> This is applicable to all the ovn-nbctl and ovn-sbctl commands.
>>>
>>>
>>> I think its better to optimize with the follow up patch as its already
>> in multiple places e.g
>> https://github.com/ovn-org/ovn/blob/main/tests/ovn.at#L10342 .That is
>> why I retained original way.
>>
>
Followup patch is also fine to adjust all of those tests.


> > +         [0], [stdout], [ignore])
>>> > +AT_CHECK([ovn-nbctl --db=ssl:127.0.0.1:$TCP_PORT \
>>> > +                    --private-key=$PKIDIR/testpki-test-privkey.pem \
>>> > +                    --certificate=$PKIDIR/testpki-test-cert.pem \
>>> > +                    --ca-cert=$PKIDIR/testpki-cacert.pem \
>>> > +                    --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1' \
>>> > +                    --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
>>> > +          list Connection],
>>> > +         [0], [stdout], [ignore])
>>> > +AT_CHECK([ovn-nbctl --db=ssl:127.0.0.1:$TCP_PORT \
>>> > +                    --private-key=$PKIDIR/testpki-test-privkey.pem \
>>> > +                    --certificate=$PKIDIR/testpki-test-cert.pem \
>>> > +                    --ca-cert=$PKIDIR/testpki-cacert.pem \
>>> > +                    --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1' \
>>> > +                    --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
>>> > +          get-connection],
>>> > +         [0], [stdout], [ignore])
>>> > +
>>> > +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
>>> > +AT_CLEANUP
>>> > +
>>> > +AT_SETUP([sb connection/ssl commands with ssl-ciphers and
>>> ssl-protocols])
>>> > +AT_SKIP_IF([test "$HAVE_OPENSSL" = no])
>>> > +PKIDIR="$(cd $abs_top_builddir/tests && pwd)"
>>> > +AT_SKIP_IF([expr "$PKIDIR" : ".*[[     '\"
>>> > +\\]]"])
>>> > +
>>> > +: > .$1.db.~lock~
>>> > +ovsdb-tool create ovn-sb.db "$abs_top_srcdir"/ovn-sb.ovsschema
>>> > +
>>> > +# Start sb db server using db connection/ssl entries (unpopulated
>>> > initially)
>>> > +start_daemon ovsdb-server --remote=punix:ovnsb_db.sock \
>>> > +
>>> > --remote=db:OVN_Southbound,SB_Global,connections \
>>> > +
>>> --private-key=db:OVN_Southbound,SSL,private_key
>>> > \
>>> > +
>>> --certificate=db:OVN_Southbound,SSL,certificate
>>> > \
>>> > +                          --ca-cert=db:OVN_Southbound,SSL,ca_cert \
>>> > +                          --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1'
>>> \
>>> > +                          --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
>>> > +                          ovn-sb.db
>>> > +
>>> > +# Populate SSL configuration entries in sb db
>>> > +AT_CHECK(
>>> > +    [ovn-sbctl set-ssl $PKIDIR/testpki-test-privkey.pem \
>>> > +                       $PKIDIR/testpki-test-cert.pem \
>>> > +                       $PKIDIR/testpki-cacert.pem], [0], [stdout],
>>> > [ignore])
>>> > +
>>> > +# Populate a passive SSL connection in sb db
>>> > +AT_CHECK([ovn-sbctl set-connection pssl:0:127.0.0.1], [0], [stdout],
>>> > [ignore])
>>> > +
>>> > +PARSE_LISTENING_PORT([ovsdb-server.log], [TCP_PORT])
>>> > +
>>> > +# Verify SSL connetivity to sb db server
>>> > +AT_CHECK([ovn-sbctl --db=ssl:127.0.0.1:$TCP_PORT \
>>> > +                    --private-key=$PKIDIR/testpki-test-privkey.pem \
>>> > +                    --certificate=$PKIDIR/testpki-test-cert.pem \
>>> > +                    --ca-cert=$PKIDIR/testpki-cacert.pem \
>>> > +                    --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1' \
>>> > +                    --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
>>> > +          list SB_Global],
>>> > +         [0], [stdout], [ignore])
>>> > +AT_CHECK([ovn-sbctl --db=ssl:127.0.0.1:$TCP_PORT \
>>> > +                    --private-key=$PKIDIR/testpki-test-privkey.pem \
>>> > +                    --certificate=$PKIDIR/testpki-test-cert.pem \
>>> > +                    --ca-cert=$PKIDIR/testpki-cacert.pem \
>>> > +                    --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1' \
>>> > +                    --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
>>> > +          list Connection],
>>> > +         [0], [stdout], [ignore])
>>> > +AT_CHECK([ovn-sbctl --db=ssl:127.0.0.1:$TCP_PORT \
>>> > +                    --private-key=$PKIDIR/testpki-test-privkey.pem \
>>> > +                    --certificate=$PKIDIR/testpki-test-cert.pem \
>>> > +                    --ca-cert=$PKIDIR/testpki-cacert.pem \
>>> > +                    --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1' \
>>> > +                    --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
>>> > +          get-connection],
>>> > +         [0], [stdout], [ignore])
>>> > +
>>> > +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
>>> > +AT_CLEANUP
>>> > --
>>> > 2.39.3 (Apple Git-145)
>>> >
>>> > _______________________________________________
>>> > dev mailing list
>>> > dev@openvswitch.org
>>> >
>>> https://urldefense.com/v3/__https://mail.openvswitch.org/mailman/listinfo/ovs-dev__;!!IKRxdwAv5BmarQ!b9pUg4X974A51WZFKZfkdEpZfonrVQK6Py8Vn8OwxGmUPEMOPQt5bxmdT-lSbLg4SRGxmwmpDKmTpQ$
>>> >
>>> >
>>> Thanks,
>>> Ales
>>>
>>> --
>>>
>>> Ales Musil
>>>
>>> Senior Software Engineer - OVN Core
>>>
>>> Red Hat EMEA <
>>> https://urldefense.com/v3/__https://www.redhat.com__;!!IKRxdwAv5BmarQ!b9pUg4X974A51WZFKZfkdEpZfonrVQK6Py8Vn8OwxGmUPEMOPQt5bxmdT-lSbLg4SRGxmwncGrVSrg$
>>> >
>>>
>>> amusil@redhat.com
>>> <
>>> https://urldefense.com/v3/__https://red.ht/sig__;!!IKRxdwAv5BmarQ!b9pUg4X974A51WZFKZfkdEpZfonrVQK6Py8Vn8OwxGmUPEMOPQt5bxmdT-lSbLg4SRGxmwlOrm0WvQ$
>>> >
>>> _______________________________________________
>>> dev mailing list
>>> dev@openvswitch.org
>>>
>>> https://urldefense.com/v3/__https://mail.openvswitch.org/mailman/listinfo/ovs-dev__;!!IKRxdwAv5BmarQ!b9pUg4X974A51WZFKZfkdEpZfonrVQK6Py8Vn8OwxGmUPEMOPQt5bxmdT-lSbLg4SRGxmwmpDKmTpQ$
>>>
>>
Thanks,
Ales
aginwala Jan. 17, 2024, 8:14 p.m. UTC | #5
Thank Ales:

 Addressed 2 changes:


   - Merged into a single commit to include code+tests.
   - Addressed comment for ovn-controller to remove PKIDIR.


PTAL.

Thanks,
Ali



On Tue, Jan 16, 2024 at 10:21 PM Ales Musil <amusil@redhat.com> wrote:

>
>
> On Wed, Jan 17, 2024 at 6:22 AM aginwala <aginwala@asu.edu> wrote:
>
>> Hi Ales:
>>
>> Ping. Following up for the test coverage bits.  I would like to keep it
>> slick for the bug fix/feature gap first. Please confirm so I can amend
>> accordingly.
>>
>
> Sorry for the late response, please see reply inline.
>
>
>>
>>
>> Ali
>>
>> On Fri, Jan 12, 2024 at 8:11 AM aginwala <aginwala@asu.edu> wrote:
>>
>>> Thanks for review.
>>>
>>> On Fri, Jan 12, 2024 at 2:08 AM Ales Musil <amusil@redhat.com> wrote:
>>>
>>>> On Thu, Jan 11, 2024 at 2:30 PM <amginwal@gmail.com> wrote:
>>>>
>>>> > From: Aliasgar Ginwala <aginwala@ebay.com>
>>>> >
>>>> > Signed-off-by: Aliasgar Ginwala <aginwala@ebay.com>
>>>> > ---
>>>> >
>>>>
>>>> Hi Aliasgar,
>>>>
>>>> thank you for the series. Those two patches should be squashed to one
>>>> and I
>>>> have a couple of small comments down below.
>>>>
>>> Can it be done while applying. I realized I didnt add tests so added an
>>> add on commit for the same. Would be neat to keep it that way too. Let me
>>> know I can still proceed to squash in one commit if not acceptable.
>>>
>>>>
>>>>
>>>> >  tests/ovn-controller.at
>>>> <https://urldefense.com/v3/__http://ovn-controller.at__;!!IKRxdwAv5BmarQ!ZwJ7_neCq8fDpNnZjJ7z24A2TJz7OqaM_rR7hhj7jOac1EZSE-uEaRlLGP5RF56IieFsyoFCbfWlnA$>
>>>> |  26 ++++++
>>>> >  tests/ovn.at
>>>> <https://urldefense.com/v3/__http://ovn.at__;!!IKRxdwAv5BmarQ!ZwJ7_neCq8fDpNnZjJ7z24A2TJz7OqaM_rR7hhj7jOac1EZSE-uEaRlLGP5RF56IieFsyoGo_P5dbA$>
>>>>           | 182 ++++++++++++++++++++++++++++++++++++++++
>>>> >  2 files changed, 208 insertions(+)
>>>> >
>>>> > diff --git a/tests/ovn-controller.at
>>>> <https://urldefense.com/v3/__http://ovn-controller.at__;!!IKRxdwAv5BmarQ!ZwJ7_neCq8fDpNnZjJ7z24A2TJz7OqaM_rR7hhj7jOac1EZSE-uEaRlLGP5RF56IieFsyoFCbfWlnA$>
>>>> b/tests/ovn-controller.at
>>>> <https://urldefense.com/v3/__http://ovn-controller.at__;!!IKRxdwAv5BmarQ!ZwJ7_neCq8fDpNnZjJ7z24A2TJz7OqaM_rR7hhj7jOac1EZSE-uEaRlLGP5RF56IieFsyoFCbfWlnA$>
>>>> > index 9d2a37c72..df5662527 100644
>>>> > --- a/tests/ovn-controller.at
>>>> <https://urldefense.com/v3/__http://ovn-controller.at__;!!IKRxdwAv5BmarQ!ZwJ7_neCq8fDpNnZjJ7z24A2TJz7OqaM_rR7hhj7jOac1EZSE-uEaRlLGP5RF56IieFsyoFCbfWlnA$>
>>>> > +++ b/tests/ovn-controller.at
>>>> <https://urldefense.com/v3/__http://ovn-controller.at__;!!IKRxdwAv5BmarQ!ZwJ7_neCq8fDpNnZjJ7z24A2TJz7OqaM_rR7hhj7jOac1EZSE-uEaRlLGP5RF56IieFsyoFCbfWlnA$>
>>>> > @@ -2712,3 +2712,29 @@ AT_CHECK([as hv1 ovs-ofctl dump-flows br-int
>>>> > table=40 | grep -q controller], [1]
>>>> >  OVN_CLEANUP([hv1])
>>>> >  AT_CLEANUP
>>>> >  ])
>>>> > +
>>>> > +
>>>> > +AT_SETUP([ovn-controller - ssl ciphers using command line options])
>>>> > +AT_KEYWORDS([ovn])
>>>> > +AT_SKIP_IF([test "$HAVE_OPENSSL" = no])
>>>> > +PKIDIR="$(cd $abs_top_builddir/tests && pwd)"
>>>> > +AT_SKIP_IF([expr "$PKIDIR" : ".*[[     '\"
>>>> > +\\]]"])
>>>> >
>>>>
>>>> This check is not needed, as we don't operate with the PKI files
>>>> directly.
>>>>
>>>> I would like to keep it to cover at-least one case for ssl-cipher arg
>>> for ovn-controller. Its needed as it didnt execute ovs-vsctl del-ssl
>>> command so ovn will use the pki bits from ovsdb and -ssl-ciphers will be
>>> passed explicitly
>>>
>>
> There is probably some misunderstanding, the test case is fine and should
> stay. By not needed I meant only the following part:
>
> PKIDIR="$(cd $abs_top_builddir/tests && pwd)"
> AT_SKIP_IF([expr "$PKIDIR" : ".*[[     '\"
> \\]]"])
>
>
>>
>>>> > +ovn_start
>>>> > +
>>>> > +net_add n1
>>>> > +sim_add hv1
>>>> > +ovs-vsctl add-br br-phys
>>>> > +ovn_attach n1 br-phys 192.168.0.20
>>>> > +
>>>> > +# Set cipher and and it should connect
>>>> > +OVS_APP_EXIT_AND_WAIT([ovn-controller])
>>>> > +start_daemon ovn-controller --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL
>>>> =1'
>>>> > --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2'
>>>> > +
>>>> > +OVS_WAIT_FOR_OUTPUT([ovn-appctl -t ovn-controller connection-status],
>>>> > [0], [connected
>>>> > +])
>>>> > +
>>>> > +cat hv1/ovn-controller.log
>>>> > +
>>>> > +OVN_CLEANUP([hv1])
>>>> > +AT_CLEANUP
>>>> > diff --git a/tests/ovn.at
>>>> <https://urldefense.com/v3/__http://ovn.at__;!!IKRxdwAv5BmarQ!ZwJ7_neCq8fDpNnZjJ7z24A2TJz7OqaM_rR7hhj7jOac1EZSE-uEaRlLGP5RF56IieFsyoGo_P5dbA$>
>>>> b/tests/ovn.at
>>>> <https://urldefense.com/v3/__http://ovn.at__;!!IKRxdwAv5BmarQ!ZwJ7_neCq8fDpNnZjJ7z24A2TJz7OqaM_rR7hhj7jOac1EZSE-uEaRlLGP5RF56IieFsyoGo_P5dbA$>
>>>> > index c3644ac78..34f277ef9 100644
>>>> > --- a/tests/ovn.at
>>>> <https://urldefense.com/v3/__http://ovn.at__;!!IKRxdwAv5BmarQ!ZwJ7_neCq8fDpNnZjJ7z24A2TJz7OqaM_rR7hhj7jOac1EZSE-uEaRlLGP5RF56IieFsyoGo_P5dbA$>
>>>> > +++ b/tests/ovn.at
>>>> <https://urldefense.com/v3/__http://ovn.at__;!!IKRxdwAv5BmarQ!ZwJ7_neCq8fDpNnZjJ7z24A2TJz7OqaM_rR7hhj7jOac1EZSE-uEaRlLGP5RF56IieFsyoGo_P5dbA$>
>>>> > @@ -37588,3 +37588,185 @@ OVN_CLEANUP([hv1])
>>>> >
>>>> >  AT_CLEANUP
>>>> >  ])
>>>> > +
>>>> > +AT_SETUP([read-only sb db:pssl access with ssl-ciphers and
>>>> ssl-protocols])
>>>> > +AT_SKIP_IF([test "$HAVE_OPENSSL" = no])
>>>> > +PKIDIR="$(cd $abs_top_builddir/tests && pwd)"
>>>> > +AT_SKIP_IF([expr "$PKIDIR" : ".*[[     '\"
>>>> > +\\]]"])
>>>> > +
>>>> > +: > .$1.db.~lock~
>>>> > +ovsdb-tool create ovn-sb.db "$abs_top_srcdir"/ovn-sb.ovsschema
>>>> > +
>>>> > +# Add read-only remote to sb ovsdb-server
>>>> > +AT_CHECK(
>>>> > +  [ovsdb-tool transact ovn-sb.db \
>>>> > +     ['["OVN_Southbound",
>>>> > +       {"op": "insert",
>>>> > +        "table": "SB_Global",
>>>> > +        "row": {
>>>> > +          "connections": ["set", [["named-uuid", "xyz"]]]}},
>>>> > +       {"op": "insert",
>>>> > +        "table": "Connection",
>>>> > +        "uuid-name": "xyz",
>>>> > +        "row": {"target": "pssl:0:127.0.0.1",
>>>> > +               "read_only": true}}]']], [0], [ignore], [ignore])
>>>> > +
>>>> > +start_daemon ovsdb-server --remote=punix:ovn-sb.sock \
>>>> > +
>>>> > --remote=db:OVN_Southbound,SB_Global,connections \
>>>> > +
>>>> > --private-key="$PKIDIR/testpki-test2-privkey.pem" \
>>>> > +
>>>> --certificate="$PKIDIR/testpki-test2-cert.pem" \
>>>> > +                          --ca-cert="$PKIDIR/testpki-cacert.pem" \
>>>> > +                          --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1'
>>>> \
>>>> > +                          --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
>>>> > +                          ovn-sb.db
>>>> > +
>>>> > +PARSE_LISTENING_PORT([ovsdb-server.log], [TCP_PORT])
>>>> > +
>>>> > +# read-only accesses should succeed
>>>> > +AT_CHECK([ovn-sbctl --db=ssl:127.0.0.1:$TCP_PORT \
>>>> > +                    --private-key=$PKIDIR/testpki-test-privkey.pem \
>>>> > +                    --certificate=$PKIDIR/testpki-test-cert.pem \
>>>> > +                    --ca-cert=$PKIDIR/testpki-cacert.pem \
>>>> > +                    --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1' \
>>>> > +                    --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
>>>> > +                    list SB_Global], [0], [stdout], [ignore])
>>>> > +AT_CHECK([ovn-sbctl --db=ssl:127.0.0.1:$TCP_PORT \
>>>> > +                    --private-key=$PKIDIR/testpki-test-privkey.pem \
>>>> > +                    --certificate=$PKIDIR/testpki-test-cert.pem \
>>>> > +                    --ca-cert=$PKIDIR/testpki-cacert.pem \
>>>> > +                    --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1' \
>>>> > +                    --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
>>>> > +                    list Connection], [0], [stdout], [ignore])
>>>> > +
>>>> > +# write access should fail
>>>> > +AT_CHECK([ovn-sbctl --db=ssl:127.0.0.1:$TCP_PORT \
>>>> > +                    --private-key=$PKIDIR/testpki-test-privkey.pem \
>>>> > +                    --certificate=$PKIDIR/testpki-test-cert.pem \
>>>> > +                    --ca-cert=$PKIDIR/testpki-cacert.pem \
>>>> > +                    --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1' \
>>>> > +                    --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
>>>> > +                    chassis-add ch vxlan 1.2.4.8], [1], [ignore],
>>>> > +[ovn-sbctl: transaction error: {"details":"insert operation not
>>>> allowed
>>>> > when database server is in read only mode","error":"not allowed"}
>>>> > +])
>>>> > +
>>>> > +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
>>>> > +AT_CLEANUP
>>>> >
>>>>
>>>> I don't think that this test has anything to do with allowed
>>>> ciphers/protocols. We actually have a read only test so we can most
>>>> likely
>>>> drop this one.
>>>>
>>>> I kept it to cover both read and write part of ssl as we dont have much
>>> coverage in the repo so kept it. Doesnt harm.
>>>
>>
> It doesn't harm, but arguably it has still nothing to do with the
> ciphers/protocols change. However I won't block the patch on this.
>
>
>>
>>>> > +
>>>> > +AT_SETUP([nb connection/ssl commands with ssl-ciphers and
>>>> ssl-protocols])
>>>> > +AT_SKIP_IF([test "$HAVE_OPENSSL" = no])
>>>> > +PKIDIR="$(cd $abs_top_builddir/tests && pwd)"
>>>> > +AT_SKIP_IF([expr "$PKIDIR" : ".*[[     '\"
>>>> > +\\]]"])
>>>> > +
>>>> > +: > .$1.db.~lock~
>>>> > +ovsdb-tool create ovn-nb.db "$abs_top_srcdir"/ovn-nb.ovsschema
>>>> >
>>>>
>>>> nit: To slightly simplify the test you could use something like the
>>>> below:
>>>>
>>>> m4_define([SSL_PARAMS], [--ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1' \
>>>>                          --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2'])
>>>> m4_define([PKI], [--private-key=$PKIDIR/testpki-test-privkey.pem \
>>>>                   --certificate=$PKIDIR/testpki-test-cert.pem \
>>>>                   --ca-cert=$PKIDIR/testpki-cacert.pem])
>>>>
>>>> +
>>>> > +# Start nb db server using db connection/ssl entries (unpopulated
>>>> > initially)
>>>> > +start_daemon ovsdb-server --remote=punix:ovnnb_db.sock \
>>>> > +
>>>> > --remote=db:OVN_Northbound,NB_Global,connections \
>>>> > +
>>>> --private-key=db:OVN_Northbound,SSL,private_key
>>>> > \
>>>> > +
>>>> --certificate=db:OVN_Northbound,SSL,certificate
>>>> > \
>>>> > +                          --ca-cert=db:OVN_Northbound,SSL,ca_cert \
>>>> > +                          --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1'
>>>> \
>>>> > +                          --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
>>>> > +                          ovn-nb.db
>>>> > +
>>>> > +# Populate SSL configuration entries in nb db
>>>> > +AT_CHECK(
>>>> > +    [ovn-nbctl set-ssl $PKIDIR/testpki-test-privkey.pem \
>>>> > +                       $PKIDIR/testpki-test-cert.pem \
>>>> > +                       $PKIDIR/testpki-cacert.pem], [0], [stdout],
>>>> > [ignore])
>>>> > +
>>>> > +# Populate a passive SSL connection in nb db
>>>> > +AT_CHECK([ovn-nbctl set-connection pssl:0:127.0.0.1], [0], [stdout],
>>>> > [ignore])
>>>> > +
>>>> > +PARSE_LISTENING_PORT([ovsdb-server.log], [TCP_PORT])
>>>> > +
>>>> > +# Verify SSL connetivity to nb db server
>>>> > +AT_CHECK([ovn-nbctl --db=ssl:127.0.0.1:$TCP_PORT \
>>>> > +                    --private-key=$PKIDIR/testpki-test-privkey.pem \
>>>> > +                    --certificate=$PKIDIR/testpki-test-cert.pem \
>>>> > +                    --ca-cert=$PKIDIR/testpki-cacert.pem \
>>>> > +                    --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1' \
>>>> > +                    --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
>>>> > +          list NB_Global],
>>>> >
>>>>
>>>> With the m4_define above this could be reduced to "ovn-nbctl
>>>> --db=ssl:127.0.0.1:$TCP_PORT PKI SSL_PARAMS list NB_Global".
>>>> This is applicable to all the ovn-nbctl and ovn-sbctl commands.
>>>>
>>>>
>>>> I think its better to optimize with the follow up patch as its already
>>> in multiple places e.g
>>> https://github.com/ovn-org/ovn/blob/main/tests/ovn.at#L10342
>>> <https://urldefense.com/v3/__https://github.com/ovn-org/ovn/blob/main/tests/ovn.at*L10342__;Iw!!IKRxdwAv5BmarQ!ZwJ7_neCq8fDpNnZjJ7z24A2TJz7OqaM_rR7hhj7jOac1EZSE-uEaRlLGP5RF56IieFsyoFu9F12fQ$>
>>> .That is why I retained original way.
>>>
>>
> Followup patch is also fine to adjust all of those tests.
>
>
>> > +         [0], [stdout], [ignore])
>>>> > +AT_CHECK([ovn-nbctl --db=ssl:127.0.0.1:$TCP_PORT \
>>>> > +                    --private-key=$PKIDIR/testpki-test-privkey.pem \
>>>> > +                    --certificate=$PKIDIR/testpki-test-cert.pem \
>>>> > +                    --ca-cert=$PKIDIR/testpki-cacert.pem \
>>>> > +                    --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1' \
>>>> > +                    --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
>>>> > +          list Connection],
>>>> > +         [0], [stdout], [ignore])
>>>> > +AT_CHECK([ovn-nbctl --db=ssl:127.0.0.1:$TCP_PORT \
>>>> > +                    --private-key=$PKIDIR/testpki-test-privkey.pem \
>>>> > +                    --certificate=$PKIDIR/testpki-test-cert.pem \
>>>> > +                    --ca-cert=$PKIDIR/testpki-cacert.pem \
>>>> > +                    --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1' \
>>>> > +                    --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
>>>> > +          get-connection],
>>>> > +         [0], [stdout], [ignore])
>>>> > +
>>>> > +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
>>>> > +AT_CLEANUP
>>>> > +
>>>> > +AT_SETUP([sb connection/ssl commands with ssl-ciphers and
>>>> ssl-protocols])
>>>> > +AT_SKIP_IF([test "$HAVE_OPENSSL" = no])
>>>> > +PKIDIR="$(cd $abs_top_builddir/tests && pwd)"
>>>> > +AT_SKIP_IF([expr "$PKIDIR" : ".*[[     '\"
>>>> > +\\]]"])
>>>> > +
>>>> > +: > .$1.db.~lock~
>>>> > +ovsdb-tool create ovn-sb.db "$abs_top_srcdir"/ovn-sb.ovsschema
>>>> > +
>>>> > +# Start sb db server using db connection/ssl entries (unpopulated
>>>> > initially)
>>>> > +start_daemon ovsdb-server --remote=punix:ovnsb_db.sock \
>>>> > +
>>>> > --remote=db:OVN_Southbound,SB_Global,connections \
>>>> > +
>>>> --private-key=db:OVN_Southbound,SSL,private_key
>>>> > \
>>>> > +
>>>> --certificate=db:OVN_Southbound,SSL,certificate
>>>> > \
>>>> > +                          --ca-cert=db:OVN_Southbound,SSL,ca_cert \
>>>> > +                          --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1'
>>>> \
>>>> > +                          --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
>>>> > +                          ovn-sb.db
>>>> > +
>>>> > +# Populate SSL configuration entries in sb db
>>>> > +AT_CHECK(
>>>> > +    [ovn-sbctl set-ssl $PKIDIR/testpki-test-privkey.pem \
>>>> > +                       $PKIDIR/testpki-test-cert.pem \
>>>> > +                       $PKIDIR/testpki-cacert.pem], [0], [stdout],
>>>> > [ignore])
>>>> > +
>>>> > +# Populate a passive SSL connection in sb db
>>>> > +AT_CHECK([ovn-sbctl set-connection pssl:0:127.0.0.1], [0], [stdout],
>>>> > [ignore])
>>>> > +
>>>> > +PARSE_LISTENING_PORT([ovsdb-server.log], [TCP_PORT])
>>>> > +
>>>> > +# Verify SSL connetivity to sb db server
>>>> > +AT_CHECK([ovn-sbctl --db=ssl:127.0.0.1:$TCP_PORT \
>>>> > +                    --private-key=$PKIDIR/testpki-test-privkey.pem \
>>>> > +                    --certificate=$PKIDIR/testpki-test-cert.pem \
>>>> > +                    --ca-cert=$PKIDIR/testpki-cacert.pem \
>>>> > +                    --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1' \
>>>> > +                    --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
>>>> > +          list SB_Global],
>>>> > +         [0], [stdout], [ignore])
>>>> > +AT_CHECK([ovn-sbctl --db=ssl:127.0.0.1:$TCP_PORT \
>>>> > +                    --private-key=$PKIDIR/testpki-test-privkey.pem \
>>>> > +                    --certificate=$PKIDIR/testpki-test-cert.pem \
>>>> > +                    --ca-cert=$PKIDIR/testpki-cacert.pem \
>>>> > +                    --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1' \
>>>> > +                    --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
>>>> > +          list Connection],
>>>> > +         [0], [stdout], [ignore])
>>>> > +AT_CHECK([ovn-sbctl --db=ssl:127.0.0.1:$TCP_PORT \
>>>> > +                    --private-key=$PKIDIR/testpki-test-privkey.pem \
>>>> > +                    --certificate=$PKIDIR/testpki-test-cert.pem \
>>>> > +                    --ca-cert=$PKIDIR/testpki-cacert.pem \
>>>> > +                    --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1' \
>>>> > +                    --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
>>>> > +          get-connection],
>>>> > +         [0], [stdout], [ignore])
>>>> > +
>>>> > +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
>>>> > +AT_CLEANUP
>>>> > --
>>>> > 2.39.3 (Apple Git-145)
>>>> >
>>>> > _______________________________________________
>>>> > dev mailing list
>>>> > dev@openvswitch.org
>>>> >
>>>> https://urldefense.com/v3/__https://mail.openvswitch.org/mailman/listinfo/ovs-dev__;!!IKRxdwAv5BmarQ!b9pUg4X974A51WZFKZfkdEpZfonrVQK6Py8Vn8OwxGmUPEMOPQt5bxmdT-lSbLg4SRGxmwmpDKmTpQ$
>>>> >
>>>> >
>>>> Thanks,
>>>> Ales
>>>>
>>>> --
>>>>
>>>> Ales Musil
>>>>
>>>> Senior Software Engineer - OVN Core
>>>>
>>>> Red Hat EMEA <
>>>> https://urldefense.com/v3/__https://www.redhat.com__;!!IKRxdwAv5BmarQ!b9pUg4X974A51WZFKZfkdEpZfonrVQK6Py8Vn8OwxGmUPEMOPQt5bxmdT-lSbLg4SRGxmwncGrVSrg$
>>>> >
>>>>
>>>> amusil@redhat.com
>>>> <
>>>> https://urldefense.com/v3/__https://red.ht/sig__;!!IKRxdwAv5BmarQ!b9pUg4X974A51WZFKZfkdEpZfonrVQK6Py8Vn8OwxGmUPEMOPQt5bxmdT-lSbLg4SRGxmwlOrm0WvQ$
>>>> >
>>>> _______________________________________________
>>>> dev mailing list
>>>> dev@openvswitch.org
>>>>
>>>> https://urldefense.com/v3/__https://mail.openvswitch.org/mailman/listinfo/ovs-dev__;!!IKRxdwAv5BmarQ!b9pUg4X974A51WZFKZfkdEpZfonrVQK6Py8Vn8OwxGmUPEMOPQt5bxmdT-lSbLg4SRGxmwmpDKmTpQ$
>>>>
>>>
> Thanks,
> Ales
>
> --
>
> Ales Musil
>
> Senior Software Engineer - OVN Core
>
> Red Hat EMEA
> <https://urldefense.com/v3/__https://www.redhat.com__;!!IKRxdwAv5BmarQ!ZwJ7_neCq8fDpNnZjJ7z24A2TJz7OqaM_rR7hhj7jOac1EZSE-uEaRlLGP5RF56IieFsyoFAOdgdyw$>
>
> amusil@redhat.com
>
> <https://urldefense.com/v3/__https://red.ht/sig__;!!IKRxdwAv5BmarQ!ZwJ7_neCq8fDpNnZjJ7z24A2TJz7OqaM_rR7hhj7jOac1EZSE-uEaRlLGP5RF56IieFsyoEuS3-tpA$>
>
diff mbox series

Patch

diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at
index 9d2a37c72..df5662527 100644
--- a/tests/ovn-controller.at
+++ b/tests/ovn-controller.at
@@ -2712,3 +2712,29 @@  AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=40 | grep -q controller], [1]
 OVN_CLEANUP([hv1])
 AT_CLEANUP
 ])
+
+
+AT_SETUP([ovn-controller - ssl ciphers using command line options])
+AT_KEYWORDS([ovn])
+AT_SKIP_IF([test "$HAVE_OPENSSL" = no])
+PKIDIR="$(cd $abs_top_builddir/tests && pwd)"
+AT_SKIP_IF([expr "$PKIDIR" : ".*[[ 	'\"
+\\]]"])
+ovn_start
+
+net_add n1
+sim_add hv1
+ovs-vsctl add-br br-phys
+ovn_attach n1 br-phys 192.168.0.20
+
+# Set cipher and and it should connect
+OVS_APP_EXIT_AND_WAIT([ovn-controller])
+start_daemon ovn-controller --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1' --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2'
+
+OVS_WAIT_FOR_OUTPUT([ovn-appctl -t ovn-controller connection-status], [0], [connected
+])
+
+cat hv1/ovn-controller.log
+
+OVN_CLEANUP([hv1])
+AT_CLEANUP
diff --git a/tests/ovn.at b/tests/ovn.at
index c3644ac78..34f277ef9 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -37588,3 +37588,185 @@  OVN_CLEANUP([hv1])
 
 AT_CLEANUP
 ])
+
+AT_SETUP([read-only sb db:pssl access with ssl-ciphers and ssl-protocols])
+AT_SKIP_IF([test "$HAVE_OPENSSL" = no])
+PKIDIR="$(cd $abs_top_builddir/tests && pwd)"
+AT_SKIP_IF([expr "$PKIDIR" : ".*[[ 	'\"
+\\]]"])
+
+: > .$1.db.~lock~
+ovsdb-tool create ovn-sb.db "$abs_top_srcdir"/ovn-sb.ovsschema
+
+# Add read-only remote to sb ovsdb-server
+AT_CHECK(
+  [ovsdb-tool transact ovn-sb.db \
+     ['["OVN_Southbound",
+       {"op": "insert",
+        "table": "SB_Global",
+        "row": {
+          "connections": ["set", [["named-uuid", "xyz"]]]}},
+       {"op": "insert",
+        "table": "Connection",
+        "uuid-name": "xyz",
+        "row": {"target": "pssl:0:127.0.0.1",
+               "read_only": true}}]']], [0], [ignore], [ignore])
+
+start_daemon ovsdb-server --remote=punix:ovn-sb.sock \
+                          --remote=db:OVN_Southbound,SB_Global,connections \
+                          --private-key="$PKIDIR/testpki-test2-privkey.pem" \
+                          --certificate="$PKIDIR/testpki-test2-cert.pem" \
+                          --ca-cert="$PKIDIR/testpki-cacert.pem" \
+                          --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1' \
+                          --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
+                          ovn-sb.db
+
+PARSE_LISTENING_PORT([ovsdb-server.log], [TCP_PORT])
+
+# read-only accesses should succeed
+AT_CHECK([ovn-sbctl --db=ssl:127.0.0.1:$TCP_PORT \
+                    --private-key=$PKIDIR/testpki-test-privkey.pem \
+                    --certificate=$PKIDIR/testpki-test-cert.pem \
+                    --ca-cert=$PKIDIR/testpki-cacert.pem \
+                    --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1' \
+                    --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
+                    list SB_Global], [0], [stdout], [ignore])
+AT_CHECK([ovn-sbctl --db=ssl:127.0.0.1:$TCP_PORT \
+                    --private-key=$PKIDIR/testpki-test-privkey.pem \
+                    --certificate=$PKIDIR/testpki-test-cert.pem \
+                    --ca-cert=$PKIDIR/testpki-cacert.pem \
+                    --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1' \
+                    --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
+                    list Connection], [0], [stdout], [ignore])
+
+# write access should fail
+AT_CHECK([ovn-sbctl --db=ssl:127.0.0.1:$TCP_PORT \
+                    --private-key=$PKIDIR/testpki-test-privkey.pem \
+                    --certificate=$PKIDIR/testpki-test-cert.pem \
+                    --ca-cert=$PKIDIR/testpki-cacert.pem \
+                    --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1' \
+                    --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
+                    chassis-add ch vxlan 1.2.4.8], [1], [ignore],
+[ovn-sbctl: transaction error: {"details":"insert operation not allowed when database server is in read only mode","error":"not allowed"}
+])
+
+OVS_APP_EXIT_AND_WAIT([ovsdb-server])
+AT_CLEANUP
+
+AT_SETUP([nb connection/ssl commands with ssl-ciphers and ssl-protocols])
+AT_SKIP_IF([test "$HAVE_OPENSSL" = no])
+PKIDIR="$(cd $abs_top_builddir/tests && pwd)"
+AT_SKIP_IF([expr "$PKIDIR" : ".*[[ 	'\"
+\\]]"])
+
+: > .$1.db.~lock~
+ovsdb-tool create ovn-nb.db "$abs_top_srcdir"/ovn-nb.ovsschema
+
+# Start nb db server using db connection/ssl entries (unpopulated initially)
+start_daemon ovsdb-server --remote=punix:ovnnb_db.sock \
+                          --remote=db:OVN_Northbound,NB_Global,connections \
+                          --private-key=db:OVN_Northbound,SSL,private_key \
+                          --certificate=db:OVN_Northbound,SSL,certificate \
+                          --ca-cert=db:OVN_Northbound,SSL,ca_cert \
+                          --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1' \
+                          --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
+                          ovn-nb.db
+
+# Populate SSL configuration entries in nb db
+AT_CHECK(
+    [ovn-nbctl set-ssl $PKIDIR/testpki-test-privkey.pem \
+                       $PKIDIR/testpki-test-cert.pem \
+                       $PKIDIR/testpki-cacert.pem], [0], [stdout], [ignore])
+
+# Populate a passive SSL connection in nb db
+AT_CHECK([ovn-nbctl set-connection pssl:0:127.0.0.1], [0], [stdout], [ignore])
+
+PARSE_LISTENING_PORT([ovsdb-server.log], [TCP_PORT])
+
+# Verify SSL connetivity to nb db server
+AT_CHECK([ovn-nbctl --db=ssl:127.0.0.1:$TCP_PORT \
+                    --private-key=$PKIDIR/testpki-test-privkey.pem \
+                    --certificate=$PKIDIR/testpki-test-cert.pem \
+                    --ca-cert=$PKIDIR/testpki-cacert.pem \
+                    --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1' \
+                    --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
+          list NB_Global],
+         [0], [stdout], [ignore])
+AT_CHECK([ovn-nbctl --db=ssl:127.0.0.1:$TCP_PORT \
+                    --private-key=$PKIDIR/testpki-test-privkey.pem \
+                    --certificate=$PKIDIR/testpki-test-cert.pem \
+                    --ca-cert=$PKIDIR/testpki-cacert.pem \
+                    --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1' \
+                    --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
+          list Connection],
+         [0], [stdout], [ignore])
+AT_CHECK([ovn-nbctl --db=ssl:127.0.0.1:$TCP_PORT \
+                    --private-key=$PKIDIR/testpki-test-privkey.pem \
+                    --certificate=$PKIDIR/testpki-test-cert.pem \
+                    --ca-cert=$PKIDIR/testpki-cacert.pem \
+                    --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1' \
+                    --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
+          get-connection],
+         [0], [stdout], [ignore])
+
+OVS_APP_EXIT_AND_WAIT([ovsdb-server])
+AT_CLEANUP
+
+AT_SETUP([sb connection/ssl commands with ssl-ciphers and ssl-protocols])
+AT_SKIP_IF([test "$HAVE_OPENSSL" = no])
+PKIDIR="$(cd $abs_top_builddir/tests && pwd)"
+AT_SKIP_IF([expr "$PKIDIR" : ".*[[ 	'\"
+\\]]"])
+
+: > .$1.db.~lock~
+ovsdb-tool create ovn-sb.db "$abs_top_srcdir"/ovn-sb.ovsschema
+
+# Start sb db server using db connection/ssl entries (unpopulated initially)
+start_daemon ovsdb-server --remote=punix:ovnsb_db.sock \
+                          --remote=db:OVN_Southbound,SB_Global,connections \
+                          --private-key=db:OVN_Southbound,SSL,private_key \
+                          --certificate=db:OVN_Southbound,SSL,certificate \
+                          --ca-cert=db:OVN_Southbound,SSL,ca_cert \
+                          --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1' \
+                          --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
+                          ovn-sb.db
+
+# Populate SSL configuration entries in sb db
+AT_CHECK(
+    [ovn-sbctl set-ssl $PKIDIR/testpki-test-privkey.pem \
+                       $PKIDIR/testpki-test-cert.pem \
+                       $PKIDIR/testpki-cacert.pem], [0], [stdout], [ignore])
+
+# Populate a passive SSL connection in sb db
+AT_CHECK([ovn-sbctl set-connection pssl:0:127.0.0.1], [0], [stdout], [ignore])
+
+PARSE_LISTENING_PORT([ovsdb-server.log], [TCP_PORT])
+
+# Verify SSL connetivity to sb db server
+AT_CHECK([ovn-sbctl --db=ssl:127.0.0.1:$TCP_PORT \
+                    --private-key=$PKIDIR/testpki-test-privkey.pem \
+                    --certificate=$PKIDIR/testpki-test-cert.pem \
+                    --ca-cert=$PKIDIR/testpki-cacert.pem \
+                    --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1' \
+                    --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
+          list SB_Global],
+         [0], [stdout], [ignore])
+AT_CHECK([ovn-sbctl --db=ssl:127.0.0.1:$TCP_PORT \
+                    --private-key=$PKIDIR/testpki-test-privkey.pem \
+                    --certificate=$PKIDIR/testpki-test-cert.pem \
+                    --ca-cert=$PKIDIR/testpki-cacert.pem \
+                    --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1' \
+                    --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
+          list Connection],
+         [0], [stdout], [ignore])
+AT_CHECK([ovn-sbctl --db=ssl:127.0.0.1:$TCP_PORT \
+                    --private-key=$PKIDIR/testpki-test-privkey.pem \
+                    --certificate=$PKIDIR/testpki-test-cert.pem \
+                    --ca-cert=$PKIDIR/testpki-cacert.pem \
+                    --ssl-ciphers='HIGH:!aNULL:!MD5:@SECLEVEL=1' \
+                    --ssl-protocols='TLSv1,TLSv1.1,TLSv1.2' \
+          get-connection],
+         [0], [stdout], [ignore])
+
+OVS_APP_EXIT_AND_WAIT([ovsdb-server])
+AT_CLEANUP