diff mbox series

[ovs-dev,v2,2/2] test-stream: Add ssl tests for stream open block

Message ID dc55b876581fc817a63a1099fa77859cb50c9589.camel@cloudandheat.com
State Changes Requested
Headers show
Series [ovs-dev,v2,1/2] tests-ovsdb: switch OVSDB_START_IDLTEST to macro | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/intel-ovs-compilation success test: success

Commit Message

Stefan Hoffmann May 9, 2023, 8:25 a.m. UTC
This tests stream.c and stream.py with ssl connection at
CHECK_STREAM_OPEN_BLOCK.
For the tests, ovsdb needs to be build with libssl.

Signed-off-by: Stefan Hoffmann <stefan.hoffmann@cloudandheat.com>
---
 tests/ovsdb-idl.at   | 32 ++++++++++++++++++++++++++++----
 tests/test-stream.c  | 12 +++++++++++-
 tests/test-stream.py | 18 ++++++++++++++++++
 3 files changed, 57 insertions(+), 5 deletions(-)

Comments

Ilya Maximets May 10, 2023, 7:22 p.m. UTC | #1
On 5/9/23 10:25, Stefan Hoffmann wrote:
> This tests stream.c and stream.py with ssl connection at
> CHECK_STREAM_OPEN_BLOCK.
> For the tests, ovsdb needs to be build with libssl.
> 
> Signed-off-by: Stefan Hoffmann <stefan.hoffmann@cloudandheat.com>
> ---
>  tests/ovsdb-idl.at   | 32 ++++++++++++++++++++++++++++----
>  tests/test-stream.c  | 12 +++++++++++-
>  tests/test-stream.py | 18 ++++++++++++++++++
>  3 files changed, 57 insertions(+), 5 deletions(-)
> 
> diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at
> index ba05ba985..ef8c6a7cc 100644
> --- a/tests/ovsdb-idl.at
> +++ b/tests/ovsdb-idl.at
> @@ -10,8 +10,14 @@ m4_define([OVSDB_START_IDLTEST],
>  [
>    AT_CHECK([ovsdb-tool create db dnl
>                m4_if([$2], [], [$abs_srcdir/idltest.ovsschema], [$2])])
> +  PKIDIR=$abs_top_builddir/tests
> +  m4_define([REMOTE_PROTOCOL], [m4_substr($1, 0, 6)])
>    AT_CHECK([ovsdb-server -vconsole:warn --log-file --detach --no-chdir dnl
>                --pidfile --remote=punix:socket dnl
> +              m4_if(REMOTE_PROTOCOL, "p"ssl,

It's better to bracketify the arguments.

And "p"ssl looks a bit strange. :)
You may use m4_join with an empty separator to concatenate strings instead.

And there is no real need to define REMOTE_PROTOCOL, you may just
use m4_substr($1, 0, 6) on the spot.  But no strong opinion on that.

> +                    [--private-key=$PKIDIR/testpki-privkey2.pem dnl
> +                    --certificate=$PKIDIR/testpki-cert2.pem dnl
> +                    --ca-cert=$PKIDIR/testpki-cacert.pem], []) dnl

I'd shift above 2 lines 1 space to the right.

>                m4_if([$1], [], [], [--remote=$1]) db dnl
>    ])
>    on_exit 'kill `cat ovsdb-server.pid`'
> @@ -2284,14 +2290,26 @@ m4_define([CHECK_STREAM_OPEN_BLOCK],
>    [AT_SETUP([Check stream open block - $1 - $3])
>     AT_SKIP_IF([test "$3" = "tcp6" && test "$IS_WIN32" = "yes"])
>     AT_SKIP_IF([test "$3" = "tcp6" && test "$HAVE_IPV6" = "no"])
> +   AT_SKIP_IF([test "$3" = "ssl6" && test "$IS_WIN32" = "yes"])
> +   AT_SKIP_IF([test "$3" = "ssl6" && test "$HAVE_IPV6" = "no"])
> +   AT_SKIP_IF([test "$3" = "ssl" && test "$HAVE_OPENSSL" = "no"])
> +   $PYTHON3 -c "import ssl"
> +   SSL_PRESENT=$?
> +   AT_SKIP_IF([test "$3" = "ssl" && test $SSL_PRESENT != 0])
> +   AT_SKIP_IF([test "$3" = "ssl6" && test "$HAVE_OPENSSL" = "no"])
> +   AT_SKIP_IF([test "$3" = "ssl6" && test $SSL_PRESENT != 0])
>     AT_KEYWORDS([ovsdb server stream open_block $3])
> -   OVSDB_START_IDLTEST(["ptcp:0:$4"])
> +   PKIDIR=$abs_top_builddir/tests
> +   m4_define([PROTOCOL], [m4_substr([$3], [0], [3])])
> +   OVSDB_START_IDLTEST(["p"PROTOCOL":0:"$4])

Something like this (not tested):

  OVSDB_START_IDLTEST([m4_join([], [p], [PROTOCOL], [:0:], [$4]])

or

  OVSDB_START_IDLTEST([m4_join([], [p], [m4_join([:], [PROTOCOL], [0], [$4])])

Best regards, Ilya Maximets.
Stefan Hoffmann May 11, 2023, 12:37 p.m. UTC | #2
On Wed, 2023-05-10 at 21:22 +0200, Ilya Maximets wrote:
> On 5/9/23 10:25, Stefan Hoffmann wrote:
> > This tests stream.c and stream.py with ssl connection at
> > CHECK_STREAM_OPEN_BLOCK.
> > For the tests, ovsdb needs to be build with libssl.
> > 
> > Signed-off-by: Stefan Hoffmann <stefan.hoffmann@cloudandheat.com>
> > ---
> >  tests/ovsdb-idl.at   | 32 ++++++++++++++++++++++++++++----
> >  tests/test-stream.c  | 12 +++++++++++-
> >  tests/test-stream.py | 18 ++++++++++++++++++
> >  3 files changed, 57 insertions(+), 5 deletions(-)
> > 
> > diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at
> > index ba05ba985..ef8c6a7cc 100644
> > --- a/tests/ovsdb-idl.at
> > +++ b/tests/ovsdb-idl.at
> > @@ -10,8 +10,14 @@ m4_define([OVSDB_START_IDLTEST],
> >  [
> >    AT_CHECK([ovsdb-tool create db dnl
> >                m4_if([$2], [], [$abs_srcdir/idltest.ovsschema], [$2])])
> > +  PKIDIR=$abs_top_builddir/tests
> > +  m4_define([REMOTE_PROTOCOL], [m4_substr($1, 0, 6)])
> >    AT_CHECK([ovsdb-server -vconsole:warn --log-file --detach --no-chdir dnl
> >                --pidfile --remote=punix:socket dnl
> > +              m4_if(REMOTE_PROTOCOL, "p"ssl,
> 
> It's better to bracketify the arguments.

I can bracketify the secund argument "pssl", but the first one don't
work (also with replacing REMOTE_PROTOCOL with m4_substr() ), I guess
else m4 interprets the argument as text, not function/variable.

> 
> And "p"ssl looks a bit strange. :)
> You may use m4_join with an empty separator to concatenate strings instead.
> 
> And there is no real need to define REMOTE_PROTOCOL, you may just
> use m4_substr($1, 0, 6) on the spot.  But no strong opinion on that.

You are right, will use the function directly

> 
> > +                    [--private-key=$PKIDIR/testpki-privkey2.pem dnl
> > +                    --certificate=$PKIDIR/testpki-cert2.pem dnl
> > +                    --ca-cert=$PKIDIR/testpki-cacert.pem], []) dnl
> 
> I'd shift above 2 lines 1 space to the right.
> 
> >                m4_if([$1], [], [], [--remote=$1]) db dnl
> >    ])
> >    on_exit 'kill `cat ovsdb-server.pid`'
> > @@ -2284,14 +2290,26 @@ m4_define([CHECK_STREAM_OPEN_BLOCK],
> >    [AT_SETUP([Check stream open block - $1 - $3])
> >     AT_SKIP_IF([test "$3" = "tcp6" && test "$IS_WIN32" = "yes"])
> >     AT_SKIP_IF([test "$3" = "tcp6" && test "$HAVE_IPV6" = "no"])
> > +   AT_SKIP_IF([test "$3" = "ssl6" && test "$IS_WIN32" = "yes"])
> > +   AT_SKIP_IF([test "$3" = "ssl6" && test "$HAVE_IPV6" = "no"])
> > +   AT_SKIP_IF([test "$3" = "ssl" && test "$HAVE_OPENSSL" = "no"])
> > +   $PYTHON3 -c "import ssl"
> > +   SSL_PRESENT=$?
> > +   AT_SKIP_IF([test "$3" = "ssl" && test $SSL_PRESENT != 0])
> > +   AT_SKIP_IF([test "$3" = "ssl6" && test "$HAVE_OPENSSL" = "no"])
> > +   AT_SKIP_IF([test "$3" = "ssl6" && test $SSL_PRESENT != 0])
> >     AT_KEYWORDS([ovsdb server stream open_block $3])
> > -   OVSDB_START_IDLTEST(["ptcp:0:$4"])
> > +   PKIDIR=$abs_top_builddir/tests
> > +   m4_define([PROTOCOL], [m4_substr([$3], [0], [3])])
> > +   OVSDB_START_IDLTEST(["p"PROTOCOL":0:"$4])
> 
> Something like this (not tested):
> 
>   OVSDB_START_IDLTEST([m4_join([], [p], [PROTOCOL], [:0:], [$4]])

This works, but PROTOCOL must be without brackets, not sure, why.

> 
> or
> 
>   OVSDB_START_IDLTEST([m4_join([], [p], [m4_join([:], [PROTOCOL], [0], [$4])])

Thanks for the hint. Both ways work (besides with nested m4_join, no
brackets must be used). I choosed the first one, as this is shorter.

> 
> Best regards, Ilya Maximets.

Thanks for the good hints, new patch is on it's way (hopefully without
confusing the automation this time)

Best regards,
Stefan
diff mbox series

Patch

diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at
index ba05ba985..ef8c6a7cc 100644
--- a/tests/ovsdb-idl.at
+++ b/tests/ovsdb-idl.at
@@ -10,8 +10,14 @@  m4_define([OVSDB_START_IDLTEST],
 [
   AT_CHECK([ovsdb-tool create db dnl
               m4_if([$2], [], [$abs_srcdir/idltest.ovsschema], [$2])])
+  PKIDIR=$abs_top_builddir/tests
+  m4_define([REMOTE_PROTOCOL], [m4_substr($1, 0, 6)])
   AT_CHECK([ovsdb-server -vconsole:warn --log-file --detach --no-chdir dnl
               --pidfile --remote=punix:socket dnl
+              m4_if(REMOTE_PROTOCOL, "p"ssl,
+                    [--private-key=$PKIDIR/testpki-privkey2.pem dnl
+                    --certificate=$PKIDIR/testpki-cert2.pem dnl
+                    --ca-cert=$PKIDIR/testpki-cacert.pem], []) dnl
               m4_if([$1], [], [], [--remote=$1]) db dnl
   ])
   on_exit 'kill `cat ovsdb-server.pid`'
@@ -2284,14 +2290,26 @@  m4_define([CHECK_STREAM_OPEN_BLOCK],
   [AT_SETUP([Check stream open block - $1 - $3])
    AT_SKIP_IF([test "$3" = "tcp6" && test "$IS_WIN32" = "yes"])
    AT_SKIP_IF([test "$3" = "tcp6" && test "$HAVE_IPV6" = "no"])
+   AT_SKIP_IF([test "$3" = "ssl6" && test "$IS_WIN32" = "yes"])
+   AT_SKIP_IF([test "$3" = "ssl6" && test "$HAVE_IPV6" = "no"])
+   AT_SKIP_IF([test "$3" = "ssl" && test "$HAVE_OPENSSL" = "no"])
+   $PYTHON3 -c "import ssl"
+   SSL_PRESENT=$?
+   AT_SKIP_IF([test "$3" = "ssl" && test $SSL_PRESENT != 0])
+   AT_SKIP_IF([test "$3" = "ssl6" && test "$HAVE_OPENSSL" = "no"])
+   AT_SKIP_IF([test "$3" = "ssl6" && test $SSL_PRESENT != 0])
    AT_KEYWORDS([ovsdb server stream open_block $3])
-   OVSDB_START_IDLTEST(["ptcp:0:$4"])
+   PKIDIR=$abs_top_builddir/tests
+   m4_define([PROTOCOL], [m4_substr([$3], [0], [3])])
+   OVSDB_START_IDLTEST(["p"PROTOCOL":0:"$4])
    PARSE_LISTENING_PORT([ovsdb-server.log], [TCP_PORT])
    WRONG_PORT=$(($TCP_PORT + 101))
-   AT_CHECK([$2 tcp:$4:$TCP_PORT], [0], [ignore])
-   AT_CHECK([$2 tcp:$4:$WRONG_PORT], [1], [ignore], [ignore])
+   SSL_KEY_ARGS="$PKIDIR/testpki-privkey.pem $PKIDIR/testpki-cert.pem $PKIDIR/testpki-cacert.pem"
+   AT_CHECK([$2 PROTOCOL:$4:$TCP_PORT $SSL_KEY_ARGS], [0], [ignore])
+   AT_CHECK([$2 PROTOCOL:$4:$WRONG_PORT $SSL_KEY_ARGS], [1], [ignore],
+            [ignore])
    OVSDB_SERVER_SHUTDOWN
-   AT_CHECK([$2 tcp:$4:$TCP_PORT], [1], [ignore], [ignore])
+   AT_CHECK([$2 PROTOCOL:$4:$TCP_PORT $SSL_KEY_ARGS], [1], [ignore], [ignore])
    AT_CLEANUP])
 
 CHECK_STREAM_OPEN_BLOCK([C], [test-stream], [tcp], [127.0.0.1])
@@ -2300,6 +2318,12 @@  CHECK_STREAM_OPEN_BLOCK([Python3], [$PYTHON3 $srcdir/test-stream.py],
                         [tcp], [127.0.0.1])
 CHECK_STREAM_OPEN_BLOCK([Python3], [$PYTHON3 $srcdir/test-stream.py],
                         [tcp6], [[[::1]]])
+CHECK_STREAM_OPEN_BLOCK([C], [test-stream], [ssl], [127.0.0.1])
+CHECK_STREAM_OPEN_BLOCK([C], [test-stream], [ssl6], [[[::1]]])
+CHECK_STREAM_OPEN_BLOCK([Python3], [$PYTHON3 $srcdir/test-stream.py],
+                        [ssl], [127.0.0.1])
+CHECK_STREAM_OPEN_BLOCK([Python3], [$PYTHON3 $srcdir/test-stream.py],
+                        [ssl6], [[[::1]]])
 
 # same as OVSDB_CHECK_IDL but uses Python IDL implementation with tcp
 # with multiple remotes to assert the idl connects to the leader of the Raft cluster
diff --git a/tests/test-stream.c b/tests/test-stream.c
index 68ce2c544..14e3bfe38 100644
--- a/tests/test-stream.c
+++ b/tests/test-stream.c
@@ -19,6 +19,7 @@ 
 #include "fatal-signal.h"
 #include "openvswitch/vlog.h"
 #include "stream.h"
+#include "stream-ssl.h"
 #include "util.h"
 
 VLOG_DEFINE_THIS_MODULE(test_stream);
@@ -33,7 +34,16 @@  main(int argc, char *argv[])
     set_program_name(argv[0]);
 
     if (argc < 2) {
-        ovs_fatal(0, "usage: %s REMOTE", argv[0]);
+        ovs_fatal(0, "usage: %s REMOTE [SSL_KEY] [SSL_CERT] [SSL_CA]",
+                  argv[0]);
+    }
+    if (strncmp("ssl:", argv[1], 4) == 0) {
+        if (argc < 5) {
+            ovs_fatal(0, "usage with ssl: %s REMOTE SSL_KEY SSL_CERT SSL_CA",
+                      argv[0]);
+        }
+        stream_ssl_set_ca_cert_file(argv[4], false);
+        stream_ssl_set_key_and_cert(argv[2], argv[3]);
     }
 
     error = stream_open_block(stream_open(argv[1], &stream, DSCP_DEFAULT),
diff --git a/tests/test-stream.py b/tests/test-stream.py
index 93d63c019..a6a9c18b2 100644
--- a/tests/test-stream.py
+++ b/tests/test-stream.py
@@ -15,10 +15,28 @@ 
 import sys
 
 import ovs.stream
+import ovs.util
 
 
 def main(argv):
+    if len(argv) < 2:
+        ovs.util.ovs_fatal(0,
+                           "usage: %s REMOTE [SSL_KEY] [SSL_CERT] [SSL_CA]",
+                           argv[0],
+                           )
     remote = argv[1]
+
+    if remote.startswith("ssl:"):
+        if len(argv) < 5:
+            ovs.util.ovs_fatal(
+                0,
+                "usage with ssl: %s REMOTE [SSL_KEY] [SSL_CERT] [SSL_CA]",
+                argv[0],
+            )
+        ovs.stream.SSLStream.ssl_set_ca_cert_file(argv[4])
+        ovs.stream.SSLStream.ssl_set_certificate_file(argv[3])
+        ovs.stream.SSLStream.ssl_set_private_key_file(argv[2])
+
     err, stream = ovs.stream.Stream.open_block(
             ovs.stream.Stream.open(remote), 10000)