* [PATCH v3 0/9] tests: introduce testing coverage for TLS with migration
@ 2022-04-26 16:00 Daniel P. Berrangé
2022-04-26 16:00 ` [PATCH v3 1/9] tests: fix encoding of IP addresses in x509 certs Daniel P. Berrangé
` (10 more replies)
0 siblings, 11 replies; 19+ messages in thread
From: Daniel P. Berrangé @ 2022-04-26 16:00 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Thomas Huth, Daniel P. Berrangé,
Juan Quintela, Dr. David Alan Gilbert, Peter Xu, Paolo Bonzini
This significantly expands the migration test suite to cover testing
with TLS over TCP and UNIX sockets, with both PSK (pre shared keys)
and x509 credentials, and for both single and multifd scenarios.
It identified one bug in handling PSK credentials with UNIX sockets,
but other than that everything was operating as expected.
To minimize the impact on code duplication alopt of refactoring is
done of the migration tests to introduce a common helper for running
the migration process. The various tests mostly just have to provide
a callback to set a few parameters/capabilities before migration
starts, and sometimes a callback to cleanup or validate after
completion/failure.
Changed in v3:
- Trivial rebase dropping already merged patches
Changed in v2:
- Use structs to pass around most parameters
- Hide expected errors from stderr
Daniel P. Berrangé (9):
tests: fix encoding of IP addresses in x509 certs
tests: add more helper macros for creating TLS x509 certs
tests: add migration tests of TLS with PSK credentials
tests: add migration tests of TLS with x509 credentials
tests: convert XBZRLE migration test to use common helper
tests: convert multifd migration tests to use common helper
tests: add multifd migration tests of TLS with PSK credentials
tests: add multifd migration tests of TLS with x509 credentials
tests: ensure migration status isn't reported as failed
meson.build | 1 +
tests/qtest/meson.build | 12 +-
tests/qtest/migration-helpers.c | 13 +
tests/qtest/migration-helpers.h | 1 +
tests/qtest/migration-test.c | 866 ++++++++++++++++++++++++---
tests/unit/crypto-tls-psk-helpers.c | 18 +-
tests/unit/crypto-tls-psk-helpers.h | 1 +
tests/unit/crypto-tls-x509-helpers.c | 16 +-
tests/unit/crypto-tls-x509-helpers.h | 53 ++
tests/unit/test-crypto-tlssession.c | 11 +-
10 files changed, 897 insertions(+), 95 deletions(-)
--
2.35.1
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v3 1/9] tests: fix encoding of IP addresses in x509 certs
2022-04-26 16:00 [PATCH v3 0/9] tests: introduce testing coverage for TLS with migration Daniel P. Berrangé
@ 2022-04-26 16:00 ` Daniel P. Berrangé
2022-04-28 9:46 ` Dr. David Alan Gilbert
2022-04-26 16:00 ` [PATCH v3 2/9] tests: add more helper macros for creating TLS " Daniel P. Berrangé
` (9 subsequent siblings)
10 siblings, 1 reply; 19+ messages in thread
From: Daniel P. Berrangé @ 2022-04-26 16:00 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Thomas Huth, Daniel P. Berrangé,
Juan Quintela, Dr. David Alan Gilbert, Peter Xu, Paolo Bonzini
We need to encode just the address bytes, not the whole struct sockaddr
data. Add a test case to validate that we're matching on SAN IP
addresses correctly.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
tests/unit/crypto-tls-x509-helpers.c | 16 +++++++++++++---
tests/unit/test-crypto-tlssession.c | 11 +++++++++--
2 files changed, 22 insertions(+), 5 deletions(-)
diff --git a/tests/unit/crypto-tls-x509-helpers.c b/tests/unit/crypto-tls-x509-helpers.c
index fc609b3fd4..e9937f60d8 100644
--- a/tests/unit/crypto-tls-x509-helpers.c
+++ b/tests/unit/crypto-tls-x509-helpers.c
@@ -168,9 +168,19 @@ test_tls_get_ipaddr(const char *addrstr,
hints.ai_flags = AI_NUMERICHOST;
g_assert(getaddrinfo(addrstr, NULL, &hints, &res) == 0);
- *datalen = res->ai_addrlen;
- *data = g_new(char, *datalen);
- memcpy(*data, res->ai_addr, *datalen);
+ if (res->ai_family == AF_INET) {
+ struct sockaddr_in *in = (struct sockaddr_in *)res->ai_addr;
+ *datalen = sizeof(in->sin_addr);
+ *data = g_new(char, *datalen);
+ memcpy(*data, &in->sin_addr, *datalen);
+ } else if (res->ai_family == AF_INET6) {
+ struct sockaddr_in6 *in = (struct sockaddr_in6 *)res->ai_addr;
+ *datalen = sizeof(in->sin6_addr);
+ *data = g_new(char, *datalen);
+ memcpy(*data, &in->sin6_addr, *datalen);
+ } else {
+ g_assert_not_reached();
+ }
freeaddrinfo(res);
}
diff --git a/tests/unit/test-crypto-tlssession.c b/tests/unit/test-crypto-tlssession.c
index 5f0da9192c..a6935d8497 100644
--- a/tests/unit/test-crypto-tlssession.c
+++ b/tests/unit/test-crypto-tlssession.c
@@ -512,12 +512,19 @@ int main(int argc, char **argv)
false, true, "wiki.qemu.org", NULL);
TEST_SESS_REG(altname4, cacertreq.filename,
+ servercertalt1req.filename, clientcertreq.filename,
+ false, false, "192.168.122.1", NULL);
+ TEST_SESS_REG(altname5, cacertreq.filename,
+ servercertalt1req.filename, clientcertreq.filename,
+ false, false, "fec0::dead:beaf", NULL);
+
+ TEST_SESS_REG(altname6, cacertreq.filename,
servercertalt2req.filename, clientcertreq.filename,
false, true, "qemu.org", NULL);
- TEST_SESS_REG(altname5, cacertreq.filename,
+ TEST_SESS_REG(altname7, cacertreq.filename,
servercertalt2req.filename, clientcertreq.filename,
false, false, "www.qemu.org", NULL);
- TEST_SESS_REG(altname6, cacertreq.filename,
+ TEST_SESS_REG(altname8, cacertreq.filename,
servercertalt2req.filename, clientcertreq.filename,
false, false, "wiki.qemu.org", NULL);
--
2.35.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v3 2/9] tests: add more helper macros for creating TLS x509 certs
2022-04-26 16:00 [PATCH v3 0/9] tests: introduce testing coverage for TLS with migration Daniel P. Berrangé
2022-04-26 16:00 ` [PATCH v3 1/9] tests: fix encoding of IP addresses in x509 certs Daniel P. Berrangé
@ 2022-04-26 16:00 ` Daniel P. Berrangé
2022-04-28 13:09 ` Eric Blake
2022-04-26 16:00 ` [PATCH v3 3/9] tests: add migration tests of TLS with PSK credentials Daniel P. Berrangé
` (8 subsequent siblings)
10 siblings, 1 reply; 19+ messages in thread
From: Daniel P. Berrangé @ 2022-04-26 16:00 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Thomas Huth, Daniel P. Berrangé,
Juan Quintela, Dr. David Alan Gilbert, Peter Xu, Paolo Bonzini
These macros are more suited to the general consumers of certs in the
test suite, where we don't need to exercise every single possible
permutation.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
tests/unit/crypto-tls-x509-helpers.h | 53 ++++++++++++++++++++++++++++
1 file changed, 53 insertions(+)
diff --git a/tests/unit/crypto-tls-x509-helpers.h b/tests/unit/crypto-tls-x509-helpers.h
index cf6329e653..247e7160eb 100644
--- a/tests/unit/crypto-tls-x509-helpers.h
+++ b/tests/unit/crypto-tls-x509-helpers.h
@@ -26,6 +26,9 @@
#include <libtasn1.h>
+#define QCRYPTO_TLS_TEST_CLIENT_NAME "ACME QEMU Client"
+#define QCRYPTO_TLS_TEST_CLIENT_HOSTILE_NAME "ACME Hostile Client"
+
/*
* This contains parameter about how to generate
* certificates.
@@ -118,6 +121,56 @@ void test_tls_cleanup(const char *keyfile);
}; \
test_tls_generate_cert(&varname, NULL)
+# define TLS_ROOT_REQ_SIMPLE(varname, fname) \
+ QCryptoTLSTestCertReq varname = { \
+ .filename = fname, \
+ .cn = "qemu-CA", \
+ .basicConstraintsEnable = true, \
+ .basicConstraintsCritical = true, \
+ .basicConstraintsIsCA = true, \
+ .keyUsageEnable = true, \
+ .keyUsageCritical = true, \
+ .keyUsageValue = GNUTLS_KEY_KEY_CERT_SIGN, \
+ }; \
+ test_tls_generate_cert(&varname, NULL)
+
+# define TLS_CERT_REQ_SIMPLE_CLIENT(varname, cavarname, cname, fname) \
+ QCryptoTLSTestCertReq varname = { \
+ .filename = fname, \
+ .cn = cname, \
+ .basicConstraintsEnable = true, \
+ .basicConstraintsCritical = true, \
+ .basicConstraintsIsCA = false, \
+ .keyUsageEnable = true, \
+ .keyUsageCritical = true, \
+ .keyUsageValue = \
+ GNUTLS_KEY_DIGITAL_SIGNATURE | GNUTLS_KEY_KEY_ENCIPHERMENT, \
+ .keyPurposeEnable = true, \
+ .keyPurposeCritical = true, \
+ .keyPurposeOID1 = GNUTLS_KP_TLS_WWW_CLIENT, \
+ }; \
+ test_tls_generate_cert(&varname, cavarname.crt)
+
+# define TLS_CERT_REQ_SIMPLE_SERVER(varname, cavarname, fname, \
+ hostname, ipaddr) \
+ QCryptoTLSTestCertReq varname = { \
+ .filename = fname, \
+ .cn = hostname ? hostname : ipaddr, \
+ .altname1 = hostname, \
+ .ipaddr1 = ipaddr, \
+ .basicConstraintsEnable = true, \
+ .basicConstraintsCritical = true, \
+ .basicConstraintsIsCA = false, \
+ .keyUsageEnable = true, \
+ .keyUsageCritical = true, \
+ .keyUsageValue = \
+ GNUTLS_KEY_DIGITAL_SIGNATURE | GNUTLS_KEY_KEY_ENCIPHERMENT, \
+ .keyPurposeEnable = true, \
+ .keyPurposeCritical = true, \
+ .keyPurposeOID1 = GNUTLS_KP_TLS_WWW_SERVER, \
+ }; \
+ test_tls_generate_cert(&varname, cavarname.crt)
+
extern const asn1_static_node pkix_asn1_tab[];
#endif
--
2.35.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v3 3/9] tests: add migration tests of TLS with PSK credentials
2022-04-26 16:00 [PATCH v3 0/9] tests: introduce testing coverage for TLS with migration Daniel P. Berrangé
2022-04-26 16:00 ` [PATCH v3 1/9] tests: fix encoding of IP addresses in x509 certs Daniel P. Berrangé
2022-04-26 16:00 ` [PATCH v3 2/9] tests: add more helper macros for creating TLS " Daniel P. Berrangé
@ 2022-04-26 16:00 ` Daniel P. Berrangé
2022-04-28 13:46 ` Eric Blake
2022-04-26 16:00 ` [PATCH v3 4/9] tests: add migration tests of TLS with x509 credentials Daniel P. Berrangé
` (7 subsequent siblings)
10 siblings, 1 reply; 19+ messages in thread
From: Daniel P. Berrangé @ 2022-04-26 16:00 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Thomas Huth, Daniel P. Berrangé,
Juan Quintela, Dr. David Alan Gilbert, Peter Xu, Paolo Bonzini
This validates that we correctly handle migration success and failure
scenarios when using TLS with pre shared keys.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
tests/qtest/meson.build | 7 +-
tests/qtest/migration-test.c | 159 +++++++++++++++++++++++++++-
tests/unit/crypto-tls-psk-helpers.c | 18 +++-
tests/unit/crypto-tls-psk-helpers.h | 1 +
4 files changed, 177 insertions(+), 8 deletions(-)
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 22e1361210..ec14559e73 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -271,13 +271,18 @@ endif
tpmemu_files = ['tpm-emu.c', 'tpm-util.c', 'tpm-tests.c']
+migration_files = [files('migration-helpers.c')]
+if gnutls.found()
+ migration_files += [files('../unit/crypto-tls-psk-helpers.c'), gnutls]
+endif
+
qtests = {
'bios-tables-test': [io, 'boot-sector.c', 'acpi-utils.c', 'tpm-emu.c'],
'cdrom-test': files('boot-sector.c'),
'dbus-vmstate-test': files('migration-helpers.c') + dbus_vmstate1,
'erst-test': files('erst-test.c'),
'ivshmem-test': [rt, '../../contrib/ivshmem-server/ivshmem-server.c'],
- 'migration-test': files('migration-helpers.c'),
+ 'migration-test': migration_files,
'pxe-test': files('boot-sector.c'),
'qos-test': [chardev, io, qos_test_ss.apply(config_host, strict: false).sources()],
'tpm-crb-swtpm-test': [io, tpmemu_files],
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 2af36c16a3..f733aa352e 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -23,9 +23,13 @@
#include "qapi/qapi-visit-sockets.h"
#include "qapi/qobject-input-visitor.h"
#include "qapi/qobject-output-visitor.h"
+#include "crypto/tlscredspsk.h"
#include "migration-helpers.h"
#include "tests/migration/migration-test.h"
+#ifdef CONFIG_GNUTLS
+# include "tests/unit/crypto-tls-psk-helpers.h"
+#endif /* CONFIG_GNUTLS */
/* For dirty ring test; so far only x86_64 is supported */
#if defined(__linux__) && defined(HOST_X86_64)
@@ -640,6 +644,100 @@ static void test_migrate_end(QTestState *from, QTestState *to, bool test_dest)
cleanup("dest_serial");
}
+#ifdef CONFIG_GNUTLS
+struct TestMigrateTLSPSKData {
+ char *workdir;
+ char *workdiralt;
+ char *pskfile;
+ char *pskfilealt;
+};
+
+static void *
+test_migrate_tls_psk_start_common(QTestState *from,
+ QTestState *to,
+ bool mismatch)
+{
+ struct TestMigrateTLSPSKData *data =
+ g_new0(struct TestMigrateTLSPSKData, 1);
+ QDict *rsp;
+
+ data->workdir = g_strdup_printf("%s/tlscredspsk0", tmpfs);
+ data->pskfile = g_strdup_printf("%s/%s", data->workdir,
+ QCRYPTO_TLS_CREDS_PSKFILE);
+ mkdir(data->workdir, 0700);
+ test_tls_psk_init(data->pskfile);
+
+ if (mismatch) {
+ data->workdiralt = g_strdup_printf("%s/tlscredspskalt0", tmpfs);
+ data->pskfilealt = g_strdup_printf("%s/%s", data->workdiralt,
+ QCRYPTO_TLS_CREDS_PSKFILE);
+ mkdir(data->workdiralt, 0700);
+ test_tls_psk_init_alt(data->pskfilealt);
+ }
+
+ rsp = wait_command(from,
+ "{ 'execute': 'object-add',"
+ " 'arguments': { 'qom-type': 'tls-creds-psk',"
+ " 'id': 'tlscredspsk0',"
+ " 'endpoint': 'client',"
+ " 'dir': %s,"
+ " 'username': 'qemu'} }",
+ data->workdir);
+ qobject_unref(rsp);
+
+ rsp = wait_command(to,
+ "{ 'execute': 'object-add',"
+ " 'arguments': { 'qom-type': 'tls-creds-psk',"
+ " 'id': 'tlscredspsk0',"
+ " 'endpoint': 'server',"
+ " 'dir': %s } }",
+ mismatch ? data->workdiralt : data->workdir);
+ qobject_unref(rsp);
+
+ migrate_set_parameter_str(from, "tls-creds", "tlscredspsk0");
+ migrate_set_parameter_str(to, "tls-creds", "tlscredspsk0");
+
+ return data;
+}
+
+static void *
+test_migrate_tls_psk_start_match(QTestState *from,
+ QTestState *to)
+{
+ return test_migrate_tls_psk_start_common(from, to, false);
+}
+
+static void *
+test_migrate_tls_psk_start_mismatch(QTestState *from,
+ QTestState *to)
+{
+ return test_migrate_tls_psk_start_common(from, to, true);
+}
+
+static void
+test_migrate_tls_psk_finish(QTestState *from,
+ QTestState *to,
+ void *opaque)
+{
+ struct TestMigrateTLSPSKData *data = opaque;
+
+ test_tls_psk_cleanup(data->pskfile);
+ if (data->pskfilealt) {
+ test_tls_psk_cleanup(data->pskfilealt);
+ }
+ rmdir(data->workdir);
+ if (data->workdiralt) {
+ rmdir(data->workdiralt);
+ }
+
+ g_free(data->workdiralt);
+ g_free(data->pskfilealt);
+ g_free(data->workdir);
+ g_free(data->pskfile);
+ g_free(data);
+}
+#endif /* CONFIG_GNUTLS */
+
static int migrate_postcopy_prepare(QTestState **from_ptr,
QTestState **to_ptr,
MigrateStart *args)
@@ -911,7 +1009,7 @@ static void test_precopy_common(MigrateCommon *args)
test_migrate_end(from, to, args->result == MIG_TEST_SUCCEED);
}
-static void test_precopy_unix(void)
+static void test_precopy_unix_plain(void)
{
g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
MigrateCommon args = {
@@ -922,6 +1020,19 @@ static void test_precopy_unix(void)
test_precopy_common(&args);
}
+static void test_precopy_unix_tls_psk(void)
+{
+ g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
+ MigrateCommon args = {
+ .connect_uri = uri,
+ .listen_uri = uri,
+ .start_hook = test_migrate_tls_psk_start_match,
+ .finish_hook = test_migrate_tls_psk_finish,
+ };
+
+ test_precopy_common(&args);
+}
+
static void test_precopy_unix_dirty_ring(void)
{
g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
@@ -1026,7 +1137,7 @@ static void test_xbzrle_unix(void)
test_xbzrle(uri);
}
-static void test_precopy_tcp(void)
+static void test_precopy_tcp_plain(void)
{
MigrateCommon args = {
.listen_uri = "tcp:127.0.0.1:0",
@@ -1035,6 +1146,34 @@ static void test_precopy_tcp(void)
test_precopy_common(&args);
}
+#ifdef CONFIG_GNUTLS
+static void test_precopy_tcp_tls_psk_match(void)
+{
+ MigrateCommon args = {
+ .listen_uri = "tcp:127.0.0.1:0",
+ .start_hook = test_migrate_tls_psk_start_match,
+ .finish_hook = test_migrate_tls_psk_finish,
+ };
+
+ test_precopy_common(&args);
+}
+
+static void test_precopy_tcp_tls_psk_mismatch(void)
+{
+ MigrateCommon args = {
+ .start = {
+ .hide_stderr = true,
+ },
+ .listen_uri = "tcp:127.0.0.1:0",
+ .start_hook = test_migrate_tls_psk_start_mismatch,
+ .finish_hook = test_migrate_tls_psk_finish,
+ .result = MIG_TEST_FAIL,
+ };
+
+ test_precopy_common(&args);
+}
+#endif /* CONFIG_GNUTLS */
+
static void *test_migrate_fd_start_hook(QTestState *from,
QTestState *to)
{
@@ -1497,8 +1636,20 @@ int main(int argc, char **argv)
qtest_add_func("/migration/postcopy/unix", test_postcopy);
qtest_add_func("/migration/postcopy/recovery", test_postcopy_recovery);
qtest_add_func("/migration/bad_dest", test_baddest);
- qtest_add_func("/migration/precopy/unix", test_precopy_unix);
- qtest_add_func("/migration/precopy/tcp", test_precopy_tcp);
+ qtest_add_func("/migration/precopy/unix/plain", test_precopy_unix_plain);
+#ifdef CONFIG_GNUTLS
+ qtest_add_func("/migration/precopy/unix/tls/psk",
+ test_precopy_unix_tls_psk);
+#endif /* CONFIG_GNUTLS */
+
+ qtest_add_func("/migration/precopy/tcp/plain", test_precopy_tcp_plain);
+#ifdef CONFIG_GNUTLS
+ qtest_add_func("/migration/precopy/tcp/tls/psk/match",
+ test_precopy_tcp_tls_psk_match);
+ qtest_add_func("/migration/precopy/tcp/tls/psk/mismatch",
+ test_precopy_tcp_tls_psk_mismatch);
+#endif /* CONFIG_GNUTLS */
+
/* qtest_add_func("/migration/ignore_shared", test_ignore_shared); */
qtest_add_func("/migration/xbzrle/unix", test_xbzrle_unix);
qtest_add_func("/migration/fd_proto", test_migrate_fd_proto);
diff --git a/tests/unit/crypto-tls-psk-helpers.c b/tests/unit/crypto-tls-psk-helpers.c
index 4bea7c6fa2..511e08cc9c 100644
--- a/tests/unit/crypto-tls-psk-helpers.c
+++ b/tests/unit/crypto-tls-psk-helpers.c
@@ -24,7 +24,8 @@
#include "crypto-tls-psk-helpers.h"
#include "qemu/sockets.h"
-void test_tls_psk_init(const char *pskfile)
+static void
+test_tls_psk_init_common(const char *pskfile, const char *user, const char *key)
{
FILE *fp;
@@ -33,11 +34,22 @@ void test_tls_psk_init(const char *pskfile)
g_critical("Failed to create pskfile %s: %s", pskfile, strerror(errno));
abort();
}
- /* Don't do this in real applications! Use psktool. */
- fprintf(fp, "qemu:009d5638c40fde0c\n");
+ fprintf(fp, "%s:%s\n", user, key);
fclose(fp);
}
+void test_tls_psk_init(const char *pskfile)
+{
+ /* Don't hard code a key like this in real applications! Use psktool. */
+ test_tls_psk_init_common(pskfile, "qemu", "009d5638c40fde0c");
+}
+
+void test_tls_psk_init_alt(const char *pskfile)
+{
+ /* Don't hard code a key like this in real applications! Use psktool. */
+ test_tls_psk_init_common(pskfile, "qemu", "10ffa6a2c42f0388");
+}
+
void test_tls_psk_cleanup(const char *pskfile)
{
unlink(pskfile);
diff --git a/tests/unit/crypto-tls-psk-helpers.h b/tests/unit/crypto-tls-psk-helpers.h
index faa645c629..67f8bdda71 100644
--- a/tests/unit/crypto-tls-psk-helpers.h
+++ b/tests/unit/crypto-tls-psk-helpers.h
@@ -24,6 +24,7 @@
#include <gnutls/gnutls.h>
void test_tls_psk_init(const char *keyfile);
+void test_tls_psk_init_alt(const char *keyfile);
void test_tls_psk_cleanup(const char *keyfile);
#endif
--
2.35.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v3 4/9] tests: add migration tests of TLS with x509 credentials
2022-04-26 16:00 [PATCH v3 0/9] tests: introduce testing coverage for TLS with migration Daniel P. Berrangé
` (2 preceding siblings ...)
2022-04-26 16:00 ` [PATCH v3 3/9] tests: add migration tests of TLS with PSK credentials Daniel P. Berrangé
@ 2022-04-26 16:00 ` Daniel P. Berrangé
2022-04-28 13:59 ` Eric Blake
2022-04-26 16:00 ` [PATCH v3 5/9] tests: convert XBZRLE migration test to use common helper Daniel P. Berrangé
` (6 subsequent siblings)
10 siblings, 1 reply; 19+ messages in thread
From: Daniel P. Berrangé @ 2022-04-26 16:00 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Thomas Huth, Daniel P. Berrangé,
Juan Quintela, Dr. David Alan Gilbert, Peter Xu, Paolo Bonzini
This validates that we correctly handle migration success and failure
scenarios when using TLS with x509 certificates. There are quite a few
different scenarios that matter in relation to hostname validation.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
meson.build | 1 +
tests/qtest/meson.build | 5 +
tests/qtest/migration-test.c | 382 ++++++++++++++++++++++++++++++++++-
3 files changed, 386 insertions(+), 2 deletions(-)
diff --git a/meson.build b/meson.build
index d083c6b7bf..d1231b23ae 100644
--- a/meson.build
+++ b/meson.build
@@ -1565,6 +1565,7 @@ config_host_data.set('CONFIG_KEYUTILS', keyutils.found())
config_host_data.set('CONFIG_GETTID', has_gettid)
config_host_data.set('CONFIG_GNUTLS', gnutls.found())
config_host_data.set('CONFIG_GNUTLS_CRYPTO', gnutls_crypto.found())
+config_host_data.set('CONFIG_TASN1', tasn1.found())
config_host_data.set('CONFIG_GCRYPT', gcrypt.found())
config_host_data.set('CONFIG_NETTLE', nettle.found())
config_host_data.set('CONFIG_QEMU_PRIVATE_XTS', xts == 'private')
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index ec14559e73..af7c31d611 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -274,6 +274,11 @@ tpmemu_files = ['tpm-emu.c', 'tpm-util.c', 'tpm-tests.c']
migration_files = [files('migration-helpers.c')]
if gnutls.found()
migration_files += [files('../unit/crypto-tls-psk-helpers.c'), gnutls]
+
+ if tasn1.found()
+ migration_files += [files('../unit/crypto-tls-x509-helpers.c',
+ '../unit/pkix_asn1_tab.c'), tasn1]
+ endif
endif
qtests = {
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index f733aa352e..c730697f74 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -29,6 +29,9 @@
#include "tests/migration/migration-test.h"
#ifdef CONFIG_GNUTLS
# include "tests/unit/crypto-tls-psk-helpers.h"
+# ifdef CONFIG_TASN1
+# include "tests/unit/crypto-tls-x509-helpers.h"
+# endif /* CONFIG_TASN1 */
#endif /* CONFIG_GNUTLS */
/* For dirty ring test; so far only x86_64 is supported */
@@ -736,6 +739,234 @@ test_migrate_tls_psk_finish(QTestState *from,
g_free(data->pskfile);
g_free(data);
}
+
+#ifdef CONFIG_TASN1
+typedef struct {
+ char *workdir;
+ char *keyfile;
+ char *cacert;
+ char *servercert;
+ char *serverkey;
+ char *clientcert;
+ char *clientkey;
+} TestMigrateTLSX509Data;
+
+typedef struct {
+ bool verifyclient;
+ bool clientcert;
+ bool hostileclient;
+ bool authzclient;
+ const char *certhostname;
+ const char *certipaddr;
+} TestMigrateTLSX509;
+
+static void *
+test_migrate_tls_x509_start_common(QTestState *from,
+ QTestState *to,
+ TestMigrateTLSX509 *args)
+{
+ TestMigrateTLSX509Data *data = g_new0(TestMigrateTLSX509Data, 1);
+ QDict *rsp;
+
+ data->workdir = g_strdup_printf("%s/tlscredsx5090", tmpfs);
+ data->keyfile = g_strdup_printf("%s/key.pem", data->workdir);
+
+ data->cacert = g_strdup_printf("%s/ca-cert.pem", data->workdir);
+ data->serverkey = g_strdup_printf("%s/server-key.pem", data->workdir);
+ data->servercert = g_strdup_printf("%s/server-cert.pem", data->workdir);
+ if (args->clientcert) {
+ data->clientkey = g_strdup_printf("%s/client-key.pem", data->workdir);
+ data->clientcert = g_strdup_printf("%s/client-cert.pem", data->workdir);
+ }
+
+ mkdir(data->workdir, 0700);
+
+ test_tls_init(data->keyfile);
+ g_assert(link(data->keyfile, data->serverkey) == 0);
+ if (args->clientcert) {
+ g_assert(link(data->keyfile, data->clientkey) == 0);
+ }
+
+ TLS_ROOT_REQ_SIMPLE(cacertreq, data->cacert);
+ if (args->clientcert) {
+ TLS_CERT_REQ_SIMPLE_CLIENT(servercertreq, cacertreq,
+ args->hostileclient ?
+ QCRYPTO_TLS_TEST_CLIENT_HOSTILE_NAME :
+ QCRYPTO_TLS_TEST_CLIENT_NAME,
+ data->clientcert);
+ }
+
+ TLS_CERT_REQ_SIMPLE_SERVER(clientcertreq, cacertreq,
+ data->servercert,
+ args->certhostname,
+ args->certipaddr);
+
+ rsp = wait_command(from,
+ "{ 'execute': 'object-add',"
+ " 'arguments': { 'qom-type': 'tls-creds-x509',"
+ " 'id': 'tlscredsx509client0',"
+ " 'endpoint': 'client',"
+ " 'dir': %s,"
+ " 'sanity-check': true,"
+ " 'verify-peer': true} }",
+ data->workdir);
+ qobject_unref(rsp);
+ migrate_set_parameter_str(from, "tls-creds", "tlscredsx509client0");
+ if (args->certhostname) {
+ migrate_set_parameter_str(from, "tls-hostname", args->certhostname);
+ }
+
+ rsp = wait_command(to,
+ "{ 'execute': 'object-add',"
+ " 'arguments': { 'qom-type': 'tls-creds-x509',"
+ " 'id': 'tlscredsx509server0',"
+ " 'endpoint': 'server',"
+ " 'dir': %s,"
+ " 'sanity-check': true,"
+ " 'verify-peer': %i} }",
+ data->workdir, args->verifyclient);
+ qobject_unref(rsp);
+ migrate_set_parameter_str(to, "tls-creds", "tlscredsx509server0");
+
+ if (args->authzclient) {
+ rsp = wait_command(to,
+ "{ 'execute': 'object-add',"
+ " 'arguments': { 'qom-type': 'authz-simple',"
+ " 'id': 'tlsauthz0',"
+ " 'identity': %s} }",
+ "CN=" QCRYPTO_TLS_TEST_CLIENT_NAME);
+ migrate_set_parameter_str(to, "tls-authz", "tlsauthz0");
+ }
+
+ return data;
+}
+
+/*
+ * The normal case: match server's cert hostname against
+ * whatever host we were telling QEMU to connect to (if any)
+ */
+static void *
+test_migrate_tls_x509_start_default_host(QTestState *from,
+ QTestState *to)
+{
+ TestMigrateTLSX509 args = {
+ .verifyclient = true,
+ .clientcert = true,
+ .certipaddr = "127.0.0.1"
+ };
+ return test_migrate_tls_x509_start_common(from, to, &args);
+}
+
+/*
+ * The unusual case: the server's cert is different from
+ * the address we're telling QEMU to connect to (if any),
+ * so we must give QEMU an explicit hostname to validate
+ */
+static void *
+test_migrate_tls_x509_start_override_host(QTestState *from,
+ QTestState *to)
+{
+ TestMigrateTLSX509 args = {
+ .verifyclient = true,
+ .clientcert = true,
+ .certhostname = "qemu.org",
+ };
+ return test_migrate_tls_x509_start_common(from, to, &args);
+}
+
+/*
+ * The unusual case: the server's cert is different from
+ * the address we're telling QEMU to connect to, and so we
+ * expect the client to reject the server
+ */
+static void *
+test_migrate_tls_x509_start_mismatch_host(QTestState *from,
+ QTestState *to)
+{
+ TestMigrateTLSX509 args = {
+ .verifyclient = true,
+ .clientcert = true,
+ .certipaddr = "10.0.0.1",
+ };
+ return test_migrate_tls_x509_start_common(from, to, &args);
+}
+
+static void *
+test_migrate_tls_x509_start_friendly_client(QTestState *from,
+ QTestState *to)
+{
+ TestMigrateTLSX509 args = {
+ .verifyclient = true,
+ .clientcert = true,
+ .authzclient = true,
+ .certipaddr = "127.0.0.1",
+ };
+ return test_migrate_tls_x509_start_common(from, to, &args);
+}
+
+static void *
+test_migrate_tls_x509_start_hostile_client(QTestState *from,
+ QTestState *to)
+{
+ TestMigrateTLSX509 args = {
+ .verifyclient = true,
+ .clientcert = true,
+ .hostileclient = true,
+ .authzclient = true,
+ .certipaddr = "127.0.0.1",
+ };
+ return test_migrate_tls_x509_start_common(from, to, &args);
+}
+
+/*
+ * The case with no client certificate presented,
+ * and no server verification
+ */
+static void *
+test_migrate_tls_x509_start_allow_anon_client(QTestState *from,
+ QTestState *to)
+{
+ TestMigrateTLSX509 args = {
+ .certipaddr = "127.0.0.1",
+ };
+ return test_migrate_tls_x509_start_common(from, to, &args);
+}
+
+/*
+ * The case with no client certificate presented,
+ * and server verification rejecting
+ */
+static void *
+test_migrate_tls_x509_start_reject_anon_client(QTestState *from,
+ QTestState *to)
+{
+ TestMigrateTLSX509 args = {
+ .verifyclient = true,
+ .certipaddr = "127.0.0.1",
+ };
+ return test_migrate_tls_x509_start_common(from, to, &args);
+}
+
+static void
+test_migrate_tls_x509_finish(QTestState *from,
+ QTestState *to,
+ void *opaque)
+{
+ TestMigrateTLSX509Data *data = opaque;
+
+ test_tls_cleanup(data->keyfile);
+ unlink(data->cacert);
+ unlink(data->servercert);
+ unlink(data->serverkey);
+ unlink(data->clientcert);
+ unlink(data->clientkey);
+ rmdir(data->workdir);
+
+ g_free(data->workdir);
+ g_free(data->keyfile);
+ g_free(data);
+}
+#endif /* CONFIG_TASN1 */
#endif /* CONFIG_GNUTLS */
static int migrate_postcopy_prepare(QTestState **from_ptr,
@@ -1020,6 +1251,21 @@ static void test_precopy_unix_plain(void)
test_precopy_common(&args);
}
+static void test_precopy_unix_dirty_ring(void)
+{
+ g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
+ MigrateCommon args = {
+ .start = {
+ .use_dirty_ring = true,
+ },
+ .listen_uri = uri,
+ .connect_uri = uri,
+ };
+
+ test_precopy_common(&args);
+}
+
+#ifdef CONFIG_GNUTLS
static void test_precopy_unix_tls_psk(void)
{
g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
@@ -1033,19 +1279,38 @@ static void test_precopy_unix_tls_psk(void)
test_precopy_common(&args);
}
-static void test_precopy_unix_dirty_ring(void)
+#ifdef CONFIG_TASN1
+static void test_precopy_unix_tls_x509_default_host(void)
{
g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
MigrateCommon args = {
.start = {
- .use_dirty_ring = true,
+ .hide_stderr = true,
},
+ .connect_uri = uri,
.listen_uri = uri,
+ .start_hook = test_migrate_tls_x509_start_default_host,
+ .finish_hook = test_migrate_tls_x509_finish,
+ .result = MIG_TEST_FAIL_DEST_QUIT_ERR,
+ };
+
+ test_precopy_common(&args);
+}
+
+static void test_precopy_unix_tls_x509_override_host(void)
+{
+ g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
+ MigrateCommon args = {
.connect_uri = uri,
+ .listen_uri = uri,
+ .start_hook = test_migrate_tls_x509_start_override_host,
+ .finish_hook = test_migrate_tls_x509_finish,
};
test_precopy_common(&args);
}
+#endif /* CONFIG_TASN1 */
+#endif /* CONFIG_GNUTLS */
#if 0
/* Currently upset on aarch64 TCG */
@@ -1172,6 +1437,97 @@ static void test_precopy_tcp_tls_psk_mismatch(void)
test_precopy_common(&args);
}
+
+#ifdef CONFIG_TASN1
+static void test_precopy_tcp_tls_x509_default_host(void)
+{
+ MigrateCommon args = {
+ .listen_uri = "tcp:127.0.0.1:0",
+ .start_hook = test_migrate_tls_x509_start_default_host,
+ .finish_hook = test_migrate_tls_x509_finish,
+ };
+
+ test_precopy_common(&args);
+}
+
+static void test_precopy_tcp_tls_x509_override_host(void)
+{
+ MigrateCommon args = {
+ .listen_uri = "tcp:127.0.0.1:0",
+ .start_hook = test_migrate_tls_x509_start_override_host,
+ .finish_hook = test_migrate_tls_x509_finish,
+ };
+
+ test_precopy_common(&args);
+}
+
+static void test_precopy_tcp_tls_x509_mismatch_host(void)
+{
+ MigrateCommon args = {
+ .start = {
+ .hide_stderr = true,
+ },
+ .listen_uri = "tcp:127.0.0.1:0",
+ .start_hook = test_migrate_tls_x509_start_mismatch_host,
+ .finish_hook = test_migrate_tls_x509_finish,
+ .result = MIG_TEST_FAIL_DEST_QUIT_ERR,
+ };
+
+ test_precopy_common(&args);
+}
+
+static void test_precopy_tcp_tls_x509_friendly_client(void)
+{
+ MigrateCommon args = {
+ .listen_uri = "tcp:127.0.0.1:0",
+ .start_hook = test_migrate_tls_x509_start_friendly_client,
+ .finish_hook = test_migrate_tls_x509_finish,
+ };
+
+ test_precopy_common(&args);
+}
+
+static void test_precopy_tcp_tls_x509_hostile_client(void)
+{
+ MigrateCommon args = {
+ .start = {
+ .hide_stderr = true,
+ },
+ .listen_uri = "tcp:127.0.0.1:0",
+ .start_hook = test_migrate_tls_x509_start_hostile_client,
+ .finish_hook = test_migrate_tls_x509_finish,
+ .result = MIG_TEST_FAIL,
+ };
+
+ test_precopy_common(&args);
+}
+
+static void test_precopy_tcp_tls_x509_allow_anon_client(void)
+{
+ MigrateCommon args = {
+ .listen_uri = "tcp:127.0.0.1:0",
+ .start_hook = test_migrate_tls_x509_start_allow_anon_client,
+ .finish_hook = test_migrate_tls_x509_finish,
+ };
+
+ test_precopy_common(&args);
+}
+
+static void test_precopy_tcp_tls_x509_reject_anon_client(void)
+{
+ MigrateCommon args = {
+ .start = {
+ .hide_stderr = true,
+ },
+ .listen_uri = "tcp:127.0.0.1:0",
+ .start_hook = test_migrate_tls_x509_start_reject_anon_client,
+ .finish_hook = test_migrate_tls_x509_finish,
+ .result = MIG_TEST_FAIL,
+ };
+
+ test_precopy_common(&args);
+}
+#endif /* CONFIG_TASN1 */
#endif /* CONFIG_GNUTLS */
static void *test_migrate_fd_start_hook(QTestState *from,
@@ -1640,6 +1996,12 @@ int main(int argc, char **argv)
#ifdef CONFIG_GNUTLS
qtest_add_func("/migration/precopy/unix/tls/psk",
test_precopy_unix_tls_psk);
+#ifdef CONFIG_TASN1
+ qtest_add_func("/migration/precopy/unix/tls/x509/default-host",
+ test_precopy_unix_tls_x509_default_host);
+ qtest_add_func("/migration/precopy/unix/tls/x509/override-host",
+ test_precopy_unix_tls_x509_override_host);
+#endif /* CONFIG_TASN1 */
#endif /* CONFIG_GNUTLS */
qtest_add_func("/migration/precopy/tcp/plain", test_precopy_tcp_plain);
@@ -1648,6 +2010,22 @@ int main(int argc, char **argv)
test_precopy_tcp_tls_psk_match);
qtest_add_func("/migration/precopy/tcp/tls/psk/mismatch",
test_precopy_tcp_tls_psk_mismatch);
+#ifdef CONFIG_TASN1
+ qtest_add_func("/migration/precopy/tcp/tls/x509/default-host",
+ test_precopy_tcp_tls_x509_default_host);
+ qtest_add_func("/migration/precopy/tcp/tls/x509/override-host",
+ test_precopy_tcp_tls_x509_override_host);
+ qtest_add_func("/migration/precopy/tcp/tls/x509/mismatch-host",
+ test_precopy_tcp_tls_x509_mismatch_host);
+ qtest_add_func("/migration/precopy/tcp/tls/x509/friendly-client",
+ test_precopy_tcp_tls_x509_friendly_client);
+ qtest_add_func("/migration/precopy/tcp/tls/x509/hostile-client",
+ test_precopy_tcp_tls_x509_hostile_client);
+ qtest_add_func("/migration/precopy/tcp/tls/x509/allow-anon-client",
+ test_precopy_tcp_tls_x509_allow_anon_client);
+ qtest_add_func("/migration/precopy/tcp/tls/x509/reject-anon-client",
+ test_precopy_tcp_tls_x509_reject_anon_client);
+#endif /* CONFIG_TASN1 */
#endif /* CONFIG_GNUTLS */
/* qtest_add_func("/migration/ignore_shared", test_ignore_shared); */
--
2.35.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v3 5/9] tests: convert XBZRLE migration test to use common helper
2022-04-26 16:00 [PATCH v3 0/9] tests: introduce testing coverage for TLS with migration Daniel P. Berrangé
` (3 preceding siblings ...)
2022-04-26 16:00 ` [PATCH v3 4/9] tests: add migration tests of TLS with x509 credentials Daniel P. Berrangé
@ 2022-04-26 16:00 ` Daniel P. Berrangé
2022-04-26 16:00 ` [PATCH v3 6/9] tests: convert multifd migration tests " Daniel P. Berrangé
` (5 subsequent siblings)
10 siblings, 0 replies; 19+ messages in thread
From: Daniel P. Berrangé @ 2022-04-26 16:00 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Thomas Huth, Daniel P. Berrangé,
Juan Quintela, Dr. David Alan Gilbert, Peter Xu, Paolo Bonzini
Most of the XBZRLE migration test logic is common with the rest of the
precopy tests, so it can use the helper with just one small tweak.
Reviewed-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
tests/qtest/migration-test.c | 67 ++++++++++++++----------------------
1 file changed, 25 insertions(+), 42 deletions(-)
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index c730697f74..043ae94089 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -1174,6 +1174,9 @@ typedef struct {
/* This test should fail, dest qemu should fail with abnormal status */
MIG_TEST_FAIL_DEST_QUIT_ERR,
} result;
+
+ /* Optional: set number of migration passes to wait for */
+ unsigned int iterations;
} MigrateCommon;
static void test_precopy_common(MigrateCommon *args)
@@ -1219,7 +1222,13 @@ static void test_precopy_common(MigrateCommon *args)
qtest_set_expected_status(to, 1);
}
} else {
- wait_for_migration_pass(from);
+ if (args->iterations) {
+ while (args->iterations--) {
+ wait_for_migration_pass(from);
+ }
+ } else {
+ wait_for_migration_pass(from);
+ }
migrate_set_parameter_int(from, "downtime-limit", CONVERGE_DOWNTIME);
@@ -1349,57 +1358,31 @@ static void test_ignore_shared(void)
}
#endif
-static void test_xbzrle(const char *uri)
+static void *
+test_migrate_xbzrle_start(QTestState *from,
+ QTestState *to)
{
- MigrateStart args = {};
- QTestState *from, *to;
-
- if (test_migrate_start(&from, &to, uri, &args)) {
- return;
- }
-
- /*
- * We want to pick a speed slow enough that the test completes
- * quickly, but that it doesn't complete precopy even on a slow
- * machine, so also set the downtime.
- */
- /* 1 ms should make it not converge*/
- migrate_set_parameter_int(from, "downtime-limit", 1);
- /* 1GB/s */
- migrate_set_parameter_int(from, "max-bandwidth", 1000000000);
-
migrate_set_parameter_int(from, "xbzrle-cache-size", 33554432);
migrate_set_capability(from, "xbzrle", true);
migrate_set_capability(to, "xbzrle", true);
- /* Wait for the first serial output from the source */
- wait_for_serial("src_serial");
- migrate_qmp(from, uri, "{}");
-
- wait_for_migration_pass(from);
- /* Make sure we have 2 passes, so the xbzrle cache gets a workout */
- wait_for_migration_pass(from);
-
- /* 1000ms should converge */
- migrate_set_parameter_int(from, "downtime-limit", 1000);
-
- if (!got_stop) {
- qtest_qmp_eventwait(from, "STOP");
- }
- qtest_qmp_eventwait(to, "RESUME");
-
- wait_for_serial("dest_serial");
- wait_for_migration_complete(from);
-
- test_migrate_end(from, to, true);
+ return NULL;
}
-static void test_xbzrle_unix(void)
+static void test_precopy_unix_xbzrle(void)
{
g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
+ MigrateCommon args = {
+ .connect_uri = uri,
+ .listen_uri = uri,
+
+ .start_hook = test_migrate_xbzrle_start,
- test_xbzrle(uri);
+ .iterations = 2,
+ };
+
+ test_precopy_common(&args);
}
static void test_precopy_tcp_plain(void)
@@ -1993,6 +1976,7 @@ int main(int argc, char **argv)
qtest_add_func("/migration/postcopy/recovery", test_postcopy_recovery);
qtest_add_func("/migration/bad_dest", test_baddest);
qtest_add_func("/migration/precopy/unix/plain", test_precopy_unix_plain);
+ qtest_add_func("/migration/precopy/unix/xbzrle", test_precopy_unix_xbzrle);
#ifdef CONFIG_GNUTLS
qtest_add_func("/migration/precopy/unix/tls/psk",
test_precopy_unix_tls_psk);
@@ -2029,7 +2013,6 @@ int main(int argc, char **argv)
#endif /* CONFIG_GNUTLS */
/* qtest_add_func("/migration/ignore_shared", test_ignore_shared); */
- qtest_add_func("/migration/xbzrle/unix", test_xbzrle_unix);
qtest_add_func("/migration/fd_proto", test_migrate_fd_proto);
qtest_add_func("/migration/validate_uuid", test_validate_uuid);
qtest_add_func("/migration/validate_uuid_error", test_validate_uuid_error);
--
2.35.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v3 6/9] tests: convert multifd migration tests to use common helper
2022-04-26 16:00 [PATCH v3 0/9] tests: introduce testing coverage for TLS with migration Daniel P. Berrangé
` (4 preceding siblings ...)
2022-04-26 16:00 ` [PATCH v3 5/9] tests: convert XBZRLE migration test to use common helper Daniel P. Berrangé
@ 2022-04-26 16:00 ` Daniel P. Berrangé
2022-04-26 16:00 ` [PATCH v3 7/9] tests: add multifd migration tests of TLS with PSK credentials Daniel P. Berrangé
` (4 subsequent siblings)
10 siblings, 0 replies; 19+ messages in thread
From: Daniel P. Berrangé @ 2022-04-26 16:00 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Thomas Huth, Daniel P. Berrangé,
Juan Quintela, Dr. David Alan Gilbert, Peter Xu, Paolo Bonzini
Most of the multifd migration test logic is common with the rest of the
precopy tests, so it can use the helper without difficulty. The only
exception of the multifd cancellation test which tries to run multiple
migrations in a row.
Reviewed-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
tests/qtest/migration-test.c | 77 +++++++++++++++++++-----------------
1 file changed, 40 insertions(+), 37 deletions(-)
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 043ae94089..c1b0b3aca4 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -1739,26 +1739,12 @@ static void test_migrate_auto_converge(void)
test_migrate_end(from, to, true);
}
-static void test_multifd_tcp(const char *method)
+static void *
+test_migrate_precopy_tcp_multifd_start_common(QTestState *from,
+ QTestState *to,
+ const char *method)
{
- MigrateStart args = {};
- QTestState *from, *to;
QDict *rsp;
- g_autofree char *uri = NULL;
-
- if (test_migrate_start(&from, &to, "defer", &args)) {
- return;
- }
-
- /*
- * We want to pick a speed slow enough that the test completes
- * quickly, but that it doesn't complete precopy even on a slow
- * machine, so also set the downtime.
- */
- /* 1 ms should make it not converge*/
- migrate_set_parameter_int(from, "downtime-limit", 1);
- /* 1GB/s */
- migrate_set_parameter_int(from, "max-bandwidth", 1000000000);
migrate_set_parameter_int(from, "multifd-channels", 16);
migrate_set_parameter_int(to, "multifd-channels", 16);
@@ -1774,41 +1760,58 @@ static void test_multifd_tcp(const char *method)
" 'arguments': { 'uri': 'tcp:127.0.0.1:0' }}");
qobject_unref(rsp);
- /* Wait for the first serial output from the source */
- wait_for_serial("src_serial");
-
- uri = migrate_get_socket_address(to, "socket-address");
-
- migrate_qmp(from, uri, "{}");
-
- wait_for_migration_pass(from);
+ return NULL;
+}
- migrate_set_parameter_int(from, "downtime-limit", CONVERGE_DOWNTIME);
+static void *
+test_migrate_precopy_tcp_multifd_start(QTestState *from,
+ QTestState *to)
+{
+ return test_migrate_precopy_tcp_multifd_start_common(from, to, "none");
+}
- if (!got_stop) {
- qtest_qmp_eventwait(from, "STOP");
- }
- qtest_qmp_eventwait(to, "RESUME");
+static void *
+test_migrate_precopy_tcp_multifd_zlib_start(QTestState *from,
+ QTestState *to)
+{
+ return test_migrate_precopy_tcp_multifd_start_common(from, to, "zlib");
+}
- wait_for_serial("dest_serial");
- wait_for_migration_complete(from);
- test_migrate_end(from, to, true);
+#ifdef CONFIG_ZSTD
+static void *
+test_migrate_precopy_tcp_multifd_zstd_start(QTestState *from,
+ QTestState *to)
+{
+ return test_migrate_precopy_tcp_multifd_start_common(from, to, "zstd");
}
+#endif /* CONFIG_ZSTD */
static void test_multifd_tcp_none(void)
{
- test_multifd_tcp("none");
+ MigrateCommon args = {
+ .listen_uri = "defer",
+ .start_hook = test_migrate_precopy_tcp_multifd_start,
+ };
+ test_precopy_common(&args);
}
static void test_multifd_tcp_zlib(void)
{
- test_multifd_tcp("zlib");
+ MigrateCommon args = {
+ .listen_uri = "defer",
+ .start_hook = test_migrate_precopy_tcp_multifd_zlib_start,
+ };
+ test_precopy_common(&args);
}
#ifdef CONFIG_ZSTD
static void test_multifd_tcp_zstd(void)
{
- test_multifd_tcp("zstd");
+ MigrateCommon args = {
+ .listen_uri = "defer",
+ .start_hook = test_migrate_precopy_tcp_multifd_zstd_start,
+ };
+ test_precopy_common(&args);
}
#endif
--
2.35.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v3 7/9] tests: add multifd migration tests of TLS with PSK credentials
2022-04-26 16:00 [PATCH v3 0/9] tests: introduce testing coverage for TLS with migration Daniel P. Berrangé
` (5 preceding siblings ...)
2022-04-26 16:00 ` [PATCH v3 6/9] tests: convert multifd migration tests " Daniel P. Berrangé
@ 2022-04-26 16:00 ` Daniel P. Berrangé
2022-04-28 14:05 ` Eric Blake
2022-04-26 16:00 ` [PATCH v3 8/9] tests: add multifd migration tests of TLS with x509 credentials Daniel P. Berrangé
` (3 subsequent siblings)
10 siblings, 1 reply; 19+ messages in thread
From: Daniel P. Berrangé @ 2022-04-26 16:00 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Thomas Huth, Daniel P. Berrangé,
Juan Quintela, Dr. David Alan Gilbert, Peter Xu, Paolo Bonzini
This validates that we correctly handle multifd migration success
and failure scenarios when using TLS with pre shared keys.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
tests/qtest/migration-test.c | 60 +++++++++++++++++++++++++++++++++---
1 file changed, 56 insertions(+), 4 deletions(-)
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index c1b0b3aca4..f47e4797e2 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -1815,6 +1815,48 @@ static void test_multifd_tcp_zstd(void)
}
#endif
+#ifdef CONFIG_GNUTLS
+static void *
+test_migrate_multifd_tcp_tls_psk_start_match(QTestState *from,
+ QTestState *to)
+{
+ test_migrate_precopy_tcp_multifd_start_common(from, to, "none");
+ return test_migrate_tls_psk_start_match(from, to);
+}
+
+static void *
+test_migrate_multifd_tcp_tls_psk_start_mismatch(QTestState *from,
+ QTestState *to)
+{
+ test_migrate_precopy_tcp_multifd_start_common(from, to, "none");
+ return test_migrate_tls_psk_start_mismatch(from, to);
+}
+
+static void test_multifd_tcp_tls_psk_match(void)
+{
+ MigrateCommon args = {
+ .listen_uri = "defer",
+ .start_hook = test_migrate_multifd_tcp_tls_psk_start_match,
+ .finish_hook = test_migrate_tls_psk_finish,
+ };
+ test_precopy_common(&args);
+}
+
+static void test_multifd_tcp_tls_psk_mismatch(void)
+{
+ MigrateCommon args = {
+ .start = {
+ .hide_stderr = true,
+ },
+ .listen_uri = "defer",
+ .start_hook = test_migrate_multifd_tcp_tls_psk_start_mismatch,
+ .finish_hook = test_migrate_tls_psk_finish,
+ .result = MIG_TEST_FAIL,
+ };
+ test_precopy_common(&args);
+}
+#endif /* CONFIG_GNUTLS */
+
/*
* This test does:
* source target
@@ -2025,12 +2067,22 @@ int main(int argc, char **argv)
test_validate_uuid_dst_not_set);
qtest_add_func("/migration/auto_converge", test_migrate_auto_converge);
- qtest_add_func("/migration/multifd/tcp/none", test_multifd_tcp_none);
- qtest_add_func("/migration/multifd/tcp/cancel", test_multifd_tcp_cancel);
- qtest_add_func("/migration/multifd/tcp/zlib", test_multifd_tcp_zlib);
+ qtest_add_func("/migration/multifd/tcp/plain/none",
+ test_multifd_tcp_none);
+ qtest_add_func("/migration/multifd/tcp/plain/cancel",
+ test_multifd_tcp_cancel);
+ qtest_add_func("/migration/multifd/tcp/plain/zlib",
+ test_multifd_tcp_zlib);
#ifdef CONFIG_ZSTD
- qtest_add_func("/migration/multifd/tcp/zstd", test_multifd_tcp_zstd);
+ qtest_add_func("/migration/multifd/tcp/plain/zstd",
+ test_multifd_tcp_zstd);
#endif
+#ifdef CONFIG_GNUTLS
+ qtest_add_func("/migration/multifd/tcp/tls/psk/match",
+ test_multifd_tcp_tls_psk_match);
+ qtest_add_func("/migration/multifd/tcp/tls/psk/mismatch",
+ test_multifd_tcp_tls_psk_mismatch);
+#endif /* CONFIG_GNUTLS */
if (kvm_dirty_ring_supported()) {
qtest_add_func("/migration/dirty_ring",
--
2.35.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v3 8/9] tests: add multifd migration tests of TLS with x509 credentials
2022-04-26 16:00 [PATCH v3 0/9] tests: introduce testing coverage for TLS with migration Daniel P. Berrangé
` (6 preceding siblings ...)
2022-04-26 16:00 ` [PATCH v3 7/9] tests: add multifd migration tests of TLS with PSK credentials Daniel P. Berrangé
@ 2022-04-26 16:00 ` Daniel P. Berrangé
2022-04-28 14:13 ` Eric Blake
2022-04-26 16:00 ` [PATCH v3 9/9] tests: ensure migration status isn't reported as failed Daniel P. Berrangé
` (2 subsequent siblings)
10 siblings, 1 reply; 19+ messages in thread
From: Daniel P. Berrangé @ 2022-04-26 16:00 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Thomas Huth, Daniel P. Berrangé,
Juan Quintela, Dr. David Alan Gilbert, Peter Xu, Paolo Bonzini
This validates that we correctly handle multifd migration success
and failure scenarios when using TLS with x509 certificates. There
are quite a few different scenarios that matter in relation to
hostname validation, but we skip a couple as we can assume that
the non-multifd coverage applies to some extent.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
tests/qtest/migration-test.c | 127 +++++++++++++++++++++++++++++++++++
1 file changed, 127 insertions(+)
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index f47e4797e2..5ea0b9360a 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -1832,6 +1832,48 @@ test_migrate_multifd_tcp_tls_psk_start_mismatch(QTestState *from,
return test_migrate_tls_psk_start_mismatch(from, to);
}
+#ifdef CONFIG_TASN1
+static void *
+test_migrate_multifd_tls_x509_start_default_host(QTestState *from,
+ QTestState *to)
+{
+ test_migrate_precopy_tcp_multifd_start_common(from, to, "none");
+ return test_migrate_tls_x509_start_default_host(from, to);
+}
+
+static void *
+test_migrate_multifd_tls_x509_start_override_host(QTestState *from,
+ QTestState *to)
+{
+ test_migrate_precopy_tcp_multifd_start_common(from, to, "none");
+ return test_migrate_tls_x509_start_override_host(from, to);
+}
+
+static void *
+test_migrate_multifd_tls_x509_start_mismatch_host(QTestState *from,
+ QTestState *to)
+{
+ test_migrate_precopy_tcp_multifd_start_common(from, to, "none");
+ return test_migrate_tls_x509_start_mismatch_host(from, to);
+}
+
+static void *
+test_migrate_multifd_tls_x509_start_allow_anon_client(QTestState *from,
+ QTestState *to)
+{
+ test_migrate_precopy_tcp_multifd_start_common(from, to, "none");
+ return test_migrate_tls_x509_start_allow_anon_client(from, to);
+}
+
+static void *
+test_migrate_multifd_tls_x509_start_reject_anon_client(QTestState *from,
+ QTestState *to)
+{
+ test_migrate_precopy_tcp_multifd_start_common(from, to, "none");
+ return test_migrate_tls_x509_start_reject_anon_client(from, to);
+}
+#endif /* CONFIG_TASN1 */
+
static void test_multifd_tcp_tls_psk_match(void)
{
MigrateCommon args = {
@@ -1855,6 +1897,79 @@ static void test_multifd_tcp_tls_psk_mismatch(void)
};
test_precopy_common(&args);
}
+
+#ifdef CONFIG_TASN1
+static void test_multifd_tcp_tls_x509_default_host(void)
+{
+ MigrateCommon args = {
+ .listen_uri = "defer",
+ .start_hook = test_migrate_multifd_tls_x509_start_default_host,
+ .finish_hook = test_migrate_tls_x509_finish,
+ };
+ test_precopy_common(&args);
+}
+
+static void test_multifd_tcp_tls_x509_override_host(void)
+{
+ MigrateCommon args = {
+ .listen_uri = "defer",
+ .start_hook = test_migrate_multifd_tls_x509_start_override_host,
+ .finish_hook = test_migrate_tls_x509_finish,
+ };
+ test_precopy_common(&args);
+}
+
+static void test_multifd_tcp_tls_x509_mismatch_host(void)
+{
+ /*
+ * This has different behaviour to the non-multifd case.
+ *
+ * In non-multifd case when client aborts due to mismatched
+ * cert host, the server has already started trying to load
+ * migration state, and so it exits with I/O failure.
+ *
+ * In multifd case when client aborts due to mismatched
+ * cert host, the server is still waiting for the other
+ * multifd connections to arrive so hasn't started trying
+ * to load migration state, and thus just aborts the migration
+ * without exiting
+ */
+ MigrateCommon args = {
+ .start = {
+ .hide_stderr = true,
+ },
+ .listen_uri = "defer",
+ .start_hook = test_migrate_multifd_tls_x509_start_mismatch_host,
+ .finish_hook = test_migrate_tls_x509_finish,
+ .result = MIG_TEST_FAIL,
+ };
+ test_precopy_common(&args);
+}
+
+static void test_multifd_tcp_tls_x509_allow_anon_client(void)
+{
+ MigrateCommon args = {
+ .listen_uri = "defer",
+ .start_hook = test_migrate_multifd_tls_x509_start_allow_anon_client,
+ .finish_hook = test_migrate_tls_x509_finish,
+ };
+ test_precopy_common(&args);
+}
+
+static void test_multifd_tcp_tls_x509_reject_anon_client(void)
+{
+ MigrateCommon args = {
+ .start = {
+ .hide_stderr = true,
+ },
+ .listen_uri = "defer",
+ .start_hook = test_migrate_multifd_tls_x509_start_reject_anon_client,
+ .finish_hook = test_migrate_tls_x509_finish,
+ .result = MIG_TEST_FAIL,
+ };
+ test_precopy_common(&args);
+}
+#endif /* CONFIG_TASN1 */
#endif /* CONFIG_GNUTLS */
/*
@@ -2082,6 +2197,18 @@ int main(int argc, char **argv)
test_multifd_tcp_tls_psk_match);
qtest_add_func("/migration/multifd/tcp/tls/psk/mismatch",
test_multifd_tcp_tls_psk_mismatch);
+#ifdef CONFIG_TASN1
+ qtest_add_func("/migration/multifd/tcp/tls/x509/default-host",
+ test_multifd_tcp_tls_x509_default_host);
+ qtest_add_func("/migration/multifd/tcp/tls/x509/override-host",
+ test_multifd_tcp_tls_x509_override_host);
+ qtest_add_func("/migration/multifd/tcp/tls/x509/mismatch-host",
+ test_multifd_tcp_tls_x509_mismatch_host);
+ qtest_add_func("/migration/multifd/tcp/tls/x509/allow-anon-client",
+ test_multifd_tcp_tls_x509_allow_anon_client);
+ qtest_add_func("/migration/multifd/tcp/tls/x509/reject-anon-client",
+ test_multifd_tcp_tls_x509_reject_anon_client);
+#endif /* CONFIG_TASN1 */
#endif /* CONFIG_GNUTLS */
if (kvm_dirty_ring_supported()) {
--
2.35.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v3 9/9] tests: ensure migration status isn't reported as failed
2022-04-26 16:00 [PATCH v3 0/9] tests: introduce testing coverage for TLS with migration Daniel P. Berrangé
` (7 preceding siblings ...)
2022-04-26 16:00 ` [PATCH v3 8/9] tests: add multifd migration tests of TLS with x509 credentials Daniel P. Berrangé
@ 2022-04-26 16:00 ` Daniel P. Berrangé
2022-04-28 11:40 ` [PATCH v3 0/9] tests: introduce testing coverage for TLS with migration Dr. David Alan Gilbert
2022-05-09 14:11 ` Dr. David Alan Gilbert
10 siblings, 0 replies; 19+ messages in thread
From: Daniel P. Berrangé @ 2022-04-26 16:00 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Thomas Huth, Daniel P. Berrangé,
Juan Quintela, Dr. David Alan Gilbert, Peter Xu, Paolo Bonzini
Various methods in the migration test call 'query_migrate' to fetch the
current status and then access a particular field. Almost all of these
cases expect the migration to be in a non-failed state. In the case of
'wait_for_migration_pass' in particular, if the status is 'failed' then
it will get into an infinite loop. By validating that the status is
not 'failed' the test suite will assert rather than hang when getting
into an unexpected state.
Reviewed-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
tests/qtest/migration-helpers.c | 13 +++++++++++++
tests/qtest/migration-helpers.h | 1 +
tests/qtest/migration-test.c | 6 +++---
3 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/tests/qtest/migration-helpers.c b/tests/qtest/migration-helpers.c
index 4ee26014b7..a6aa59e4e6 100644
--- a/tests/qtest/migration-helpers.c
+++ b/tests/qtest/migration-helpers.c
@@ -107,6 +107,19 @@ QDict *migrate_query(QTestState *who)
return wait_command(who, "{ 'execute': 'query-migrate' }");
}
+QDict *migrate_query_not_failed(QTestState *who)
+{
+ const char *status;
+ QDict *rsp = migrate_query(who);
+ status = qdict_get_str(rsp, "status");
+ if (g_str_equal(status, "failed")) {
+ g_printerr("query-migrate shows failed migration: %s\n",
+ qdict_get_str(rsp, "error-desc"));
+ }
+ g_assert(!g_str_equal(status, "failed"));
+ return rsp;
+}
+
/*
* Note: caller is responsible to free the returned object via
* g_free() after use
diff --git a/tests/qtest/migration-helpers.h b/tests/qtest/migration-helpers.h
index 555adafce1..d07e0fb748 100644
--- a/tests/qtest/migration-helpers.h
+++ b/tests/qtest/migration-helpers.h
@@ -26,6 +26,7 @@ G_GNUC_PRINTF(3, 4)
void migrate_qmp(QTestState *who, const char *uri, const char *fmt, ...);
QDict *migrate_query(QTestState *who);
+QDict *migrate_query_not_failed(QTestState *who);
void wait_for_migration_status(QTestState *who,
const char *goal, const char **ungoals);
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 5ea0b9360a..d9f444ea14 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -181,7 +181,7 @@ static int64_t read_ram_property_int(QTestState *who, const char *property)
QDict *rsp_return, *rsp_ram;
int64_t result;
- rsp_return = migrate_query(who);
+ rsp_return = migrate_query_not_failed(who);
if (!qdict_haskey(rsp_return, "ram")) {
/* Still in setup */
result = 0;
@@ -198,7 +198,7 @@ static int64_t read_migrate_property_int(QTestState *who, const char *property)
QDict *rsp_return;
int64_t result;
- rsp_return = migrate_query(who);
+ rsp_return = migrate_query_not_failed(who);
result = qdict_get_try_int(rsp_return, property, 0);
qobject_unref(rsp_return);
return result;
@@ -213,7 +213,7 @@ static void read_blocktime(QTestState *who)
{
QDict *rsp_return;
- rsp_return = migrate_query(who);
+ rsp_return = migrate_query_not_failed(who);
g_assert(qdict_haskey(rsp_return, "postcopy-blocktime"));
qobject_unref(rsp_return);
}
--
2.35.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v3 1/9] tests: fix encoding of IP addresses in x509 certs
2022-04-26 16:00 ` [PATCH v3 1/9] tests: fix encoding of IP addresses in x509 certs Daniel P. Berrangé
@ 2022-04-28 9:46 ` Dr. David Alan Gilbert
0 siblings, 0 replies; 19+ messages in thread
From: Dr. David Alan Gilbert @ 2022-04-28 9:46 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: Laurent Vivier, Thomas Huth, Juan Quintela, qemu-devel, Peter Xu,
Paolo Bonzini
* Daniel P. Berrangé (berrange@redhat.com) wrote:
> We need to encode just the address bytes, not the whole struct sockaddr
> data. Add a test case to validate that we're matching on SAN IP
> addresses correctly.
Lets see:
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> tests/unit/crypto-tls-x509-helpers.c | 16 +++++++++++++---
> tests/unit/test-crypto-tlssession.c | 11 +++++++++--
> 2 files changed, 22 insertions(+), 5 deletions(-)
>
> diff --git a/tests/unit/crypto-tls-x509-helpers.c b/tests/unit/crypto-tls-x509-helpers.c
> index fc609b3fd4..e9937f60d8 100644
> --- a/tests/unit/crypto-tls-x509-helpers.c
> +++ b/tests/unit/crypto-tls-x509-helpers.c
> @@ -168,9 +168,19 @@ test_tls_get_ipaddr(const char *addrstr,
> hints.ai_flags = AI_NUMERICHOST;
> g_assert(getaddrinfo(addrstr, NULL, &hints, &res) == 0);
test_tls_get_ipaddr is passed a char** data ptr that's then passed to
gnutls_x509_crt_set_subject_alt_name with GNUTLS_SAN_IPADDRESS, none of
which I know about, bu tthe manpage says:
'GNUTLS_SAN_IPADDRESS as a binary IP address (4 or 16 bytes)'
so yes, it wants the IP not the full structure.
>
> - *datalen = res->ai_addrlen;
> - *data = g_new(char, *datalen);
> - memcpy(*data, res->ai_addr, *datalen);
> + if (res->ai_family == AF_INET) {
> + struct sockaddr_in *in = (struct sockaddr_in *)res->ai_addr;
> + *datalen = sizeof(in->sin_addr);
> + *data = g_new(char, *datalen);
> + memcpy(*data, &in->sin_addr, *datalen);
> + } else if (res->ai_family == AF_INET6) {
> + struct sockaddr_in6 *in = (struct sockaddr_in6 *)res->ai_addr;
> + *datalen = sizeof(in->sin6_addr);
> + *data = g_new(char, *datalen);
> + memcpy(*data, &in->sin6_addr, *datalen);
> + } else {
> + g_assert_not_reached();
> + }
> freeaddrinfo(res);
Yes, you could use g_memdup,
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> }
>
> diff --git a/tests/unit/test-crypto-tlssession.c b/tests/unit/test-crypto-tlssession.c
> index 5f0da9192c..a6935d8497 100644
> --- a/tests/unit/test-crypto-tlssession.c
> +++ b/tests/unit/test-crypto-tlssession.c
> @@ -512,12 +512,19 @@ int main(int argc, char **argv)
> false, true, "wiki.qemu.org", NULL);
>
> TEST_SESS_REG(altname4, cacertreq.filename,
> + servercertalt1req.filename, clientcertreq.filename,
> + false, false, "192.168.122.1", NULL);
> + TEST_SESS_REG(altname5, cacertreq.filename,
> + servercertalt1req.filename, clientcertreq.filename,
> + false, false, "fec0::dead:beaf", NULL);
> +
> + TEST_SESS_REG(altname6, cacertreq.filename,
> servercertalt2req.filename, clientcertreq.filename,
> false, true, "qemu.org", NULL);
> - TEST_SESS_REG(altname5, cacertreq.filename,
> + TEST_SESS_REG(altname7, cacertreq.filename,
> servercertalt2req.filename, clientcertreq.filename,
> false, false, "www.qemu.org", NULL);
> - TEST_SESS_REG(altname6, cacertreq.filename,
> + TEST_SESS_REG(altname8, cacertreq.filename,
> servercertalt2req.filename, clientcertreq.filename,
> false, false, "wiki.qemu.org", NULL);
>
> --
> 2.35.1
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 0/9] tests: introduce testing coverage for TLS with migration
2022-04-26 16:00 [PATCH v3 0/9] tests: introduce testing coverage for TLS with migration Daniel P. Berrangé
` (8 preceding siblings ...)
2022-04-26 16:00 ` [PATCH v3 9/9] tests: ensure migration status isn't reported as failed Daniel P. Berrangé
@ 2022-04-28 11:40 ` Dr. David Alan Gilbert
2022-05-09 14:11 ` Dr. David Alan Gilbert
10 siblings, 0 replies; 19+ messages in thread
From: Dr. David Alan Gilbert @ 2022-04-28 11:40 UTC (permalink / raw)
To: Daniel P. Berrangé, eblake
Cc: Laurent Vivier, Thomas Huth, Juan Quintela, qemu-devel, Peter Xu,
Paolo Bonzini
* Daniel P. Berrangé (berrange@redhat.com) wrote:
> This significantly expands the migration test suite to cover testing
> with TLS over TCP and UNIX sockets, with both PSK (pre shared keys)
> and x509 credentials, and for both single and multifd scenarios.
>
> It identified one bug in handling PSK credentials with UNIX sockets,
> but other than that everything was operating as expected.
>
> To minimize the impact on code duplication alopt of refactoring is
> done of the migration tests to introduce a common helper for running
> the migration process. The various tests mostly just have to provide
> a callback to set a few parameters/capabilities before migration
> starts, and sometimes a callback to cleanup or validate after
> completion/failure.
I've queued:
tests: ensure migration status isn't reported as failed
tests: convert multifd migration tests to use common helper
tests: convert XBZRLE migration test to use common helper
tests: fix encoding of IP addresses in x509 certs
I'd appreciate some TLS people to review the other parts.
Dave
> Changed in v3:
>
> - Trivial rebase dropping already merged patches
>
> Changed in v2:
>
> - Use structs to pass around most parameters
> - Hide expected errors from stderr
>
> Daniel P. Berrangé (9):
> tests: fix encoding of IP addresses in x509 certs
> tests: add more helper macros for creating TLS x509 certs
> tests: add migration tests of TLS with PSK credentials
> tests: add migration tests of TLS with x509 credentials
> tests: convert XBZRLE migration test to use common helper
> tests: convert multifd migration tests to use common helper
> tests: add multifd migration tests of TLS with PSK credentials
> tests: add multifd migration tests of TLS with x509 credentials
> tests: ensure migration status isn't reported as failed
>
> meson.build | 1 +
> tests/qtest/meson.build | 12 +-
> tests/qtest/migration-helpers.c | 13 +
> tests/qtest/migration-helpers.h | 1 +
> tests/qtest/migration-test.c | 866 ++++++++++++++++++++++++---
> tests/unit/crypto-tls-psk-helpers.c | 18 +-
> tests/unit/crypto-tls-psk-helpers.h | 1 +
> tests/unit/crypto-tls-x509-helpers.c | 16 +-
> tests/unit/crypto-tls-x509-helpers.h | 53 ++
> tests/unit/test-crypto-tlssession.c | 11 +-
> 10 files changed, 897 insertions(+), 95 deletions(-)
>
> --
> 2.35.1
>
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 2/9] tests: add more helper macros for creating TLS x509 certs
2022-04-26 16:00 ` [PATCH v3 2/9] tests: add more helper macros for creating TLS " Daniel P. Berrangé
@ 2022-04-28 13:09 ` Eric Blake
0 siblings, 0 replies; 19+ messages in thread
From: Eric Blake @ 2022-04-28 13:09 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: Laurent Vivier, Thomas Huth, Juan Quintela, qemu-devel, Peter Xu,
Dr. David Alan Gilbert, Paolo Bonzini
On Tue, Apr 26, 2022 at 05:00:41PM +0100, Daniel P. Berrangé wrote:
> These macros are more suited to the general consumers of certs in the
> test suite, where we don't need to exercise every single possible
> permutation.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> tests/unit/crypto-tls-x509-helpers.h | 53 ++++++++++++++++++++++++++++
> 1 file changed, 53 insertions(+)
>
No impact until a later patch uses the macros, but the look reasonable.
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 3/9] tests: add migration tests of TLS with PSK credentials
2022-04-26 16:00 ` [PATCH v3 3/9] tests: add migration tests of TLS with PSK credentials Daniel P. Berrangé
@ 2022-04-28 13:46 ` Eric Blake
2022-05-09 13:29 ` Dr. David Alan Gilbert
0 siblings, 1 reply; 19+ messages in thread
From: Eric Blake @ 2022-04-28 13:46 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: Laurent Vivier, Thomas Huth, Juan Quintela, qemu-devel, Peter Xu,
Dr. David Alan Gilbert, Paolo Bonzini
On Tue, Apr 26, 2022 at 05:00:42PM +0100, Daniel P. Berrangé wrote:
> This validates that we correctly handle migration success and failure
> scenarios when using TLS with pre shared keys.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> tests/qtest/meson.build | 7 +-
> tests/qtest/migration-test.c | 159 +++++++++++++++++++++++++++-
> tests/unit/crypto-tls-psk-helpers.c | 18 +++-
> tests/unit/crypto-tls-psk-helpers.h | 1 +
> 4 files changed, 177 insertions(+), 8 deletions(-)
>
>
> -static void test_precopy_unix(void)
> +static void test_precopy_unix_plain(void)
> {
> g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
> MigrateCommon args = {
> @@ -922,6 +1020,19 @@ static void test_precopy_unix(void)
> test_precopy_common(&args);
> }
>
> +static void test_precopy_unix_tls_psk(void)
> +{
This function is unguarded...
> + g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
> + MigrateCommon args = {
> + .connect_uri = uri,
> + .listen_uri = uri,
> + .start_hook = test_migrate_tls_psk_start_match,
> + .finish_hook = test_migrate_tls_psk_finish,
> + };
> +
> + test_precopy_common(&args);
> +}
> +
> static void test_precopy_unix_dirty_ring(void)
> {
> g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
> @@ -1026,7 +1137,7 @@ static void test_xbzrle_unix(void)
> test_xbzrle(uri);
> }
>
> -static void test_precopy_tcp(void)
> +static void test_precopy_tcp_plain(void)
> {
> MigrateCommon args = {
> .listen_uri = "tcp:127.0.0.1:0",
> @@ -1035,6 +1146,34 @@ static void test_precopy_tcp(void)
> test_precopy_common(&args);
> }
>
> +#ifdef CONFIG_GNUTLS
> +static void test_precopy_tcp_tls_psk_match(void)
...while everything else is #ifdef CONFIG_GNUTLS...
> static void *test_migrate_fd_start_hook(QTestState *from,
> QTestState *to)
> {
> @@ -1497,8 +1636,20 @@ int main(int argc, char **argv)
> qtest_add_func("/migration/postcopy/unix", test_postcopy);
> qtest_add_func("/migration/postcopy/recovery", test_postcopy_recovery);
> qtest_add_func("/migration/bad_dest", test_baddest);
> - qtest_add_func("/migration/precopy/unix", test_precopy_unix);
> - qtest_add_func("/migration/precopy/tcp", test_precopy_tcp);
> + qtest_add_func("/migration/precopy/unix/plain", test_precopy_unix_plain);
> +#ifdef CONFIG_GNUTLS
> + qtest_add_func("/migration/precopy/unix/tls/psk",
> + test_precopy_unix_tls_psk);
> +#endif /* CONFIG_GNUTLS */
> +
> + qtest_add_func("/migration/precopy/tcp/plain", test_precopy_tcp_plain);
> +#ifdef CONFIG_GNUTLS
> + qtest_add_func("/migration/precopy/tcp/tls/psk/match",
> + test_precopy_tcp_tls_psk_match);
...including use of the function.
> + qtest_add_func("/migration/precopy/tcp/tls/psk/mismatch",
> + test_precopy_tcp_tls_psk_mismatch);
> +#endif /* CONFIG_GNUTLS */
> +
With the missing #ifdef added in,
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 4/9] tests: add migration tests of TLS with x509 credentials
2022-04-26 16:00 ` [PATCH v3 4/9] tests: add migration tests of TLS with x509 credentials Daniel P. Berrangé
@ 2022-04-28 13:59 ` Eric Blake
0 siblings, 0 replies; 19+ messages in thread
From: Eric Blake @ 2022-04-28 13:59 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: Laurent Vivier, Thomas Huth, Juan Quintela, qemu-devel, Peter Xu,
Dr. David Alan Gilbert, Paolo Bonzini
On Tue, Apr 26, 2022 at 05:00:43PM +0100, Daniel P. Berrangé wrote:
> This validates that we correctly handle migration success and failure
> scenarios when using TLS with x509 certificates. There are quite a few
> different scenarios that matter in relation to hostname validation.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> meson.build | 1 +
> tests/qtest/meson.build | 5 +
> tests/qtest/migration-test.c | 382 ++++++++++++++++++++++++++++++++++-
> 3 files changed, 386 insertions(+), 2 deletions(-)
>
> +
> +#ifdef CONFIG_TASN1
> +typedef struct {
> + char *workdir;
> + char *keyfile;
> + char *cacert;
> + char *servercert;
> + char *serverkey;
> + char *clientcert;
> + char *clientkey;
> +} TestMigrateTLSX509Data;
> +
> +typedef struct {
> + bool verifyclient;
> + bool clientcert;
> + bool hostileclient;
> + bool authzclient;
> + const char *certhostname;
> + const char *certipaddr;
> +} TestMigrateTLSX509;
> +
> +static void *
> +test_migrate_tls_x509_start_common(QTestState *from,
> + QTestState *to,
> + TestMigrateTLSX509 *args)
> +{
> + TestMigrateTLSX509Data *data = g_new0(TestMigrateTLSX509Data, 1);
> + QDict *rsp;
> +
> + data->workdir = g_strdup_printf("%s/tlscredsx5090", tmpfs);
> + data->keyfile = g_strdup_printf("%s/key.pem", data->workdir);
> +
> + data->cacert = g_strdup_printf("%s/ca-cert.pem", data->workdir);
> + data->serverkey = g_strdup_printf("%s/server-key.pem", data->workdir);
> + data->servercert = g_strdup_printf("%s/server-cert.pem", data->workdir);
> + if (args->clientcert) {
> + data->clientkey = g_strdup_printf("%s/client-key.pem", data->workdir);
> + data->clientcert = g_strdup_printf("%s/client-cert.pem", data->workdir);
> + }
> +
> + mkdir(data->workdir, 0700);
> +
> + test_tls_init(data->keyfile);
> + g_assert(link(data->keyfile, data->serverkey) == 0);
Is relying on side effects inside a g_assert() wise? But not the
first time our testsuite has done it, so I guess you're okay.
> + if (args->clientcert) {
> + g_assert(link(data->keyfile, data->clientkey) == 0);
> + }
> +
> + TLS_ROOT_REQ_SIMPLE(cacertreq, data->cacert);
> + if (args->clientcert) {
> + TLS_CERT_REQ_SIMPLE_CLIENT(servercertreq, cacertreq,
> + args->hostileclient ?
> + QCRYPTO_TLS_TEST_CLIENT_HOSTILE_NAME :
> + QCRYPTO_TLS_TEST_CLIENT_NAME,
> + data->clientcert);
> + }
> +
> + TLS_CERT_REQ_SIMPLE_SERVER(clientcertreq, cacertreq,
> + data->servercert,
> + args->certhostname,
> + args->certipaddr);
The new macros from 2/9 came in handy.
> +
> +/*
> + * The normal case: match server's cert hostname against
> + * whatever host we were telling QEMU to connect to (if any)
> + */
> +static void *
> +test_migrate_tls_x509_start_default_host(QTestState *from,
> + QTestState *to)
> +{
> + TestMigrateTLSX509 args = {
> + .verifyclient = true,
> + .clientcert = true,
> + .certipaddr = "127.0.0.1"
> + };
> + return test_migrate_tls_x509_start_common(from, to, &args);
> +}
> +
> +/*
> + * The unusual case: the server's cert is different from
> + * the address we're telling QEMU to connect to (if any),
> + * so we must give QEMU an explicit hostname to validate
> + */
> +static void *
> +test_migrate_tls_x509_start_override_host(QTestState *from,
> + QTestState *to)
Useful comments, and good coverage of a number of cases.
> +{
> + TestMigrateTLSX509 args = {
> + .verifyclient = true,
> + .clientcert = true,
> + .certhostname = "qemu.org",
> + };
> + return test_migrate_tls_x509_start_common(from, to, &args);
> +}
> +
> +/*
> + * The unusual case: the server's cert is different from
> + * the address we're telling QEMU to connect to, and so we
> + * expect the client to reject the server
> + */
> +static void *
> +test_migrate_tls_x509_start_mismatch_host(QTestState *from,
> + QTestState *to)
> +{
> + TestMigrateTLSX509 args = {
> + .verifyclient = true,
> + .clientcert = true,
> + .certipaddr = "10.0.0.1",
> + };
> + return test_migrate_tls_x509_start_common(from, to, &args);
> +}
> +
> +static void *
> +test_migrate_tls_x509_start_friendly_client(QTestState *from,
> + QTestState *to)
> +{
> + TestMigrateTLSX509 args = {
> + .verifyclient = true,
> + .clientcert = true,
> + .authzclient = true,
> + .certipaddr = "127.0.0.1",
> + };
> + return test_migrate_tls_x509_start_common(from, to, &args);
> +}
> +
> +static void *
> +test_migrate_tls_x509_start_hostile_client(QTestState *from,
> + QTestState *to)
Worth a comment on these two?
> @@ -1020,6 +1251,21 @@ static void test_precopy_unix_plain(void)
> test_precopy_common(&args);
> }
>
> +static void test_precopy_unix_dirty_ring(void)
> +{
> + g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
> + MigrateCommon args = {
> + .start = {
> + .use_dirty_ring = true,
> + },
> + .listen_uri = uri,
> + .connect_uri = uri,
> + };
> +
> + test_precopy_common(&args);
> +}
> +
Looks like unintended code motion? Should this be squashed into 3/9...
> +#ifdef CONFIG_GNUTLS
> static void test_precopy_unix_tls_psk(void)
...especially since this is fixing the missing #ifdef I pointed out there?
> {
> g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
> @@ -1033,19 +1279,38 @@ static void test_precopy_unix_tls_psk(void)
> test_precopy_common(&args);
> }
>
> -static void test_precopy_unix_dirty_ring(void)
> +#ifdef CONFIG_TASN1
> +static void test_precopy_unix_tls_x509_default_host(void)
> {
> g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
> MigrateCommon args = {
> .start = {
> - .use_dirty_ring = true,
> + .hide_stderr = true,
> },
> + .connect_uri = uri,
> .listen_uri = uri,
> + .start_hook = test_migrate_tls_x509_start_default_host,
> + .finish_hook = test_migrate_tls_x509_finish,
> + .result = MIG_TEST_FAIL_DEST_QUIT_ERR,
> + };
The code motion mixed in made this a bit harder to read.
> +
> + test_precopy_common(&args);
> +}
> +
> +static void test_precopy_unix_tls_x509_override_host(void)
> +{
> + g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
> + MigrateCommon args = {
> .connect_uri = uri,
> + .listen_uri = uri,
> + .start_hook = test_migrate_tls_x509_start_override_host,
> + .finish_hook = test_migrate_tls_x509_finish,
> };
>
> test_precopy_common(&args);
> }
> +#endif /* CONFIG_TASN1 */
> +#endif /* CONFIG_GNUTLS */
>
> #if 0
> /* Currently upset on aarch64 TCG */
> @@ -1172,6 +1437,97 @@ static void test_precopy_tcp_tls_psk_mismatch(void)
>
> test_precopy_common(&args);
> }
> +
> +#ifdef CONFIG_TASN1
> +static void test_precopy_tcp_tls_x509_default_host(void)
> +static void test_precopy_tcp_tls_x509_reject_anon_client(void)
> +{
> + MigrateCommon args = {
> + .start = {
> + .hide_stderr = true,
> + },
> + .listen_uri = "tcp:127.0.0.1:0",
> + .start_hook = test_migrate_tls_x509_start_reject_anon_client,
> + .finish_hook = test_migrate_tls_x509_finish,
> + .result = MIG_TEST_FAIL,
> + };
> +
> + test_precopy_common(&args);
> +}
> +#endif /* CONFIG_TASN1 */
> #endif /* CONFIG_GNUTLS */
>
> static void *test_migrate_fd_start_hook(QTestState *from,
> @@ -1640,6 +1996,12 @@ int main(int argc, char **argv)
> #ifdef CONFIG_GNUTLS
> qtest_add_func("/migration/precopy/unix/tls/psk",
> test_precopy_unix_tls_psk);
> +#ifdef CONFIG_TASN1
> + qtest_add_func("/migration/precopy/unix/tls/x509/default-host",
> + test_precopy_unix_tls_x509_default_host);
> + qtest_add_func("/migration/precopy/unix/tls/x509/override-host",
> + test_precopy_unix_tls_x509_override_host);
> +#endif /* CONFIG_TASN1 */
> #endif /* CONFIG_GNUTLS */
>
> qtest_add_func("/migration/precopy/tcp/plain", test_precopy_tcp_plain);
> @@ -1648,6 +2010,22 @@ int main(int argc, char **argv)
> test_precopy_tcp_tls_psk_match);
> qtest_add_func("/migration/precopy/tcp/tls/psk/mismatch",
> test_precopy_tcp_tls_psk_mismatch);
> +#ifdef CONFIG_TASN1
> + qtest_add_func("/migration/precopy/tcp/tls/x509/default-host",
> + test_precopy_tcp_tls_x509_default_host);
> + qtest_add_func("/migration/precopy/tcp/tls/x509/override-host",
> + test_precopy_tcp_tls_x509_override_host);
> + qtest_add_func("/migration/precopy/tcp/tls/x509/mismatch-host",
> + test_precopy_tcp_tls_x509_mismatch_host);
> + qtest_add_func("/migration/precopy/tcp/tls/x509/friendly-client",
> + test_precopy_tcp_tls_x509_friendly_client);
> + qtest_add_func("/migration/precopy/tcp/tls/x509/hostile-client",
> + test_precopy_tcp_tls_x509_hostile_client);
> + qtest_add_func("/migration/precopy/tcp/tls/x509/allow-anon-client",
> + test_precopy_tcp_tls_x509_allow_anon_client);
> + qtest_add_func("/migration/precopy/tcp/tls/x509/reject-anon-client",
> + test_precopy_tcp_tls_x509_reject_anon_client);
> +#endif /* CONFIG_TASN1 */
> #endif /* CONFIG_GNUTLS */
Other than the potential mess with having to rebase the code motion of
test_precopy_unix_dirty_ring when fixing the #ifdef in 3, it looks
like these test additions are good.
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 7/9] tests: add multifd migration tests of TLS with PSK credentials
2022-04-26 16:00 ` [PATCH v3 7/9] tests: add multifd migration tests of TLS with PSK credentials Daniel P. Berrangé
@ 2022-04-28 14:05 ` Eric Blake
0 siblings, 0 replies; 19+ messages in thread
From: Eric Blake @ 2022-04-28 14:05 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: Laurent Vivier, Thomas Huth, Juan Quintela, qemu-devel, Peter Xu,
Dr. David Alan Gilbert, Paolo Bonzini
On Tue, Apr 26, 2022 at 05:00:46PM +0100, Daniel P. Berrangé wrote:
> This validates that we correctly handle multifd migration success
> and failure scenarios when using TLS with pre shared keys.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> tests/qtest/migration-test.c | 60 +++++++++++++++++++++++++++++++++---
> 1 file changed, 56 insertions(+), 4 deletions(-)
>
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 8/9] tests: add multifd migration tests of TLS with x509 credentials
2022-04-26 16:00 ` [PATCH v3 8/9] tests: add multifd migration tests of TLS with x509 credentials Daniel P. Berrangé
@ 2022-04-28 14:13 ` Eric Blake
0 siblings, 0 replies; 19+ messages in thread
From: Eric Blake @ 2022-04-28 14:13 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: Laurent Vivier, Thomas Huth, Juan Quintela, qemu-devel, Peter Xu,
Dr. David Alan Gilbert, Paolo Bonzini
On Tue, Apr 26, 2022 at 05:00:47PM +0100, Daniel P. Berrangé wrote:
> This validates that we correctly handle multifd migration success
> and failure scenarios when using TLS with x509 certificates. There
> are quite a few different scenarios that matter in relation to
> hostname validation, but we skip a couple as we can assume that
> the non-multifd coverage applies to some extent.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> tests/qtest/migration-test.c | 127 +++++++++++++++++++++++++++++++++++
> 1 file changed, 127 insertions(+)
>
> +
> +static void test_multifd_tcp_tls_x509_mismatch_host(void)
> +{
> + /*
> + * This has different behaviour to the non-multifd case.
> + *
> + * In non-multifd case when client aborts due to mismatched
> + * cert host, the server has already started trying to load
> + * migration state, and so it exits with I/O failure.
odd double space
> + *
> + * In multifd case when client aborts due to mismatched
> + * cert host, the server is still waiting for the other
> + * multifd connections to arrive so hasn't started trying
> + * to load migration state, and thus just aborts the migration
> + * without exiting
Worth a trailing .
> + */
> + MigrateCommon args = {
> + .start = {
> + .hide_stderr = true,
> + },
> + .listen_uri = "defer",
> + .start_hook = test_migrate_multifd_tls_x509_start_mismatch_host,
> + .finish_hook = test_migrate_tls_x509_finish,
> + .result = MIG_TEST_FAIL,
> + };
> + test_precopy_common(&args);
> +}
Definitely a good example of why this was worth testing, and the
comment explains why the difference in observed failure scenarios is
good.
Comment fixes are trivial, so
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 3/9] tests: add migration tests of TLS with PSK credentials
2022-04-28 13:46 ` Eric Blake
@ 2022-05-09 13:29 ` Dr. David Alan Gilbert
0 siblings, 0 replies; 19+ messages in thread
From: Dr. David Alan Gilbert @ 2022-05-09 13:29 UTC (permalink / raw)
To: Eric Blake
Cc: Daniel P. Berrangé,
qemu-devel, Laurent Vivier, Thomas Huth, Juan Quintela, Peter Xu,
Paolo Bonzini
* Eric Blake (eblake@redhat.com) wrote:
> On Tue, Apr 26, 2022 at 05:00:42PM +0100, Daniel P. Berrangé wrote:
> > This validates that we correctly handle migration success and failure
> > scenarios when using TLS with pre shared keys.
> >
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> > tests/qtest/meson.build | 7 +-
> > tests/qtest/migration-test.c | 159 +++++++++++++++++++++++++++-
> > tests/unit/crypto-tls-psk-helpers.c | 18 +++-
> > tests/unit/crypto-tls-psk-helpers.h | 1 +
> > 4 files changed, 177 insertions(+), 8 deletions(-)
> >
>
> >
> > -static void test_precopy_unix(void)
> > +static void test_precopy_unix_plain(void)
> > {
> > g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
> > MigrateCommon args = {
> > @@ -922,6 +1020,19 @@ static void test_precopy_unix(void)
> > test_precopy_common(&args);
> > }
> >
> > +static void test_precopy_unix_tls_psk(void)
> > +{
>
> This function is unguarded...
>
> > + g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
> > + MigrateCommon args = {
> > + .connect_uri = uri,
> > + .listen_uri = uri,
> > + .start_hook = test_migrate_tls_psk_start_match,
> > + .finish_hook = test_migrate_tls_psk_finish,
> > + };
> > +
> > + test_precopy_common(&args);
> > +}
> > +
> > static void test_precopy_unix_dirty_ring(void)
> > {
> > g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
> > @@ -1026,7 +1137,7 @@ static void test_xbzrle_unix(void)
> > test_xbzrle(uri);
> > }
> >
> > -static void test_precopy_tcp(void)
> > +static void test_precopy_tcp_plain(void)
> > {
> > MigrateCommon args = {
> > .listen_uri = "tcp:127.0.0.1:0",
> > @@ -1035,6 +1146,34 @@ static void test_precopy_tcp(void)
> > test_precopy_common(&args);
> > }
> >
> > +#ifdef CONFIG_GNUTLS
> > +static void test_precopy_tcp_tls_psk_match(void)
>
> ...while everything else is #ifdef CONFIG_GNUTLS...
>
> > static void *test_migrate_fd_start_hook(QTestState *from,
> > QTestState *to)
> > {
> > @@ -1497,8 +1636,20 @@ int main(int argc, char **argv)
> > qtest_add_func("/migration/postcopy/unix", test_postcopy);
> > qtest_add_func("/migration/postcopy/recovery", test_postcopy_recovery);
> > qtest_add_func("/migration/bad_dest", test_baddest);
> > - qtest_add_func("/migration/precopy/unix", test_precopy_unix);
> > - qtest_add_func("/migration/precopy/tcp", test_precopy_tcp);
> > + qtest_add_func("/migration/precopy/unix/plain", test_precopy_unix_plain);
> > +#ifdef CONFIG_GNUTLS
> > + qtest_add_func("/migration/precopy/unix/tls/psk",
> > + test_precopy_unix_tls_psk);
> > +#endif /* CONFIG_GNUTLS */
> > +
> > + qtest_add_func("/migration/precopy/tcp/plain", test_precopy_tcp_plain);
> > +#ifdef CONFIG_GNUTLS
> > + qtest_add_func("/migration/precopy/tcp/tls/psk/match",
> > + test_precopy_tcp_tls_psk_match);
>
> ...including use of the function.
>
> > + qtest_add_func("/migration/precopy/tcp/tls/psk/mismatch",
> > + test_precopy_tcp_tls_psk_mismatch);
> > +#endif /* CONFIG_GNUTLS */
> > +
>
> With the missing #ifdef added in,
OK, ifdef added in queuing
> Reviewed-by: Eric Blake <eblake@redhat.com>
>
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc. +1-919-301-3266
> Virtualization: qemu.org | libvirt.org
>
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 0/9] tests: introduce testing coverage for TLS with migration
2022-04-26 16:00 [PATCH v3 0/9] tests: introduce testing coverage for TLS with migration Daniel P. Berrangé
` (9 preceding siblings ...)
2022-04-28 11:40 ` [PATCH v3 0/9] tests: introduce testing coverage for TLS with migration Dr. David Alan Gilbert
@ 2022-05-09 14:11 ` Dr. David Alan Gilbert
10 siblings, 0 replies; 19+ messages in thread
From: Dr. David Alan Gilbert @ 2022-05-09 14:11 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: qemu-devel, Peter Xu, Juan Quintela, Thomas Huth, Paolo Bonzini,
Laurent Vivier
* Daniel P. Berrangé (berrange@redhat.com) wrote:
> This significantly expands the migration test suite to cover testing
> with TLS over TCP and UNIX sockets, with both PSK (pre shared keys)
> and x509 credentials, and for both single and multifd scenarios.
>
> It identified one bug in handling PSK credentials with UNIX sockets,
> but other than that everything was operating as expected.
>
> To minimize the impact on code duplication alopt of refactoring is
> done of the migration tests to introduce a common helper for running
> the migration process. The various tests mostly just have to provide
> a callback to set a few parameters/capabilities before migration
> starts, and sometimes a callback to cleanup or validate after
> completion/failure.
Full set now queued again
> Changed in v3:
>
> - Trivial rebase dropping already merged patches
>
> Changed in v2:
>
> - Use structs to pass around most parameters
> - Hide expected errors from stderr
>
> Daniel P. Berrangé (9):
> tests: fix encoding of IP addresses in x509 certs
> tests: add more helper macros for creating TLS x509 certs
> tests: add migration tests of TLS with PSK credentials
> tests: add migration tests of TLS with x509 credentials
> tests: convert XBZRLE migration test to use common helper
> tests: convert multifd migration tests to use common helper
> tests: add multifd migration tests of TLS with PSK credentials
> tests: add multifd migration tests of TLS with x509 credentials
> tests: ensure migration status isn't reported as failed
>
> meson.build | 1 +
> tests/qtest/meson.build | 12 +-
> tests/qtest/migration-helpers.c | 13 +
> tests/qtest/migration-helpers.h | 1 +
> tests/qtest/migration-test.c | 866 ++++++++++++++++++++++++---
> tests/unit/crypto-tls-psk-helpers.c | 18 +-
> tests/unit/crypto-tls-psk-helpers.h | 1 +
> tests/unit/crypto-tls-x509-helpers.c | 16 +-
> tests/unit/crypto-tls-x509-helpers.h | 53 ++
> tests/unit/test-crypto-tlssession.c | 11 +-
> 10 files changed, 897 insertions(+), 95 deletions(-)
>
> --
> 2.35.1
>
>
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2022-05-09 14:13 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-26 16:00 [PATCH v3 0/9] tests: introduce testing coverage for TLS with migration Daniel P. Berrangé
2022-04-26 16:00 ` [PATCH v3 1/9] tests: fix encoding of IP addresses in x509 certs Daniel P. Berrangé
2022-04-28 9:46 ` Dr. David Alan Gilbert
2022-04-26 16:00 ` [PATCH v3 2/9] tests: add more helper macros for creating TLS " Daniel P. Berrangé
2022-04-28 13:09 ` Eric Blake
2022-04-26 16:00 ` [PATCH v3 3/9] tests: add migration tests of TLS with PSK credentials Daniel P. Berrangé
2022-04-28 13:46 ` Eric Blake
2022-05-09 13:29 ` Dr. David Alan Gilbert
2022-04-26 16:00 ` [PATCH v3 4/9] tests: add migration tests of TLS with x509 credentials Daniel P. Berrangé
2022-04-28 13:59 ` Eric Blake
2022-04-26 16:00 ` [PATCH v3 5/9] tests: convert XBZRLE migration test to use common helper Daniel P. Berrangé
2022-04-26 16:00 ` [PATCH v3 6/9] tests: convert multifd migration tests " Daniel P. Berrangé
2022-04-26 16:00 ` [PATCH v3 7/9] tests: add multifd migration tests of TLS with PSK credentials Daniel P. Berrangé
2022-04-28 14:05 ` Eric Blake
2022-04-26 16:00 ` [PATCH v3 8/9] tests: add multifd migration tests of TLS with x509 credentials Daniel P. Berrangé
2022-04-28 14:13 ` Eric Blake
2022-04-26 16:00 ` [PATCH v3 9/9] tests: ensure migration status isn't reported as failed Daniel P. Berrangé
2022-04-28 11:40 ` [PATCH v3 0/9] tests: introduce testing coverage for TLS with migration Dr. David Alan Gilbert
2022-05-09 14:11 ` Dr. David Alan Gilbert
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.