[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v5 25/28] migration: define 'tls-creds' and 'tls
From: |
Dr. David Alan Gilbert |
Subject: |
Re: [Qemu-devel] [PATCH v5 25/28] migration: define 'tls-creds' and 'tls-hostname' migration parameters |
Date: |
Fri, 18 Mar 2016 17:03:58 +0000 |
User-agent: |
Mutt/1.5.24 (2015-08-30) |
* Daniel P. Berrange (address@hidden) wrote:
> Define two new migration parameters to be used with TLS encryption.
> The 'tls-creds' parameter provides the ID of an instance of the
> 'tls-creds' object type, or rather a subclass such as 'tls-creds-x509'.
> Providing these credentials will enable use of TLS on the migration
> data stream.
>
> If using x509 certificates, together with a migration URI that does
> not include a hostname, the 'tls-hostname' parameter provides the
> hostname to use when verifying the server's x509 certificate. This
> allows TLS to be used in combination with fd: and exec: protocols
> where a TCP connection is established by a 3rd party outside of
> QEMU.
>
> NB, this requires changing the migrate_set_parameter method in the
> HMP to accept a 's' (string) value instead of 'i' (integer). This
> is backwards compatible, because the parsing of strings allows the
> quotes to be optional, thus any integer is also a valid string.
>
> Signed-off-by: Daniel P. Berrange <address@hidden>
> ---
> hmp-commands.hx | 2 +-
> hmp.c | 44 ++++++++++++++++++++++++++++++++------
> migration/migration.c | 14 +++++++++++++
> qapi-schema.json | 58
> ++++++++++++++++++++++++++++++++++++++++++++++++---
> 4 files changed, 108 insertions(+), 10 deletions(-)
>
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 4f4f60a..98b4b1a 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1008,7 +1008,7 @@ ETEXI
>
> {
> .name = "migrate_set_parameter",
> - .args_type = "parameter:s,value:i",
> + .args_type = "parameter:s,value:s",
> .params = "parameter value",
> .help = "Set the parameter for migration",
> .mhandler.cmd = hmp_migrate_set_parameter,
> diff --git a/hmp.c b/hmp.c
> index 7126f17..8ace9c2 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -293,6 +293,12 @@ void hmp_info_migrate_parameters(Monitor *mon, const
> QDict *qdict)
> monitor_printf(mon, " %s: %" PRId64,
>
> MigrationParameter_lookup[MIGRATION_PARAMETER_X_CPU_THROTTLE_INCREMENT],
> params->x_cpu_throttle_increment);
> + monitor_printf(mon, " %s: '%s'",
> + MigrationParameter_lookup[MIGRATION_PARAMETER_TLS_CREDS],
> + params->tls_creds ? : "");
> + monitor_printf(mon, " %s: '%s'",
> + MigrationParameter_lookup[MIGRATION_PARAMETER_TLS_HOSTNAME],
> + params->tls_hostname ? : "");
> monitor_printf(mon, "\n");
> }
>
> @@ -1242,13 +1248,17 @@ void hmp_migrate_set_capability(Monitor *mon, const
> QDict *qdict)
> void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
> {
> const char *param = qdict_get_str(qdict, "parameter");
> - int value = qdict_get_int(qdict, "value");
> + const char *valuestr = qdict_get_str(qdict, "value");
> + long valueint = 0;
> Error *err = NULL;
> bool has_compress_level = false;
> bool has_compress_threads = false;
> bool has_decompress_threads = false;
> bool has_x_cpu_throttle_initial = false;
> bool has_x_cpu_throttle_increment = false;
> + bool has_tls_creds = false;
> + bool has_tls_hostname = false;
> + bool use_int_value = false;
> int i;
>
> for (i = 0; i < MIGRATION_PARAMETER__MAX; i++) {
> @@ -1256,25 +1266,46 @@ void hmp_migrate_set_parameter(Monitor *mon, const
> QDict *qdict)
> switch (i) {
> case MIGRATION_PARAMETER_COMPRESS_LEVEL:
> has_compress_level = true;
> + use_int_value = true;
> break;
> case MIGRATION_PARAMETER_COMPRESS_THREADS:
> has_compress_threads = true;
> + use_int_value = true;
> break;
> case MIGRATION_PARAMETER_DECOMPRESS_THREADS:
> has_decompress_threads = true;
> + use_int_value = true;
> break;
> case MIGRATION_PARAMETER_X_CPU_THROTTLE_INITIAL:
> has_x_cpu_throttle_initial = true;
> + use_int_value = true;
> break;
> case MIGRATION_PARAMETER_X_CPU_THROTTLE_INCREMENT:
> has_x_cpu_throttle_increment = true;
> break;
> + case MIGRATION_PARAMETER_TLS_CREDS:
> + has_tls_creds = true;
> + break;
> + case MIGRATION_PARAMETER_TLS_HOSTNAME:
> + has_tls_hostname = true;
> + break;
> + }
> +
> + if (use_int_value) {
> + if (qemu_strtol(valuestr, NULL, 10, &valueint) < 0) {
> + error_setg(&err, "Unable to parse '%s' as an int",
> + valuestr);
> + goto cleanup;
> + }
OK, yes, I might have suggested qemu_strtosz_suffix instead which would avoid
use of a long, but as far as I can tell the 'i' on monitor only
promised a 32bit int anyway.
But,
Reviewed-by: Dr. David Alan Gilbert <address@hidden>
Dave
> }
> - qmp_migrate_set_parameters(has_compress_level, value,
> - has_compress_threads, value,
> - has_decompress_threads, value,
> - has_x_cpu_throttle_initial, value,
> - has_x_cpu_throttle_increment, value,
> +
> + qmp_migrate_set_parameters(has_compress_level, valueint,
> + has_compress_threads, valueint,
> + has_decompress_threads, valueint,
> + has_x_cpu_throttle_initial, valueint,
> + has_x_cpu_throttle_increment,
> valueint,
> + has_tls_creds, valuestr,
> + has_tls_hostname, valuestr,
> &err);
> break;
> }
> @@ -1284,6 +1315,7 @@ void hmp_migrate_set_parameter(Monitor *mon, const
> QDict *qdict)
> error_setg(&err, QERR_INVALID_PARAMETER, param);
> }
>
> + cleanup:
> if (err) {
> error_report_err(err);
> }
> diff --git a/migration/migration.c b/migration/migration.c
> index 7a06111..a2b0fb5 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -537,6 +537,8 @@ MigrationParameters *qmp_query_migrate_parameters(Error
> **errp)
> params->decompress_threads = s->parameters.decompress_threads;
> params->x_cpu_throttle_initial = s->parameters.x_cpu_throttle_initial;
> params->x_cpu_throttle_increment =
> s->parameters.x_cpu_throttle_increment;
> + params->tls_creds = g_strdup(s->parameters.tls_creds);
> + params->tls_hostname = g_strdup(s->parameters.tls_hostname);
>
> return params;
> }
> @@ -738,6 +740,10 @@ void qmp_migrate_set_parameters(bool has_compress_level,
> int64_t x_cpu_throttle_initial,
> bool has_x_cpu_throttle_increment,
> int64_t x_cpu_throttle_increment,
> + bool has_tls_creds,
> + const char *tls_creds,
> + bool has_tls_hostname,
> + const char *tls_hostname,
> Error **errp)
> {
> MigrationState *s = migrate_get_current();
> @@ -789,6 +795,14 @@ void qmp_migrate_set_parameters(bool has_compress_level,
> if (has_x_cpu_throttle_increment) {
> s->parameters.x_cpu_throttle_increment = x_cpu_throttle_increment;
> }
> + if (has_tls_creds) {
> + g_free(s->parameters.tls_creds);
> + s->parameters.tls_creds = g_strdup(tls_creds);
> + }
> + if (has_tls_hostname) {
> + g_free(s->parameters.tls_hostname);
> + s->parameters.tls_hostname = g_strdup(tls_hostname);
> + }
> }
>
>
> diff --git a/qapi-schema.json b/qapi-schema.json
> index a3fe948..604171d 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -617,11 +617,28 @@
> # @x-cpu-throttle-increment: throttle percentage increase each time
> # auto-converge detects that migration is not
> making
> # progress. The default value is 10. (Since 2.5)
> +#
> +# @tls-creds: ID of the 'tls-creds' object that provides credentials for
> +# establishing a TLS connection over the migration data channel.
> +# On the outgoing side of the migration, the credentials must
> +# be for a 'client' endpoint, while for the incoming side the
> +# credentials must be for a 'server' endpoint. Setting this
> +# will enable TLS for all migrations. The default is unset,
> +# resulting in unsecured migration at the QEMU level. (Since 2.6)
> +#
> +# @tls-hostname: hostname of the target host for the migration. This is
> +# required when using x509 based TLS credentials and the
> +# migration URI does not already include a hostname. For
> +# example if using fd: or exec: based migration, the
> +# hostname must be provided so that the server's x509
> +# certificate identity canbe validated. (Since 2.6)
> +#
> # Since: 2.4
> ##
> { 'enum': 'MigrationParameter',
> 'data': ['compress-level', 'compress-threads', 'decompress-threads',
> - 'x-cpu-throttle-initial', 'x-cpu-throttle-increment'] }
> + 'x-cpu-throttle-initial', 'x-cpu-throttle-increment',
> + 'tls-creds', 'tls-hostname'] }
>
> #
> # @migrate-set-parameters
> @@ -641,6 +658,22 @@
> # @x-cpu-throttle-increment: throttle percentage increase each time
> # auto-converge detects that migration is not
> making
> # progress. The default value is 10. (Since 2.5)
> +#
> +# @tls-creds: ID of the 'tls-creds' object that provides credentials for
> +# establishing a TLS connection over the migration data channel.
> +# On the outgoing side of the migration, the credentials must
> +# be for a 'client' endpoint, while for the incoming side the
> +# credentials must be for a 'server' endpoint. Setting this
> +# will enable TLS for all migrations. The default is unset,
> +# resulting in unsecured migration at the QEMU level. (Since 2.6)
> +#
> +# @tls-hostname: hostname of the target host for the migration. This is
> +# required when using x509 based TLS credentials and the
> +# migration URI does not already include a hostname. For
> +# example if using fd: or exec: based migration, the
> +# hostname must be provided so that the server's x509
> +# certificate identity canbe validated. (Since 2.6)
> +#
> # Since: 2.4
> ##
> { 'command': 'migrate-set-parameters',
> @@ -648,7 +681,9 @@
> '*compress-threads': 'int',
> '*decompress-threads': 'int',
> '*x-cpu-throttle-initial': 'int',
> - '*x-cpu-throttle-increment': 'int'} }
> + '*x-cpu-throttle-increment': 'int',
> + '*tls-creds': 'str',
> + '*tls-hostname': 'str'} }
>
> #
> # @MigrationParameters
> @@ -667,6 +702,21 @@
> # auto-converge detects that migration is not
> making
> # progress. The default value is 10. (Since 2.5)
> #
> +# @tls-creds: ID of the 'tls-creds' object that provides credentials for
> +# establishing a TLS connection over the migration data channel.
> +# On the outgoing side of the migration, the credentials must
> +# be for a 'client' endpoint, while for the incoming side the
> +# credentials must be for a 'server' endpoint. Setting this
> +# will enable TLS for all migrations. The default is unset,
> +# resulting in unsecured migration at the QEMU level. (Since 2.6)
> +#
> +# @tls-hostname: hostname of the target host for the migration. This is
> +# required when using x509 based TLS credentials and the
> +# migration URI does not already include a hostname. For
> +# example if using fd: or exec: based migration, the
> +# hostname must be provided so that the server's x509
> +# certificate identity canbe validated. (Since 2.6)
> +#
> # Since: 2.4
> ##
> { 'struct': 'MigrationParameters',
> @@ -674,7 +724,9 @@
> 'compress-threads': 'int',
> 'decompress-threads': 'int',
> 'x-cpu-throttle-initial': 'int',
> - 'x-cpu-throttle-increment': 'int'} }
> + 'x-cpu-throttle-increment': 'int',
> + 'tls-creds': 'str',
> + 'tls-hostname': 'str'} }
> ##
> # @query-migrate-parameters
> #
> --
> 2.5.0
>
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK
- [Qemu-devel] [PATCH v5 14/28] migration: convert tcp socket protocol to use QIOChannel, (continued)
- [Qemu-devel] [PATCH v5 14/28] migration: convert tcp socket protocol to use QIOChannel, Daniel P. Berrange, 2016/03/18
- [Qemu-devel] [PATCH v5 18/28] migration: convert savevm to use QIOChannel for writing to files, Daniel P. Berrange, 2016/03/18
- [Qemu-devel] [PATCH v5 17/28] migration: convert RDMA to use QIOChannel interface, Daniel P. Berrange, 2016/03/18
- [Qemu-devel] [PATCH v5 19/28] migration: delete QEMUFile buffer implementation, Daniel P. Berrange, 2016/03/18
- [Qemu-devel] [PATCH v5 21/28] migration: delete QEMUFile sockets implementation, Daniel P. Berrange, 2016/03/18
- [Qemu-devel] [PATCH v5 20/28] migration: delete QEMUSizedBuffer struct, Daniel P. Berrange, 2016/03/18
- [Qemu-devel] [PATCH v5 23/28] migration: move definition of struct QEMUFile back into qemu-file.c, Daniel P. Berrange, 2016/03/18
- [Qemu-devel] [PATCH v5 22/28] migration: delete QEMUFile stdio implementation, Daniel P. Berrange, 2016/03/18
- [Qemu-devel] [PATCH v5 24/28] migration: don't use an array for storing migrate parameters, Daniel P. Berrange, 2016/03/18
- [Qemu-devel] [PATCH v5 25/28] migration: define 'tls-creds' and 'tls-hostname' migration parameters, Daniel P. Berrange, 2016/03/18
- Re: [Qemu-devel] [PATCH v5 25/28] migration: define 'tls-creds' and 'tls-hostname' migration parameters,
Dr. David Alan Gilbert <=
- [Qemu-devel] [PATCH v5 28/28] migration: remove qemu_get_fd method from QEMUFile, Daniel P. Berrange, 2016/03/18
- [Qemu-devel] [PATCH v5 26/28] migration: add support for encrypting data with TLS, Daniel P. Berrange, 2016/03/18
- [Qemu-devel] [PATCH v5 27/28] migration: remove support for non-iovec based write handlers, Daniel P. Berrange, 2016/03/18