On Thu, 2023-04-27 at 18:27 +0200, 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 <[email protected]>
Some questions from myself:
Are there other places, that should also get ssl or python checks and
should be included here? Maybe something (python) client related.
Besides that I add some comments to discuss my change below.
> ---
> tests/ovsdb-idl.at | 45 +++++++++++++++++++++++++++++++++++++++-----
> tests/test-stream.c | 12 +++++++++++-
> tests/test-stream.py | 6 ++++++
> 3 files changed, 57 insertions(+), 6 deletions(-)
>
> diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at
> index 5a7e76eaa..b40a167eb 100644
> --- a/tests/ovsdb-idl.at
> +++ b/tests/ovsdb-idl.at
> @@ -8,7 +8,15 @@ m4_divert_text([PREPARE_TESTS], [
> # specified).
> ovsdb_start_idltest () {
> ovsdb-tool create db ${2:-$abs_srcdir/idltest.ovsschema} || return $?
> - ovsdb-server -vconsole:warn --log-file --detach --no-chdir --pidfile
> --remote=punix:socket ${1:+--remote=$1} db || return $?
> + SSL_FLAGS=""
> + if [[ "${1::4}" == "pssl" ]]; then
> + SSL_FLAGS="--private-key=$PKIDIR/testpki-privkey2.pem \
> + --certificate=$PKIDIR/testpki-cert2.pem \
> + --ca-cert=$PKIDIR/testpki-cacert.pem"
> + fi
> + ovsdb-server -vconsole:warn --log-file --detach --no-chdir --pidfile \
> + $SSL_FLAGS --remote=punix:socket ${1:+--remote=$1} \
> + db || return $?
> on_exit 'kill `cat ovsdb-server.pid`'
> }
I like this approach more than OVSDB_CHECK_IDL_SSL_PY, where the server
is started at the m4_define section, still I could move the server
start also to CHECK_STREAM_OPEN_BLOCK directly.
>
> @@ -2279,14 +2287,35 @@ 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])
> - AT_CHECK([ovsdb_start_idltest "ptcp:0:$4"])
> + PROTOCOL=$3
> + PROTOCOL=${PROTOCOL::3}
> + LISTEN_PROTOCOL=p$PROTOCOL
> + PKIDIR=$abs_top_builddir/tests
> + AT_CHECK([ovsdb_start_idltest "$LISTEN_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])
> + AT_CHECK([$2 $PROTOCOL:$4:$TCP_PORT \
> + $PKIDIR/testpki-privkey.pem $PKIDIR/testpki-cert.pem \
> + $PKIDIR/testpki-cacert.pem],
> + [0], [ignore])
> + AT_CHECK([$2 $PROTOCOL:$4:$WRONG_PORT \
> + $PKIDIR/testpki-privkey.pem $PKIDIR/testpki-cert.pem \
> + $PKIDIR/testpki-cacert.pem],
> + [1], [ignore], [ignore])
> OVSDB_SERVER_SHUTDOWN
> - AT_CHECK([$2 tcp:$4:$TCP_PORT], [1], [ignore], [ignore])
> + AT_CHECK([$2 $PROTOCOL:$4:$TCP_PORT \
> + $PKIDIR/testpki-privkey.pem $PKIDIR/testpki-cert.pem \
> + $PKIDIR/testpki-cacert.pem],
> + [1], [ignore], [ignore])
> AT_CLEANUP])
>
> CHECK_STREAM_OPEN_BLOCK([C], [test-stream], [tcp], [127.0.0.1])
> @@ -2295,6 +2324,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]]])
Is it fine to add the tests this way or should I build a new
CHECK_STREAM_OPEN_BLOCK_SSL (and maybe CHECK_STREAM_OPEN_BLOCK_TCP)
section and merge it together, like OVSDB_CHECK_IDL_SSL_PY to
OVSDB_CHECK_IDL.
Upside would be, that we check if python ssl module is installed only
at ssl tests, but we would need to define all the checks and server
management twice.
>
> # 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..fb4af58b9 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: %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..73f26d0b9 100644
> --- a/tests/test-stream.py
> +++ b/tests/test-stream.py
> @@ -19,6 +19,12 @@ import ovs.stream
>
> def main(argv):
> remote = argv[1]
> +
> + if remote.startswith("ssl"):
> + 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)
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
