May 2016 - Devel - libvirt List Archives

[libvirt] 撤回: error: internal error unable to execute QEMU command 'getfd': No file descriptor supplied via SCM_RIGHTS
by 饶俊明(云平台事业部) 14 May '16

14 May '16

饶俊明(云平台事业部) 将撤回邮件“error: internal error unable to execute QEMU command 'getfd': No file descriptor supplied via SCM_RIGHTS”。 ******************************************************************************************************************************** The information in this email is confidential and may be legally privileged. If you have received this email in error or are not the intended recipient, please immediately notify the sender and delete this message from your computer. Any use, distribution, or copying of this email other than by the intended recipient is strictly prohibited. All messages sent to and from us may be monitored to ensure compliance with internal policies and to protect our business. Emails are not secure and cannot be guaranteed to be error free as they can be intercepted, amended, lost or destroyed, or contain viruses. Anyone who communicates with us by email is taken to accept these risks. 收发邮件者请注意：本邮件含保密信息，若误收本邮件，请务必通知发送人并直接删去，不得使用、传播或复制本邮件。进出邮件均受到本公司合规监控。邮件可能发生被截留、被修改、丢失、被破坏或包含计算机病毒等不安全情况。 ********************************************************************************************************************************

1 0

[libvirt] error: internal error unable to execute QEMU command 'getfd': No file descriptor supplied via SCM_RIGHTS
by 饶俊明(云平台事业部) 14 May '16

14 May '16

Hi, I am getting the following error when I try to migrate VM on my network using virsh migrate; error : qemuMonitorJSONCheckError:359 : internal error unable to execute QEMU command 'getfd': No file descriptor supplied via SCM_RIGHTS but, other vm's running on the server successfully migrated to remote server; Libvirt Version: libvirt-0.10.2-46.el6.x86_64 Qemu-kvm Version: qemu-kvm-0.12.1.2-2.448.el6.3.x86_64 OS Version: Red Hat Enterprise Linux Server release 6.6 Thanks~ ******************************************************************************************************************************** The information in this email is confidential and may be legally privileged. If you have received this email in error or are not the intended recipient, please immediately notify the sender and delete this message from your computer. Any use, distribution, or copying of this email other than by the intended recipient is strictly prohibited. All messages sent to and from us may be monitored to ensure compliance with internal policies and to protect our business. Emails are not secure and cannot be guaranteed to be error free as they can be intercepted, amended, lost or destroyed, or contain viruses. Anyone who communicates with us by email is taken to accept these risks. 收发邮件者请注意：本邮件含保密信息，若误收本邮件，请务必通知发送人并直接删去，不得使用、传播或复制本邮件。进出邮件均受到本公司合规监控。邮件可能发生被截留、被修改、丢失、被破坏或包含计算机病毒等不安全情况。 ********************************************************************************************************************************

1 0

Re: [libvirt] [libvirt-users] libvirt + openvswitch, <parameters interfaceid='x'/> seems less-than-useful?
by Laine Stump 14 May '16

14 May '16

On 05/13/2016 08:24 PM, Richard Harman wrote: > On 05/13/16 20:04, Laine Stump wrote: >> On 05/13/2016 09:57 AM, Richard Harman wrote: >>> I've debugged this, and I think the interfaceid stuff in network XML >>> isn't doing much besides setting a custom interfaceid in an OpenVSwitch >>> table that really has no effect on the operation of OpenVSwitch. I >>> think the -- set Interface vnet0 "external-ids:iface-id="{uuid here}" >>> command run by libvirt doesn't actually set the _uuid of a new >>> interface. :( >> I don't use Open vSwitch, but I was around when the support was added, >> and reviewed the patches (which were authored by Kyle Mestery, whose >> email has changed since then, so I'm not sure how to Cc him). My >> understanding at the time was that the interfaceid was intended for >> exactly the purpose that you say doesn't work (i.e. so that OVS itself >> could recognize that interface as it appeared and disappeared at >> different locations on the network). I would classify that as a bug, >> but that's just my understanding as an OVS outsider. >> > Further evidence, now that I figured out how to dump the external-ids > stuff in OVS: > > Here's the relevant ports of the domain XML: > > <domain type='kvm' id='5'> > <name>cuckoo</name> > <uuid>cb4903aa-eaf4-432d-aa57-e9f74f1c8b79</uuid> > .... > <interface type='bridge'> > <mac address='52:54:00:f5:29:72'/> > <source network='malware' bridge='malware0'/> > <virtualport type='openvswitch'> > <parameters interfaceid='23e982f3-f344-48a2-8bed-b584acb95616'/> > </virtualport> > <target dev='monitor0'/> > <model type='virtio'/> > <alias name='net2'/> > <address type='pci' domain='0x0000' bus='0x00' slot='0x08' > function='0x0'/> > </interface> > .... > > cuckoo vm up: monitor0 _uuid is 969ef76c... > > # ovs-vsctl --columns=name,_uuid,external-ids list Interface | grep -A3 > monitor0 > name : "monitor0" > _uuid : 969ef76c-c802-4e9f-97e0-9a06ce133252 > external_ids : {attached-mac="52:54:00:f5:29:72", > iface-id="23e982f3-f344-48a2-8bed-b584acb95616", iface-status=active, > vm-id="cb4903aa-eaf4-432d-aa57-e9f74f1c8b79"} > # > > cuckoo vm down: (no output, the interface has been deleted by libvirt) > > # ovs-vsctl --columns=name,_uuid,external-ids list Interface | grep -A3 > monitor0 > # > > I found this in my log, showing libvirt deleted the interface: > ovs-vsctl[4590]: ovs|00001|vsctl|INFO|Called as ovs-vsctl --timeout=5 -- > --if-exists del-port monitor0 > > cuckoo vm up again: uuid is now fb025df0... > > # ovs-vsctl --columns=name,_uuid,external-ids list Interface | grep -A3 > monitor0 > name : "monitor0" > _uuid : fb025df0-d934-46d6-b9ed-314bc1de5ef3 > external_ids : {attached-mac="52:54:00:f5:29:72", > iface-id="23e982f3-f344-48a2-8bed-b584acb95616", iface-status=active, > vm-id="cb4903aa-eaf4-432d-aa57-e9f74f1c8b79"} > # > > Anyway, thanks for reading! Hopefully we'll get to the bottom of this, > where I can keep my span port. :) In the mean time I'm going to go > digging through the OVS source, specifically the ovs-vsctl command to > see if an argument can be used to forcibly set the internal _uuid in OVS. Right. Either the ovs-vsctl command used by libvirt needs to change, or (if it's actually a bug in OVS) ovs-ctl's use of external-ids:iface-id needs to change. I'm Cc'ing libvir-list, just in case there's an OVS person who monitors that list but not this one.

1 0

[libvirt] [PATCH] bhyve: implement virConnectGetType
by Fabian Freyer 14 May '16

14 May '16

This implements virConnectGetType for the bhyve driver. --- src/bhyve/bhyve_driver.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index 4fc504e..a853e94 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c @@ -1508,6 +1508,15 @@ bhyveDomainHasManagedSaveImage(virDomainPtr domain, unsigned int flags) return ret; } +static const char * +bhyveConnectGetType(virConnectPtr conn ATTRIBUTE_UNUSED) +{ + if (virConnectGetTypeEnsureACL(conn) < 0) + return NULL; + + return "BHYVE"; +} + static virHypervisorDriver bhyveHypervisorDriver = { .name = "bhyve", .connectOpen = bhyveConnectOpen, /* 1.2.2 */ @@ -1557,6 +1566,7 @@ static virHypervisorDriver bhyveHypervisorDriver = { .connectDomainEventRegisterAny = bhyveConnectDomainEventRegisterAny, /* 1.2.5 */ .connectDomainEventDeregisterAny = bhyveConnectDomainEventDeregisterAny, /* 1.2.5 */ .domainHasManagedSaveImage = bhyveDomainHasManagedSaveImage, /* 1.2.13 */ + .connectGetType = bhyveConnectGetType, /* 1.3.5 */ }; -- 2.1.4

3 5

[libvirt] [PATCH 0/4] Automatically choose usable GIC version
by Andrea Bolognani 14 May '16

14 May '16

Currently, if no GIC version has been provided by the user, libvirt will default to GIC v2. This is a problem when trying to create new guests on hardware that only supports GIC v3 guests: QEMU will refuse to start a GIC v2 guest, and guest installation will abort immediately. This series implements a way for libvirt to figure out a suitable GIC version itself, without relying on tools such as virt-install to parse the domain capabilities before attempting installation. Andrea Bolognani (4): qemu: Automatically choose usable GIC version qemu: Add virQEMUCapsSetGICCapabilities() tests: Prepare to have different usable GIC versions tests: Try different usable GIC versions src/conf/domain_capabilities.c | 25 +++ src/conf/domain_capabilities.h | 8 + src/libvirt_private.syms | 1 + src/qemu/qemu_capabilities.c | 29 +++- src/qemu/qemu_capabilities.h | 5 + src/qemu/qemu_domain.c | 66 ++++++-- .../qemuxml2argv-aarch64-gic-none-both.args | 1 + .../qemuxml2argv-aarch64-gic-none-both.xml | 1 + .../qemuxml2argv-aarch64-gic-none-v2.args | 1 + .../qemuxml2argv-aarch64-gic-none-v2.xml | 1 + .../qemuxml2argv-aarch64-gic-none-v3.args | 1 + .../qemuxml2argv-aarch64-gic-none-v3.xml | 1 + tests/qemuxml2argvtest.c | 153 +++++++++++++---- .../qemuxml2xmlout-aarch64-gic-none-both.xml | 1 + .../qemuxml2xmlout-aarch64-gic-none-v2.xml | 1 + .../qemuxml2xmlout-aarch64-gic-none-v3.xml | 1 + tests/qemuxml2xmltest.c | 184 +++++++++++++-------- 17 files changed, 368 insertions(+), 112 deletions(-) create mode 120000 tests/qemuxml2argvdata/qemuxml2argv-aarch64-gic-none-both.args create mode 120000 tests/qemuxml2argvdata/qemuxml2argv-aarch64-gic-none-both.xml create mode 120000 tests/qemuxml2argvdata/qemuxml2argv-aarch64-gic-none-v2.args create mode 120000 tests/qemuxml2argvdata/qemuxml2argv-aarch64-gic-none-v2.xml create mode 120000 tests/qemuxml2argvdata/qemuxml2argv-aarch64-gic-none-v3.args create mode 120000 tests/qemuxml2argvdata/qemuxml2argv-aarch64-gic-none-v3.xml create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-gic-none-both.xml create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-gic-none-v2.xml create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-gic-none-v3.xml -- 2.5.5

3 10

[libvirt] [PATCH 0/2] libxl: A few more fixes related to vram
by Jim Fehlig 14 May '16

14 May '16

Commit 3e428670 broke the xlconfigtests, which commit 96b21fb0 mostly fixed. I found that running 'make check' on a machine with Xen installed (/usr/lib/xen/bin/qemu-{dm,system-i386} exists) failed. Test files containing <emulator>/usr/lib/xen/bin/qemu-dm</emulator> resulted in vram being set to the lower default values defined by the old qemu-dm. This series takes one possible approach to fixing the problem by only using qemu-xen (aka qemu upstream) in the test files (note that libxl defaults to qemu-xen if an emulator is not specified). Actually, it's patch 2 that changes the test files. I added patch 1 after seeing errors such as libvirt: error : internal error: Child process (/usr/lib/xen/bin/qemu-dm -help) unexpected exit status 127: libvirt: error : cannot execute binary /usr/lib/xen/bin/qemu-dm: No such file or directory when specifying a non-existent emulator. Another approach, suggested by Joao, is to allow vram == 0 and when building the domain, set video_memkb in the libxl_domain_build_info struct to LIBXL_MEMKB_DEFAULT, allowing libxl to pick the correct default. For this approach, the <video> parsing logic would have to change again. Currently, it sets a vram defaults if not done by the driver post parse function. Along with adjusting the libxl driver to handle vram = 0, all the xlconfigtests would have to change. E.g. - <model type='cirrus' vram='8192' heads='1' primary='yes'/> + <model type='cirrus' heads='1' primary='yes'/> Jim Fehlig (2): libxl: don't attempt to probe a non-existent emulator xlconfigtests: use qemu-xen in all test data files src/libxl/libxl_conf.c | 3 +++ tests/xlconfigdata/test-disk-positional-parms-full.cfg | 2 +- tests/xlconfigdata/test-disk-positional-parms-full.xml | 2 +- tests/xlconfigdata/test-disk-positional-parms-partial.cfg | 2 +- tests/xlconfigdata/test-disk-positional-parms-partial.xml | 2 +- tests/xlconfigdata/test-fullvirt-multiusb.cfg | 2 +- tests/xlconfigdata/test-fullvirt-multiusb.xml | 2 +- tests/xlconfigdata/test-fullvirt-nohap.cfg | 2 +- tests/xlconfigdata/test-fullvirt-nohap.xml | 2 +- tests/xlconfigdata/test-new-disk.cfg | 2 +- tests/xlconfigdata/test-new-disk.xml | 2 +- tests/xlconfigdata/test-rbd-multihost-noauth.cfg | 2 +- tests/xlconfigdata/test-rbd-multihost-noauth.xml | 2 +- tests/xlconfigdata/test-spice-features.cfg | 2 +- tests/xlconfigdata/test-spice-features.xml | 2 +- tests/xlconfigdata/test-spice.cfg | 2 +- tests/xlconfigdata/test-spice.xml | 2 +- tests/xlconfigdata/test-vif-rate.cfg | 2 +- tests/xlconfigdata/test-vif-rate.xml | 2 +- 19 files changed, 21 insertions(+), 18 deletions(-) -- 2.8.2

2 7

[libvirt] [PATCH] virt-aa-helper: remove replace_string and use virStringReplace instead
by Pavel Hrdina 14 May '16

14 May '16

Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com> --- src/security/virt-aa-helper.c | 76 +++++-------------------------------------- 1 file changed, 9 insertions(+), 67 deletions(-) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 7eeb4ef..537e89d 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -148,62 +148,6 @@ vah_info(const char *str) } /* - * Replace @oldstr in @orig with @repstr - * @len is number of bytes allocated for @orig. Assumes @orig, @oldstr and - * @repstr are null terminated - */ -static int -replace_string(char *orig, const size_t len, const char *oldstr, - const char *repstr) -{ - int idx; - char *pos = NULL; - char *tmp = NULL; - - if ((pos = strstr(orig, oldstr)) == NULL) { - vah_error(NULL, 0, _("could not find replacement string")); - return -1; - } - - if (VIR_ALLOC_N_QUIET(tmp, len) < 0) { - vah_error(NULL, 0, _("could not allocate memory for string")); - return -1; - } - tmp[0] = '\0'; - - idx = abs(pos - orig); - - /* copy everything up to oldstr */ - strncat(tmp, orig, idx); - - /* add the replacement string */ - if (strlen(tmp) + strlen(repstr) > len - 1) { - vah_error(NULL, 0, _("not enough space in target buffer")); - VIR_FREE(tmp); - return -1; - } - strcat(tmp, repstr); - - /* add everything after oldstr */ - if (strlen(tmp) + strlen(orig) - (idx + strlen(oldstr)) > len - 1) { - vah_error(NULL, 0, _("not enough space in target buffer")); - VIR_FREE(tmp); - return -1; - } - strncat(tmp, orig + idx + strlen(oldstr), - strlen(orig) - (idx + strlen(oldstr))); - - if (virStrcpy(orig, tmp, len) == NULL) { - vah_error(NULL, 0, _("error replacing string")); - VIR_FREE(tmp); - return -1; - } - VIR_FREE(tmp); - - return 0; -} - -/* * run an apparmor_parser command */ static int @@ -340,6 +284,7 @@ create_profile(const char *profile, const char *profile_name, char *pcontent = NULL; char *replace_name = NULL; char *replace_files = NULL; + char *tmp = NULL; const char *template_name = "\nprofile LIBVIRT_TEMPLATE"; const char *template_end = "\n}"; int tlen, plen; @@ -412,19 +357,16 @@ create_profile(const char *profile, const char *profile_name, goto clean_replace; } - if (VIR_ALLOC_N_QUIET(pcontent, plen) < 0) { - vah_error(NULL, 0, _("could not allocate memory for profile")); - goto clean_replace; - } - pcontent[0] = '\0'; - strcpy(pcontent, tcontent); - - if (replace_string(pcontent, plen, template_name, replace_name) < 0) + if (!(pcontent = virStringReplace(tcontent, template_name, replace_name))) goto clean_all; - if ((virtType != VIR_DOMAIN_VIRT_LXC) && - replace_string(pcontent, plen, template_end, replace_files) < 0) - goto clean_all; + if (virtType != VIR_DOMAIN_VIRT_LXC) { + if (!(tmp = virStringReplace(pcontent, template_end, replace_files))) + goto clean_all; + VIR_FREE(pcontent); + pcontent = tmp; + tmp = NULL; + } /* write the file */ if ((fd = open(profile, O_CREAT | O_EXCL | O_WRONLY, 0644)) == -1) { -- 2.8.2

2 2

[libvirt] [PATCH] leave out the default USB controller only on i440fx during migration
by Shivaprasad G Bhat 13 May '16

13 May '16

Further followup discussions in list on commit 192a53e concluded that we should be leaving out the USB controller only for i440fx machines as default USB can be used by someone on q35 at random slots. Signed-off-by: Shivaprasad G Bhat <sbhat(a)linux.vnet.ibm.com> --- src/qemu/qemu_domain.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 173f82c..5a9b48b 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2884,7 +2884,8 @@ qemuDomainDefFormatBuf(virQEMUDriverPtr driver, * with libvirt <= 0.9.4. Limitation doesn't apply to other archs * and can cause problems on PPC64. */ - if (ARCH_IS_X86(def->os.arch) && usb && usb->idx == 0 && usb->model == -1) { + if (ARCH_IS_X86(def->os.arch) && qemuDomainMachineIsI440FX(def) && + usb && usb->idx == 0 && usb->model == -1) { VIR_DEBUG("Removing default USB controller from domain '%s'" " for migration compatibility", def->name); toremove++;

3 2

[libvirt] [PATCH v3 0/4] Introduce big test mock
by Michal Privoznik 13 May '16

13 May '16

diff to v2: - Peter's review suggestions worked in Note that patch 2/4 is ACKed already, but without the rest it makes no sense, so I've not pushed it yet and sending it here for completeness. Michal Privoznik (4): virfile: Introduce virFileRemoveLastComponent tests: Introduce global mock library virtestmock: Print invalid file accesses into a file tests: Introduce check-file-access.pl .gitignore | 1 + HACKING | 9 ++ Makefile.am | 3 + cfg.mk | 2 +- docs/hacking.html.in | 11 ++ src/libvirt_private.syms | 1 + src/util/virfile.c | 17 +++ src/util/virfile.h | 1 + src/util/virstoragefile.c | 6 +- tests/Makefile.am | 28 ++++- tests/check-file-access.pl | 104 ++++++++++++++++ tests/file_access_whitelist.txt | 23 ++++ tests/testutils.c | 32 +++-- tests/testutils.h | 10 +- tests/vircgroupmock.c | 15 +-- tests/virpcimock.c | 14 +-- tests/virtestmock.c | 266 ++++++++++++++++++++++++++++++++++++++++ 17 files changed, 503 insertions(+), 40 deletions(-) create mode 100755 tests/check-file-access.pl create mode 100644 tests/file_access_whitelist.txt create mode 100644 tests/virtestmock.c -- 2.8.1

2 10

[libvirt] [PATCH v2] secret: Alter virSecretGetSecretString
by John Ferlan 13 May '16

13 May '16

Rather than returning a "char *" indicating perhaps some sized set of characters that is NUL terminated, alter the function to return 0 or -1 for success/failure and add two parameters to handle returning the buffer and it's size. The function no longer encodes the returned secret, rather it returns the unencoded secret forcing callers to make the necessary adjustments. Alter the callers to handle the adjusted model. Add a new function virStringBufferIsPrintable to handle checking if the plaintext secret that is about to be printed on the command line has non-printable characters and cause failure if it does. Signed-off-by: John Ferlan <jferlan(a)redhat.com> --- v1: http://www.redhat.com/archives/libvir-list/2016-May/msg00870.html Changes since v1: libvirt_private.syms, virstring.c, virstring.h: - Introduce virStringBufferIsPrintable. I could make it a separate patch if so desired. libxl_conf.c, qemu_domain.c, qemu_command.c - Use new format of virSecretGetSecretString - Encode the secret after the call now when necessary - Use virStringBufferIsPrintable before printing iSCSI password secret_util.c: - Return int instead of uint8_t * - Remove 'encode' parameter - Add parameter "*ret_secret", alloc and return the secret in uint8_t * buf src/libvirt_private.syms | 1 + src/libxl/libxl_conf.c | 24 +++++++++++++++++------- src/qemu/qemu_command.c | 18 +++++++++++++++++- src/qemu/qemu_domain.c | 17 +++++------------ src/qemu/qemu_domain.h | 3 ++- src/secret/secret_util.c | 45 ++++++++++++++++++++++----------------------- src/secret/secret_util.h | 15 ++++++++------- src/util/virstring.c | 19 +++++++++++++++++++ src/util/virstring.h | 1 + 9 files changed, 92 insertions(+), 51 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index a980a32..b76d9d5 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2301,6 +2301,7 @@ virSkipSpacesBackwards; virStrcpy; virStrdup; virStringArrayHasString; +virStringBufferIsPrintable; virStringFreeList; virStringFreeListCount; virStringGetFirstWithPrefix; diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index d927b37..2dfd03d 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -47,6 +47,7 @@ #include "libxl_utils.h" #include "virstoragefile.h" #include "secret_util.h" +#include "base64.h" #define VIR_FROM_THIS VIR_FROM_LIBXL @@ -1018,7 +1019,9 @@ static int libxlMakeNetworkDiskSrc(virStorageSourcePtr src, char **srcstr) { virConnectPtr conn = NULL; - char *secret = NULL; + uint8_t *secret = NULL; + char *base64secret = NULL; + size_t secretlen; char *username = NULL; int ret = -1; @@ -1030,21 +1033,28 @@ libxlMakeNetworkDiskSrc(virStorageSourcePtr src, char **srcstr) if (!(conn = virConnectOpen("xen:///system"))) goto cleanup; - if (!(secret = virSecretGetSecretString(conn, - protocol, - true, - src->auth, - VIR_SECRET_USAGE_TYPE_CEPH))) + if (virSecretGetSecretString(conn, protocol, src->auth, + VIR_SECRET_USAGE_TYPE_CEPH, + &secret, &secretlen) < 0) goto cleanup; + + /* RBD expects an encoded secret */ + base64_encode_alloc((const char *)secret, secretlen, &base64secret); + memset(secret, 0, secretlen); + if (!base64secret) { + virReportOOMError(); + goto cleanup; + } } - if (!(*srcstr = libxlMakeNetworkDiskSrcStr(src, username, secret))) + if (!(*srcstr = libxlMakeNetworkDiskSrcStr(src, username, base64secret))) goto cleanup; ret = 0; cleanup: VIR_FREE(secret); + VIR_FREE(base64secret); virObjectUnref(conn); return ret; } diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 0d6d5f8..18b268a 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -56,6 +56,7 @@ #include "virscsi.h" #include "virnuma.h" #include "virgic.h" +#include "base64.h" #if defined(__linux__) # include <linux/capability.h> #endif @@ -628,6 +629,12 @@ qemuBuildGeneralSecinfoURI(virURIPtr uri, switch ((qemuDomainSecretInfoType) secinfo->type) { case VIR_DOMAIN_SECRET_INFO_TYPE_PLAIN: if (secinfo->s.plain.secret) { + if (!virStringBufferIsPrintable(secinfo->s.plain.secret, + secinfo->s.plain.secretlen)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("found non printable characters in secret")); + return -1; + } if (virAsprintf(&uri->user, "%s:%s", secinfo->s.plain.username, secinfo->s.plain.secret) < 0) @@ -662,6 +669,8 @@ static int qemuBuildRBDSecinfoURI(virBufferPtr buf, qemuDomainSecretInfoPtr secinfo) { + char *base64secret = NULL; + if (!secinfo) { virBufferAddLit(buf, ":auth_supported=none"); return 0; @@ -669,11 +678,18 @@ qemuBuildRBDSecinfoURI(virBufferPtr buf, switch ((qemuDomainSecretInfoType) secinfo->type) { case VIR_DOMAIN_SECRET_INFO_TYPE_PLAIN: + base64_encode_alloc((const char *)secinfo->s.plain.secret, + secinfo->s.plain.secretlen, &base64secret); + if (!base64secret) { + virReportOOMError(); + return -1; + } virBufferEscape(buf, '\\', ":", ":id=%s", secinfo->s.plain.username); virBufferEscape(buf, '\\', ":", ":key=%s:auth_supported=cephx\\;none", - secinfo->s.plain.secret); + base64secret); + VIR_FREE(base64secret); break; case VIR_DOMAIN_SECRET_INFO_TYPE_AES: diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 39a50e6..2f94f23 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -731,7 +731,7 @@ static void qemuDomainSecretPlainFree(qemuDomainSecretPlain secret) { VIR_FREE(secret.username); - memset(secret.secret, 0, strlen(secret.secret)); + memset(secret.secret, 0, secret.secretlen); VIR_FREE(secret.secret); } @@ -870,7 +870,6 @@ qemuDomainSecretPlainSetup(virConnectPtr conn, virStorageNetProtocol protocol, virStorageAuthDefPtr authdef) { - bool encode = false; int secretType = VIR_SECRET_USAGE_TYPE_ISCSI; const char *protocolstr = virStorageNetProtocolTypeToString(protocol); @@ -878,18 +877,12 @@ qemuDomainSecretPlainSetup(virConnectPtr conn, if (VIR_STRDUP(secinfo->s.plain.username, authdef->username) < 0) return -1; - if (protocol == VIR_STORAGE_NET_PROTOCOL_RBD) { - /* qemu requires the secret to be encoded for RBD */ - encode = true; + if (protocol == VIR_STORAGE_NET_PROTOCOL_RBD) secretType = VIR_SECRET_USAGE_TYPE_CEPH; - } - - if (!(secinfo->s.plain.secret = - virSecretGetSecretString(conn, protocolstr, encode, - authdef, secretType))) - return -1; - return 0; + return virSecretGetSecretString(conn, protocolstr, authdef, secretType, + &secinfo->s.plain.secret, + &secinfo->s.plain.secretlen); } diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 331ade0..f074ca5 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -251,7 +251,8 @@ typedef struct _qemuDomainSecretPlain qemuDomainSecretPlain; typedef struct _qemuDomainSecretPlain *qemuDomainSecretPlainPtr; struct _qemuDomainSecretPlain { char *username; - char *secret; + uint8_t *secret; + size_t secretlen; }; # define QEMU_DOMAIN_AES_IV_KEY_LEN 16 /* 16 bytes for 128 bit random */ diff --git a/src/secret/secret_util.c b/src/secret/secret_util.c index 217584f..5dff57c 100644 --- a/src/secret/secret_util.c +++ b/src/secret/secret_util.c @@ -27,7 +27,6 @@ #include "virlog.h" #include "virobject.h" #include "viruuid.h" -#include "base64.h" #include "datatypes.h" #define VIR_FROM_THIS VIR_FROM_SECRET @@ -38,27 +37,30 @@ VIR_LOG_INIT("secret.secret_util"); /* virSecretGetSecretString: * @conn: Pointer to the connection driver to make secret driver call * @scheme: Unique enough string for error message to help determine cause - * @encoded: Whether the returned secret needs to be base64 encoded * @authdef: Pointer to the disk storage authentication * @secretUsageType: Type of secret usage for authdef lookup + * @ret_secret: returned secret as a sized stream of unsigned chars + * @ret_secret_size: Return size of the secret - either raw text or base64 * - * Lookup the secret for the authdef usage type and return it either as - * raw text or encoded based on the caller's need. + * Lookup the secret for the authdef usage type and return it as raw text. + * It is up to the caller to encode the secret further. * - * Returns a pointer to memory that needs to be cleared and free'd after - * usage or NULL on error. + * Returns 0 on success, -1 on failure. On success the memory in ret_secret + * needs to be cleared and free'd after usage. */ -char * +int virSecretGetSecretString(virConnectPtr conn, const char *scheme, - bool encoded, virStorageAuthDefPtr authdef, - virSecretUsageType secretUsageType) + virSecretUsageType secretUsageType, + uint8_t **ret_secret, + size_t *ret_secret_size) { size_t secret_size; virSecretPtr sec = NULL; - char *secret = NULL; + unsigned char *secret = NULL; char uuidStr[VIR_UUID_STRING_BUFLEN]; + int ret = -1; /* look up secret */ switch (authdef->secretType) { @@ -85,8 +87,8 @@ virSecretGetSecretString(virConnectPtr conn, goto cleanup; } - secret = (char *)conn->secretDriver->secretGetValue(sec, &secret_size, 0, - VIR_SECRET_GET_VALUE_INTERNAL_CALL); + secret = conn->secretDriver->secretGetValue(sec, &secret_size, 0, + VIR_SECRET_GET_VALUE_INTERNAL_CALL); if (!secret) { if (authdef->secretType == VIR_STORAGE_SECRET_TYPE_UUID) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -102,19 +104,16 @@ virSecretGetSecretString(virConnectPtr conn, goto cleanup; } - if (encoded) { - char *base64 = NULL; + if (VIR_ALLOC_N(*ret_secret, secret_size) < 0) + goto cleanup; - base64_encode_alloc(secret, secret_size, &base64); - VIR_FREE(secret); - if (!base64) { - virReportOOMError(); - goto cleanup; - } - secret = base64; - } + memcpy(*ret_secret, secret, secret_size); + *ret_secret_size = secret_size; + ret = 0; cleanup: virObjectUnref(sec); - return secret; + memset(secret, 0, secret_size); + VIR_FREE(secret); + return ret; } diff --git a/src/secret/secret_util.h b/src/secret/secret_util.h index c707599..0520db3 100644 --- a/src/secret/secret_util.h +++ b/src/secret/secret_util.h @@ -25,11 +25,12 @@ # include "internal.h" # include "virstoragefile.h" -char *virSecretGetSecretString(virConnectPtr conn, - const char *scheme, - bool encoded, - virStorageAuthDefPtr authdef, - virSecretUsageType secretUsageType) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(4) - ATTRIBUTE_RETURN_CHECK; +int virSecretGetSecretString(virConnectPtr conn, + const char *scheme, + virStorageAuthDefPtr authdef, + virSecretUsageType secretUsageType, + uint8_t **ret_secret, + size_t *ret_secret_size) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) + ATTRIBUTE_NONNULL(5) ATTRIBUTE_NONNULL(6) ATTRIBUTE_RETURN_CHECK; #endif /* __VIR_SECRET_H__ */ diff --git a/src/util/virstring.c b/src/util/virstring.c index 735e65b..1679266 100644 --- a/src/util/virstring.c +++ b/src/util/virstring.c @@ -1066,3 +1066,22 @@ virStringIsPrintable(const char *str) return true; } + + +/** + * virBufferIsPrintable: + * + * Returns true if @buf of @buflen contains only printable characters + */ +bool +virStringBufferIsPrintable(const uint8_t *buf, + size_t buflen) +{ + size_t i; + + for (i = 0; i < buflen; i++) + if (!c_isprint(buf[i])) + return false; + + return true; +} diff --git a/src/util/virstring.h b/src/util/virstring.h index fd2868a..9203aa3 100644 --- a/src/util/virstring.h +++ b/src/util/virstring.h @@ -276,5 +276,6 @@ bool virStringHasControlChars(const char *str); void virStringStripControlChars(char *str); bool virStringIsPrintable(const char *str); +bool virStringBufferIsPrintable(const uint8_t *buf, size_t buflen); #endif /* __VIR_STRING_H__ */ -- 2.5.5

2 1