[libvirt] [PATCH 3/3] security_dac: Favour ACLs over chown()

[Michal Privoznik]

On 05.09.2013 16:14, Daniel P. Berrange wrote:
> On Wed, Aug 28, 2013 at 12:27:40PM +0200, Michal Privoznik wrote:
>> On filesystems supporting ACLs we don't need to do a chown but we
>> can just set ACLs to gain access for qemu. However, since we are
>> setting these on too low level, where we don't know if disk is
>> just a read only or read write, we set read write access
>> unconditionally.
>>
>> >From implementation POV, a reference counter is introduced, so ACL is
>> restored only on the last restore attempt in order to not cut off other
>> domains. And since a file may had an ACL for a user already set, we need
>> to keep this as well. Both these, the reference counter and original ACL
>> are stored as extended attributes named trusted.libvirt.dac.refCount and
>> trusted.libvirt.dac.oldACL respectively.
>>
>> However, some filesystems doesn't support ACLs, XATTRs, or both. So the
>> code is made to favour ACLs among with tracking the reference count. If
>> this fails, we fall back to chown()  with best effort to remember the
>> original owner of file.
>>
>> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
>> ---
>>  src/security/security_dac.c | 297 +++++++++++++++++++++++++++++++++++++++-----
>>  1 file changed, 268 insertions(+), 29 deletions(-)
>>
>> diff --git a/src/security/security_dac.c b/src/security/security_dac.c
>> index 8cbb083..913b68e 100644
>> --- a/src/security/security_dac.c
>> +++ b/src/security/security_dac.c
>> @@ -37,6 +37,9 @@
>>  
>>  #define VIR_FROM_THIS VIR_FROM_SECURITY
>>  #define SECURITY_DAC_NAME "dac"
>> +#define SECURITY_DAC_XATTR_OLD_ACL "trusted.libvirt.dac.oldACL"
>> +#define SECURITY_DAC_XATTR_OLD_OWNER "trusted.libvirt.dac.oldOwner"
>> +#define SECURITY_DAC_XATTR_REFCOUNT "trusted.libvirt.dac.refCount"
> 
> Use of "trusted." is non-portable, per the long mail thread last
> time this was posted[1]. The discussion there was never resolved in
> any way, so NACK to this patch as is.
> 
> Daniel
> 
> [1] https://www.redhat.com/archives/libvir-list/2013-March/msg01610.html
> 

Okay then, consider this squashed in:

diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 47af4e8..acc80c4 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -37,9 +37,9 @@

 #define VIR_FROM_THIS VIR_FROM_SECURITY
 #define SECURITY_DAC_NAME "dac"
-#define SECURITY_DAC_XATTR_OLD_ACL "trusted.libvirt.dac.oldACL"
-#define SECURITY_DAC_XATTR_OLD_OWNER "trusted.libvirt.dac.oldOwner"
-#define SECURITY_DAC_XATTR_REFCOUNT "trusted.libvirt.dac.refCount"
+#define SECURITY_DAC_XATTR_OLD_ACL "user.libvirt.dac.oldACL"
+#define SECURITY_DAC_XATTR_OLD_OWNER "user.libvirt.dac.oldOwner"
+#define SECURITY_DAC_XATTR_REFCOUNT "user.libvirt.dac.refCount"

 typedef struct _virSecurityDACData virSecurityDACData;
 typedef virSecurityDACData *virSecurityDACDataPtr;

Michal