Convert all the secret/storage encryption APIs / wire format to handle UUIDs in raw format instead of non-canonical printable format. Guarentees data format correctness. * docs/schemas/storageencryption.rng: Make UUID mandatory for a secret and validate fully * docs/schemas/secret.rng: Fully validate UUID * include/libvirt/libvirt.h, include/libvirt/libvirt.h.in, Add virSecretLookupByUUID and virSecretGetUUID. Make virSecretGetUUIDString follow normal API design pattern * python/generator.py: Skip generation of virSecretGetUUID, virSecretGetUUIDString and virSecretLookupByUUID * python/libvir.c, python/libvirt-python-api.xml: Manual impl of virSecretGetUUID,virSecretGetUUIDString and virSecretLookupByUUID * qemud/remote.c: s/virSecretLookupByUUIDString/virSecretLookupByUUID/ Fix get_nonnull_secret/make_nonnull_secret to use unsigned char * qemud/remote_protocol.x: Fix remote_nonnull_secret to use a remote_uuid instead of remote_nonnull_string for UUID field. Rename REMOTE_PROC_SECRET_LOOKUP_BY_UUID_STRING to REMOTE_PROC_SECRET_LOOKUP_BY_UUID_STRING and make it take an remote_uuid value * qemud/remote_dispatch_args.h, qemud/remote_dispatch_prototypes.h, qemud/remote_dispatch_ret.h, qemud/remote_dispatch_table.h, qemud/remote_protocol.c, qemud/remote_protocol.h: Re-generate * src/datatypes.h, src/datatypes.c: Store UUID in raw format instead of printable. Change virGetSecret to use raw format UUID * src/driver.h: Rename virDrvSecretLookupByUUIDString to virDrvSecretLookupByUUID and use raw format UUID * src/libvirt.c: Add virSecretLookupByUUID and virSecretGetUUID and re-implement virSecretLookupByUUIDString and virSecretGetUUIDString in terms of those * src/libvirt_public.syms: Add virSecretLookupByUUID and virSecretGetUUID * src/remote_internal.c: Rename remoteSecretLookupByUUIDString to remoteSecretLookupByUUID. Fix typo in args for remoteSecretDefineXML impl. Use raw UUID format for get_nonnull_secret and make_nonnull_secret * src/storage_encryption_conf.c, src/storage_encryption_conf.h: Storage UUID in raw format, and require it to be present in XML. Use UUID parser to validate. * secret_conf.h, secret_conf.c: Generate a UUID if none is provided. Storage UUID in raw format. * src/secret_driver.c: Adjust to deal with raw UUIDs. Save secrets in a filed with printable UUID, instead of base64 UUID. * src/virsh.c: Adjust for changed public API contract of virSecretGetUUIDString. * src/storage_Backend.c: DOn't undefine secret we just generated upon successful volume creation. Fix to handle raw UUIDs. Generate a non-clashing UUID * src/qemu_driver.c: Change to use lookupByUUID instead of lookupByUUIDString --- docs/schemas/secret.rng | 14 +++- docs/schemas/storageencryption.rng | 19 ++++- include/libvirt/libvirt.h | 7 ++- include/libvirt/libvirt.h.in | 7 ++- python/generator.py | 3 + python/libvir.c | 79 +++++++++++++++++++ python/libvirt-python-api.xml | 16 ++++ qemud/remote.c | 16 ++-- qemud/remote_dispatch_args.h | 2 +- qemud/remote_dispatch_prototypes.h | 6 +- qemud/remote_dispatch_ret.h | 2 +- qemud/remote_dispatch_table.h | 8 +- qemud/remote_protocol.c | 8 +- qemud/remote_protocol.h | 22 +++--- qemud/remote_protocol.x | 10 +- src/datatypes.c | 26 +++--- src/datatypes.h | 4 +- src/driver.h | 6 +- src/libvirt.c | 152 +++++++++++++++++++++++++++++------ src/libvirt_public.syms | 2 + src/qemu_driver.c | 11 +-- src/remote_internal.c | 26 +++--- src/secret_conf.c | 28 ++++++- src/secret_conf.h | 2 +- src/secret_driver.c | 129 +++++++++++-------------------- src/storage_backend.c | 56 +++++++++----- src/storage_encryption_conf.c | 29 +++++-- src/storage_encryption_conf.h | 3 +- src/virsh.c | 7 +- 29 files changed, 470 insertions(+), 230 deletions(-) diff --git a/docs/schemas/secret.rng b/docs/schemas/secret.rng index 8cfbd8f..2aab1db 100644 --- a/docs/schemas/secret.rng +++ b/docs/schemas/secret.rng @@ -25,7 +25,7 @@ <interleave> <optional> <element name='uuid'> - <text/> + <ref name='UUID'/> </element> </optional> <optional> @@ -53,4 +53,16 @@ <text/> </element> </define> + + <define name="UUID"> + <choice> + <data type="string"> + <param name="pattern">[a-fA-F0-9]{32}</param> + </data> + <data type="string"> + <param name="pattern">[a-fA-F0-9]{8}\-([a-fA-F0-9]{4}\-){3}[a-fA-F0-9]{12}</param> + </data> + </choice> + </define> + </grammar> diff --git a/docs/schemas/storageencryption.rng b/docs/schemas/storageencryption.rng index 8f9cd62..2739d79 100644 --- a/docs/schemas/storageencryption.rng +++ b/docs/schemas/storageencryption.rng @@ -23,12 +23,21 @@ <value>passphrase</value> </choice> </attribute> - <optional> - <attribute name='uuid'> - <text/> - </attribute> - </optional> + <attribute name='uuid'> + <ref name="UUID"/> + </attribute> </element> </define> + <define name="UUID"> + <choice> + <data type="string"> + <param name="pattern">[a-fA-F0-9]{32}</param> + </data> + <data type="string"> + <param name="pattern">[a-fA-F0-9]{8}\-([a-fA-F0-9]{4}\-){3}[a-fA-F0-9]{12}</param> + </data> + </choice> + </define> + </grammar> diff --git a/include/libvirt/libvirt.h b/include/libvirt/libvirt.h index 1779b08..eadf420 100644 --- a/include/libvirt/libvirt.h +++ b/include/libvirt/libvirt.h @@ -1467,12 +1467,17 @@ int virConnectNumOfSecrets (virConnectPtr conn); int virConnectListSecrets (virConnectPtr conn, char **uuids, int maxuuids); +virSecretPtr virSecretLookupByUUID(virConnectPtr conn, + const unsigned char *uuid); virSecretPtr virSecretLookupByUUIDString(virConnectPtr conn, const char *uuid); virSecretPtr virSecretDefineXML (virConnectPtr conn, const char *xml, unsigned int flags); -char * virSecretGetUUIDString (virSecretPtr secret); +int virSecretGetUUID (virSecretPtr secret, + unsigned char *buf); +int virSecretGetUUIDString (virSecretPtr secret, + char *buf); char * virSecretGetXMLDesc (virSecretPtr secret, unsigned int flags); int virSecretSetValue (virSecretPtr secret, diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 8e26e48..1391af8 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1467,12 +1467,17 @@ int virConnectNumOfSecrets (virConnectPtr conn); int virConnectListSecrets (virConnectPtr conn, char **uuids, int maxuuids); +virSecretPtr virSecretLookupByUUID(virConnectPtr conn, + const unsigned char *uuid); virSecretPtr virSecretLookupByUUIDString(virConnectPtr conn, const char *uuid); virSecretPtr virSecretDefineXML (virConnectPtr conn, const char *xml, unsigned int flags); -char * virSecretGetUUIDString (virSecretPtr secret); +int virSecretGetUUID (virSecretPtr secret, + unsigned char *buf); +int virSecretGetUUIDString (virSecretPtr secret, + char *buf); char * virSecretGetXMLDesc (virSecretPtr secret, unsigned int flags); int virSecretSetValue (virSecretPtr secret, diff --git a/python/generator.py b/python/generator.py index 4dbad1c..c25ff55 100755 --- a/python/generator.py +++ b/python/generator.py @@ -328,6 +328,9 @@ skip_impl = ( 'virDomainPinVcpu', 'virSecretGetValue', 'virSecretSetValue', + 'virSecretGetUUID', + 'virSecretGetUUIDString', + 'virSecretLookupByUUID', 'virStoragePoolGetUUID', 'virStoragePoolGetUUIDString', 'virStoragePoolLookupByUUID', diff --git a/python/libvir.c b/python/libvir.c index 0c00b80..d4f1eb2 100644 --- a/python/libvir.c +++ b/python/libvir.c @@ -1563,6 +1563,82 @@ libvirt_virNodeDeviceListCaps(PyObject *self ATTRIBUTE_UNUSED, } static PyObject * +libvirt_virSecretGetUUID(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) { + PyObject *py_retval; + unsigned char uuid[VIR_UUID_BUFLEN]; + virSecretPtr secret; + PyObject *pyobj_secret; + int c_retval; + + if (!PyArg_ParseTuple(args, (char *)"O:virSecretGetUUID", &pyobj_secret)) + return(NULL); + secret = (virSecretPtr) PyvirSecret_Get(pyobj_secret); + + if (secret == NULL) + return VIR_PY_NONE; + LIBVIRT_BEGIN_ALLOW_THREADS; + c_retval = virSecretGetUUID(secret, &uuid[0]); + LIBVIRT_END_ALLOW_THREADS; + + if (c_retval < 0) + return VIR_PY_NONE; + py_retval = PyString_FromStringAndSize((char *) &uuid[0], VIR_UUID_BUFLEN); + + return(py_retval); +} + +static PyObject * +libvirt_virSecretGetUUIDString(PyObject *self ATTRIBUTE_UNUSED, + PyObject *args) { + PyObject *py_retval; + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virSecretPtr dom; + PyObject *pyobj_dom; + int c_retval; + + if (!PyArg_ParseTuple(args, (char *)"O:virSecretGetUUIDString", + &pyobj_dom)) + return(NULL); + dom = (virSecretPtr) PyvirSecret_Get(pyobj_dom); + + if (dom == NULL) + return VIR_PY_NONE; + LIBVIRT_BEGIN_ALLOW_THREADS; + c_retval = virSecretGetUUIDString(dom, &uuidstr[0]); + LIBVIRT_END_ALLOW_THREADS; + + if (c_retval < 0) + return VIR_PY_NONE; + + py_retval = PyString_FromString((char *) &uuidstr[0]); + return(py_retval); +} + +static PyObject * +libvirt_virSecretLookupByUUID(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) { + PyObject *py_retval; + virSecretPtr c_retval; + virConnectPtr conn; + PyObject *pyobj_conn; + unsigned char * uuid; + int len; + + if (!PyArg_ParseTuple(args, (char *)"Oz#:virSecretLookupByUUID", &pyobj_conn, &uuid, &len)) + return(NULL); + conn = (virConnectPtr) PyvirConnect_Get(pyobj_conn); + + if ((uuid == NULL) || (len != VIR_UUID_BUFLEN)) + return VIR_PY_NONE; + + LIBVIRT_BEGIN_ALLOW_THREADS; + c_retval = virSecretLookupByUUID(conn, uuid); + LIBVIRT_END_ALLOW_THREADS; + py_retval = libvirt_virSecretPtrWrap((virSecretPtr) c_retval); + return(py_retval); +} + + +static PyObject * libvirt_virConnectListSecrets(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) { PyObject *py_retval; @@ -2358,6 +2434,9 @@ static PyMethodDef libvirtMethods[] = { {(char *) "virEventInvokeTimeoutCallback", libvirt_virEventInvokeTimeoutCallback, METH_VARARGS, NULL}, {(char *) "virNodeListDevices", libvirt_virNodeListDevices, METH_VARARGS, NULL}, {(char *) "virNodeDeviceListCaps", libvirt_virNodeDeviceListCaps, METH_VARARGS, NULL}, + {(char *) "virSecretGetUUID", libvirt_virSecretGetUUID, METH_VARARGS, NULL}, + {(char *) "virSecretGetUUIDString", libvirt_virSecretGetUUIDString, METH_VARARGS, NULL}, + {(char *) "virSecretLookupByUUID", libvirt_virSecretLookupByUUID, METH_VARARGS, NULL}, {(char *) "virConnectListSecrets", libvirt_virConnectListSecrets, METH_VARARGS, NULL}, {(char *) "virSecretGetValue", libvirt_virSecretGetValue, METH_VARARGS, NULL}, {(char *) "virSecretSetValue", libvirt_virSecretSetValue, METH_VARARGS, NULL}, diff --git a/python/libvirt-python-api.xml b/python/libvirt-python-api.xml index e5c1fb9..148b89b 100644 --- a/python/libvirt-python-api.xml +++ b/python/libvirt-python-api.xml @@ -190,5 +190,21 @@ <arg name='value' type='const char *' info='The secret value'/> <arg name='flags' type='unsigned int' info='flags (unused; pass 0)'/> </function> + <function name='virSecretLookupByUUID' file='python'> + <info>Try to lookup a secret on the given hypervisor based on its UUID.</info> + <return type='virSecretPtr' info='a new secret object or NULL in case of failure'/> + <arg name='conn' type='virConnectPtr' info='pointer to the hypervisor connection'/> + <arg name='uuid' type='const unsigned char *' info='the UUID string for the secret, must be 16 bytes'/> + </function> + <function name='virSecretGetUUID' file='python'> + <info>Extract the UUID unique Identifier of a secret.</info> + <return type='char *' info='the 16 bytes string or None in case of error'/> + <arg name='secret' type='virSecretPtr' info='a secret object'/> + </function> + <function name='virSecretGetUUIDString' file='python'> + <info>Fetch globally unique ID of the secret as a string.</info> + <return type='char *' info='the UUID string or None in case of error'/> + <arg name='secret' type='virSecretPtr' info='a secret object'/> + </function> </symbols> </api> diff --git a/qemud/remote.c b/qemud/remote.c index 5d58611..a9fcc58 100644 --- a/qemud/remote.c +++ b/qemud/remote.c @@ -4710,15 +4710,15 @@ remoteDispatchSecretGetXmlDesc (struct qemud_server *server ATTRIBUTE_UNUSED, } static int -remoteDispatchSecretLookupByUuidString (struct qemud_server *server ATTRIBUTE_UNUSED, - struct qemud_client *client ATTRIBUTE_UNUSED, - virConnectPtr conn, remote_error *err, - remote_secret_lookup_by_uuid_string_args *args, - remote_secret_lookup_by_uuid_string_ret *ret) +remoteDispatchSecretLookupByUuid (struct qemud_server *server ATTRIBUTE_UNUSED, + struct qemud_client *client ATTRIBUTE_UNUSED, + virConnectPtr conn, remote_error *err, + remote_secret_lookup_by_uuid_args *args, + remote_secret_lookup_by_uuid_ret *ret) { virSecretPtr secret; - secret = virSecretLookupByUUIDString (conn, args->uuid); + secret = virSecretLookupByUUID (conn, (unsigned char *)args->uuid); if (secret == NULL) { remoteDispatchConnError (err, conn); return -1; @@ -4828,7 +4828,7 @@ get_nonnull_storage_vol (virConnectPtr conn, remote_nonnull_storage_vol vol) static virSecretPtr get_nonnull_secret (virConnectPtr conn, remote_nonnull_secret secret) { - return virGetSecret (conn, secret.uuid); + return virGetSecret (conn, BAD_CAST secret.uuid); } /* Make remote_nonnull_domain and remote_nonnull_network. */ @@ -4879,5 +4879,5 @@ make_nonnull_node_device (remote_nonnull_node_device *dev_dst, virNodeDevicePtr static void make_nonnull_secret (remote_nonnull_secret *secret_dst, virSecretPtr secret_src) { - secret_dst->uuid = strdup(secret_src->uuid); + memcpy (secret_dst->uuid, secret_src->uuid, VIR_UUID_BUFLEN); } diff --git a/qemud/remote_dispatch_args.h b/qemud/remote_dispatch_args.h index dcf7ddf..9f37963 100644 --- a/qemud/remote_dispatch_args.h +++ b/qemud/remote_dispatch_args.h @@ -118,7 +118,7 @@ remote_domain_xml_to_native_args val_remote_domain_xml_to_native_args; remote_list_defined_interfaces_args val_remote_list_defined_interfaces_args; remote_list_secrets_args val_remote_list_secrets_args; - remote_secret_lookup_by_uuid_string_args val_remote_secret_lookup_by_uuid_string_args; + remote_secret_lookup_by_uuid_args val_remote_secret_lookup_by_uuid_args; remote_secret_define_xml_args val_remote_secret_define_xml_args; remote_secret_get_xml_desc_args val_remote_secret_get_xml_desc_args; remote_secret_set_value_args val_remote_secret_set_value_args; diff --git a/qemud/remote_dispatch_prototypes.h b/qemud/remote_dispatch_prototypes.h index 647f5bb..7773cd9 100644 --- a/qemud/remote_dispatch_prototypes.h +++ b/qemud/remote_dispatch_prototypes.h @@ -807,13 +807,13 @@ static int remoteDispatchSecretGetXmlDesc( remote_error *err, remote_secret_get_xml_desc_args *args, remote_secret_get_xml_desc_ret *ret); -static int remoteDispatchSecretLookupByUuidString( +static int remoteDispatchSecretLookupByUuid( struct qemud_server *server, struct qemud_client *client, virConnectPtr conn, remote_error *err, - remote_secret_lookup_by_uuid_string_args *args, - remote_secret_lookup_by_uuid_string_ret *ret); + remote_secret_lookup_by_uuid_args *args, + remote_secret_lookup_by_uuid_ret *ret); static int remoteDispatchSecretSetValue( struct qemud_server *server, struct qemud_client *client, diff --git a/qemud/remote_dispatch_ret.h b/qemud/remote_dispatch_ret.h index 9d74a27..15d3386 100644 --- a/qemud/remote_dispatch_ret.h +++ b/qemud/remote_dispatch_ret.h @@ -101,7 +101,7 @@ remote_list_defined_interfaces_ret val_remote_list_defined_interfaces_ret; remote_num_of_secrets_ret val_remote_num_of_secrets_ret; remote_list_secrets_ret val_remote_list_secrets_ret; - remote_secret_lookup_by_uuid_string_ret val_remote_secret_lookup_by_uuid_string_ret; + remote_secret_lookup_by_uuid_ret val_remote_secret_lookup_by_uuid_ret; remote_secret_define_xml_ret val_remote_secret_define_xml_ret; remote_secret_get_xml_desc_ret val_remote_secret_get_xml_desc_ret; remote_secret_get_value_ret val_remote_secret_get_value_ret; diff --git a/qemud/remote_dispatch_table.h b/qemud/remote_dispatch_table.h index 02d7bb5..07e36bb 100644 --- a/qemud/remote_dispatch_table.h +++ b/qemud/remote_dispatch_table.h @@ -707,10 +707,10 @@ .args_filter = (xdrproc_t) xdr_remote_list_secrets_args, .ret_filter = (xdrproc_t) xdr_remote_list_secrets_ret, }, -{ /* SecretLookupByUuidString => 141 */ - .fn = (dispatch_fn) remoteDispatchSecretLookupByUuidString, - .args_filter = (xdrproc_t) xdr_remote_secret_lookup_by_uuid_string_args, - .ret_filter = (xdrproc_t) xdr_remote_secret_lookup_by_uuid_string_ret, +{ /* SecretLookupByUuid => 141 */ + .fn = (dispatch_fn) remoteDispatchSecretLookupByUuid, + .args_filter = (xdrproc_t) xdr_remote_secret_lookup_by_uuid_args, + .ret_filter = (xdrproc_t) xdr_remote_secret_lookup_by_uuid_ret, }, { /* SecretDefineXml => 142 */ .fn = (dispatch_fn) remoteDispatchSecretDefineXml, diff --git a/qemud/remote_protocol.c b/qemud/remote_protocol.c index db4d794..b6666a1 100644 --- a/qemud/remote_protocol.c +++ b/qemud/remote_protocol.c @@ -107,7 +107,7 @@ bool_t xdr_remote_nonnull_secret (XDR *xdrs, remote_nonnull_secret *objp) { - if (!xdr_remote_nonnull_string (xdrs, &objp->uuid)) + if (!xdr_remote_uuid (xdrs, objp->uuid)) return FALSE; return TRUE; } @@ -2572,16 +2572,16 @@ xdr_remote_list_secrets_ret (XDR *xdrs, remote_list_secrets_ret *objp) } bool_t -xdr_remote_secret_lookup_by_uuid_string_args (XDR *xdrs, remote_secret_lookup_by_uuid_string_args *objp) +xdr_remote_secret_lookup_by_uuid_args (XDR *xdrs, remote_secret_lookup_by_uuid_args *objp) { - if (!xdr_remote_nonnull_string (xdrs, &objp->uuid)) + if (!xdr_remote_uuid (xdrs, objp->uuid)) return FALSE; return TRUE; } bool_t -xdr_remote_secret_lookup_by_uuid_string_ret (XDR *xdrs, remote_secret_lookup_by_uuid_string_ret *objp) +xdr_remote_secret_lookup_by_uuid_ret (XDR *xdrs, remote_secret_lookup_by_uuid_ret *objp) { if (!xdr_remote_nonnull_secret (xdrs, &objp->secret)) diff --git a/qemud/remote_protocol.h b/qemud/remote_protocol.h index b54b3ae..4b73ee1 100644 --- a/qemud/remote_protocol.h +++ b/qemud/remote_protocol.h @@ -86,7 +86,7 @@ struct remote_nonnull_node_device { typedef struct remote_nonnull_node_device remote_nonnull_node_device; struct remote_nonnull_secret { - remote_nonnull_string uuid; + remote_uuid uuid; }; typedef struct remote_nonnull_secret remote_nonnull_secret; @@ -1453,15 +1453,15 @@ struct remote_list_secrets_ret { }; typedef struct remote_list_secrets_ret remote_list_secrets_ret; -struct remote_secret_lookup_by_uuid_string_args { - remote_nonnull_string uuid; +struct remote_secret_lookup_by_uuid_args { + remote_uuid uuid; }; -typedef struct remote_secret_lookup_by_uuid_string_args remote_secret_lookup_by_uuid_string_args; +typedef struct remote_secret_lookup_by_uuid_args remote_secret_lookup_by_uuid_args; -struct remote_secret_lookup_by_uuid_string_ret { +struct remote_secret_lookup_by_uuid_ret { remote_nonnull_secret secret; }; -typedef struct remote_secret_lookup_by_uuid_string_ret remote_secret_lookup_by_uuid_string_ret; +typedef struct remote_secret_lookup_by_uuid_ret remote_secret_lookup_by_uuid_ret; struct remote_secret_define_xml_args { remote_nonnull_string xml; @@ -1657,7 +1657,7 @@ enum remote_procedure { REMOTE_PROC_LIST_DEFINED_INTERFACES = 138, REMOTE_PROC_NUM_OF_SECRETS = 139, REMOTE_PROC_LIST_SECRETS = 140, - REMOTE_PROC_SECRET_LOOKUP_BY_UUID_STRING = 141, + REMOTE_PROC_SECRET_LOOKUP_BY_UUID = 141, REMOTE_PROC_SECRET_DEFINE_XML = 142, REMOTE_PROC_SECRET_GET_XML_DESC = 143, REMOTE_PROC_SECRET_SET_VALUE = 144, @@ -1929,8 +1929,8 @@ extern bool_t xdr_remote_domain_xml_to_native_ret (XDR *, remote_domain_xml_to_ extern bool_t xdr_remote_num_of_secrets_ret (XDR *, remote_num_of_secrets_ret*); extern bool_t xdr_remote_list_secrets_args (XDR *, remote_list_secrets_args*); extern bool_t xdr_remote_list_secrets_ret (XDR *, remote_list_secrets_ret*); -extern bool_t xdr_remote_secret_lookup_by_uuid_string_args (XDR *, remote_secret_lookup_by_uuid_string_args*); -extern bool_t xdr_remote_secret_lookup_by_uuid_string_ret (XDR *, remote_secret_lookup_by_uuid_string_ret*); +extern bool_t xdr_remote_secret_lookup_by_uuid_args (XDR *, remote_secret_lookup_by_uuid_args*); +extern bool_t xdr_remote_secret_lookup_by_uuid_ret (XDR *, remote_secret_lookup_by_uuid_ret*); extern bool_t xdr_remote_secret_define_xml_args (XDR *, remote_secret_define_xml_args*); extern bool_t xdr_remote_secret_define_xml_ret (XDR *, remote_secret_define_xml_ret*); extern bool_t xdr_remote_secret_get_xml_desc_args (XDR *, remote_secret_get_xml_desc_args*); @@ -2181,8 +2181,8 @@ extern bool_t xdr_remote_domain_xml_to_native_ret (); extern bool_t xdr_remote_num_of_secrets_ret (); extern bool_t xdr_remote_list_secrets_args (); extern bool_t xdr_remote_list_secrets_ret (); -extern bool_t xdr_remote_secret_lookup_by_uuid_string_args (); -extern bool_t xdr_remote_secret_lookup_by_uuid_string_ret (); +extern bool_t xdr_remote_secret_lookup_by_uuid_args (); +extern bool_t xdr_remote_secret_lookup_by_uuid_ret (); extern bool_t xdr_remote_secret_define_xml_args (); extern bool_t xdr_remote_secret_define_xml_ret (); extern bool_t xdr_remote_secret_get_xml_desc_args (); diff --git a/qemud/remote_protocol.x b/qemud/remote_protocol.x index 006dfa1..5712d98 100644 --- a/qemud/remote_protocol.x +++ b/qemud/remote_protocol.x @@ -188,7 +188,7 @@ struct remote_nonnull_node_device { /* A secret which may not be null. */ struct remote_nonnull_secret { - remote_nonnull_string uuid; + remote_uuid uuid; }; /* A domain or network which may be NULL. */ @@ -1293,11 +1293,11 @@ struct remote_list_secrets_ret { remote_nonnull_string uuids<REMOTE_SECRET_UUID_LIST_MAX>; }; -struct remote_secret_lookup_by_uuid_string_args { - remote_nonnull_string uuid; +struct remote_secret_lookup_by_uuid_args { + remote_uuid uuid; }; -struct remote_secret_lookup_by_uuid_string_ret { +struct remote_secret_lookup_by_uuid_ret { remote_nonnull_secret secret; }; @@ -1500,7 +1500,7 @@ enum remote_procedure { REMOTE_PROC_NUM_OF_SECRETS = 139, REMOTE_PROC_LIST_SECRETS = 140, - REMOTE_PROC_SECRET_LOOKUP_BY_UUID_STRING = 141, + REMOTE_PROC_SECRET_LOOKUP_BY_UUID = 141, REMOTE_PROC_SECRET_DEFINE_XML = 142, REMOTE_PROC_SECRET_GET_XML_DESC = 143, REMOTE_PROC_SECRET_SET_VALUE = 144, diff --git a/src/datatypes.c b/src/datatypes.c index 2e85196..b0067f6 100644 --- a/src/datatypes.c +++ b/src/datatypes.c @@ -25,6 +25,7 @@ #include "virterror_internal.h" #include "logging.h" #include "memory.h" +#include "uuid.h" #define VIR_FROM_THIS VIR_FROM_NONE @@ -1169,9 +1170,10 @@ virUnrefNodeDevice(virNodeDevicePtr dev) { * Returns a pointer to the secret, or NULL in case of failure */ virSecretPtr -virGetSecret(virConnectPtr conn, const char *uuid) +virGetSecret(virConnectPtr conn, const unsigned char *uuid) { virSecretPtr ret = NULL; + char uuidstr[VIR_UUID_STRING_BUFLEN]; if (!VIR_IS_CONNECT(conn) || uuid == NULL) { virLibConnError(NULL, VIR_ERR_INVALID_ARG, __FUNCTION__); @@ -1179,7 +1181,9 @@ virGetSecret(virConnectPtr conn, const char *uuid) } virMutexLock(&conn->lock); - ret = virHashLookup(conn->secrets, uuid); + virUUIDFormat(uuid, uuidstr); + + ret = virHashLookup(conn->secrets, uuidstr); if (ret == NULL) { if (VIR_ALLOC(ret) < 0) { virMutexUnlock(&conn->lock); @@ -1188,14 +1192,9 @@ virGetSecret(virConnectPtr conn, const char *uuid) } ret->magic = VIR_SECRET_MAGIC; ret->conn = conn; - ret->uuid = strdup(uuid); - if (ret->uuid == NULL) { - virMutexUnlock(&conn->lock); - virReportOOMError(conn); - goto error; - } + memcpy(&(ret->uuid[0]), uuid, VIR_UUID_BUFLEN); - if (virHashAddEntry(conn->secrets, uuid, ret) < 0) { + if (virHashAddEntry(conn->secrets, uuidstr, ret) < 0) { virMutexUnlock(&conn->lock); virLibConnError(conn, VIR_ERR_INTERNAL_ERROR, "%s", _("failed to add secret to conn hash table")); @@ -1229,9 +1228,11 @@ error: static void virReleaseSecret(virSecretPtr secret) { virConnectPtr conn = secret->conn; - DEBUG("release secret %p %s", secret, secret->uuid); + char uuidstr[VIR_UUID_STRING_BUFLEN]; + DEBUG("release secret %p %p", secret, secret->uuid); - if (virHashRemoveEntry(conn->secrets, secret->uuid, NULL) < 0) { + virUUIDFormat(secret->uuid, uuidstr); + if (virHashRemoveEntry(conn->secrets, uuidstr, NULL) < 0) { virMutexUnlock(&conn->lock); virLibConnError(conn, VIR_ERR_INTERNAL_ERROR, "%s", _("secret missing from connection hash table")); @@ -1239,7 +1240,6 @@ virReleaseSecret(virSecretPtr secret) { } secret->magic = -1; - VIR_FREE(secret->uuid); VIR_FREE(secret); if (conn) { @@ -1272,7 +1272,7 @@ virUnrefSecret(virSecretPtr secret) { return -1; } virMutexLock(&secret->conn->lock); - DEBUG("unref secret %p %s %d", secret, secret->uuid, secret->refs); + DEBUG("unref secret %p %p %d", secret, secret->uuid, secret->refs); secret->refs--; refs = secret->refs; if (refs == 0) { diff --git a/src/datatypes.h b/src/datatypes.h index aa60b63..5319308 100644 --- a/src/datatypes.h +++ b/src/datatypes.h @@ -255,7 +255,7 @@ struct _virSecret { unsigned int magic; /* specific value to check */ int refs; /* reference count */ virConnectPtr conn; /* pointer back to the connection */ - char *uuid; /* ID of the secret */ + unsigned char uuid[VIR_UUID_BUFLEN]; /* the domain unique identifier */ }; @@ -296,7 +296,7 @@ virNodeDevicePtr virGetNodeDevice(virConnectPtr conn, int virUnrefNodeDevice(virNodeDevicePtr dev); virSecretPtr virGetSecret(virConnectPtr conn, - const char *uuid); + const unsigned char *uuid); int virUnrefSecret(virSecretPtr secret); #endif diff --git a/src/driver.h b/src/driver.h index 447b7a2..9f197d9 100644 --- a/src/driver.h +++ b/src/driver.h @@ -819,8 +819,8 @@ verify((VIR_SECRET_GET_VALUE_INTERNAL_CALL & VIR_SECRET_GET_VALUE_FLAGS_MASK) == 0); typedef virSecretPtr - (*virDrvSecretLookupByUUIDString) (virConnectPtr conn, - const char *uuid); + (*virDrvSecretLookupByUUID) (virConnectPtr conn, + const unsigned char *uuid); typedef virSecretPtr (*virDrvSecretDefineXML) (virConnectPtr conn, const char *xml, @@ -866,7 +866,7 @@ struct _virSecretDriver { virDrvSecretNumOfSecrets numOfSecrets; virDrvSecretListSecrets listSecrets; - virDrvSecretLookupByUUIDString lookupByUUIDString; + virDrvSecretLookupByUUID lookupByUUID; virDrvSecretDefineXML defineXML; virDrvSecretGetXMLDesc getXMLDesc; virDrvSecretSetValue setValue; diff --git a/src/libvirt.c b/src/libvirt.c index 96d204c..7a915fa 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -8811,36 +8811,36 @@ error: } /** - * virSecretLookupByUUIDString: - * @conn: virConnect connection - * @uuid: ID of a secret + * virSecretLookupByUUID: + * @conn: pointer to the hypervisor connection + * @uuid: the raw UUID for the secret * - * Fetches a secret based on uuid. + * Try to lookup a secret on the given hypervisor based on its UUID. * - * Returns the secret on success, or NULL on failure. + * Returns a new secret object or NULL in case of failure. If the + * secret cannot be found, then VIR_ERR_NO_SECRET error is raised. */ virSecretPtr -virSecretLookupByUUIDString(virConnectPtr conn, const char *uuid) +virSecretLookupByUUID(virConnectPtr conn, const unsigned char *uuid) { - VIR_DEBUG("conn=%p, uuid=%s", conn, uuid); + DEBUG("conn=%p, uuid=%s", conn, uuid); virResetLastError(); if (!VIR_IS_CONNECT(conn)) { virLibConnError(NULL, VIR_ERR_INVALID_CONN, __FUNCTION__); - return NULL; + return (NULL); } if (uuid == NULL) { virLibConnError(conn, VIR_ERR_INVALID_ARG, __FUNCTION__); goto error; } - if (conn->secretDriver != NULL && - conn->secretDriver->lookupByUUIDString != NULL) { + if (conn->secretDriver && + conn->secretDriver->lookupByUUID) { virSecretPtr ret; - - ret = conn->secretDriver->lookupByUUIDString(conn, uuid); - if (ret == NULL) + ret = conn->secretDriver->lookupByUUID (conn, uuid); + if (!ret) goto error; return ret; } @@ -8854,6 +8854,65 @@ error: } /** + * virSecretLookupByUUIDString: + * @conn: pointer to the hypervisor connection + * @uuidstr: the string UUID for the secret + * + * Try to lookup a secret on the given hypervisor based on its UUID. + * + * Returns a new secret object or NULL in case of failure. If the + * secret cannot be found, then VIR_ERR_NO_SECRET error is raised. + */ +virSecretPtr +virSecretLookupByUUIDString(virConnectPtr conn, const char *uuidstr) +{ + int raw[VIR_UUID_BUFLEN], i; + unsigned char uuid[VIR_UUID_BUFLEN]; + int ret; + + DEBUG("conn=%p, uuidstr=%s", conn, uuidstr); + + virResetLastError(); + + if (!VIR_IS_CONNECT(conn)) { + virLibConnError(NULL, VIR_ERR_INVALID_CONN, __FUNCTION__); + return (NULL); + } + if (uuidstr == NULL) { + virLibConnError(conn, VIR_ERR_INVALID_ARG, __FUNCTION__); + goto error; + } + /* XXX: sexpr_uuid() also supports 'xxxx-xxxx-xxxx-xxxx' format. + * We needn't it here. Right? + */ + ret = sscanf(uuidstr, + "%02x%02x%02x%02x-" + "%02x%02x-" + "%02x%02x-" + "%02x%02x-" + "%02x%02x%02x%02x%02x%02x", + raw + 0, raw + 1, raw + 2, raw + 3, + raw + 4, raw + 5, raw + 6, raw + 7, + raw + 8, raw + 9, raw + 10, raw + 11, + raw + 12, raw + 13, raw + 14, raw + 15); + + if (ret!=VIR_UUID_BUFLEN) { + virLibConnError(conn, VIR_ERR_INVALID_ARG, __FUNCTION__); + goto error; + } + for (i = 0; i < VIR_UUID_BUFLEN; i++) + uuid[i] = raw[i] & 0xFF; + + return virSecretLookupByUUID(conn, &uuid[0]); + +error: + /* Copy to connection error object for back compatability */ + virSetConnError(conn); + return NULL; +} + + +/** * virSecretDefineXML: * @conn: virConnect connection * @xml: XML describing the secret. @@ -8906,37 +8965,78 @@ error: } /** - * virSecretGetUUIDString: + * virSecretGetUUID: * @secret: A virSecret secret + * @uuid: buffer of VIR_UUID_BUFLEN bytes in size * * Fetches the UUID of the secret. * - * Returns ID of the secret (not necessarily in the UUID format) on success, - * NULL on failure. The caller must free() the ID. + * Returns 0 on success with the uuid buffer being filled, or + * -1 upon failure. */ -char * -virSecretGetUUIDString(virSecretPtr secret) +int +virSecretGetUUID(virSecretPtr secret, unsigned char *uuid) { - char *ret; - VIR_DEBUG("secret=%p", secret); virResetLastError(); if (!VIR_IS_CONNECTED_SECRET(secret)) { virLibSecretError(NULL, VIR_ERR_INVALID_SECRET, __FUNCTION__); - return NULL; + return -1; + } + if (uuid == NULL) { + virLibSecretError(secret, VIR_ERR_INVALID_ARG, __FUNCTION__); + /* Copy to connection error object for back compatability */ + virSetConnError(secret->conn); + return -1; } - ret = strdup(secret->uuid); - if (ret != NULL) - return ret; + memcpy(uuid, &secret->uuid[0], VIR_UUID_BUFLEN); + + return 0; +} - virReportOOMError(secret->conn); +/** + * virSecretGetUUIDString: + * @secret: a secret object + * @buf: pointer to a VIR_UUID_STRING_BUFLEN bytes array + * + * Get the UUID for a secret as string. For more information about + * UUID see RFC4122. + * + * Returns -1 in case of error, 0 in case of success + */ +int +virSecretGetUUIDString(virSecretPtr secret, char *buf) +{ + unsigned char uuid[VIR_UUID_BUFLEN]; + DEBUG("secret=%p, buf=%p", secret, buf); + + virResetLastError(); + + if (!VIR_IS_SECRET(secret)) { + virLibSecretError(NULL, VIR_ERR_INVALID_SECRET, __FUNCTION__); + return (-1); + } + if (buf == NULL) { + virLibSecretError(secret, VIR_ERR_INVALID_ARG, __FUNCTION__); + goto error; + } + + if (virSecretGetUUID(secret, &uuid[0])) + goto error; + + virUUIDFormat(uuid, buf); + return (0); + +error: + /* Copy to connection error object for back compatability */ virSetConnError(secret->conn); - return NULL; + return -1; } + /** * virSecretGetXMLDesc: * @secret: A virSecret secret diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 65080ed..c34bbae 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -298,8 +298,10 @@ LIBVIRT_0.7.1 { virSecretGetConnect; virConnectNumOfSecrets; virConnectListSecrets; + virSecretLookupByUUID; virSecretLookupByUUIDString; virSecretDefineXML; + virSecretGetUUID; virSecretGetUUIDString; virSecretGetXMLDesc; virSecretSetValue; diff --git a/src/qemu_driver.c b/src/qemu_driver.c index 5a8a686..0e7d8d4 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -2695,7 +2695,7 @@ findVolumeQcowPassphrase(virConnectPtr conn, virDomainObjPtr vm, size_t size; if (conn->secretDriver == NULL || - conn->secretDriver->lookupByUUIDString == NULL || + conn->secretDriver->lookupByUUID == NULL || conn->secretDriver->getValue == NULL) { qemudReportError(conn, NULL, NULL, VIR_ERR_NO_SUPPORT, "%s", _("secret storage not supported")); @@ -2715,13 +2715,8 @@ findVolumeQcowPassphrase(virConnectPtr conn, virDomainObjPtr vm, return NULL; } - if (enc->secrets[0]->uuid == NULL) { - qemudReportError(conn, NULL, NULL, VIR_ERR_INVALID_DOMAIN, - _("missing secret uuid for volume %s"), path); - return NULL; - } - secret = conn->secretDriver->lookupByUUIDString(conn, - enc->secrets[0]->uuid); + secret = conn->secretDriver->lookupByUUID(conn, + enc->secrets[0]->uuid); if (secret == NULL) return NULL; data = conn->secretDriver->getValue(secret, &size, diff --git a/src/remote_internal.c b/src/remote_internal.c index 3dd4609..e7f0186 100644 --- a/src/remote_internal.c +++ b/src/remote_internal.c @@ -6475,25 +6475,25 @@ done: } static virSecretPtr -remoteSecretLookupByUUIDString (virConnectPtr conn, const char *uuid) +remoteSecretLookupByUUID (virConnectPtr conn, const unsigned char *uuid) { virSecretPtr rv = NULL; - remote_secret_lookup_by_uuid_string_args args; - remote_secret_lookup_by_uuid_string_ret ret; + remote_secret_lookup_by_uuid_args args; + remote_secret_lookup_by_uuid_ret ret; struct private_data *priv = conn->secretPrivateData; remoteDriverLock (priv); - args.uuid = (char *) uuid; + memcpy (args.uuid, uuid, VIR_UUID_BUFLEN); memset (&ret, 0, sizeof (ret)); - if (call (conn, priv, 0, REMOTE_PROC_SECRET_LOOKUP_BY_UUID_STRING, - (xdrproc_t) xdr_remote_secret_lookup_by_uuid_string_args, (char *) &args, - (xdrproc_t) xdr_remote_secret_lookup_by_uuid_string_ret, (char *) &ret) == -1) + if (call (conn, priv, 0, REMOTE_PROC_SECRET_LOOKUP_BY_UUID, + (xdrproc_t) xdr_remote_secret_lookup_by_uuid_args, (char *) &args, + (xdrproc_t) xdr_remote_secret_lookup_by_uuid_ret, (char *) &ret) == -1) goto done; rv = get_nonnull_secret (conn, ret.secret); - xdr_free ((xdrproc_t) xdr_remote_secret_lookup_by_uuid_string_ret, + xdr_free ((xdrproc_t) xdr_remote_secret_lookup_by_uuid_ret, (char *) &ret); done: @@ -6506,7 +6506,7 @@ remoteSecretDefineXML (virConnectPtr conn, const char *xml, unsigned int flags) { virSecretPtr rv = NULL; remote_secret_define_xml_args args; - remote_secret_lookup_by_uuid_string_ret ret; + remote_secret_define_xml_ret ret; struct private_data *priv = conn->secretPrivateData; remoteDriverLock (priv); @@ -6521,7 +6521,7 @@ remoteSecretDefineXML (virConnectPtr conn, const char *xml, unsigned int flags) goto done; rv = get_nonnull_secret (conn, ret.secret); - xdr_free ((xdrproc_t) xdr_remote_secret_lookup_by_uuid_string_ret, + xdr_free ((xdrproc_t) xdr_remote_secret_define_xml_ret, (char *) &ret); done: @@ -7733,7 +7733,7 @@ get_nonnull_node_device (virConnectPtr conn, remote_nonnull_node_device dev) static virSecretPtr get_nonnull_secret (virConnectPtr conn, remote_nonnull_secret secret) { - return virGetSecret(conn, secret.uuid); + return virGetSecret(conn, BAD_CAST secret.uuid); } /* Make remote_nonnull_domain and remote_nonnull_network. */ @@ -7778,7 +7778,7 @@ make_nonnull_storage_vol (remote_nonnull_storage_vol *vol_dst, virStorageVolPtr static void make_nonnull_secret (remote_nonnull_secret *secret_dst, virSecretPtr secret_src) { - secret_dst->uuid = secret_src->uuid; + memcpy (secret_dst->uuid, secret_src->uuid, VIR_UUID_BUFLEN); } /*----------------------------------------------------------------------*/ @@ -7940,7 +7940,7 @@ static virSecretDriver secret_driver = { .close = remoteSecretClose, .numOfSecrets = remoteSecretNumOfSecrets, .listSecrets = remoteSecretListSecrets, - .lookupByUUIDString = remoteSecretLookupByUUIDString, + .lookupByUUID = remoteSecretLookupByUUID, .defineXML = remoteSecretDefineXML, .getXMLDesc = remoteSecretGetXMLDesc, .setValue = remoteSecretSetValue, diff --git a/src/secret_conf.c b/src/secret_conf.c index c7b11a9..51ac13d 100644 --- a/src/secret_conf.c +++ b/src/secret_conf.c @@ -31,6 +31,7 @@ #include "virterror_internal.h" #include "util.h" #include "xml.h" +#include "uuid.h" #define VIR_FROM_THIS VIR_FROM_SECRET @@ -42,7 +43,6 @@ virSecretDefFree(virSecretDefPtr def) if (def == NULL) return; - VIR_FREE(def->id); VIR_FREE(def->description); switch (def->usage_type) { case VIR_SECRET_USAGE_TYPE_NONE: @@ -105,6 +105,7 @@ secretXMLParseNode(virConnectPtr conn, xmlDocPtr xml, xmlNodePtr root) xmlXPathContextPtr ctxt = NULL; virSecretDefPtr def = NULL, ret = NULL; char *prop = NULL; + char *uuidstr = NULL; if (!xmlStrEqual(root->name, BAD_CAST "secret")) { virSecretReportError(conn, VIR_ERR_XML_ERROR, "%s", @@ -152,7 +153,22 @@ secretXMLParseNode(virConnectPtr conn, xmlDocPtr xml, xmlNodePtr root) VIR_FREE(prop); } - def->id = virXPathString(conn, "string(./uuid)", ctxt); + uuidstr = virXPathString(conn, "string(./uuid)", ctxt); + if (!uuidstr) { + if (virUUIDGenerate(def->uuid)) { + virSecretReportError(conn, VIR_ERR_INTERNAL_ERROR, + "%s", _("Failed to generate UUID")); + goto cleanup; + } + } else { + if (virUUIDParse(uuidstr, def->uuid) < 0) { + virSecretReportError(conn, VIR_ERR_INTERNAL_ERROR, + "%s", _("malformed uuid element")); + goto cleanup; + } + VIR_FREE(uuidstr); + } + def->description = virXPathString(conn, "string(./description)", ctxt); if (virXPathNode(conn, "./usage", ctxt) != NULL && virSecretDefParseUsage(conn, ctxt, def) < 0) @@ -280,13 +296,17 @@ char * virSecretDefFormat(virConnectPtr conn, const virSecretDefPtr def) { virBuffer buf = VIR_BUFFER_INITIALIZER; + unsigned char *uuid; + char uuidstr[VIR_UUID_STRING_BUFLEN]; char *tmp; virBufferVSprintf(&buf, "<secret ephemeral='%s' private='%s'>\n", def->ephemeral ? "yes" : "no", def->private ? "yes" : "no"); - if (def->id != NULL) - virBufferEscapeString(&buf, " <uuid>%s</uuid>\n", def->id); + + uuid = def->uuid; + virUUIDFormat(uuid, uuidstr); + virBufferEscapeString(&buf, " <uuid>%s</uuid>\n", uuidstr); if (def->description != NULL) virBufferEscapeString(&buf, " <description>%s</description>\n", def->description); diff --git a/src/secret_conf.h b/src/secret_conf.h index 95beedf..556f5a4 100644 --- a/src/secret_conf.h +++ b/src/secret_conf.h @@ -43,7 +43,7 @@ typedef virSecretDef *virSecretDefPtr; struct _virSecretDef { unsigned ephemeral : 1; unsigned private : 1; - char *id; /* May be NULL */ + unsigned char uuid[VIR_UUID_BUFLEN]; char *description; /* May be NULL */ int usage_type; union { diff --git a/src/secret_driver.c b/src/secret_driver.c index 40c3ede..c1e0c67 100644 --- a/src/secret_driver.c +++ b/src/secret_driver.c @@ -111,13 +111,13 @@ secretFree(virSecretEntryPtr secret) } static virSecretEntryPtr * -secretFind(virSecretDriverStatePtr driver, const char *uuid) +secretFind(virSecretDriverStatePtr driver, const unsigned char *uuid) { virSecretEntryPtr *pptr, s; for (pptr = &driver->secrets; *pptr != NULL; pptr = &s->next) { s = *pptr; - if (STREQ(s->def->id, uuid)) + if (memcmp(s->def->uuid, uuid, VIR_UUID_BUFLEN) == 0) return pptr; } return NULL; @@ -125,15 +125,13 @@ secretFind(virSecretDriverStatePtr driver, const char *uuid) static virSecretEntryPtr secretCreate(virConnectPtr conn, virSecretDriverStatePtr driver, - const char *uuid) + const unsigned char *uuid) { virSecretEntryPtr secret = NULL; if (VIR_ALLOC(secret) < 0 || VIR_ALLOC(secret->def)) goto no_memory; - secret->def->id = strdup(uuid); - if (secret->def->id == NULL) - goto no_memory; + memcpy(secret->def->uuid, uuid, VIR_UUID_BUFLEN); listInsert(&driver->secrets, secret); return secret; @@ -145,7 +143,7 @@ secretCreate(virConnectPtr conn, virSecretDriverStatePtr driver, static virSecretEntryPtr secretFindOrCreate(virConnectPtr conn, virSecretDriverStatePtr driver, - const char *uuid, bool *created_new) + const unsigned char *uuid, bool *created_new) { virSecretEntryPtr *pptr, secret; @@ -223,18 +221,15 @@ static char * secretComputePath(virConnectPtr conn, virSecretDriverStatePtr driver, const virSecretEntry *secret, const char *suffix) { - char *ret, *base64_id; + char *ret; + char uuidstr[VIR_UUID_STRING_BUFLEN]; - base64_encode_alloc(secret->def->id, strlen(secret->def->id), &base64_id); - if (base64_id == NULL) { - virReportOOMError(conn); - return NULL; - } + virUUIDFormat(secret->def->uuid, uuidstr); - if (virAsprintf(&ret, "%s/%s%s", driver->directory, base64_id, suffix) < 0) + if (virAsprintf(&ret, "%s/%s%s", driver->directory, uuidstr, suffix) < 0) /* ret is NULL */ virReportOOMError(conn); - VIR_FREE(base64_id); + return ret; } @@ -357,28 +352,16 @@ static int secretLoadValidateUUID(virConnectPtr conn, virSecretDefPtr def, const char *xml_basename) { - char *base64_id; + char uuidstr[VIR_UUID_STRING_BUFLEN]; - if (def->id == NULL) { - virSecretReportError(conn, VIR_ERR_INTERNAL_ERROR, - _("<uuid> missing in secret description in '%s'"), - xml_basename); - return -1; - } + virUUIDFormat(def->uuid, uuidstr); - base64_encode_alloc(def->id, strlen(def->id), &base64_id); - if (base64_id == NULL) { - virReportOOMError(conn); - return -1; - } - if (!virFileMatchesNameSuffix(xml_basename, base64_id, ".xml")) { + if (!virFileMatchesNameSuffix(xml_basename, uuidstr, ".xml")) { virSecretReportError(conn, VIR_ERR_INTERNAL_ERROR, _("<uuid> does not match secret file name '%s'"), xml_basename); - VIR_FREE(base64_id); return -1; } - VIR_FREE(base64_id); return 0; } @@ -604,11 +587,13 @@ secretListSecrets(virConnectPtr conn, char **uuids, int maxuuids) i = 0; for (secret = driver->secrets; secret != NULL; secret = secret->next) { + char *uuidstr; if (i == maxuuids) break; - uuids[i] = strdup(secret->def->id); - if (uuids[i] == NULL) + if (VIR_ALLOC_N(uuidstr, VIR_UUID_STRING_BUFLEN) < 0) goto cleanup; + virUUIDFormat(secret->def->uuid, uuidstr); + uuids[i] = uuidstr; i++; } @@ -625,7 +610,7 @@ cleanup: } static virSecretPtr -secretLookupByUUIDString(virConnectPtr conn, const char *uuid) +secretLookupByUUID(virConnectPtr conn, const unsigned char *uuid) { virSecretDriverStatePtr driver = conn->secretPrivateData; virSecretPtr ret = NULL; @@ -635,49 +620,25 @@ secretLookupByUUIDString(virConnectPtr conn, const char *uuid) pptr = secretFind(driver, uuid); if (pptr == NULL) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(uuid, uuidstr); virSecretReportError(conn, VIR_ERR_NO_SECRET, - _("no secret with matching id '%s'"), uuid); + _("no secret with matching uuid '%s'"), uuidstr); goto cleanup; } - ret = virGetSecret(conn, (*pptr)->def->id); + ret = virGetSecret(conn, (*pptr)->def->uuid); cleanup: + if (1) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(uuid, uuidstr); + VIR_ERROR("Lookup %s got %p", uuidstr, ret); + } secretDriverUnlock(driver); return ret; } -static char * -secretGenerateUUID(virConnectPtr conn, virSecretDriverStatePtr driver) -{ - char *uuid = NULL; - unsigned attempt; - - if (VIR_ALLOC_N(uuid, VIR_UUID_STRING_BUFLEN) < 0) { - virReportOOMError(conn); - goto error; - } - - for (attempt = 0; attempt < 65536; attempt++) { - unsigned char uuid_data[VIR_UUID_BUFLEN]; - - if (virUUIDGenerate(uuid_data) < 0) { - virSecretReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s", - _("unable to generate uuid")); - goto error; - } - virUUIDFormat(uuid_data, uuid); - if (secretFind(driver, uuid) == NULL) - return uuid; - } - virSecretReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s", - _("too many conflicts when generating an uuid")); - goto error; - -error: - VIR_FREE(uuid); - return NULL; -} static virSecretPtr secretDefineXML(virConnectPtr conn, const char *xml, @@ -695,16 +656,8 @@ secretDefineXML(virConnectPtr conn, const char *xml, secretDriverLock(driver); - if (new_attrs->id != NULL) - secret = secretFindOrCreate(conn, driver, new_attrs->id, - &secret_is_new); - else { - new_attrs->id = secretGenerateUUID(conn, driver); - if (new_attrs->id == NULL) - goto cleanup; - secret = secretCreate(conn, driver, new_attrs->id); - secret_is_new = true; - } + secret = secretFindOrCreate(conn, driver, new_attrs->uuid, + &secret_is_new); if (secret == NULL) goto cleanup; @@ -743,7 +696,7 @@ secretDefineXML(virConnectPtr conn, const char *xml, new_attrs = NULL; virSecretDefFree(backup); - ret = virGetSecret(conn, secret->def->id); + ret = virGetSecret(conn, secret->def->uuid); goto cleanup; restore_backup: @@ -776,8 +729,10 @@ secretGetXMLDesc(virSecretPtr obj, unsigned int flags ATTRIBUTE_UNUSED) pptr = secretFind(driver, obj->uuid); if (pptr == NULL) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(obj->uuid, uuidstr); virSecretReportError(obj->conn, VIR_ERR_NO_SECRET, - _("no secret with matching id '%s'"), obj->uuid); + _("no secret with matching uuid '%s'"), uuidstr); goto cleanup; } @@ -808,8 +763,10 @@ secretSetValue(virSecretPtr obj, const unsigned char *value, pptr = secretFind(driver, obj->uuid); if (pptr == NULL) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(obj->uuid, uuidstr); virSecretReportError(obj->conn, VIR_ERR_NO_SECRET, - _("no secret with matching id '%s'"), obj->uuid); + _("no secret with matching uuid '%s'"), uuidstr); goto cleanup; } secret = *pptr; @@ -859,14 +816,18 @@ secretGetValue(virSecretPtr obj, size_t *value_size, unsigned int flags) pptr = secretFind(driver, obj->uuid); if (pptr == NULL) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(obj->uuid, uuidstr); virSecretReportError(obj->conn, VIR_ERR_NO_SECRET, - _("no secret with matching id '%s'"), obj->uuid); + _("no secret with matching uuid '%s'"), uuidstr); goto cleanup; } secret = *pptr; if (secret->value == NULL) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(obj->uuid, uuidstr); virSecretReportError(obj->conn, VIR_ERR_NO_SECRET, - _("secret '%s' does not have a value"), obj->uuid); + _("secret '%s' does not have a value"), uuidstr); goto cleanup; } @@ -901,8 +862,10 @@ secretUndefine(virSecretPtr obj) pptr = secretFind(driver, obj->uuid); if (pptr == NULL) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(obj->uuid, uuidstr); virSecretReportError(obj->conn, VIR_ERR_NO_SECRET, - _("no secret with matching id '%s'"), obj->uuid); + _("no secret with matching uuid '%s'"), uuidstr); goto cleanup; } @@ -1036,7 +999,7 @@ static virSecretDriver secretDriver = { .close = secretClose, .numOfSecrets = secretNumOfSecrets, .listSecrets = secretListSecrets, - .lookupByUUIDString = secretLookupByUUIDString, + .lookupByUUID = secretLookupByUUID, .defineXML = secretDefineXML, .getXMLDesc = secretGetXMLDesc, .setValue = secretSetValue, diff --git a/src/storage_backend.c b/src/storage_backend.c index cf5a593..800d4ea 100644 --- a/src/storage_backend.c +++ b/src/storage_backend.c @@ -50,8 +50,9 @@ #include "node_device.h" #include "internal.h" #include "secret_conf.h" - +#include "uuid.h" #include "storage_backend.h" +#include "logging.h" #if WITH_STORAGE_LVM #include "storage_backend_logical.h" @@ -338,6 +339,32 @@ cleanup: } static int +virStorageGenerateSecretUUID(virConnectPtr conn, + unsigned char *uuid) +{ + unsigned attempt; + + for (attempt = 0; attempt < 65536; attempt++) { + virSecretPtr tmp; + if (virUUIDGenerate(uuid) < 0) { + virSecretReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s", + _("unable to generate uuid")); + return -1; + } + tmp = conn->secretDriver->lookupByUUID(conn, uuid); + if (tmp == NULL) + return 0; + + virSecretFree(tmp); + } + + virSecretReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s", + _("too many conflicts when generating an uuid")); + + return -1; +} + +static int virStorageGenerateQcowEncryption(virConnectPtr conn, virStorageVolDefPtr vol) { @@ -346,11 +373,13 @@ virStorageGenerateQcowEncryption(virConnectPtr conn, virStorageEncryptionPtr enc; virStorageEncryptionSecretPtr enc_secret = NULL; virSecretPtr secret = NULL; - char *uuid = NULL, *xml; + char *xml; unsigned char value[VIR_STORAGE_QCOW_PASSPHRASE_SIZE]; int ret = -1; - if (conn->secretDriver == NULL || conn->secretDriver->defineXML == NULL || + if (conn->secretDriver == NULL || + conn->secretDriver->lookupByUUID == NULL || + conn->secretDriver->defineXML == NULL || conn->secretDriver->setValue == NULL) { virStorageReportError(conn, VIR_ERR_NO_SUPPORT, "%s", _("secret storage not supported")); @@ -372,12 +401,9 @@ virStorageGenerateQcowEncryption(virConnectPtr conn, def->ephemeral = 0; def->private = 0; - def->id = NULL; /* Chosen by the secret driver */ - if (virAsprintf(&def->description, "qcow passphrase for %s", - vol->target.path) == -1) { - virReportOOMError(conn); + if (virStorageGenerateSecretUUID(conn, def->uuid) < 0) goto cleanup; - } + def->usage_type = VIR_SECRET_USAGE_TYPE_VOLUME; def->usage.volume = strdup(vol->target.path); if (def->usage.volume == NULL) { @@ -397,22 +423,14 @@ virStorageGenerateQcowEncryption(virConnectPtr conn, } VIR_FREE(xml); - uuid = strdup(secret->uuid); - if (uuid == NULL) { - virReportOOMError(conn); - goto cleanup; - } - if (virStorageGenerateQcowPassphrase(conn, value) < 0) goto cleanup; if (conn->secretDriver->setValue(secret, value, sizeof(value), 0) < 0) goto cleanup; - secret = NULL; enc_secret->type = VIR_STORAGE_ENCRYPTION_SECRET_TYPE_PASSPHRASE; - enc_secret->uuid = uuid; - uuid = NULL; + memcpy(enc_secret->uuid, secret->uuid, VIR_UUID_BUFLEN); enc->format = VIR_STORAGE_ENCRYPTION_FORMAT_QCOW; enc->secrets[0] = enc_secret; /* Space for secrets[0] allocated above */ enc_secret = NULL; @@ -421,9 +439,9 @@ virStorageGenerateQcowEncryption(virConnectPtr conn, ret = 0; cleanup: - VIR_FREE(uuid); if (secret != NULL) { - if (conn->secretDriver->undefine != NULL) + if (ret != 0 && + conn->secretDriver->undefine != NULL) conn->secretDriver->undefine(secret); virSecretFree(secret); } diff --git a/src/storage_encryption_conf.c b/src/storage_encryption_conf.c index 1ce76b5..b97b989 100644 --- a/src/storage_encryption_conf.c +++ b/src/storage_encryption_conf.c @@ -34,6 +34,7 @@ #include "util.h" #include "xml.h" #include "virterror_internal.h" +#include "uuid.h" #define VIR_FROM_THIS VIR_FROM_STORAGE @@ -49,7 +50,6 @@ virStorageEncryptionSecretFree(virStorageEncryptionSecretPtr secret) { if (!secret) return; - VIR_FREE(secret->uuid); VIR_FREE(secret); } @@ -77,6 +77,7 @@ virStorageEncryptionSecretParse(virConnectPtr conn, xmlXPathContextPtr ctxt, virStorageEncryptionSecretPtr ret; char *type_str; int type; + char *uuidstr = NULL; if (VIR_ALLOC(ret) < 0) { virReportOOMError(conn); @@ -103,12 +104,25 @@ virStorageEncryptionSecretParse(virConnectPtr conn, xmlXPathContextPtr ctxt, VIR_FREE(type_str); ret->type = type; - ret->uuid = virXPathString(conn, "string(./@uuid)", ctxt); + uuidstr = virXPathString(conn, "string(./@uuid)", ctxt); + if (uuidstr) { + if (virUUIDParse(uuidstr, ret->uuid) < 0) { + virStorageReportError(conn, VIR_ERR_XML_ERROR, + _("malformed volume encryption uuid '%s'"), + uuidstr); + goto cleanup; + } + } else { + virStorageReportError(conn, VIR_ERR_XML_ERROR, "%s", + _("missing volume encryption uuid")); + goto cleanup; + } ctxt->node = old_node; return ret; cleanup: virStorageEncryptionSecretFree(ret); + VIR_FREE(uuidstr); ctxt->node = old_node; return NULL; } @@ -205,6 +219,7 @@ virStorageEncryptionSecretFormat(virConnectPtr conn, virStorageEncryptionSecretPtr secret) { const char *type; + char uuidstr[VIR_UUID_STRING_BUFLEN]; type = virStorageEncryptionSecretTypeTypeToString(secret->type); if (!type) { @@ -213,10 +228,8 @@ virStorageEncryptionSecretFormat(virConnectPtr conn, return -1; } - virBufferVSprintf(buf, " <secret type='%s'", type); - if (secret->uuid != NULL) - virBufferEscapeString(buf, " uuid='%s'", secret->uuid); - virBufferAddLit(buf, "/>\n"); + virUUIDFormat(secret->uuid, uuidstr); + virBufferVSprintf(buf, " <secret type='%s' uuid='%s'/>\n", type, uuidstr); return 0; } @@ -234,14 +247,14 @@ virStorageEncryptionFormat(virConnectPtr conn, "%s", _("unexpected encryption format")); return -1; } - virBufferVSprintf(buf, " <encryption format='%s'>\n", format); + virBufferVSprintf(buf, " <encryption format='%s'>\n", format); for (i = 0; i < enc->nsecrets; i++) { if (virStorageEncryptionSecretFormat(conn, buf, enc->secrets[i]) < 0) return -1; } - virBufferAddLit(buf, " </encryption>\n"); + virBufferAddLit(buf, " </encryption>\n"); return 0; } diff --git a/src/storage_encryption_conf.h b/src/storage_encryption_conf.h index cdcf625..5d0bc3c 100644 --- a/src/storage_encryption_conf.h +++ b/src/storage_encryption_conf.h @@ -41,7 +41,7 @@ typedef struct _virStorageEncryptionSecret virStorageEncryptionSecret; typedef virStorageEncryptionSecret *virStorageEncryptionSecretPtr; struct _virStorageEncryptionSecret { int type; /* enum virStorageEncryptionSecretType */ - char *uuid; + unsigned char uuid[VIR_UUID_BUFLEN]; }; enum virStorageEncryptionFormat { @@ -63,6 +63,7 @@ struct _virStorageEncryption { }; void virStorageEncryptionFree(virStorageEncryptionPtr enc); + virStorageEncryptionPtr virStorageEncryptionParseNode(virConnectPtr conn, xmlDocPtr xml, xmlNodePtr root); diff --git a/src/virsh.c b/src/virsh.c index 9dc8857..74147da 100644 --- a/src/virsh.c +++ b/src/virsh.c @@ -5270,8 +5270,9 @@ static const vshCmdOptDef opts_secret_define[] = { static int cmdSecretDefine(vshControl *ctl, const vshCmd *cmd) { - char *from, *buffer, *uuid; + char *from, *buffer; virSecretPtr res; + char uuid[VIR_UUID_STRING_BUFLEN]; if (!vshConnectionUsability(ctl, ctl->conn, TRUE)) return FALSE; @@ -5290,15 +5291,13 @@ cmdSecretDefine(vshControl *ctl, const vshCmd *cmd) vshError(ctl, FALSE, _("Failed to set attributes from %s"), from); return FALSE; } - uuid = virSecretGetUUIDString(res); - if (uuid == NULL) { + if (virSecretGetUUIDString(res, &(uuid[0])) < 0) { vshError(ctl, FALSE, "%s", _("Failed to get UUID of created secret")); virSecretFree(res); return FALSE; } vshPrint(ctl, _("Secret %s created\n"), uuid); - free(uuid); virSecretFree(res); return TRUE; } -- 1.6.2.5

On Fri, Sep 11, 2009 at 08:28:59PM -0400, Miloslav Trmac wrote: Opps, yes that's killed now Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

On Fri, Sep 11, 2009 at 03:19:18PM +0100, Daniel P. Berrange wrote: [...] Looking from the outside I find that hard to graps especially the last one. virSecretGetUsageID return value supposed to be an UUID , the comment let the user expect this but it seems to be actually free form. I'm not against the patch but I think this needs some example or improved comments to really set properly the mechanism and expectations for the user. This may come after as a set of documentations, but if the function description could be clarified a bit this would be nice. Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

On Fri, Sep 11, 2009 at 03:19:19PM +0100, Daniel P. Berrange wrote: ACK, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

On Fri, Sep 11, 2009 at 03:19:20PM +0100, Daniel P. Berrange wrote: ACK thanks ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/