On 4/25/23 11:54 AM, Erik Skultety wrote:
On Tue, Apr 25, 2023 at 10:40:45AM -0500, Jonathon Jongsma wrote:
On 4/25/23 9:43 AM, Jonathon Jongsma wrote:
On 4/25/23 8:11 AM, Erik Skultety wrote:
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.
Yeah, I probably should have commented on why I used this 'partial'
approach. I tried a few different ways, including adding a positional
argument to argparse that would capture the target executable and its
arguments like so:
argsparse.add_argument("args",
nargs="+")
and then parsing with parser.parse_args() rather than
parse_known_args(). But that prevented me from passing arguments to the
target binary without inserting a '--' in to indicate that the run
script should stop parsing:
Fails:
# ./_build/run --selinux ./_build/src/libvirtd --verbose
usage: run [--selinux] args [args ...]
run: error: unrecognized arguments: --verbose
Works:
# ./_build/run --selinux -- ./_build/src/libvirtd --verbose
2023-04-25 14:26:32.175+0000: 1530463: info : libvirt version: 9.3.0
...
That seemed annoying to me.
Maybe we could keep using parse_known_args() with the 'args' argument
defined above, but I have a vague recollection that this caused some
other undesirable behavior so I switched back to the version I
submitted. I'll try to refresh my memory.
So here's one case that becomes more annoying when using the above setup
(the new 'args' catchall argument combined with parse_known_args()):
./run gdb -- libvirtd --verbose
gdb: unrecognized option '--verbose'
the ./run script eats the '--' that should go to gdb, so gdb tries to
interpret the --verbose option rather than passing it on to libvirtd.
I see. Without trying it/playing with it myself, I don't have a better proposal
at the moment, let me have a look at this tomorrow, I'll take your patch and
see if anything I'm familiar with in argparse would work, if not or if you can
figure out a better way in the meantime, I'll retract my comments to this
patch, how about that?
I did play around a little more and couldn't immediately find a better
way to do it. If you find a better approach, I'm happy to consider it. I
didn't initially consider using something like a a subparser (as you
mentioned in a different email) because that would change the way that
the script is invoked and I didn't want to disrupt others' development
workflow. But if people are OK with that, then that obviously opens up
more options.
Jonathon
