Libguestfs - May 2014 - Re: [PATCH 2/2] Use setfiles from the appliance for the SELinux relabel (RHBZ#1089100).

On Tuesday 27 May 2014 09:08:27 Richard W.M. Jones
wrote:
> On Mon, May 26, 2014 at 11:21:59AM +0200, Pino Toscano wrote:
> > Rewrite the relabel API to read the policy configured in the guest,
> > invoking setfiles (added as part of the appliance, as part of
> > policycoreutils) to relabel the specified root. In case of failure
> > at
> > any point of the process, a touch of .autorelabel in the root is
> > tried as last-attempt measure to do the relabel.
> > 
> > Considering that running SELinux tools in the appliance might be
> > affected by the SELinux state (leading to wrong results),
> > selinux_relabel now bails out if SELinux is enabled in the
> > appliance.
> > As a result of this, virt-builder and virt-customize explicitly
> > disable it if the relabel is enabled.
> > 
> > -    g#set_selinux ops.flags.selinux_relabel;
> > +    (* If a relabel is needed, make sure to turn SELinux off to
> > avoid +     * awkward interactions with the relabel process.
> > +     *)
> > +    if ops.flags.selinux_relabel then g#set_selinux false;
> 
> This defaults to false, so AFAICT you could just remove this hunk.  Or
> call g#set_selinux false unconditionally to make your intention
> explicit?
> 
> (Same for the customize_main.ml hunk)
Yes, that together its comment above is done to make that setting 
explicit, so it is not changed in the future creating issues.
> > +  len = length_without_training_slash (root);
> > +
> > +  if (asprintf (&selinux_config,
"%s%.*s/etc/selinux/config",
> > +                sysroot, len, root) == -1) {
> > +    if (verbose)
> > +      fprintf (stderr, "asprintf/selinux_config failed\n");
> > +    goto do_autorelabel;
> > +  }
> > +
> > +  r = read_selinux_policy (selinux_config, &policy);
> > +  if (r == -1) {
> > +    if (verbose)
> > +      fprintf (stderr, "cannot read policy from %s\n",
> > selinux_config); +    goto do_autorelabel;
> > +  }
> > +  if (verbose)
> > +    fprintf (stderr, "policy in %s: %s\n", root, policy);
> > +
> > +  if (policy[0] == '\0')
> > +    goto do_autorelabel;
> 
> You'll probably find this is much easier to write and a lot more
> robust using augeas calls.
Good idea, I will use it.

Thanks,
-- 
Pino Toscano

I'm still anxious for Colin to tell us if this API is suitable for his
needs.

I'm guessing that OStree does not have /etc/selinux/config, so this
API would not work for him ...  (Unfortunately I deleted my very old
OStree VM so I'll have to download a new one to see how it works).

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW

[Including libguestfs mailing list this time]

On Tue, May 27, 2014 at 06:05:15AM -0700, Colin Walters
wrote:
> On Tue, May 27, 2014, at 02:04 AM, Richard W.M. Jones wrote:
> > 
> > I'm still anxious for Colin to tell us if this API is suitable for
his
> > needs.
> 
> I'd like an API that allows me to only relabel *unlabeled* files.
> The use case here is: I have an existing disk image with an OS,
> I want to inject e.g. a systemd service into it.  If I do this offline
> from libguestfs, the injected /usr/libexec/mydaemon and
> /usr/lib/systemd/system/mydaemon.service
> won't be labeled, but everything else will be.
>
> > I'm guessing that OStree does not have /etc/selinux/config,
> 
> Right, it's in the "deployment root" of
> /ostree/deploy/$osname/deploy/$checksum/etc/selinux/config
Got it:
><fs> ll
/ostree/deploy/project-atomic-controller/deploy/afc1794b4b42df77edf1988897b167573b99e299fa39a15b07b235a0e7387d02.0/etc/selinux/targeted/contexts/files/file_contexts
-rw-r--r--. 1 root root 352240 Apr 14 20:14
/sysroot/ostree/deploy/project-atomic-controller/deploy/afc1794b4b42df77edf1988897b167573b99e299fa39a15b07b235a0e7387d02.0/etc/selinux/targeted/contexts/files/file_contexts
> To figure that out you'd want to use the OSTree APIs; and then it
> introduces
> further questions around *which* deployments you want to relabel.  All? 
> Only
> one (the default?).
> 
> What I do currently in my scripts is only relabel the default, and that
> would
> be the best default for an API.
> 
> But a totally valid thing to do with OSTree is - say you're running
> RHEL7,
> and you want to check whether the latest Fedora kernel fixes an issue
> you're seeing.   You can use ostree to dynamically parallel install
> Fedora content in a new deployment root, try it with near-total
> safety[1],
> and then if it doesn't work, just delete it and free up the space.
So I think an API which looks like this ...

 required params:

   None

 optional params:

   path =>
     Either a directory to be relabelled recursively, or a single
     file (defaults to "/").

   root =>
     Inspection root of guest.  Optional, only makes sense when
     'contexts' param is *omitted*.

   contexts =>
     The `file_contexts' file.  Defaults to
     /etc/selinux/$selinux_type/contexts/files/file_contexts

     OSTree would probably want to pass:
    
/ostree/deploy/$osname/deploy/$checksum/etc/selinux/targeted/contexts/files/file_contexts

Inspection could be updated to parse /etc/selinux/config in order to
get the default SELinux policy and pass it back through an API such as
`inspect-get-selinux-type'.

If 'contexts' is omitted, 'root' must be supplied, and it causes
an
internal call to guestfs_inspect_get_selinux_type (g, root) in order
to get the default policy.

What do you think?

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW

On Tuesday 27 May 2014 14:25:08 Richard W.M. Jones
wrote:
> So I think an API which looks like this ...
> 
>  required params:
> 
>    None
> 
>  optional params:
> 
>    path =>
>      Either a directory to be relabelled recursively, or a single
>      file (defaults to "/").
> 
>    root =>
>      Inspection root of guest.  Optional, only makes sense when
>      'contexts' param is *omitted*.
> 
>    contexts =>
>      The `file_contexts' file.  Defaults to
>      /etc/selinux/$selinux_type/contexts/files/file_contexts
> 
>      OSTree would probably want to pass:
>     
> /ostree/deploy/$osname/deploy/$checksum/etc/selinux/targeted/contexts
> /files/file_contexts
> 
> Inspection could be updated to parse /etc/selinux/config in order to
> get the default SELinux policy and pass it back through an API such as
> `inspect-get-selinux-type'.
> 
> If 'contexts' is omitted, 'root' must be supplied, and it
causes an
> internal call to guestfs_inspect_get_selinux_type (g, root) in order
> to get the default policy.
Note that not specifying a root could lead to issues, as the file 
contexts are relative to a root. So if I say to relabel the path 
/guestmountpoint/etc/myconfig according to some /path/of/file_contexts 
without specifying what is the root, how should setfiles know that path 
is /etc/myconfig mounted at /guestmountpoint?

At this point I'm thinking the best option would be making the root a 
normal (mandatory) argument, leaving path and contexts as optional (with 
the former being "/" as default value, and the latter as "find it
from
the root").

In the situation above, path would become a relative path to the 
specified root (so if I mount a guest into /guest and I want to relabel 
it only under /etc, I would pass root=/guest and path=/etc).

-- 
Pino Toscano