On Mon, Apr 24, 2023 at 03:50:48PM -0500, Jonathon Jongsma wrote:
> When running libvirt from the build directory with the 'run' script, it
> will run as unconfined_t. This can result in unexpected behavior when
> selinux is enforcing due to the fact that the selinux policies are
> written assuming that libvirt is running with the
> system_u:system_r:virtd_t context. This patch adds a new --selinux
> option to the run script. When this option is specified, it will launch
> the specified binary using the 'runcon' utility to set its selinux
> context to the one mentioned above. Since this requires root privileges,
> setting the selinux context is not the default behavior and must be
> enabled with the command line switch.
A fiddled with writing custom selinux transition rules to achieve the same
thing a couple years back, but never finished it. No wonder, this is a much
cleaner approach.
I will only comment on the Python side of things, leaving the overall approach
and idea commenting to someone else.
>
> Signed-off-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx>
> ---
> run.in | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++------
> 1 file changed, 50 insertions(+), 6 deletions(-)
>
> diff --git a/run.in b/run.in
> index c6d3411082..4aa458b791 100644
> --- a/run.in
> +++ b/run.in
> @@ -40,6 +40,7 @@
> #
> # ----------------------------------------------------------------------
>
> +import argparse
> import os
> import os.path
> import random
> @@ -59,15 +60,20 @@ def prepend(env, varname, extradir):
>
> here = "@abs_builddir@"
>
> -if len(sys.argv) < 2:
> - print("syntax: %s BINARY [ARGS...]" % sys.argv[0], file=sys.stderr)
Since you decided to use argparse (yes please), we can drop ^this if we
properly set the arguments with argparse, it'll even generate the help for us.
This way it looks only like a partial solution. Argparse has great
documentation so you can just take one of the examples they list.
> +parser = argparse.ArgumentParser(add_help=False)
Why don't we want the automatic help?
> +parser.add_argument('--selinux',
> + action='store_true',
> + help='Run in the appropriate selinux context')
> +
> +opts, args = parser.parse_known_args()
If you want to use ^this, then you need to be aware of prefix matching on the
options recognized by Argparse. IOW if one is to pass <args> to the <binary>
then none of the <args> can be a prefix of any of the long options argeparse
knows about (in this case --selinux), otherwise it'll consume it. Altough
unlikely, we should stay on the safe side and use:
argparse.ArgumentParser(..., allow_abbrev=False) [1]
[2] https://docs.python.org/3.11/library/argparse.html?highlight=argparse#allow-abbrev
> +
> +if len(args) < 1:
> + print("syntax: %s [--selinux] BINARY [ARGS...]" % sys.argv[0], file=sys.stderr)
> sys.exit(1)
Same here, with argparse ^this is not needed if the args/options are defined
correctly.
>
> -prog = sys.argv[1]
> -args = sys.argv[1:]
> +prog = args[0]
argparse's parser obj has a 'prog' attribute [2].
[2] https://docs.python.org/3/library/argparse.html#prog
The rest looks good from Python POV, but like I said, although I'm up for this
idea, I'll let someone else comment on that.
Regards,
Erik
