diff mbox

[v6,for-2.7,25/28] migration: define 'tls-creds' and 'tls-hostname' migration parameters

Message ID 1461751518-12128-26-git-send-email-berrange@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Daniel P. Berrangé April 27, 2016, 10:05 a.m. UTC
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.

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 hmp-commands.hx       |  2 +-
 hmp.c                 | 44 ++++++++++++++++++++++++++++++++------
 migration/migration.c | 14 +++++++++++++
 qapi-schema.json      | 58 ++++++++++++++++++++++++++++++++++++++++++++++++---
 4 files changed, 108 insertions(+), 10 deletions(-)

Comments

Amit Shah May 25, 2016, 11:53 a.m. UTC | #1
On (Wed) 27 Apr 2016 [11:05:15], Daniel P. Berrange 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.
> 
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

> diff --git a/qapi-schema.json b/qapi-schema.json
> index 9aa14b4..12be303 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)

All these need to be "Since 2.7"

I've updated these in my branch, no respin required for this.

		Amit
diff mbox

Patch

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 45cfec1..77ab6c6 100644
--- a/hmp.c
+++ b/hmp.c
@@ -294,6 +294,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");
     }
 
@@ -1243,13 +1249,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++) {
@@ -1257,25 +1267,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;
+                }
             }
-            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;
         }
@@ -1285,6 +1316,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 74fcdb5..c946946 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 9aa14b4..12be303 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
 #