[Libvirt-cim] [PATCH V3 11/11] allow ssh based migration with non root's key file
Wenchao Xia
xiawenc at linux.vnet.ibm.com
Tue Dec 18 10:17:40 UTC 2012
This patch allow libvirt-cim to use non-root's ssh key in migration
to avoid exposing root's ssh login on server. In some case server are
forbidden to expose or provide any root ssh login, and still use ssh
encryption between two migration nodes with key of special account
created for virtual machine management.
Usage:
1 set temp file in libvirt-cim.conf or not:
migrate_ssh_temp_key=[DIR]/[FILE]
Note that [DIR] must be full owned by root user and exist, otherwise
libvirt will reject the request later. For example set it to
/root/VControll_id_rsa
If it is set, it tells libvirt-cim a copy is needed to make keyfile
in a directory owned by root. If not set, that means keyfile is already
in a valid place can be used directly.
2 call migrate API with ssh destination as before, but add one
additonal parameter for example:
SSH_Key="/home/VController/.ssh/id_rsa"
Then that key would be used intead of default root's ssh key.
Details:
This patch contains two parts, first is adding common code to
read config include non-root ssh migration configuration,
second is migration logic code.
For first part, now config reading is done only once instead
improving performance, so change config now needs restart the
service. Also abstracted the common config reading code to make
each property access more gracefully.
For second part, libvirt-cim would run shell command if needed:
"cp -f [SSH_Key] [migrate_ssh_temp_key]",
then use [migrate_ssh_temp_key] to generate uri suffix for remote
connection to migration destination. If copy is not needed,
then directly use [SSH_Key].
Note:
Test was done when SELinux was shutdown.
V2:
Rebased and Better documentation.
v3:
Added some common code to read configuration, Added an option which
can disable this function, use just one parameter SSH_Key in API,
forbid copy any file to avoid security problem for that now config
file controls it.
Signed-off-by: Wenchao Xia <xiawenc at linux.vnet.ibm.com>
---
libvirt-cim.conf | 14 +++++
libxkutil/misc_util.c | 119 ++++++++++++++++++++++++++++++++++++-----
libxkutil/misc_util.h | 3 +
src/Virt_VSMigrationService.c | 114 +++++++++++++++++++++++++++++++++++++--
4 files changed, 231 insertions(+), 19 deletions(-)
diff --git a/libvirt-cim.conf b/libvirt-cim.conf
index d3cb2c3..97e6918 100644
--- a/libvirt-cim.conf
+++ b/libvirt-cim.conf
@@ -11,3 +11,17 @@
# Default value: false
#
# readonly = false;
+
+# migrate_ssh_temp_key (string)
+# Defines a temp key file which would be used as ssh key in ssh migration,
+# Only valid when SSH_Key is set in migration call.
+# If SSH_Key is not in a directory owned by root, set this value to a path
+# owned by root, tells libvirt-cim copy it there before use it. The directory
+# in the path must exist and totally owned by root.
+# If SSH_Key is already in a valid place, don't set it to to tell libvirt-cim
+# use SSH_Key directly.
+#
+# Possible values: {any path plus filename on host}
+# Default value: NULL, that is not set.
+#
+# migrate_ssh_temp_key = "/root/vm_migrate_tmp_id_rsa";
diff --git a/libxkutil/misc_util.c b/libxkutil/misc_util.c
index 2d149ae..4a16ec2 100644
--- a/libxkutil/misc_util.c
+++ b/libxkutil/misc_util.c
@@ -106,14 +106,28 @@ static const char *cn_to_uri(const char *classname)
return NULL;
}
-static int is_read_only(void)
-{
- int readonly = 0;
+typedef enum LibvirtcimConfigType {
+ CONFIG_BOOL,
+ CONFIG_STRING,
+} LibvirtcimConfigType;
+
+typedef struct LibvirtcimConfigProperty {
+ const char *name;
+ LibvirtcimConfigType value_type;
+ union {
+ int value_bool;
+ char *value_string;
+ };
+ int have_read;
+} LibvirtcimConfigProperty;
+static int libvirt_cim_config_get(LibvirtcimConfigProperty *prop)
+{
#ifdef HAVE_LIBCONFIG
config_t conf;
- int ret;
- const char *readonly_str = "readonly";
+ int ret = 0;
+ int error = 0;
+ const char *value_string = NULL;
config_init(&conf);
@@ -121,24 +135,101 @@ static int is_read_only(void)
if (ret == CONFIG_FALSE) {
CU_DEBUG("Error reading config file at line %d: '%s'\n",
conf.error_line, conf.error_text);
+ error = -1;
goto out;
}
- ret = config_lookup_bool(&conf, readonly_str, &readonly);
- if (ret == CONFIG_FALSE) {
- CU_DEBUG("'%s' not found in config file, assuming false\n",
- readonly_str);
- goto out;
+ switch (prop->value_type) {
+ case CONFIG_BOOL:
+ ret = config_lookup_bool(&conf,
+ prop->name, &prop->value_bool);
+ if (ret == CONFIG_FALSE) {
+ CU_DEBUG("Bool property '%s' in config file '%s' "
+ "not found.",
+ prop->name, LIBVIRTCIM_CONF);
+ error = -1;
+ goto out;
+ }
+
+ CU_DEBUG("Bool property '%s' in config file '%s' is '%d'.",
+ prop->name, LIBVIRTCIM_CONF, prop->value_bool);
+ break;
+ case CONFIG_STRING:
+ ret = config_lookup_string(&conf,
+ prop->name, &value_string);
+ if (ret == CONFIG_FALSE) {
+ CU_DEBUG("String property '%s' in config file '%s' "
+ "not found.",
+ prop->name, LIBVIRTCIM_CONF);
+ error = -1;
+ goto out;
+ }
+
+ CU_DEBUG("String property '%s' in config file '%s' is '%s'.",
+ prop->name, LIBVIRTCIM_CONF, value_string);
+
+ if (prop->value_string) {
+ free(prop->value_string);
+ }
+ prop->value_string = strdup(value_string);
+ if (!prop->value_string) {
+ CU_DEBUG("Failed in duplicate value '%s'",
+ value_string);
+ error = -1;
+ goto out;
+ }
+ break;
+ default:
+ CU_DEBUG("Got invalid property type request %d.",
+ prop->value_type);
+ error = -1;
+ break;
}
- CU_DEBUG("'%s' value in '%s' config file: %d\n", readonly_str,
- LIBVIRTCIM_CONF, readonly);
out:
config_destroy(&conf);
+ return error;
+#else
+ CU_DEBUG("Built without libconfig, can't read '%s'.",prop->name);
+ return -2;
#endif
+}
+
+static int is_read_only(void)
+{
+ static LibvirtcimConfigProperty prop;
+
+ if (prop.have_read == 1) {
+ goto ret;
+ }
+
+ prop.name = "readonly";
+ prop.value_type = CONFIG_BOOL;
+ prop.value_bool = 0;
+ libvirt_cim_config_get(&prop);
+ /* try only once */
+ prop.have_read = 1;
+
+ret:
+ return prop.value_bool;
+}
+
+const char *get_mig_ssh_tmp_key(void)
+{
+ static LibvirtcimConfigProperty prop;
+
+ if (prop.have_read == 1) {
+ goto ret;
+ }
+
+ prop.name = "migrate_ssh_temp_key";
+ prop.value_type = CONFIG_STRING;
+ libvirt_cim_config_get(&prop);
+ /* try only once */
+ prop.have_read = 1;
- /* Default value is 0 (false) */
- return readonly;
+ret:
+ return prop.value_string;
}
virConnectPtr connect_by_classname(const CMPIBroker *broker,
diff --git a/libxkutil/misc_util.h b/libxkutil/misc_util.h
index c7a2122..6ce60a0 100644
--- a/libxkutil/misc_util.h
+++ b/libxkutil/misc_util.h
@@ -151,6 +151,9 @@ int virt_set_status(const CMPIBroker *broker,
#define REF2STR(r) CMGetCharPtr(CMObjectPathToString(r, NULL))
+/* get libvirt-cim config */
+const char *get_mig_ssh_tmp_key(void);
+
/*
* Local Variables:
* mode: C
diff --git a/src/Virt_VSMigrationService.c b/src/Virt_VSMigrationService.c
index 76e3d25..14a3004 100644
--- a/src/Virt_VSMigrationService.c
+++ b/src/Virt_VSMigrationService.c
@@ -150,6 +150,7 @@ static CMPIStatus get_migration_uri(CMPIInstance *msd,
static char *dest_uri(const char *cn,
const char *dest,
+ const char *dest_params,
uint16_t transport)
{
const char *prefix;
@@ -157,6 +158,7 @@ static char *dest_uri(const char *cn,
const char *param = "";
char *uri = NULL;
int rc;
+ int param_labeled = 0;
if (STARTS_WITH(cn, "Xen"))
prefix = "xen";
@@ -197,16 +199,79 @@ static char *dest_uri(const char *cn,
goto out;
}
- if (!STREQC(param, ""))
+ if (!STREQC(param, "")) {
rc = asprintf(&uri, "%s/%s", uri, param);
+ param_labeled = 1;
+ }
- if (rc == -1)
+ if (rc == -1) {
uri = NULL;
+ goto out;
+ }
+ if (dest_params) {
+ if (param_labeled == 0) {
+ rc = asprintf(&uri, "%s?%s", uri, dest_params);
+ } else {
+ /* ? is already added */
+ rc = asprintf(&uri, "%s%s", uri, dest_params);
+ }
+ if (rc == -1) {
+ uri = NULL;
+ goto out;
+ }
+ }
out:
return uri;
}
+/* libvirt need private key specified must be placed in a directory owned by
+ root, because libvirt-cim now runs as root. So here the key would be copied,
+ up layer need to delete that key after migration. This method could allow
+ libvirt-cim borrow a non-root ssh private key, instead of using root's private
+ key, avoid security risk. */
+static int ssh_key_cp(const char *src, const char *dest)
+{
+ char *cmd = NULL;
+ int rc;
+ int ret = 0;
+ FILE *stream = NULL;
+ char buf[256];
+
+ rc = asprintf(&cmd, "cp -f %s %s", src, dest);
+ if (rc == -1) {
+ cmd = NULL;
+ ret = -1;
+ goto out;
+ }
+
+ CU_DEBUG("executing system cmd [%s].", cmd);
+ /* Todo: We need a better popen, currently stdout have been closed
+ and SIGCHILD is handled by tog-pegasus, so fgets always got NULL
+ making error detection not possible. */
+ stream = popen(cmd, "r");
+ if (stream == NULL) {
+ CU_DEBUG("Failed to open pipe to run command");
+ ret = -2;
+ goto out;
+ }
+ usleep(10000);
+
+ buf[255] = 0;
+ while (fgets(buf, sizeof(buf), stream) != NULL) {
+ CU_DEBUG("Exception got: %s.", buf);
+ ret = -3;
+ goto out;
+ }
+
+ out:
+ if (stream != NULL) {
+ pclose(stream);
+ }
+ free(cmd);
+ return ret;
+}
+
static CMPIStatus get_msd_values(const CMPIObjectPath *ref,
const char *destination,
const CMPIArgs *argsin,
@@ -217,6 +282,11 @@ static CMPIStatus get_msd_values(const CMPIObjectPath *ref,
CMPIInstance *msd;
uint16_t uri_type;
char *uri = NULL;
+ const char *ssh_key = NULL;
+ char *dest_params = NULL;
+ int ret;
+
+ cu_get_str_arg(argsin, "SSH_Key", &ssh_key);
s = get_msd(ref, argsin, &msd);
if (s.rc != CMPI_RC_OK)
@@ -230,7 +300,38 @@ static CMPIStatus get_msd_values(const CMPIObjectPath *ref,
if (s.rc != CMPI_RC_OK)
goto out;
- uri = dest_uri(CLASSNAME(ref), destination, uri_type);
+ if (ssh_key) {
+ CU_DEBUG("Trying migrate with specified ssh key file '%s'.",
+ ssh_key);
+ const char * tmp_keyfile = get_mig_ssh_tmp_key();
+ const char * final_keyfile = NULL;
+ const char *errstr;
+ if (tmp_keyfile) {
+ CU_DEBUG("Copying ssh keys src '%s', dest '%s'.",
+ ssh_key, tmp_keyfile);
+ ret = ssh_key_cp(ssh_key, tmp_keyfile);
+ if (ret < 0) {
+ errstr = "Failed to copy ssh key files";
+ CU_DEBUG("%s", errstr);
+ cu_statusf(_BROKER, &s, CMPI_RC_ERR_FAILED,
+ "%s", errstr);
+ goto out;
+ }
+ final_keyfile = tmp_keyfile;
+ } else {
+ final_keyfile = ssh_key;
+ CU_DEBUG("Skip key copy.");
+ }
+ CU_DEBUG("Generating param string with key '%s'.",
+ final_keyfile);
+ ret = asprintf(&dest_params, "keyfile=%s", final_keyfile);
+ if (ret < 0) {
+ CU_DEBUG("Failed in generating param string.");
+ goto out;
+ }
+ }
+
+ uri = dest_uri(CLASSNAME(ref), destination, dest_params, uri_type);
if (uri == NULL) {
cu_statusf(_BROKER, &s,
CMPI_RC_ERR_FAILED,
@@ -238,6 +339,7 @@ static CMPIStatus get_msd_values(const CMPIObjectPath *ref,
goto out;
}
+ CU_DEBUG("Migrate tring to connect remote host with uri %s.", uri);
*conn = virConnectOpen(uri);
if (*conn == NULL) {
CU_DEBUG("Failed to connect to remote host (%s)", uri);
@@ -249,7 +351,7 @@ static CMPIStatus get_msd_values(const CMPIObjectPath *ref,
out:
free(uri);
-
+ free(dest_params);
return s;
}
@@ -1537,7 +1639,7 @@ static CMPIStatus migrate_vs_host(CMPIMethodMI *self,
const char *dhost = NULL;
CMPIObjectPath *system;
const char *name = NULL;
-
+
cu_get_str_arg(argsin, "DestinationHost", &dhost);
cu_get_ref_arg(argsin, "ComputerSystem", &system);
@@ -1608,6 +1710,7 @@ static struct method_handler vsimth = {
.handler = vs_migratable_host,
.args = {{"ComputerSystem", CMPI_ref, false},
{"DestinationHost", CMPI_string, false},
+ {"SSH_Key", CMPI_string, true},
{"MigrationSettingData", CMPI_instance, true},
{"NewSystemSettingData", CMPI_instance, true},
{"NewResourceSettingData", CMPI_instanceA, true},
@@ -1632,6 +1735,7 @@ static struct method_handler mvsth = {
.handler = migrate_vs_host,
.args = {{"ComputerSystem", CMPI_ref, false},
{"DestinationHost", CMPI_string, false},
+ {"SSH_Key", CMPI_string, true},
{"MigrationSettingData", CMPI_instance, true},
{"NewSystemSettingData", CMPI_instance, true},
{"NewResourceSettingData", CMPI_instanceA, true},
--
1.7.1
More information about the Libvirt-cim
mailing list