Re: [PATCH v9 12/26] tests: Enable crypto tests under msys2/mingw

["Daniel P. Berrangé" <berrange@redhat.com>]

On Tue, Sep 15, 2020 at 08:13:04PM +0800, Yonggang Luo wrote:
> Fixes following tests on msys2/mingw
> 'test-crypto-tlscredsx509'
> test-crypto-tlssession'
> 'test-io-channel-tls'
> 
> These tests are failure with:
> ERROR test-crypto-tlscredsx509 - missing test plan
> ERROR test-crypto-tlssession - missing test plan
> ERROR test-io-channel-tls - missing test plan
> 
> Because on win32 those test case are all disabled in the header
> 
> Add qemu_socket_pair for cross platform support
> Convert file system handling functions to glib
> Add qemu_link function instead posix only link function.
> Use send ad recv from qemu that convert Windows Socks error to errno properly.
> Use g_remove instead unlink
> Use g_mkdir instead mkdir
> 
> Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
> ---
>  tests/crypto-tls-psk-helpers.c   |   2 +-
>  tests/crypto-tls-x509-helpers.c  | 106 ++++++++++++++++++++++++++++++-
>  tests/crypto-tls-x509-helpers.h  |   9 ++-
>  tests/test-crypto-tlscredsx509.c |  47 +++++++-------
>  tests/test-crypto-tlssession.c   |  68 +++++++++++---------
>  tests/test-io-channel-tls.c      |  51 ++++++++-------
>  6 files changed, 204 insertions(+), 79 deletions(-)
> 
> diff --git a/tests/crypto-tls-psk-helpers.c b/tests/crypto-tls-psk-helpers.c
> index 4f3bd446ad..58888d5537 100644
> --- a/tests/crypto-tls-psk-helpers.c
> +++ b/tests/crypto-tls-psk-helpers.c
> @@ -46,7 +46,7 @@ void test_tls_psk_init(const char *pskfile)
>  
>  void test_tls_psk_cleanup(const char *pskfile)
>  {
> -    unlink(pskfile);
> +    g_remove(pskfile);
>  }
>  
>  #endif /* QCRYPTO_HAVE_TLS_TEST_SUPPORT */
> diff --git a/tests/crypto-tls-x509-helpers.c b/tests/crypto-tls-x509-helpers.c
> index 01b3daf358..1238983755 100644
> --- a/tests/crypto-tls-x509-helpers.c
> +++ b/tests/crypto-tls-x509-helpers.c
> @@ -23,6 +23,8 @@
>  #include "crypto-tls-x509-helpers.h"
>  #include "crypto/init.h"
>  #include "qemu/sockets.h"
> +#include <glib.h>
> +#include <glib/gstdio.h>
>  
>  #ifdef QCRYPTO_HAVE_TLS_TEST_SUPPORT
>  
> @@ -133,7 +135,7 @@ void test_tls_init(const char *keyfile)
>  void test_tls_cleanup(const char *keyfile)
>  {
>      asn1_delete_structure(&pkix_asn1);
> -    unlink(keyfile);
> +    g_remove(keyfile);
>  }
>  
>  /*
> @@ -501,8 +503,108 @@ void test_tls_discard_cert(QCryptoTLSTestCertReq *req)
>      req->crt = NULL;
>  
>      if (getenv("QEMU_TEST_DEBUG_CERTS") == NULL) {
> -        unlink(req->filename);
> +        g_remove(req->filename);
>      }
>  }
>  
> +int qemu_link(const char *exist_path1, const char *new_path2)
> +{
> +#if defined(_WIN32)
> +    g_autofree gchar *current_dir = g_get_current_dir();
> +    g_autofree gchar *full_path = g_build_filename(current_dir, exist_path1, NULL);
> +    return CreateSymbolicLinkA(
> +        new_path2, full_path, 0 | SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE) ? 0 : -1;
> +#else
> +    return link(exist_path1, new_path2);
> +#endif
> +}
> +
> +#if defined(_WIN32)
> +
> +int qemu_socketpair(int family, int type, int protocol, int channel[2])
> +{
> +    struct addrinfo addr_data;
> +    struct addrinfo *addr = NULL;
> +    int sock_listener = -1;
> +    int sock_client = -1;
> +    int sock_server = -1;
> +    int one = 1;
> +
> +    memset(&addr_data, 0, sizeof(addr_data));
> +    addr_data.ai_family = AF_INET;
> +    addr_data.ai_socktype = type;
> +    addr_data.ai_protocol = protocol;
> +    if (0 != getaddrinfo("127.0.0.1", "0", &addr_data, &addr)) {


This test is backwards - QEMU style is "getaddrinfo(...) != 0"


> +        goto error;
> +    }
> +
> +    if (NULL == addr) {
> +        goto error;
> +    }
> +
> +    sock_listener = socket(addr->ai_family, addr->ai_socktype, addr->ai_protocol);
> +    if (-1 == sock_listener) {
> +        goto error;
> +    }

Normal code style is  "sock_listener < 0"

> +
> +    if (-1 == setsockopt(sock_listener, SOL_SOCKET, SO_REUSEADDR, (const char *)&one, sizeof(one))) {
> +        goto error;
> +    }

SO_REUSEADDR is not desirable on Windows -  see os-win32.c comments about
it.

> +    if (-1 == bind(sock_listener, addr->ai_addr, addr->ai_addrlen)) {
> +        goto error;
> +    }
> +    if (-1 == getsockname(sock_listener, addr->ai_addr, (int *)&(addr->ai_addrlen))) {
> +        goto error;
> +    }
> +    if (-1 == listen(sock_listener, 1)) {
> +        goto error;
> +    }
> +
> +    sock_client = socket(addr->ai_family, addr->ai_socktype, addr->ai_protocol);
> +
> +    if (-1 == sock_client) {
> +        goto error;
> +    }
> +
> +    if (-1 == connect(sock_client, addr->ai_addr, addr->ai_addrlen)) {
> +        goto error;
> +    }
> +
> +    sock_server = accept(sock_listener, 0, 0);
> +
> +    if (-1 == sock_server) {
> +        goto error;
> +    }
> +
> +    closesocket(sock_listener);
> +
> +    channel[0] = sock_client;
> +    channel[1] = sock_server;
> +    return 0;
> +
> +error:
> +    if (-1 != sock_server) {
> +        closesocket(sock_server);
> +    }
> +    if (-1 != sock_client) {
> +        closesocket(sock_client);
> +    }
> +    if (-1 != sock_listener) {
> +        closesocket(sock_listener);
> +    }
> +    if (NULL != addr) {
> +        freeaddrinfo(addr);
> +    }
> +    return -1;
> +}
> +
> +#else
> +
> +int qemu_socketpair(int family, int type, int protocol, int recv[2])
> +{
> +    return socketpair(family, type, protocol, recv);
> +}
> +
> +#endif
> +
>  #endif /* QCRYPTO_HAVE_TLS_TEST_SUPPORT */
> diff --git a/tests/crypto-tls-x509-helpers.h b/tests/crypto-tls-x509-helpers.h
> index 08efba4e19..6902d43af7 100644
> --- a/tests/crypto-tls-x509-helpers.h
> +++ b/tests/crypto-tls-x509-helpers.h
> @@ -24,8 +24,9 @@
>  #include <gnutls/gnutls.h>
>  #include <gnutls/x509.h>
>  
> -#if !(defined WIN32) && \
> -    defined(CONFIG_TASN1)
> +#include "qemu/osdep.h"
> +
> +#if defined(CONFIG_TASN1)
>  # define QCRYPTO_HAVE_TLS_TEST_SUPPORT
>  #endif
>  
> @@ -127,6 +128,10 @@ void test_tls_cleanup(const char *keyfile);
>  
>  extern const ASN1_ARRAY_TYPE pkix_asn1_tab[];
>  
> +int qemu_link(const char *exist_path1, const char *new_path2);
> +
> +int qemu_socketpair(int family, int type, int protocol, int channel[2]);
> +

THese aren't really related t the TLS tests - they're platform
portability wrappers, so osdep.{ch} and qemu-sockets.c is where
they ought to live

>  #endif /* QCRYPTO_HAVE_TLS_TEST_SUPPORT */
>  
>  #endif


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|