[libvirt] [PATCH] selinux: Don't fail RestoreAll if file doesn't have a default label
When restoring selinux labels after a VM is stopped, any non-standard path that doesn't have a default selinux label causes the process to stop and exit early. This isn't really an error condition IMO. Of course the selinux API could be erroring for some other reason but hopefully that's rare enough to not need explicit handling. Common example here is storing disk images in a non-standard location like under /mnt. --- src/security/security_selinux.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index eee8d71..7681f1b 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -936,7 +936,11 @@ virSecuritySELinuxRestoreSecurityFileLabel(const char *path) } if (getContext(newpath, buf.st_mode, &fcon) < 0) { + /* Any user created path likely does not have a default label, + * which makes this an expected non error + */ VIR_WARN("cannot lookup default selinux label for %s", newpath); + rc = 0; } else { rc = virSecuritySELinuxSetFilecon(newpath, fcon); } -- 1.7.11.7
On 2012年10月22日 04:44, Cole Robinson wrote:
When restoring selinux labels after a VM is stopped, any non-standard path that doesn't have a default selinux label causes the process to stop and exit early. This isn't really an error condition IMO.
Of course the selinux API could be erroring for some other reason but hopefully that's rare enough to not need explicit handling.
Common example here is storing disk images in a non-standard location like under /mnt. --- src/security/security_selinux.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index eee8d71..7681f1b 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -936,7 +936,11 @@ virSecuritySELinuxRestoreSecurityFileLabel(const char *path) }
if (getContext(newpath, buf.st_mode,&fcon)< 0) { + /* Any user created path likely does not have a default label, + * which makes this an expected non error + */ VIR_WARN("cannot lookup default selinux label for %s", newpath); + rc = 0;
ACK, it's fair to set the return code to 0, per it already tends to give a warning.
On 10/21/2012 02:44 PM, Cole Robinson wrote:
When restoring selinux labels after a VM is stopped, any non-standard path that doesn't have a default selinux label causes the process to stop and exit early. This isn't really an error condition IMO.
Of course the selinux API could be erroring for some other reason but hopefully that's rare enough to not need explicit handling.
Common example here is storing disk images in a non-standard location like under /mnt. --- src/security/security_selinux.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index eee8d71..7681f1b 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -936,7 +936,11 @@ virSecuritySELinuxRestoreSecurityFileLabel(const char *path) }
if (getContext(newpath, buf.st_mode, &fcon) < 0) { + /* Any user created path likely does not have a default label, + * which makes this an expected non error + */ VIR_WARN("cannot lookup default selinux label for %s", newpath); + rc = 0;
In the case where there is no default label to restore, shouldn't we still be removing our sVirt label rather than just ignoring the failure but leaving our label intact? -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 10/22/2012 11:51 AM, Eric Blake wrote:
On 10/21/2012 02:44 PM, Cole Robinson wrote:
When restoring selinux labels after a VM is stopped, any non-standard path that doesn't have a default selinux label causes the process to stop and exit early. This isn't really an error condition IMO.
Of course the selinux API could be erroring for some other reason but hopefully that's rare enough to not need explicit handling.
Common example here is storing disk images in a non-standard location like under /mnt. --- src/security/security_selinux.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index eee8d71..7681f1b 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -936,7 +936,11 @@ virSecuritySELinuxRestoreSecurityFileLabel(const char *path) }
if (getContext(newpath, buf.st_mode, &fcon) < 0) { + /* Any user created path likely does not have a default label, + * which makes this an expected non error + */ VIR_WARN("cannot lookup default selinux label for %s", newpath); + rc = 0;
In the case where there is no default label to restore, shouldn't we still be removing our sVirt label rather than just ignoring the failure but leaving our label intact?
I don't know if we can just 'remove' a label, we have to replace it with a different label, right? If I create a file under /mnt/foo the catch all label is unconfined_u:object_r:file_t:s0 but not sure if we can hardcode that. dwalsh, is there a way to programmatically determine the fallback default label? - Cole
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 10/22/2012 04:13 PM, Cole Robinson wrote:
On 10/22/2012 11:51 AM, Eric Blake wrote:
On 10/21/2012 02:44 PM, Cole Robinson wrote:
When restoring selinux labels after a VM is stopped, any non-standard path that doesn't have a default selinux label causes the process to stop and exit early. This isn't really an error condition IMO.
Of course the selinux API could be erroring for some other reason but hopefully that's rare enough to not need explicit handling.
Common example here is storing disk images in a non-standard location like under /mnt. --- src/security/security_selinux.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index eee8d71..7681f1b 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -936,7 +936,11 @@ virSecuritySELinuxRestoreSecurityFileLabel(const char *path) }
if (getContext(newpath, buf.st_mode, &fcon) < 0) { + /* Any user created path likely does not have a default label, + * which makes this an expected non error + */ VIR_WARN("cannot lookup default selinux label for %s", newpath); + rc = 0;
In the case where there is no default label to restore, shouldn't we still be removing our sVirt label rather than just ignoring the failure but leaving our label intact?
I don't know if we can just 'remove' a label, we have to replace it with a different label, right? If I create a file under /mnt/foo the catch all label is unconfined_u:object_r:file_t:s0 but not sure if we can hardcode that.
dwalsh, is there a way to programmatically determine the fallback default label?
- Cole
I would guess you could walk the stack until you got a file system with a label. dirs(newpath) ... For example in /mnt/a/b/c Check /mnt/a/b/c, then /mnt/a/b then /mnt/a then /mnt then / It should definitely not set it to file_t. I will send this question off to the selinux list to see if they have any better suggestions. -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.12 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://www.enigmail.net/ iEYEARECAAYFAlCGd0YACgkQrlYvE4MpobORcACgg8Ctj7tX5qtlSEtyOzaOArvw rWUAniGx8b73oIhFJpssbid3oexmO46i =Vz87 -----END PGP SIGNATURE-----
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 10/22/2012 04:13 PM, Cole Robinson wrote:
On 10/22/2012 11:51 AM, Eric Blake wrote:
On 10/21/2012 02:44 PM, Cole Robinson wrote:
When restoring selinux labels after a VM is stopped, any non-standard path that doesn't have a default selinux label causes the process to stop and exit early. This isn't really an error condition IMO.
Of course the selinux API could be erroring for some other reason but hopefully that's rare enough to not need explicit handling.
Common example here is storing disk images in a non-standard location like under /mnt. --- src/security/security_selinux.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index eee8d71..7681f1b 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -936,7 +936,11 @@ virSecuritySELinuxRestoreSecurityFileLabel(const char *path) }
if (getContext(newpath, buf.st_mode, &fcon) < 0) { + /* Any user created path likely does not have a default label, + * which makes this an expected non error + */ VIR_WARN("cannot lookup default selinux label for %s", newpath); + rc = 0;
In the case where there is no default label to restore, shouldn't we still be removing our sVirt label rather than just ignoring the failure but leaving our label intact?
I don't know if we can just 'remove' a label, we have to replace it with a different label, right? If I create a file under /mnt/foo the catch all label is unconfined_u:object_r:file_t:s0 but not sure if we can hardcode that.
dwalsh, is there a way to programmatically determine the fallback default label?
- Cole
Another option would be to get the label before the virtual machine starts and then restore it to the old label, if we have kept the label around. -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.12 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://www.enigmail.net/ iEYEARECAAYFAlCGd9kACgkQrlYvE4MpobOTqgCfX7IlWThWpHRbPIopZ4BUIerB e/MAn2YNe6wBNA6X7OGjDHqIqGv2E+7B =78kR -----END PGP SIGNATURE-----
On 10/23/2012 06:56 AM, Daniel J Walsh wrote:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 10/22/2012 04:13 PM, Cole Robinson wrote:
On 10/22/2012 11:51 AM, Eric Blake wrote:
On 10/21/2012 02:44 PM, Cole Robinson wrote:
When restoring selinux labels after a VM is stopped, any non-standard path that doesn't have a default selinux label causes the process to stop and exit early. This isn't really an error condition IMO.
Of course the selinux API could be erroring for some other reason but hopefully that's rare enough to not need explicit handling.
Common example here is storing disk images in a non-standard location like under /mnt. --- src/security/security_selinux.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index eee8d71..7681f1b 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -936,7 +936,11 @@ virSecuritySELinuxRestoreSecurityFileLabel(const char *path) }
if (getContext(newpath, buf.st_mode, &fcon) < 0) { + /* Any user created path likely does not have a default label, + * which makes this an expected non error + */ VIR_WARN("cannot lookup default selinux label for %s", newpath); + rc = 0;
In the case where there is no default label to restore, shouldn't we still be removing our sVirt label rather than just ignoring the failure but leaving our label intact?
I don't know if we can just 'remove' a label, we have to replace it with a different label, right? If I create a file under /mnt/foo the catch all label is unconfined_u:object_r:file_t:s0 but not sure if we can hardcode that.
dwalsh, is there a way to programmatically determine the fallback default label?
- Cole
Another option would be to get the label before the virtual machine starts and then restore it to the old label, if we have kept the label around.
That would take some work. I think it's fair to say that if users have a file with a non default label, and they want it restored after svirt usage, they just install their own custom policy so our equivalent of 'restorecon' does the right thing. - Cole
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 10/23/2012 10:55 AM, Cole Robinson wrote:
On 10/23/2012 06:56 AM, Daniel J Walsh wrote:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 10/22/2012 04:13 PM, Cole Robinson wrote:
On 10/22/2012 11:51 AM, Eric Blake wrote:
On 10/21/2012 02:44 PM, Cole Robinson wrote:
When restoring selinux labels after a VM is stopped, any non-standard path that doesn't have a default selinux label causes the process to stop and exit early. This isn't really an error condition IMO.
Of course the selinux API could be erroring for some other reason but hopefully that's rare enough to not need explicit handling.
Common example here is storing disk images in a non-standard location like under /mnt. --- src/security/security_selinux.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index eee8d71..7681f1b 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -936,7 +936,11 @@ virSecuritySELinuxRestoreSecurityFileLabel(const char *path) }
if (getContext(newpath, buf.st_mode, &fcon) < 0) { + /* Any user created path likely does not have a default label, + * which makes this an expected non error + */ VIR_WARN("cannot lookup default selinux label for %s", newpath); + rc = 0;
In the case where there is no default label to restore, shouldn't we still be removing our sVirt label rather than just ignoring the failure but leaving our label intact?
I don't know if we can just 'remove' a label, we have to replace it with a different label, right? If I create a file under /mnt/foo the catch all label is unconfined_u:object_r:file_t:s0 but not sure if we can hardcode that.
dwalsh, is there a way to programmatically determine the fallback default label?
- Cole
Another option would be to get the label before the virtual machine starts and then restore it to the old label, if we have kept the label around.
That would take some work. I think it's fair to say that if users have a file with a non default label, and they want it restored after svirt usage, they just install their own custom policy so our equivalent of 'restorecon' does the right thing.
- Cole
One other option would be to grab the label before you change it and then set it back you are done. -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.12 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://www.enigmail.net/ iEYEARECAAYFAlCH6WsACgkQrlYvE4MpobPnQACfWXTb/5Gff2V012e9wdN8B7Xs AxEAoNeEzC/tdVOyLS/d8ZD6W0wkIgY6 =h9TN -----END PGP SIGNATURE-----
On 10/22/2012 11:51 AM, Eric Blake wrote:
On 10/21/2012 02:44 PM, Cole Robinson wrote:
When restoring selinux labels after a VM is stopped, any non-standard path that doesn't have a default selinux label causes the process to stop and exit early. This isn't really an error condition IMO.
Of course the selinux API could be erroring for some other reason but hopefully that's rare enough to not need explicit handling.
Common example here is storing disk images in a non-standard location like under /mnt. --- src/security/security_selinux.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index eee8d71..7681f1b 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -936,7 +936,11 @@ virSecuritySELinuxRestoreSecurityFileLabel(const char *path) }
if (getContext(newpath, buf.st_mode, &fcon) < 0) { + /* Any user created path likely does not have a default label, + * which makes this an expected non error + */ VIR_WARN("cannot lookup default selinux label for %s", newpath); + rc = 0;
In the case where there is no default label to restore, shouldn't we still be removing our sVirt label rather than just ignoring the failure but leaving our label intact?
I sent other mails about that. But since that topic is kind of a side point, is this patch okay to commit in the interim? It should only improve our behavior WRT restoring default labels, since we will now continue on even if something in the chain doesn't have a default. Thanks, Cole
On 10/23/2012 08:57 AM, Cole Robinson wrote:
if (getContext(newpath, buf.st_mode, &fcon) < 0) { + /* Any user created path likely does not have a default label, + * which makes this an expected non error + */ VIR_WARN("cannot lookup default selinux label for %s", newpath); + rc = 0;
In the case where there is no default label to restore, shouldn't we still be removing our sVirt label rather than just ignoring the failure but leaving our label intact?
I sent other mails about that. But since that topic is kind of a side point, is this patch okay to commit in the interim? It should only improve our behavior WRT restoring default labels, since we will now continue on even if something in the chain doesn't have a default.
Yes, that's a good argument for applying now. ACK. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 10/23/2012 11:42 AM, Eric Blake wrote:
On 10/23/2012 08:57 AM, Cole Robinson wrote:
if (getContext(newpath, buf.st_mode, &fcon) < 0) { + /* Any user created path likely does not have a default label, + * which makes this an expected non error + */ VIR_WARN("cannot lookup default selinux label for %s", newpath); + rc = 0;
In the case where there is no default label to restore, shouldn't we still be removing our sVirt label rather than just ignoring the failure but leaving our label intact?
I sent other mails about that. But since that topic is kind of a side point, is this patch okay to commit in the interim? It should only improve our behavior WRT restoring default labels, since we will now continue on even if something in the chain doesn't have a default.
Yes, that's a good argument for applying now.
ACK.
Thanks, pushed now. - Cole
participants (4)
-
Cole Robinson -
Daniel J Walsh -
Eric Blake -
Osier Yang