Libguestfs - Feb 2018 - Re: [libvirt] [PATCH tck] Relabel SELinux when customizing virt-builder image

On 02/06/2018 10:53 AM, Pino Toscano wrote:
> On Tuesday, 6 February 2018 16:40:04 CET Daniel P. Berrangé wrote:
>> When you tell virt-builder to install extra RPMs, this potentially
>> looses the SELinux labelling that Anaconda had originally setup. Thus
we
>> must tell virt-builder to enable SELinux relabelling.
>>
>> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
>> ---
>>  lib/Sys/Virt/TCK.pm | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/lib/Sys/Virt/TCK.pm b/lib/Sys/Virt/TCK.pm
>> index e9da8d2..b39f578 100644
>> --- a/lib/Sys/Virt/TCK.pm
>> +++ b/lib/Sys/Virt/TCK.pm
>> @@ -405,7 +405,7 @@ sub create_virt_builder_disk {
>>      }
>>  
>>      print "# running virt-builder $osname\n";
>> -    system "virt-builder", "--install",
"dsniff", "--root-password", "password:$password",
"--output", $target, $osname;
>> +    system "virt-builder", "--install",
"dsniff", "--selinux-relabel", "--root-password",
"password:$password", "--output", $target, $osname;
>>  
>>      die "cannot run virt-builder: $?" if $? != 0;
> 
> Reviewed-by: Pino Toscano <ptoscano@redhat.com>
> 
This change works, but since the original image came from virt-builder,
and virt-builder knows enough about the image to know that it should
install packages with dnf (or yum or apt-get or whatever is appropriate
for any given image), it should also have enough info available to
determine on its own that the selinux labels need to be redone.
Especially since the Fedora images provided by virt-builder have selinux
set to enforcing, I think the default behavior in this case should be
for virt-builder to relabel.

This patch fixes the problem for libvirt-tck, but I can imagine that
this same problem will be revisited time after time on IRC and the
libguestfs mailing list (once the user takes the obligatory
troubleshooting trip to discover the source of the problem). In this
case the initial symptom was "a guest that was never logged into by a
human was failing an automated test". There were several steps from
there to "dhcpc was failing to get an IP address due to bad selinux
labels", and then learning via IRC that the labels were incorrect
because extra packages are installed with the image mounted on the
libguestfs appliance, which runs with selinux disabled.

What is preventing virt-builder from automatically making a correct
determination about whether or not relabeling must be done?

On Tue, Feb 06, 2018 at 12:50:51PM -0500, Laine Stump
wrote:
> On 02/06/2018 10:53 AM, Pino Toscano wrote:
> > On Tuesday, 6 February 2018 16:40:04 CET Daniel P. Berrangé wrote:
> >> When you tell virt-builder to install extra RPMs, this potentially
> >> looses the SELinux labelling that Anaconda had originally setup.
Thus we
> >> must tell virt-builder to enable SELinux relabelling.
> >>
> >> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> >> ---
> >>  lib/Sys/Virt/TCK.pm | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/lib/Sys/Virt/TCK.pm b/lib/Sys/Virt/TCK.pm
> >> index e9da8d2..b39f578 100644
> >> --- a/lib/Sys/Virt/TCK.pm
> >> +++ b/lib/Sys/Virt/TCK.pm
> >> @@ -405,7 +405,7 @@ sub create_virt_builder_disk {
> >>      }
> >>  
> >>      print "# running virt-builder $osname\n";
> >> -    system "virt-builder", "--install",
"dsniff", "--root-password", "password:$password",
"--output", $target, $osname;
> >> +    system "virt-builder", "--install",
"dsniff", "--selinux-relabel", "--root-password",
"password:$password", "--output", $target, $osname;
> >>  
> >>      die "cannot run virt-builder: $?" if $? != 0;
> > 
> > Reviewed-by: Pino Toscano <ptoscano@redhat.com>
> > 
> 
> This change works, but since the original image came from virt-builder,
> and virt-builder knows enough about the image to know that it should
> install packages with dnf (or yum or apt-get or whatever is appropriate
> for any given image), it should also have enough info available to
> determine on its own that the selinux labels need to be redone.
> Especially since the Fedora images provided by virt-builder have selinux
> set to enforcing, I think the default behavior in this case should be
> for virt-builder to relabel.
> 
> This patch fixes the problem for libvirt-tck, but I can imagine that
> this same problem will be revisited time after time on IRC and the
> libguestfs mailing list (once the user takes the obligatory
> troubleshooting trip to discover the source of the problem). In this
> case the initial symptom was "a guest that was never logged into by a
> human was failing an automated test". There were several steps from
> there to "dhcpc was failing to get an IP address due to bad selinux
> labels", and then learning via IRC that the labels were incorrect
> because extra packages are installed with the image mounted on the
> libguestfs appliance, which runs with selinux disabled.
> 
> What is preventing virt-builder from automatically making a correct
> determination about whether or not relabeling must be done?
Yes, in fact I think it could go further and just call
SELinux_relabel.relabel on every guest, since that code just ignores
non-SELinux guests.

Basically the reasons it doesn't do this are historical and possibly a
fear of breaking if some guest has broken SELinux files.  We could
retain the ‘--no-selinux-relabel’ flag to mean don't do any
relabelling.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org

On Wed, Feb 07, 2018 at 11:10:25AM +0000, Richard W.M. Jones
wrote:
> Yes, in fact I think it could go further and just call
> SELinux_relabel.relabel on every guest, since that code just ignores
> non-SELinux guests.
> 
> Basically the reasons it doesn't do this are historical and possibly a
> fear of breaking if some guest has broken SELinux files.  We could
> retain the ‘--no-selinux-relabel’ flag to mean don't do any
> relabelling.
There's now a bug to track this feature request:

https://bugzilla.redhat.com/show_bug.cgi?id=1554735

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/