[common PATCH 0/3] SELinux_relabel: relabel only if enforcing (RHBZ#1828952)
Continuation/rework of: https://www.redhat.com/archives/libguestfs/2020-May/msg00020.html This is my approach, as I explained here: https://bugzilla.redhat.com/show_bug.cgi?id=1828952#c4 https://www.redhat.com/archives/libguestfs/2020-May/msg00035.html IOW: do not attempt to relabel if the guest is not enforcing, as it is either useless or may fail; few words more are in the comments of patch #3. Pino Toscano (2): mlcustomize: refactor reading from /etc/selinux/config mlcustomize: do not relabel if not enforcing (RHBZ#1828952) Richard W.M. Jones (1): mlcustomize: Refactor SELinux_relabel code. mlcustomize/SELinux_relabel.ml | 153 ++++++++++++++++++++------------- 1 file changed, 94 insertions(+), 59 deletions(-) -- 2.26.2
From: "Richard W.M. Jones" <rjones@redhat.com> This shouldn't change the effect of this code. --- mlcustomize/SELinux_relabel.ml | 121 ++++++++++++++++++--------------- 1 file changed, 65 insertions(+), 56 deletions(-) diff --git a/mlcustomize/SELinux_relabel.ml b/mlcustomize/SELinux_relabel.ml index 44995df..5df1f08 100644 --- a/mlcustomize/SELinux_relabel.ml +++ b/mlcustomize/SELinux_relabel.ml @@ -28,65 +28,74 @@ module G = Guestfs let array_find a l = List.mem a (Array.to_list l) -let relabel (g : G.guestfs) = - (* Is the guest using SELinux? *) - if g#is_file ~followsymlinks:true "/usr/sbin/load_policy" && - g#is_file ~followsymlinks:true "/etc/selinux/config" then ( - (* Is setfiles / SELinux relabelling functionality available? *) - if g#feature_available [| "selinuxrelabel" |] then ( - (* Use Augeas to parse /etc/selinux/config. *) - g#aug_init "/" (16+32) (* AUG_SAVE_NOOP | AUG_NO_LOAD *); - (* See: https://bugzilla.redhat.com/show_bug.cgi?id=975412#c0 *) - ignore (g#aug_rm "/augeas/load/*[\"/etc/selinux/config/\" !~ regexp('^') + glob(incl) + regexp('/.*')]"); - g#aug_load (); - debug_augeas_errors g; - - (* Get the SELinux policy name, eg. "targeted", "minimum". - * Use "targeted" if not specified, just like libselinux does. +let rec relabel (g : G.guestfs) = + (* Is the guest using SELinux? (Otherwise this is a no-op). *) + if is_selinux_guest g then ( + try + use_setfiles g; + (* That worked, so we don't need to autorelabel. *) + g#rm_f "/.autorelabel" + with Failure _ -> + (* This is the fallback in case something in the setfiles + * method didn't work. That includes the case where a non-SELinux + * host is processing an SELinux guest, and other things. *) - let policy = - let config_path = "/files/etc/selinux/config" in - let selinuxtype_path = config_path ^ "/SELINUXTYPE" in - let keys = g#aug_ls config_path in - if array_find selinuxtype_path keys then - g#aug_get selinuxtype_path - else - "targeted" in + g#touch "/.autorelabel" + ) - g#aug_close (); +and is_selinux_guest g = + g#is_file ~followsymlinks:true "/usr/sbin/load_policy" && + g#is_file ~followsymlinks:true "/etc/selinux/config" - (* Get the spec file name. *) - let specfile = - sprintf "/etc/selinux/%s/contexts/files/file_contexts" policy in +and use_setfiles g = + (* Is setfiles / SELinux relabelling functionality available? *) + if not (g#feature_available [| "selinuxrelabel" |]) then + failwith "no selinux relabel feature"; - (* RHEL 6.2 - 6.5 had a malformed specfile that contained the - * invalid regular expression "/var/run/spice-vdagentd.\pid" - * (instead of "\.p"). This stops setfiles from working on - * the guest. - * - * Because an SELinux relabel writes all over the filesystem, - * it seems reasonable to fix this problem in the specfile - * at the same time. (RHBZ#1374232) - *) - if g#grep ~fixed:true "vdagentd.\\pid" specfile <> [||] then ( - debug "fixing invalid regular expression in %s" specfile; - let old_specfile = specfile ^ "~" in - g#mv specfile old_specfile; - let content = g#read_file old_specfile in - let content = - String.replace content "vdagentd.\\pid" "vdagentd\\.pid" in - g#write specfile content; - g#copy_attributes ~all:true old_specfile specfile - ); + (* Use Augeas to parse /etc/selinux/config. *) + g#aug_init "/" (16+32) (* AUG_SAVE_NOOP | AUG_NO_LOAD *); + (* See: https://bugzilla.redhat.com/show_bug.cgi?id=975412#c0 *) + ignore (g#aug_rm "/augeas/load/*[\"/etc/selinux/config/\" !~ regexp('^') + glob(incl) + regexp('/.*')]"); + g#aug_load (); + debug_augeas_errors g; - (* Relabel everything. *) - g#selinux_relabel ~force:true specfile "/"; + (* Get the SELinux policy name, eg. "targeted", "minimum". + * Use "targeted" if not specified, just like libselinux does. + *) + let policy = + let config_path = "/files/etc/selinux/config" in + let selinuxtype_path = config_path ^ "/SELINUXTYPE" in + let keys = g#aug_ls config_path in + if array_find selinuxtype_path keys then + g#aug_get selinuxtype_path + else + "targeted" in - (* If that worked, we don't need to autorelabel. *) - g#rm_f "/.autorelabel" - ) - else ( - (* SELinux guest, but not SELinux host. Fallback to this. *) - g#touch "/.autorelabel" - ) - ) + g#aug_close (); + + (* Get the spec file name. *) + let specfile = + sprintf "/etc/selinux/%s/contexts/files/file_contexts" policy in + + (* RHEL 6.2 - 6.5 had a malformed specfile that contained the + * invalid regular expression "/var/run/spice-vdagentd.\pid" + * (instead of "\.p"). This stops setfiles from working on + * the guest. + * + * Because an SELinux relabel writes all over the filesystem, + * it seems reasonable to fix this problem in the specfile + * at the same time. (RHBZ#1374232) + *) + if g#grep ~fixed:true "vdagentd.\\pid" specfile <> [||] then ( + debug "fixing invalid regular expression in %s" specfile; + let old_specfile = specfile ^ "~" in + g#mv specfile old_specfile; + let content = g#read_file old_specfile in + let content = + String.replace content "vdagentd.\\pid" "vdagentd\\.pid" in + g#write specfile content; + g#copy_attributes ~all:true old_specfile specfile + ); + + (* Relabel everything. *) + g#selinux_relabel ~force:true specfile "/" -- 2.26.2
Move the reading of values from /etc/selinux/config to an own function, so it can be easily reused. This is a simple refactoring. --- mlcustomize/SELinux_relabel.ml | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/mlcustomize/SELinux_relabel.ml b/mlcustomize/SELinux_relabel.ml index 5df1f08..647aeda 100644 --- a/mlcustomize/SELinux_relabel.ml +++ b/mlcustomize/SELinux_relabel.ml @@ -62,14 +62,7 @@ and use_setfiles g = (* Get the SELinux policy name, eg. "targeted", "minimum". * Use "targeted" if not specified, just like libselinux does. *) - let policy = - let config_path = "/files/etc/selinux/config" in - let selinuxtype_path = config_path ^ "/SELINUXTYPE" in - let keys = g#aug_ls config_path in - if array_find selinuxtype_path keys then - g#aug_get selinuxtype_path - else - "targeted" in + let policy = read_selinux_config_key g "SELINUXTYPE" "targeted" in g#aug_close (); @@ -99,3 +92,12 @@ and use_setfiles g = (* Relabel everything. *) g#selinux_relabel ~force:true specfile "/" + +and read_selinux_config_key g key defval = + let config_path = "/files/etc/selinux/config" in + let key_path = config_path ^ "/" ^ key in + let keys = g#aug_ls config_path in + if array_find key_path keys then + g#aug_get key_path + else + defval -- 2.26.2
Do not attempt to relabel a guest in case its SELinux enforcing mode is not "enforcing", as it is either pointless, or it may fail because of an invalid policy configured. --- mlcustomize/SELinux_relabel.ml | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/mlcustomize/SELinux_relabel.ml b/mlcustomize/SELinux_relabel.ml index 647aeda..db00e59 100644 --- a/mlcustomize/SELinux_relabel.ml +++ b/mlcustomize/SELinux_relabel.ml @@ -24,6 +24,9 @@ open Printf module G = Guestfs +exception SELinux_not_enforcing +(* Interal exception to signal a non-enforcing SELinux. *) + (* Simple reimplementation of Array.mem, available only with OCaml >= 4.03. *) let array_find a l = List.mem a (Array.to_list l) @@ -35,12 +38,18 @@ let rec relabel (g : G.guestfs) = use_setfiles g; (* That worked, so we don't need to autorelabel. *) g#rm_f "/.autorelabel" - with Failure _ -> + with + | Failure _ -> (* This is the fallback in case something in the setfiles * method didn't work. That includes the case where a non-SELinux * host is processing an SELinux guest, and other things. *) g#touch "/.autorelabel" + | SELinux_not_enforcing -> + (* This means that SELinux was not configured to be in enforcing mode, + * so silently accept this. + *) + () ) and is_selinux_guest g = @@ -59,6 +68,21 @@ and use_setfiles g = g#aug_load (); debug_augeas_errors g; + (* Get the SELinux enforcing mode, eg "enforcing", "permissive", + * "disabled". + * Use "disabled" if not specified, just like libselinux seems to do. + *) + let typ = read_selinux_config_key g "SELINUX" "disabled" in + (* Do not attempt any relabelling if the SELinux is not "enforcing": + * - in "permissive" mode SELinux is still running, however nothing is + * enforced: this means labels can be wrong, and "it is fine" + * - when "disabled" means SELinux is not running, so any relabelling + * is pointless (other than potentially fail due to an invalid + * SELINUXTYPE configuration) + *) + if typ <> "enforcing" then + raise SELinux_not_enforcing; + (* Get the SELinux policy name, eg. "targeted", "minimum". * Use "targeted" if not specified, just like libselinux does. *) -- 2.26.2
On Wed, Sep 23, 2020 at 05:57:50PM +0200, Pino Toscano wrote:
Do not attempt to relabel a guest in case its SELinux enforcing mode is not "enforcing", as it is either pointless, or it may fail because of an invalid policy configured. --- mlcustomize/SELinux_relabel.ml | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-)
diff --git a/mlcustomize/SELinux_relabel.ml b/mlcustomize/SELinux_relabel.ml index 647aeda..db00e59 100644 --- a/mlcustomize/SELinux_relabel.ml +++ b/mlcustomize/SELinux_relabel.ml @@ -24,6 +24,9 @@ open Printf
module G = Guestfs
+exception SELinux_not_enforcing +(* Interal exception to signal a non-enforcing SELinux. *) + (* Simple reimplementation of Array.mem, available only with OCaml >= 4.03. *) let array_find a l = List.mem a (Array.to_list l) @@ -35,12 +38,18 @@ let rec relabel (g : G.guestfs) = use_setfiles g; (* That worked, so we don't need to autorelabel. *) g#rm_f "/.autorelabel" - with Failure _ -> + with + | Failure _ -> (* This is the fallback in case something in the setfiles * method didn't work. That includes the case where a non-SELinux * host is processing an SELinux guest, and other things. *) g#touch "/.autorelabel" + | SELinux_not_enforcing -> + (* This means that SELinux was not configured to be in enforcing mode, + * so silently accept this. + *) + () )
and is_selinux_guest g = @@ -59,6 +68,21 @@ and use_setfiles g = g#aug_load (); debug_augeas_errors g;
+ (* Get the SELinux enforcing mode, eg "enforcing", "permissive", + * "disabled". + * Use "disabled" if not specified, just like libselinux seems to do. + *) + let typ = read_selinux_config_key g "SELINUX" "disabled" in + (* Do not attempt any relabelling if the SELinux is not "enforcing": + * - in "permissive" mode SELinux is still running, however nothing is + * enforced: this means labels can be wrong, and "it is fine"
I don't think it's fine. As I showed here: https://www.redhat.com/archives/libguestfs/2020-June/msg00115.html in permissive mode labels are still being updated on disk. TBH I don't understand what you said here: https://www.redhat.com/archives/libguestfs/2020-June/msg00117.html about "both the labels and the policy may be all wrong". If the administrator set the policy to permissive then labels ought still to be updated when the guest is running, and we ought to try to keep them updated if we can in v2v. It's also fine for an administrator to switch a system to permissive and then back to enforcing without relabelling or rebooting.
+ * - when "disabled" means SELinux is not running, so any relabelling + * is pointless (other than potentially fail due to an invalid + * SELINUXTYPE configuration)
Here you're correct. Once the admin disabled SELinux, labels are going to quickly get out of step with reality, and so attempting to relabel in virt-v2v is indeed a waste of time. I'd accept this series if you changed "not enforcing" to "not enforcing or permissive". Rich.
+ *) + if typ <> "enforcing" then + raise SELinux_not_enforcing; + (* Get the SELinux policy name, eg. "targeted", "minimum". * Use "targeted" if not specified, just like libselinux does. *) -- 2.26.2
_______________________________________________ Libguestfs mailing list Libguestfs@redhat.com https://www.redhat.com/mailman/listinfo/libguestfs
-- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into KVM guests. http://libguestfs.org/virt-v2v
On Thu, Sep 24, 2020 at 11:15:29AM +0100, Richard W.M. Jones wrote:
On Wed, Sep 23, 2020 at 05:57:50PM +0200, Pino Toscano wrote:
Do not attempt to relabel a guest in case its SELinux enforcing mode is not "enforcing", as it is either pointless, or it may fail because of an invalid policy configured.
If invalid policy is configured, you're doomed whether you do labelling or not. If valid policy is configured, and you don't do labelling you're creating a new failure mode. IOW, always doing labelling is the right thing, because at least one of the 4 scenarios will be an improvement, and it isn't creating any downsides other than the time required to do the labelling.
--- mlcustomize/SELinux_relabel.ml | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-)
diff --git a/mlcustomize/SELinux_relabel.ml b/mlcustomize/SELinux_relabel.ml index 647aeda..db00e59 100644 --- a/mlcustomize/SELinux_relabel.ml +++ b/mlcustomize/SELinux_relabel.ml @@ -24,6 +24,9 @@ open Printf
module G = Guestfs
+exception SELinux_not_enforcing +(* Interal exception to signal a non-enforcing SELinux. *) + (* Simple reimplementation of Array.mem, available only with OCaml >= 4.03. *) let array_find a l = List.mem a (Array.to_list l) @@ -35,12 +38,18 @@ let rec relabel (g : G.guestfs) = use_setfiles g; (* That worked, so we don't need to autorelabel. *) g#rm_f "/.autorelabel" - with Failure _ -> + with + | Failure _ -> (* This is the fallback in case something in the setfiles * method didn't work. That includes the case where a non-SELinux * host is processing an SELinux guest, and other things. *) g#touch "/.autorelabel" + | SELinux_not_enforcing -> + (* This means that SELinux was not configured to be in enforcing mode, + * so silently accept this. + *) + () )
and is_selinux_guest g = @@ -59,6 +68,21 @@ and use_setfiles g = g#aug_load (); debug_augeas_errors g;
+ (* Get the SELinux enforcing mode, eg "enforcing", "permissive", + * "disabled". + * Use "disabled" if not specified, just like libselinux seems to do. + *) + let typ = read_selinux_config_key g "SELINUX" "disabled" in + (* Do not attempt any relabelling if the SELinux is not "enforcing": + * - in "permissive" mode SELinux is still running, however nothing is + * enforced: this means labels can be wrong, and "it is fine"
I don't think it's fine. As I showed here:
https://www.redhat.com/archives/libguestfs/2020-June/msg00115.html
in permissive mode labels are still being updated on disk.
TBH I don't understand what you said here:
https://www.redhat.com/archives/libguestfs/2020-June/msg00117.html
about "both the labels and the policy may be all wrong". If the administrator set the policy to permissive then labels ought still to be updated when the guest is running, and we ought to try to keep them updated if we can in v2v. It's also fine for an administrator to switch a system to permissive and then back to enforcing without relabelling or rebooting.
Yep, in permissive mode apps / admins should still work on the assumption that SELinux is operating normally. ie assume you can be switched to enforcing mode at any time. You certainly want to continue to apply labelling as normal on disk. If you don't care about labelling to be updated, it means you are not likely to ever switch to enforcing, and that's what the "disabled" state is for.
+ * - when "disabled" means SELinux is not running, so any relabelling + * is pointless (other than potentially fail due to an invalid + * SELINUXTYPE configuration)
Here you're correct. Once the admin disabled SELinux, labels are going to quickly get out of step with reality, and so attempting to relabel in virt-v2v is indeed a waste of time.
I'd accept this series if you changed "not enforcing" to "not enforcing or permissive".
Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Thursday, 24 September 2020 12:15:29 CEST Richard W.M. Jones wrote:
On Wed, Sep 23, 2020 at 05:57:50PM +0200, Pino Toscano wrote:
Do not attempt to relabel a guest in case its SELinux enforcing mode is not "enforcing", as it is either pointless, or it may fail because of an invalid policy configured. --- mlcustomize/SELinux_relabel.ml | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-)
diff --git a/mlcustomize/SELinux_relabel.ml b/mlcustomize/SELinux_relabel.ml index 647aeda..db00e59 100644 --- a/mlcustomize/SELinux_relabel.ml +++ b/mlcustomize/SELinux_relabel.ml @@ -24,6 +24,9 @@ open Printf
module G = Guestfs
+exception SELinux_not_enforcing +(* Interal exception to signal a non-enforcing SELinux. *) + (* Simple reimplementation of Array.mem, available only with OCaml >= 4.03. *) let array_find a l = List.mem a (Array.to_list l) @@ -35,12 +38,18 @@ let rec relabel (g : G.guestfs) = use_setfiles g; (* That worked, so we don't need to autorelabel. *) g#rm_f "/.autorelabel" - with Failure _ -> + with + | Failure _ -> (* This is the fallback in case something in the setfiles * method didn't work. That includes the case where a non-SELinux * host is processing an SELinux guest, and other things. *) g#touch "/.autorelabel" + | SELinux_not_enforcing -> + (* This means that SELinux was not configured to be in enforcing mode, + * so silently accept this. + *) + () )
and is_selinux_guest g = @@ -59,6 +68,21 @@ and use_setfiles g = g#aug_load (); debug_augeas_errors g;
+ (* Get the SELinux enforcing mode, eg "enforcing", "permissive", + * "disabled". + * Use "disabled" if not specified, just like libselinux seems to do. + *) + let typ = read_selinux_config_key g "SELINUX" "disabled" in + (* Do not attempt any relabelling if the SELinux is not "enforcing": + * - in "permissive" mode SELinux is still running, however nothing is + * enforced: this means labels can be wrong, and "it is fine"
I don't think it's fine. As I showed here:
https://www.redhat.com/archives/libguestfs/2020-June/msg00115.html
in permissive mode labels are still being updated on disk.
This is true for default labels, yes.
TBH I don't understand what you said here:
https://www.redhat.com/archives/libguestfs/2020-June/msg00117.html
about "both the labels and the policy may be all wrong". If the administrator set the policy to permissive then labels ought still to be updated when the guest is running, and we ought to try to keep them updated if we can in v2v.
There are various cases when, even of an enforcing system, labels are not kept up-to-date: $ getenforce Enforcing $ touch /tmp/test $ ls -lZ /tmp/test -rw-rw-r--. 1 ptoscano ptoscano unconfined_u:object_r:user_tmp_t:s0 0 Sep 24 12:26 /tmp/test $ mv /tmp/test ~/var/ $ ls -lZ ~/var/test -rw-rw-r--. 1 ptoscano ptoscano unconfined_u:object_r:user_tmp_t:s0 0 Sep 24 12:26 /home/ptoscano/var/test $ restorecon -v ~/var/test Relabeled /home/ptoscano/var/test from unconfined_u:object_r:user_tmp_t:s0 to unconfined_u:object_r:user_home_t:s0 $ ls -lZ ~/var/test -rw-rw-r--. 1 ptoscano ptoscano unconfined_u:object_r:user_home_t:s0 0 Sep 24 12:26 /home/ptoscano/var/test Considering that /tmp is a general location for temporary files, it's common that files may end with a tmp_t-alike label when moved back to the destination place (e.g. after a rename()). That is not the only situation like this that I saw in the past. In permissive mode, all these situation are logged in the audit log, yes, but they cause no blocks nor errors.
It's also fine for an administrator to switch a system to permissive and then back to enforcing without relabelling or rebooting.
A mislabelled /etc/passwd is still read and used fine in permissive mode. Switch back from permissive to enforcing without a relabelling is generally not a good idea, especially after the system ran for a lot of time after the switch to permissive. -- Pino Toscano
On Thu, Sep 24, 2020 at 12:39:02PM +0200, Pino Toscano wrote: ...
There are various cases when, even of an enforcing system, labels are not kept up-to-date:
$ getenforce Enforcing $ touch /tmp/test $ ls -lZ /tmp/test -rw-rw-r--. 1 ptoscano ptoscano unconfined_u:object_r:user_tmp_t:s0 0 Sep 24 12:26 /tmp/test $ mv /tmp/test ~/var/ $ ls -lZ ~/var/test -rw-rw-r--. 1 ptoscano ptoscano unconfined_u:object_r:user_tmp_t:s0 0 Sep 24 12:26 /home/ptoscano/var/test $ restorecon -v ~/var/test Relabeled /home/ptoscano/var/test from unconfined_u:object_r:user_tmp_t:s0 to unconfined_u:object_r:user_home_t:s0 $ ls -lZ ~/var/test -rw-rw-r--. 1 ptoscano ptoscano unconfined_u:object_r:user_home_t:s0 0 Sep 24 12:26 /home/ptoscano/var/test
That's definitely a weird thing. Bug maybe?
Considering that /tmp is a general location for temporary files, it's common that files may end with a tmp_t-alike label when moved back to the destination place (e.g. after a rename()). That is not the only situation like this that I saw in the past.
In permissive mode, all these situation are logged in the audit log, yes, but they cause no blocks nor errors.
It's also fine for an administrator to switch a system to permissive and then back to enforcing without relabelling or rebooting.
A mislabelled /etc/passwd is still read and used fine in permissive mode. Switch back from permissive to enforcing without a relabelling is generally not a good idea, especially after the system ran for a lot of time after the switch to permissive.
It's seems true from what you wrote above that someone could copy /tmp/passwd -> /etc/passwd and it would have a wrong label. But virt-v2v could fix that label, which even in permissive mode sounds like a win. My question is what's the down-side to relabelling in permissive mode? (I can see in *disabled* mode it's just a waste of time because the work we do for relabelling in virt-v2v is just going to be undone when the guest boots with SELinux disabled). Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://people.redhat.com/~rjones/virt-df/
On Thursday, 24 September 2020 13:53:57 CEST Richard W.M. Jones wrote:
Considering that /tmp is a general location for temporary files, it's common that files may end with a tmp_t-alike label when moved back to the destination place (e.g. after a rename()). That is not the only situation like this that I saw in the past.
In permissive mode, all these situation are logged in the audit log, yes, but they cause no blocks nor errors.
It's also fine for an administrator to switch a system to permissive and then back to enforcing without relabelling or rebooting.
A mislabelled /etc/passwd is still read and used fine in permissive mode. Switch back from permissive to enforcing without a relabelling is generally not a good idea, especially after the system ran for a lot of time after the switch to permissive.
It's seems true from what you wrote above that someone could copy /tmp/passwd -> /etc/passwd and it would have a wrong label. But virt-v2v could fix that label, which even in permissive mode sounds like a win.
The question is: why? If the system had wrong labels even for system files, and the administrator did not bother/want to fix them (because of permissive), why should virt-v2v? Even if virt-v2v relabels a permissive guest, the labels will get out of sync once the guest runs again and does its own stuff, so there is no gain here.
My question is what's the down-side to relabelling in permissive mode?
Time spend doing something that is not useful/used for the guest. -- Pino Toscano
On Thu, Sep 24, 2020 at 02:16:24PM +0200, Pino Toscano wrote:
On Thursday, 24 September 2020 13:53:57 CEST Richard W.M. Jones wrote:
Considering that /tmp is a general location for temporary files, it's common that files may end with a tmp_t-alike label when moved back to the destination place (e.g. after a rename()). That is not the only situation like this that I saw in the past.
In permissive mode, all these situation are logged in the audit log, yes, but they cause no blocks nor errors.
It's also fine for an administrator to switch a system to permissive and then back to enforcing without relabelling or rebooting.
A mislabelled /etc/passwd is still read and used fine in permissive mode. Switch back from permissive to enforcing without a relabelling is generally not a good idea, especially after the system ran for a lot of time after the switch to permissive.
It's seems true from what you wrote above that someone could copy /tmp/passwd -> /etc/passwd and it would have a wrong label. But virt-v2v could fix that label, which even in permissive mode sounds like a win.
The question is: why? If the system had wrong labels even for system files, and the administrator did not bother/want to fix them (because of permissive), why should virt-v2v? Even if virt-v2v relabels a permissive guest, the labels will get out of sync once the guest runs again and does its own stuff, so there is no gain here.
I don't think this part is true (except in rather contrived situations). It seems as if when setting the mode to Permissive when the guest is running normally, it *is* attempting to maintain the correct labels. So virt-v2v should do the same thing. Rich.
My question is what's the down-side to relabelling in permissive mode?
Time spend doing something that is not useful/used for the guest.
-- Pino Toscano
-- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://people.redhat.com/~rjones/virt-df/
On Thursday, 24 September 2020 14:53:25 CEST Richard W.M. Jones wrote:
On Thu, Sep 24, 2020 at 02:16:24PM +0200, Pino Toscano wrote:
On Thursday, 24 September 2020 13:53:57 CEST Richard W.M. Jones wrote:
Considering that /tmp is a general location for temporary files, it's common that files may end with a tmp_t-alike label when moved back to the destination place (e.g. after a rename()). That is not the only situation like this that I saw in the past.
In permissive mode, all these situation are logged in the audit log, yes, but they cause no blocks nor errors.
It's also fine for an administrator to switch a system to permissive and then back to enforcing without relabelling or rebooting.
A mislabelled /etc/passwd is still read and used fine in permissive mode. Switch back from permissive to enforcing without a relabelling is generally not a good idea, especially after the system ran for a lot of time after the switch to permissive.
It's seems true from what you wrote above that someone could copy /tmp/passwd -> /etc/passwd and it would have a wrong label. But virt-v2v could fix that label, which even in permissive mode sounds like a win.
The question is: why? If the system had wrong labels even for system files, and the administrator did not bother/want to fix them (because of permissive), why should virt-v2v? Even if virt-v2v relabels a permissive guest, the labels will get out of sync once the guest runs again and does its own stuff, so there is no gain here.
I don't think this part is true (except in rather contrived situations).
I asked few questions, so "don't think it is true" does not seem correct to me.
It seems as if when setting the mode to Permissive when the guest is running normally, it *is* attempting to maintain the correct labels.
There is a difference between "attempting" and "succeeding". Permissive does some effort, but there is *zero* guarantee that the labels of the system are correct. And again: setting a Fedora/RHEL/CentOS system as anything different than enforcing requires an explicit action (either in /etc/selinux/config, or at runtime with `setenforce`, or when installing via kickstart, etc), so this is a situation that was explicitly requested.
So virt-v2v should do the same thing.
Right: respect the administrator choice to not enforce the SELinux policy on filesystems in any way. -- Pino Toscano
On Wednesday, 23 September 2020 17:57:47 CET Pino Toscano wrote:
Continuation/rework of: https://www.redhat.com/archives/libguestfs/2020-May/msg00020.html
This is my approach, as I explained here: https://bugzilla.redhat.com/show_bug.cgi?id=1828952#c4 https://www.redhat.com/archives/libguestfs/2020-May/msg00035.html IOW: do not attempt to relabel if the guest is not enforcing, as it is either useless or may fail; few words more are in the comments of patch #3.
Are there any non-ideological actual technical objection to this patch series? If not, I'll push it in a week. Thanks, -- Pino Toscano
On Wed, Nov 11, 2020 at 12:52:31PM +0100, Pino Toscano wrote:
On Wednesday, 23 September 2020 17:57:47 CET Pino Toscano wrote:
Continuation/rework of: https://www.redhat.com/archives/libguestfs/2020-May/msg00020.html
This is my approach, as I explained here: https://bugzilla.redhat.com/show_bug.cgi?id=1828952#c4 https://www.redhat.com/archives/libguestfs/2020-May/msg00035.html IOW: do not attempt to relabel if the guest is not enforcing, as it is either useless or may fail; few words more are in the comments of patch #3.
Are there any non-ideological actual technical objection to this patch series?
If not, I'll push it in a week.
Yes, I have technical objections. I do not have any "ideological" objections to it. Please do not push the patch as is. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://people.redhat.com/~rjones/virt-df/
participants (3)
-
Daniel P. Berrangé -
Pino Toscano -
Richard W.M. Jones