[libvirt] [PATCH 08/11] Don't allow renaming of domains by the backdoor

Eric Blake eblake at redhat.com
Fri May 3 16:27:41 UTC 2013


On 05/02/2013 06:03 AM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange at redhat.com>
> 
> Several APIs allow for custom XML to be passed in. This is
> checked for ABI stability, which will ensure the UUID is
> not being changed. There isn't validation that the name
> did not change though. This could allow renaming of guests
> via the backdoor, which in turn could allow for bypassing
> access control restrictions based on names.
> 
> Signed-off-by: Daniel P. Berrange <berrange at redhat.com>
> ---
>  src/conf/domain_conf.c    | 11 +++++++++++
>  src/qemu/qemu_migration.c |  6 ------
>  2 files changed, 11 insertions(+), 6 deletions(-)

How does this interact with APIs like virDomainMigrate(), which
intentionally has a dname parameter for renaming the domain on the
destination?  Or is the idea that in virDomainMigrate2(), if you want to
rename the domain, you have to do so through the dname argument and NOT
via the backdoor of the dxml argument?

/me reads docs:

... The migration will fail
 * if @dxml would cause any guest-visible changes.  Pass NULL
 * if no changes are needed to the XML between source and destination.
 * @dxml cannot be used to rename the domain during migration (use
 * @dname for that purpose).  Domain name in @dxml must match the
 * original domain name.

Ah, so it looks like we indeed expect dname to be the only supported
way, and that it is applied after dxml is first checked for ABI
compatibility.

> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index a8b5dfd..d945b74 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -12560,6 +12560,17 @@ virDomainDefCheckABIStability(virDomainDefPtr src,
>          return false;
>      }
>  
> +    /* Not strictly ABI related, but we want to make sure domains
> +     * don't get silently re-named through the backdoor when passing
> +     * custom XML into various APIs, since this would create havoc
> +     */
> +    if (STRNEQ(src->name, dst->name)) {
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                       _("Target domain name '%s' does not match source '%s'"),
> +                       dst->name, src->name);
> +        return false;
> +    }

The code makes sense, but I'd feel better delaying my ack until getting
confirmation that I'm correctly interpreting that rename is
intentionally denied on 'virsh save/restore', and rename during
migration is allowed only through the dname argument, after dxml is
already validated against the pre-rename xml.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 621 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20130503/607823cb/attachment-0001.sig>


More information about the libvir-list mailing list