Bug 1794518

However on centos 7 with libguestfs-1.40.2-5.el7_7.2.x86_64 from rhev the conversion succeeds

I would really like to see the virt-v2v -v -x full output.  You can attach
it here, or if it contains sensitive data then email it to me directly.

Can you also see if this fixes the problem:

export LIBGUESTFS_MEMSIZE=4096

Thanks for sending me the full log.  As far as I can tell this really
is a bug in virt-v2v.  The setfiles command that we run when doing selinux
relabelling at the end fails if quotas are enabled, because the /quota.user
file is somehow "special" and setfiles chokes on it.

This was my attempt to reproduce the problem, but I cannot get it to fail.

-----
I had to use CentOS 6 to reproduce this because it requires an ext4 root filesystem.

$ virt-builder centos-6 --root-password password:123456
$ qemu-system-x86_64 -machine accel=kvm:tcg -cpu host -m 2048 -drive file=centos-6.img,format=raw,if=virtio

Inside the guest, enable quotas as described in this page:
https://wiki.archlinux.org/index.php/Disk_quota
This will involve rebooting the guest at least once.

Ensure after doing this you have a file called /quota.user or /aquota.user
and then power off the guest.

Convert the guest by doing:

$ virt-v2v -i disk centos-6.img -o null

Expected this step to fail at setfiles, but in fact it works fine.

As an update on this (coming back to this project after a while). A centos 7 docker container running virt-v2v 1.40.2rhel=7,release=9.el7_8.1,libvirt performs the conversion fine, a centos 8 container running virt-v2v 1.40.2rhel=8,release=25.module_el8.4.0+525+256c136b,libvirt has these selinux issues. I will try to produce a test image that I can send to you.

Using the disk image supplied by the reporter I found the error is:

commandrvf: setfiles -F -e /sysroot/dev -e /sysroot/proc -e /sysroot/selinux -e /sysroot/sys -m -r /sysroot -v /sysroot/etc/selinux/targeted/contexts/files/file_contexts /sysroot/
setfiles: Could not set context for /sysroot/etc/.ip:  Operation not permitted
guestfsd: error: setfiles: Could not set context for /sysroot/etc/.ip:  Operation not permitted

Inside the guest:

$ ls -l /etc/.ip 
-rwSr-sr-x 1 root root 4648 Nov 12  2018 /etc/.ip
$ stat /etc/.ip
  File: `/etc/.ip'
  Size: 4648      	Blocks: 16         IO Block: 4096   regular file
Device: fd00h/64768d	Inode: 20031       Links: 1
Access: (6655/-rwSr-sr-x)  Uid: (    0/    root)   Gid: (    0/    root)
Access: 2020-11-18 07:46:09.159073077 +0200
Modify: 2018-11-12 03:16:34.316119563 +0200
Change: 2019-06-29 07:41:53.116183079 +0200

(Note that /quota.user and /quota.group also exist in this guest.
They may also be causing problems for setfiles, but not in the
conversion which I did)

I tried adding -e /etc/.ip to the setfiles command but it didn't
make a difference.

yes it's affecting a number of different files on different images this was just an example..

lgetxattr("/sysroot/etc/.ip", "security.selinux", 0x56394efa0ff0, 255) = -1 ENODATA (No data available)
access("/var/run/setrans/.setrans-unix", F_OK) = -1 ENOENT (No such file or directory)
futex(0x7ffa050f15d0, FUTEX_WAKE_PRIVATE, 2147483647) = 0
lsetxattr("/sysroot/etc/.ip", "security.selinux", "system_u:object_r:etc_t:s0", 27, 0) = -1 EPERM (Operation not permitted)
write(2, "setfiles: ", 10)              = 10
write(2, "Could not set context for /sysro"..., 69) = 69

This is really a bug in setfiles.  It should either be able to relabel this
file or provide a way to ignore these errors.

I guess we could add new 'ignore errors' option to setfiles and do not set SELINUX_RESTORECON_ABORT_ON_ERROR flag.

But what is special on /sysroot/etc/.ip so that setfiles can't set the extended  attribute?

    	int rc = lsetxattr(path, XATTR_NAME_SELINUX, context, strlen(context) + 1, 0);

It's not only the file mode, is there any acl in game?

The bug reporter supplied me with the disk image (I'm afraid much too large
to easily share) so I can run commands on it.  To look for ACLs I used the
getfacl command:

# getfacl /sysroot/etc/.ip 
getfacl: Removing leading '/' from absolute path names
# file: sysroot/etc/.ip
# owner: root
# group: root
# flags: ss-
user::rw-
group::r-x
other::r-x

However you did set me on the right track, because I wondered if the
problem could be an ext4 attr, and indeed:

# lsattr /sysroot/etc/.ip 
----i---------e----- /sysroot/etc/.ip

So the file has the i (immutable) attribute.  ('e' means the file
can use ext4 extents which is not important here).

I wonder if we can ignore (or warn) on files with the immutable bit
set?  It seems wrong to fail setfiles completely.

Apparently there's behaviour change between rhel7 and rhel8. In rhel7 such file would be skipped while the new implementation aborts on it.

At this moment I'd blame https://github.com/SELinuxProject/selinux/commit/602347c7422e971a5674fe2767267a96e3b4f61c#diff-81dd45592a225693b30ab27df56bf9d298238035cf86e8fa930e20d25c8902e7

Originally there were 'goto skip' and 'return SKIP' when lsetfilecon() failed:

	ret = lsetfilecon(ftsent->fts_accpath, newcon);
	if (ret) {
        	fprintf(stderr, "%s set context %s->%s failed:'%s'\n",
			r_opts->progname, my_file, newcon, strerror(errno));
		goto skip;
	}
	ret = 0;


But the new implementation uses selinux_restorecon() with SELINUX_RESTORECON_ABORT_ON_ERROR flag which aborts immediately after lsetfilecon() fails.

We could fix it simply stop using SELINUX_RESTORECON_ABORT_ON_ERROR. But given the change happened 4 years ago, I'd like to discuss it with upstream first.

https://patchwork.kernel.org/project/selinux/patch/20210113210948.217575-1-plautrba@redhat.com/

Patch looks good here, thanks.

I tested policycoreutils-2.9-12.el8 and the original disk image supplied
by the reporter.  I see a series of errors like:

virt-v2v: error: libguestfs error: selinux_relabel: setfiles: Could not set context for /sysroot/etc/.ip: Operation not permitted
setfiles: Could not set context for /sysroot/etc/ssh/sshd_config:  Operation not permitted
setfiles: Could not set context for /sysroot/etc/exim.conf:  Operation not permitted
[...]

Previously only the first error was printed.

Unfortunately after that the conversion still fails.

Do we need to change the setfiles command in some way in order to get it to
ignore these errors?

Here is a simple(r) reproducer for this bug:

$ virt-builder fedora-30
$ guestfish -a fedora-30.img -i removexattr security.selinux /etc/passwd : removexattr security.selinux /etc/group : set-e2attrs /etc/passwd i : set-e2attrs /etc/group i
$ virt-v2v -i disk fedora-30.img -o null

You see multiple errors as there are more files which can't be labelled and setfiles try to relabel them all, setfiles then fails as there were errors during the relabel. I'd consider this correct.

But I guess we could use different error value for different errors. It's -1 for everything now, but we could change it to use 1 as a generic error and 2 as a value for processing/relabeling problem. You would need to check the error value, if it's 2 labelling happened but some files could  be still using wrong label.

The problem is we're doing this operation on N-thousand VMs at once
(while doing v2v conversions), and we don't really want to stop because
someone set an immutable bit on a file.  There's no good answer here,
but some way to ignore certain classes of errors would be welcome.  How
about some sort of "--ignore class1,class2,..." flag where certain types
of errors would be ignored while still failing on other kinds of errors?

I'm afraid it's not possible with the current code base. setfiles uses selinux_restorecon(3) which returns 0 on succes, -1 on error. In the change attached to this bug we've dropped SELINUX_RESTORECON_ABORT_ON_ERROR flag so that selinux_restorecon() doesn't abort on labeling error but it still returns -1.

I guess you could warn users when setfiles fails. Or you could implement the relabel on your own. Bellow is an example with minimal working code with hardcoded values:

#include <selinux/restorecon.h>
#include <selinux/selinux.h>
#include <selinux/label.h>

int main(int argc, char **argv) {
    struct selinux_opt selinux_opts[] = {
        { SELABEL_OPT_VALIDATE, (char *)1 },
        { SELABEL_OPT_PATH, "/sysroot/etc/selinux/targeted/contexts/files/file_contexts" },
        { SELABEL_OPT_DIGEST, (char *)1 }
    };
    unsigned int flags = SELINUX_RESTORECON_RECURSE |
        SELINUX_RESTORECON_LOG_MATCHES | SELINUX_RESTORECON_VERBOSE;
    struct selabel_handle *hndl;
    char *exclude_list[] = {
      "/sysroot/dev", "/sysroot/proc", "/sysroot/selinux", "/sysroot/sys", 0
    };
    int rc;

    hndl = selabel_open(SELABEL_CTX_FILE, selinux_opts, 3);

    selinux_restorecon_set_sehandle(hndl);
    selinux_restorecon_set_alt_rootpath("/sysroot");
    selinux_restorecon_set_exclude_list((const char **)exclude_list);
    rc = selinux_restorecon("/sysroot", flags);
    return (rc == -1 ? 1 : 0);
}

I'm going to have to think about this.  The current change is
still an improvement, so it would be nice to have this in RHEL.

If you like however you can move this bug back to component "libguestfs".

policycoreutils bug is cloned at https://bugzilla.redhat.com/show_bug.cgi?id=1926386

After evaluating this issue, there are no plans to address it further or fix it in an upcoming release.  Therefore, it is being closed.  If plans change such that this issue will be fixed in an upcoming release, then the bug can be reopened.

I'm sorry this bug was closed erroneously by an automated process that we
do not have control over.  Reopening.

Still a real bug, requires fixing per comment 25.  Moving to RHEL 9.

Moving back to libguestfs because I believe the fix will need to be made
there (in daemon/selinux-relabel.c).  The change will affect and have to be
tested against guestfs-tools (eg. virt-customize) and virt-v2v.

I've got some work-in-progress patches for this. The change will affect all of libselinux, setfiles(8), and the libguestfs daemon.

IMO part of the change really needs to go into the guts of the selinux_restorecon() function.

[PATCH for-3.5 0/5] selinux_restorecon(3), setfiles(8): skip relabeling errors
Message-Id: <20220428065354.27605-1-lersek>
https://lore.kernel.org/selinux/20220428065354.27605-1-lersek@redhat.com/

[PATCH v2 0/5] selinux_restorecon(3), setfiles(8): skip relabeling errors
Message-Id: <20220503082326.11621-1-lersek>
https://lore.kernel.org/selinux/20220503082326.11621-1-lersek@redhat.com/

(In reply to Laszlo Ersek from comment #37)
> [PATCH v2 0/5] selinux_restorecon(3), setfiles(8): skip relabeling errors
> Message-Id: <20220503082326.11621-1-lersek>
> https://lore.kernel.org/selinux/20220503082326.11621-1-lersek@redhat.com/

Merged to upstream SELinux in commit range d108226d3fc6..2b6f7e969cf6.

Next up: utilizing "setfiles -C" in "daemon/selinux-relabel.c", replacing commandv() with commandrv(), and accepting both exit statuses 0 and 1.

A reproducer based on comment 30:

(1) Prepare a disk image:

virt-builder fedora-30
guestfish -a fedora-30.img -i -- \
  touch /etc/testfile : \
  set-e2attrs /etc/testfile i

(2a) create an overlay:

guestfish -- \
  disk-create overlay.qcow2 qcow2 -1 backingfile:fedora-30.img backingformat:raw

(2b) attempt relabeling (in the overlay):

guestfish -a overlay.qcow2 -i -v -x -- \
  selinux-relabel /etc/selinux/targeted/contexts/files/file_contexts /etc/testfile force:true \
  2>&1 \
| tee -i log

(2c) check guestfish exit status:

echo ${PIPESTATUS[0]}


The idea is that step (1) needs to be done only once, but then step (2)
can be repeated an arbitrary number of times, using different versions
of libguestfs and setfiles(8), every time starting out with a pristine
overlay.

policycoreutils  |libguestfs  |(2c)|(2b)
-----------------+------------+----+------------------------------------------------------------
3.3-1.fc35       |08c4ac90f5a3|1   |commandrvf: setfiles -m
                 |            |    |commandrvf: setfiles -F
                 |            |    |  -e /sysroot/dev
                 |            |    |  -e /sysroot/proc
                 |            |    |  -e /sysroot/selinux
                 |            |    |  -e /sysroot/sys
                 |            |    |  -m
                 |            |    |  -r /sysroot
                 |            |    |  -v
                 |            |    |  /sysroot/etc/selinux/targeted/contexts/files/file_contexts
                 |            |    |  /sysroot/etc/testfile
                 |            |    |setfiles: Could not set context for /sysroot/etc/testfile:
                 |            |    |  Operation not permitted
                 |            |    |libguestfs: trace: selinux_relabel = -1 (error)
-----------------+------------+----+------------------------------------------------------------
3.4-0.rc3.1.fc35 |08c4ac90f5a3|1   |<same as above>
-----------------+------------+----+------------------------------------------------------------
3.3-1.fc35       |08c4ac90f5a3|1   |commandrvf: setfiles -m
                 |+ patches   |    |commandrvf: setfiles -C
                 |            |    |setfiles: invalid option -- 'C'
                 |            |    |commandrvf: setfiles -F
                 |            |    |  -e /sysroot/dev
                 |            |    |  -e /sysroot/proc
                 |            |    |  -e /sysroot/selinux
                 |            |    |  -e /sysroot/sys
                 |            |    |  -m
                 |            |    |  -r /sysroot
                 |            |    |  -v
                 |            |    |  /sysroot/etc/selinux/targeted/contexts/files/file_contexts
                 |            |    |  /sysroot/etc/testfile
                 |            |    |setfiles: Could not set context for /sysroot/etc/testfile:
                 |            |    |  Operation not permitted
                 |            |    |libguestfs: trace: selinux_relabel = -1 (error)
-----------------+------------+----+------------------------------------------------------------
3.4-0.rc3.1.fc35 |08c4ac90f5a3|0   |commandrvf: setfiles -m
                 |+ patches   |    |commandrvf: setfiles -C
                 |            |    |commandrvf: setfiles -F
                 |            |    |  -e /sysroot/dev
                 |            |    |  -e /sysroot/proc
                 |            |    |  -e /sysroot/selinux
                 |            |    |  -e /sysroot/sys
                 |            |    |  -m
                 |            |    |  -C
                 |            |    |  -r /sysroot
                 |            |    |  -v
                 |            |    |  /sysroot/etc/selinux/targeted/contexts/files/file_contexts
                 |            |    |  /sysroot/etc/testfile
                 |            |    |setfiles: Could not set context for /sysroot/etc/testfile:
                 |            |    |  Operation not permitted
                 |            |    |libguestfs: trace: selinux_relabel = 0

(In reply to Laszlo Ersek from comment #42)
> A reproducer based on comment 30:

Umm. I meant "based on comment 22".

A regression test -- in step (2b), specify "/etc/no-such-file" in place of "/etc/testfile":

policycoreutils  |libguestfs  |(2c)|(2b)
-----------------+------------+----+------------------------------------------------------------
3.4-0.rc3.1.fc35 |08c4ac90f5a3|1   |commandrvf: setfiles -m
                 |+ patches   |    |commandrvf: setfiles -C
                 |            |    |commandrvf: setfiles -F
                 |            |    |  -e /sysroot/dev
                 |            |    |  -e /sysroot/proc
                 |            |    |  -e /sysroot/selinux
                 |            |    |  -e /sysroot/sys
                 |            |    |  -m
                 |            |    |  -C
                 |            |    |  -r /sysroot
                 |            |    |  -v
                 |            |    |  /sysroot/etc/selinux/targeted/contexts/files/file_contexts
                 |            |    |  /sysroot/etc/no-such-file
                 |            |    |setfiles: lstat(/sysroot/etc/no-such-file) failed:
                 |            |    |  No such file or directory
                 |            |    |libguestfs: trace: selinux_relabel = -1 (error)

[libguestfs PATCH 0/2] daemon/selinux-relabel: tolerate relabeling errors
Message-Id: <20220511122345.14208-1-lersek>
https://listman.redhat.com/archives/libguestfs/2022-May/028850.html

(In reply to Laszlo Ersek from comment #47)
> [libguestfs PATCH 0/2] daemon/selinux-relabel: tolerate relabeling errors
> Message-Id: <20220511122345.14208-1-lersek>
> https://listman.redhat.com/archives/libguestfs/2022-May/028850.html

Merged up-stream as commit range 08c4ac90f5a3..a39b79f6079c.

Verified with the following packages:
libguestfs-1.48.2-2.el9.x86_64
policycoreutils-3.4-1.el9.x86_64


Steps:

1. On rhel9.1 host
$ virt-builder fedora-35

2.
$ guestfish -a fedora-35.img -i -- touch /etc/testfile : set-e2attrs /etc/testfile i

3.
$ guestfish -- disk-create overlay.qcow2 qcow2 -1 backingfile:fedora-35.img backingformat:raw

4.
$ guestfish -a overlay.qcow2 -i -v -x -- selinux-relabel /etc/selinux/targeted/contexts/files/file_contexts /etc/testfile force:true 2>&1 | tee -i log
...
commandrvf: setfiles -F -e /sysroot/dev -e /sysroot/proc -e /sysroot/selinux -e /sysroot/sys -m -C -r /sysroot -v /sysroot/etc/selinux/targeted/contexts/files/file_contexts /sysroot/etc/testfile
Can't stat exclude path "/sysroot/selinux", No such file or directory - ignoring.
setfiles: Regex version mismatch, expected: 10.40 2022-04-14 actual: 10.39 2021-10-29
setfiles: Regex version mismatch, expected: 10.40 2022-04-14 actual: 10.39 2021-10-29
setfiles: Could not set context for /sysroot/etc/testfile:  Operation not permitted
guestfsd: => selinux_relabel (0x1d3) took 0.18 secs
libguestfs: trace: selinux_relabel = 0
...

5.
$ echo ${PIPESTATUS[0]}
0

6. Test with non-existent file
$ guestfish -a overlay.qcow2 -i -v -x -- selinux-relabel /etc/selinux/targeted/contexts/files/file_contexts /etc/no-such-file force:true 2>&1 | tee -i log.no-such-file
...
commandrvf: setfiles -F -e /sysroot/dev -e /sysroot/proc -e /sysroot/selinux -e /sysroot/sys -m -C -r /sysroot -v /sysroot/etc/selinux/targeted/contexts/files/file_contexts /sysroot/etc/no-such-file
Can't stat exclude path "/sysroot/selinux", No such file or directory - ignoring.
setfiles: Regex version mismatch, expected: 10.40 2022-04-14 actual: 10.39 2021-10-29
setfiles: Regex version mismatch, expected: 10.40 2022-04-14 actual: 10.39 2021-10-29
setfiles: lstat(/sysroot/etc/no-such-file) failed: No such file or directory
guestfsd: error: Can't stat exclude path "/sysroot/selinux", No such file or directory - ignoring.
setfiles: Regex version mismatch, expected: 10.40 2022-04-14 actual: 10.39 2021-10-29
setfiles: Regex version mismatch, expected: 10.40 2022-04-14 actual: 10.39 2021-10-29
setfiles: lstat(/sysroot/etc/no-such-file) failed: No such file or directory
guestfsd: => selinux_relabel (0x1d3) took 0.18 secs
libguestfs: trace: selinux_relabel = -1 (error)
...

7.
$ echo ${PIPESTATUS[0]}
1

On virt-v2v side:

Reproduced with the versions:
libguestfs-1.48.1-1.el9.x86_64
guestfs-tools-1.48.1-1.el9.x86_64
virt-v2v-2.0.5-1.el9.x86_64

1) Prepare a disk image:
# virt-builder fedora-30

2) set immutable bits：
# guestfish -a fedora-30.img -i removexattr security.selinux /etc/passwd : removexattr security.selinux /etc/group : set-e2attrs /etc/passwd i : set-e2attrs /etc/group i

3) use virt-v2v:
# virt-v2v -i disk fedora-30.img -o null
[   0.0] Setting up the source: -i disk fedora-30.img
[   1.0] Opening the source
[   6.5] Inspecting the source
[  14.3] Checking for sufficient free disk space in the guest
[  14.3] Converting Fedora 30 (Thirty) to run on KVM
virt-v2v: warning: /files/boot/grub2/device.map/hd0 references unknown 
device "vda".  You may have to fix this entry manually after conversion.
virt-v2v: error: libguestfs error: selinux_relabel: Can't stat exclude path 
"/sysroot/selinux", No such file or directory - ignoring.
setfiles: Regex version mismatch, expected: 10.40 2022-04-14 actual: 10.33 
2019-04-16
setfiles: Regex version mismatch, expected: 10.40 2022-04-14 actual: 10.33 
2019-04-16
setfiles: Could not set context for /sysroot/etc/group:  Operation not 
permitted
setfiles: Could not set context for /sysroot/etc/passwd:  Operation not 
permitted

If reporting bugs, run virt-v2v with debugging enabled and include the 
complete output:

  virt-v2v -v -x [...]



Tried with the latest versions:
libguestfs-1.48.3-4.el9.x86_64
guestfs-tools-1.48.2-4.el9.x86_64
virt-v2v-2.0.6-3.el9.x86_64


1) Prepare a disk image:
# virt-builder fedora-35

2) set immutable bits：
# guestfish -a fedora-35.img -i removexattr security.selinux /etc/passwd : removexattr security.selinux /etc/group : set-e2attrs /etc/passwd i : set-e2attrs /etc/group i

3) use virt-v2v:
# virt-v2v -i disk fedora-35.img -o null
[   0.0] Setting up the source: -i disk fedora-35.img
[   1.0] Opening the source
[   4.8] Inspecting the source
[   8.3] Checking for sufficient free disk space in the guest
[   8.3] Converting Fedora Linux 35 (Thirty Five) to run on KVM
virt-v2v: The QEMU Guest Agent will be installed for this guest at first 
boot.
virt-v2v: warning: /files/boot/grub2/device.map/hd0 references unknown 
device "vda".  You may have to fix this entry manually after conversion.
virt-v2v: This guest has virtio drivers installed.
[  42.6] Mapping filesystem data to avoid copying unused and blank areas
[  44.6] Closing the overlay
[  44.7] Assigning disks to buses
[  44.7] Checking if the guest needs BIOS or UEFI to boot
[  44.7] Setting up the destination: -o null
[  45.8] Copying disk 1/1
█ 100% [****************************************]
[  47.9] Creating output metadata
[  47.9] Finishing off

Set the status to 'VERIFIED' per comment 57 and comment 58.

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory (Low: libguestfs security, bug fix, and enhancement update), and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHSA-2022:7958