OpenStack Compute (nova)

libvirt should not require dynamic_ownership off for secure Cinder/Quobyte settings

Bug #1609298 reported by Silvan Kaiser
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack Compute (nova)
Confirmed
Medium
Unassigned

Bug Description

tl;dr
When running Quobyte Cinder storage with nas_secure_file_* settings set to true libvirt is currently required to be configured with dynamic_ownership=0 (off). This is not recommended with Nova.

Expected results: secure settings in Cinder should work with Nova and unmodified dynamic_ownership in libvirt config
Actual results: The option in libvirt is required

More detailed:
When run with dynamic_ownership=1 libvirt changes file ownership on guest files to root:root at some point. Running Cinder with the Quobyte driver in nas_secure_file_ownership / nas_secure_file_permissions = true conflicts with this: In secure mode image files belong to the nova/cinder service users (both in a common group) and file permissions are 660 (instead of running root:root/666 as is the insecure mode for these cinder options). When libvirt changes the files ownership to root:root nova/cinder cannot access those files any longer, hurting e.g. snapshots and the like.

A correction proposal was made by Daniel Berrange at https://bugs.launchpad.net/nova/+bug/1597644/comments/22 :
"[..]If so, a much better approach is to enhance nova so that it can set a <seclabel> element against *just* the quobyte backed disks, that tells libvirt to skip ownership changes for those disks. That way operation of libvirt / QEMU in general will not be affect, thus avoiding nasty side-effects such as this console.log problem.[..]"

Silvan Kaiser (2-silvan)
Changed in nova:
assignee: nobody → Silvan Kaiser (2-silvan)
Maciej Szankin (mszankin)
Changed in nova:
status: New → In Progress
Revision history for this message
John Wood (john-wood-w) wrote :

Thanks for your comments on my CR here: https://review.openstack.org/#/c/301966/

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to nova (master)

Related fix proposed to branch: master
Review: https://review.openstack.org/392643

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to nova (master)

Reviewed: https://review.openstack.org/392643
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=1848860fd4fb6dfb2ead1f8271050d36df592205
Submitter: Jenkins
Branch: master

commit 1848860fd4fb6dfb2ead1f8271050d36df592205
Author: Stephen Finucane <email address hidden>
Date: Wed Nov 2 12:43:35 2016 +0000

    Ignore IOError when creating 'console.log'

    In 'd4e6bd8', and later refined in 'ec6ed24c', an issue with the the
    Quobyte CI was resolved by ensuring that the 'console.log' file is
    always created, even when libvirt's 'dynamic_ownership' config option
    is set to '0'. However, attempting to take ownership of this file can
    cause issues when 'dynamic_ownership' is set to '1' and a user attempts
    to reschedule an instance to a host where it was previously located. In
    this case, the permissions on the file have already been co-opted by
    libvirt, and attempting to change them results in an IOError.

    The easiest fix for this is to simply ignore the IOError, as the file
    already exists and libvirt has set correct permissions already. This is
    what do here. A longer term fix would be to avoid changing the
    permissions only for the offending Quobytes block devices, but this is
    likely not backportable and will be done in a follow-up fix.

    Change-Id: I353afd57be207330757580447a1b606c7b9b7c09
    Partial-bug: #1634282
    Related-bug: #1597644
    Related-bug: #1609298

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to nova (stable/newton)

Related fix proposed to branch: stable/newton
Review: https://review.openstack.org/454593

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to nova (stable/newton)

Reviewed: https://review.openstack.org/454593
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=a96092d0e48fef62d257acf0428ddb6daf07d3a4
Submitter: Jenkins
Branch: stable/newton

commit a96092d0e48fef62d257acf0428ddb6daf07d3a4
Author: Stephen Finucane <email address hidden>
Date: Wed Nov 2 12:43:35 2016 +0000

    Ignore IOError when creating 'console.log'

    In 'd4e6bd8', and later refined in 'ec6ed24c', an issue with the the
    Quobyte CI was resolved by ensuring that the 'console.log' file is
    always created, even when libvirt's 'dynamic_ownership' config option
    is set to '0'. However, attempting to take ownership of this file can
    cause issues when 'dynamic_ownership' is set to '1' and a user attempts
    to reschedule an instance to a host where it was previously located. In
    this case, the permissions on the file have already been co-opted by
    libvirt, and attempting to change them results in an IOError.

    The easiest fix for this is to simply ignore the IOError, as the file
    already exists and libvirt has set correct permissions already. This is
    what do here. A longer term fix would be to avoid changing the
    permissions only for the offending Quobytes block devices, but this is
    likely not backportable and will be done in a follow-up fix.

    Change-Id: I353afd57be207330757580447a1b606c7b9b7c09
    Partial-bug: #1634282
    Related-bug: #1597644
    Related-bug: #1609298
    (cherry picked from commit 1848860fd4fb6dfb2ead1f8271050d36df592205)

tags: added: in-stable-newton
Revision history for this message
Sean Dague (sdague) wrote :

There are no currently open reviews on this bug, changing
the status back to the previous state and unassigning. If
there are active reviews related to this bug, please include
links in comments.

Changed in nova:
status: In Progress → New
assignee: Silvan Kaiser (2-silvan) → nobody
Revision history for this message
Sean Dague (sdague) wrote :

Some of the patches are merged, it's not clear where things stand. Can you confirm what is still needed?

Changed in nova:
status: New → Incomplete
Revision history for this message
Launchpad Janitor (janitor) wrote :

[Expired for OpenStack Compute (nova) because there has been no activity for 60 days.]

Changed in nova:
status: Incomplete → Expired
Silvan Kaiser (2-silvan)
Changed in nova:
status: Expired → New
Revision history for this message
melanie witt (melwitt) wrote :

OK, so based on the bug report, it looks like what's needed to fix this is a change to the libvirt driver to set a <seclabel> element against only quobyte backed disks, that will tell libvirt to skip ownership changes for those disks. This would allow libvirt with dynamic_ownership=1 to work with quobyte cinder storage.

Changed in nova:
importance: Undecided → Medium
status: New → Confirmed
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.
Subscribing...

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.