Commit Graph

43 Commits

Author SHA1 Message Date
Daniel P. Berrangé
f80c009cb3 build: generate configmake.h in root directory
Currently we rely on gnulib creating configmake.h, but we
can easily create it ourselves instead.

Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2020-01-29 14:51:39 +00:00
Daniel P. Berrangé
117353f9e5 src: replace use of INT_BUFSIZE_BOUND macros
Introduce a vastly simpler VIR_INT64_STR_BUFLEN constant
which is large enough for all cases where we currently
use INT_BUFSIZE_BOUND. This eliminates most use of the
gnulib intprops.h header.

Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2020-01-17 09:58:43 +00:00
Daniel P. Berrangé
f4e1ef9d39 tools: fix regression passing command with virt-login-shell
It is documented that a command to run inside the container can be
passed with the -c arg.

  virt-login-shell -c "ls -l /"

This fixes

  commit 4feeb2d986
  Author: Daniel P. Berrangé <berrange@redhat.com>
  Date:   Thu Aug 1 10:58:31 2019 +0100

    tools: split virt-login-shell into two binaries

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2019-09-30 12:08:30 +01:00
Daniel P. Berrangé
8242ce4f45 tools: avoid accidentally using files from gnulib
The AM_CPPFLAGS setting includes the gnulib headers, which
means we can get some replacement functions defined. Since
virt-login-shell and the NSS module intentionally don't link
to gnulib, these replacement functions causes link failures.

This was seen cross-compiling on Debian for example:

virt-login-shell.o: In function `main':
/builds/libvirt/libvirt/build/tools/../../tools/virt-login-shell.c:81: undefined reference to `rpl_strerror'
/builds/libvirt/libvirt/build/tools/../../tools/virt-login-shell.c:66: undefined reference to `rpl_strerror'
/builds/libvirt/libvirt/build/tools/../../tools/virt-login-shell.c:75: undefined reference to `rpl_strerror'

The only way to avoid these replacement gnulib headers is
to drop the -Ignulib/lib flags. We do still want to use
gnulib for configmake.h and intprops.h, but those can be
included via their full path.

We must also stop using internal.h, since that expects
-Ignulib/lib to be on the include path in order to resolve
the verify.h header.

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2019-08-08 13:32:02 +01:00
Daniel P. Berrangé
4feeb2d986 tools: split virt-login-shell into two binaries
The virt-login-shell binary is a setuid program that takes
no arguments. When invoked it looks at the invoking uid,
resolves it to a username, and finds an LXC guest with the
same name. It then starts the guest and runs the shell in
side the namespaces of the container.

Given this set of tasks the virt-login-shell binary needs
to connect to libvirtd, make various other libvirt API calls.
This is a problem for setuid binaries as various libraries
that libvirt.so links to are not safe. For example, they have
constructor functions which execute an unknown amount of code
that can be influenced by env variables.

For this reason virt-login-shell doesn't use libvirt.so,
but instead links to a custom, cut down, set of source files
sufficient to be a local client only.

This introduces a problem for integrating glib2 into libvirt
though, as once integrated, there would be no way to build
virt-login-shell without an external dependancy on glib2 and
this is definitely not setuid safe.

To resolve this problem, we split the virt-login-shell binary
into two parts. The first part is setuid and does almost
nothing. It simply records the original uid+gid, and then
invokes the virt-login-shell-helper binary. Crucially when
it does this it completes scrubs all environment variables.
It is thus safe for virt-login-shell-helper to link to the
normal libvirt.so. Any things that constructor functions
do cannot be influenced by user control env vars or cli
args.

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2019-08-07 16:54:01 +01:00
Daniel P. Berrangé
46754ffb6a tools: rename source for virt-login-shell
We'll shortly be renaming the binary to virt-login-shell-helper
and introducing a new tool as virt-login-shell. Renaming the
source file first gives a much more usefull diff for the next
commit.

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2019-08-07 16:54:01 +01:00
Daniel P. Berrangé
cd1f25a9d4 tools: fix double error reporting in virt-login-shell
The public API entry points will call virDispatchError which
will print to stderr by default. We then jump to a cleanup
path which calls virDispatchError again.

We tried to stop the entry points printing to stderr, but
incorrectly called virSetErrorFunc. It needs a real function
that is a no-op, not a NULL function.

Once we fix virSetErrorFunc, then we need to use fprintf in
the cleanup path instead of virDispatchError.

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2019-08-07 16:54:01 +01:00
Daniel P. Berrangé
275bcbebab tools: fix crash in virt-login-shell if config doesn't exist
If the 'allowed_users' config setting in virt-login-shell.conf
does not exist, we dereference a NULL pointer resulting in a
crash. We should check for this case and thus ensure the user
is denied access gracefully.

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2019-08-07 16:54:01 +01:00
Michal Privoznik
c99e954973 Remove even more Author(s): lines from source files
In 600462834f we've tried to remove Author(s): lines
from comments at the beginning of our source files. Well, in some
files while we removed the "Author" line we did not remove the
actual list of authors.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
2019-01-03 13:24:18 +01:00
Erik Skultety
5165ff0971 src: More cleanup of some system headers already contained in internal.h
All of the ones being removed are pulled in by internal.h. The only
exception is sanlock which expects the application to include <stdint.h>
before sanlock's headers, because sanlock prototypes use fixed width
int, but they don't include stdint.h themselves, so we have to leave
that one in place.

Signed-off-by: Erik Skultety <eskultet@redhat.com>
Acked-by: Michal Privoznik <mprivozn@redhat.com>
2018-09-20 10:16:39 +02:00
Erik Skultety
9403b63102 internal: Move <stdio.h> include to internal.h
It doesn't really make sense for us to have stdlib.h and string.h but
not stdio.h in the internal.h header.

Signed-off-by: Erik Skultety <eskultet@redhat.com>
Acked-by: Michal Privoznik <mprivozn@redhat.com>
2018-09-20 10:16:38 +02:00
Daniel P. Berrangé
a2fd657b86 lxc: allow use of lxc:///system URI as preferred format
Historically we have used a bare lxc:/// URI for connecting to LXC. This
is different from our practice with QEMU, UML, Parallels, Libxl, BHyve
and VirtualBox drivers, which all use a path of '/system' or '/session'
or both.

By making LXC allow '/system', we have fully standardized on the use of
either '/system' or '/session' for all the stateful drivers that run
inside libvirtd.

Support for lxc:/// is of course maintained for back-compat.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2018-04-12 16:52:01 +01:00
Michal Privoznik
c2a5a4e7ea virstring: Unify string list function names
We have couple of functions that operate over NULL terminated
lits of strings. However, our naming sucks:

virStringJoin
virStringFreeList
virStringFreeListCount
virStringArrayHasString
virStringGetFirstWithPrefix

We can do better:

virStringListJoin
virStringListFree
virStringListFreeCount
virStringListHasString
virStringListGetFirstWithPrefix

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2016-11-25 13:54:05 +01:00
John Ferlan
dfb18b0afb tools: Fix comparison in virLoginShellGetShellArgv
Commit id '740e4d70' altered the logic to fetch the sysconf values and
added a new virConfGetValueStringList which returns -1 on failure, 0 if
missing, and 1 if the value was present.

However, the caller only checked !shargv which caught Coverity's attention
since the following VIR_ALLOC_N(*shargv, 2) would be a NULL ptr deref

Signed-off-by: John Ferlan <jferlan@redhat.com>
2016-07-19 07:51:10 -04:00
Daniel P. Berrange
740e4d7052 virt-login-shell: convert to typesafe virConf accessors
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2016-07-12 09:58:21 +01:00
Peter Krempa
e29f557515 tools: virt-login-shell: Fix cut'n'paste mistake in error message
Whine about 'allowed_users' having wrong format rather than 'shell'
2016-06-20 16:51:10 +02:00
Peter Krempa
94f93d7071 tools: virt-login-shell: Fix group list bounds checking
The list certainly isn't zero terminated and it would disallow usage of
group 'root'. Pass in the array size and match against it.
2016-06-20 16:48:55 +02:00
Daniel P. Berrange
f9d4280145 virt-login-shell: add ability to join the container cgroups
Prior to joining the namespaces of the container, move the
process into the containers' cgroups, so that the shell that
is subsequently launched is under the container resource
constraints.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2016-06-10 11:03:02 +01:00
Daniel P. Berrange
18a10ddc16 virt-login-shell: add ability to auto-detect shell from container
Currently the shell must be looked up from the config setting in
/etc/libvirt/virt-login-shell.conf. This is inflexible if there
are containers where different users need different shells. Add
add a new 'auto-shell' config parameter which instructs us to
query the containers' /etc/passwd for the shell to be exec'd.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2016-06-10 11:03:02 +01:00
Daniel P. Berrange
ee877b8710 virt-login-shell: fully reset container environment
The virt-login-shell environment will be initialized with
an arbitrary number of environment variables determined
by the SSH daemon and PAM configuration. Most of these are
not relevant inside the container, and at best they are
noise and at worst they'll break apps. For example if
XDG_RUNTIME_DIR is leaked to the container, it'll break
any apps using it, since  the directory it points to is
only visible to the host OS filesystem, not the container
FS.

Use clearenv() to blank out everything and then set known
good values for PATH, SHELL, USER, LOGNAME HOME and TERM.
Everything else is left up to the login shell to initialize.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2016-06-10 11:03:02 +01:00
Daniel P. Berrange
1ebe6f2434 virt-login-shell: avoid loosing error during cleanup
The virDomainFree / virConnectClose methods will reset the
last error handle, so we must save the error during cleanup

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2016-06-10 11:03:02 +01:00
Daniel P. Berrange
730466081c virt-login-shell: allow shell to be a simple string argument
Currently the shell config file parameter must be a list
giving the shell path and args. Allow it to be a plain
string argument as well.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2016-06-10 11:03:01 +01:00
Daniel P. Berrange
8a95d3df48 virt-login-shell: change way we request a login shell
Currently we request a login shell by passing the -l argument
to the shell. This is either hardcoded, or required to be
specified by the user in the virt-login-shell.conf file.

The standard way for login programs to request a shell run
as a login shell is to modify the argv passed to execve()
so that argv[0] contains the relative shell filename
prefixed with a zero. eg instead of doing

  const char **shellargs = ["/bin/bash", "-l", NULL];
  execve(shellargs[0], shellargs, env);

We should be doing

  const char **shellargs = ["-bash", NULL];
  execve("/bin/bash", shellargs, env);

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2016-06-10 11:03:01 +01:00
Daniel P. Berrange
da7396605b virt-login-shell: honour the -c option to launch commands
The virt-login-shell program is supposed to look like a
regular shell to clients. Login services like sshd
expect the shell to accept a '-c cmdstring' argument to
specify a command to launch instead of presenting an
interactive prompt.

We can implement this by simply passing the '-c cmdstring'
data straight through to the real shell we use. This does
not open any security holes, since the command is not run
until we're inside the container namespaces. This allows
scp to work for users with virt-login-shell.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2016-06-10 11:03:01 +01:00
Cole Robinson
e7db227810 util: Add virGettextInitialize, convert the code
Take setlocale/gettext error handling pattern from tools/virsh-*
and use it for all standalone binaries via a new shared
virGettextInitialize routine. The virsh* pattern differed slightly
from other callers. All users now consistently:

* Ignore setlocale errors. virsh has done this forever, presumably for
  good reason. This has been partially responsible for some bug reports:

  https://bugzilla.redhat.com/show_bug.cgi?id=1312688
  https://bugzilla.redhat.com/show_bug.cgi?id=1026514
  https://bugzilla.redhat.com/show_bug.cgi?id=1016158

* Report the failed function name
* Report strerror
2016-04-14 13:22:40 -04:00
Michal Privoznik
f55d1316ad sysconf: Include unistd.h
The manpage for sysconf() suggest including unistd.h as the
function is declared there. Even though we are not hitting any
compile issues currently, let's include the correct header file
instead of relying on some hidden include chain.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2015-12-24 18:03:50 +01:00
John Ferlan
07334ccbac Resolve Coverity CHECKED_RETURN
Coverity complained that checking the return of virDomainCreate()
was not consistent amongst the callers - so added the return check
to the objecteventtest.c and adjust the virt-login-shell to compare
< 0 rather than just non zero for the failure condition.
2014-09-15 10:44:27 -04:00
Ján Tomko
5d8793eebb Indent top-level labels by one space in tools/ 2014-03-25 14:58:41 +01:00
Eric Blake
cb9bd7963b virt-login-shell: silence coverity warning
Coverity spotted that 'nfdlist' (ssize_t) could be -1, but that we
were using 'i' (size_t) to iterate over the list at cleanup, with
crashing results because it promotes to a really big unsigned number.

* tools/virt-login-shell.c (main): Avoid treating -1 as unsigned.

Signed-off-by: Eric Blake <eblake@redhat.com>
2014-03-05 11:59:06 -07:00
Eric Blake
730fc9622b virt-login-shell: saner exit value
virt-login-shell was exiting with status 0, regardless of what the
wrapped shell returned.  This is unkind to users; we should behave
more like env(1), nice(1), su(1), and other wrapper programs, by
preserving the invoked application's status (which includes the
distinction between death due to signal vs. normal death).

* tools/virt-login-shell.c (main): Pass through child exit status.
* tools/virt-login-shell.pod: Document exit status.

Signed-off-by: Eric Blake <eblake@redhat.com>
2014-03-03 12:40:32 -07:00
Eric Blake
4594a33b4b virt-login-shell: use single instead of double fork
Note that 'virsh lxc-enter-namespace' must double-fork, for two
reasons: some namespaces can only be done from a single thread,
while virsh is multithreaded; and because virsh can be run in
batch mode where we must not corrupt the namespace of that
execution upon return from the subsidiary command.

When virt-login-shell was first written, it blindly copied from
'virsh lxc-enter-namespace', including the double-fork.  But
neither of the reasons for double forking apply to
virt-login-shell (we are single-threaded, and we have nothing to
do after the child completes that would require us to preserve a
namespace), so we can simplify life by using a single fork.
In turn, this will make it easier for a future patch to pass the
child's exit status on to the invoking shell.

In flattening to a single fork, note that closing the fds must
be done after fork, because the parent process still needs to
use fds to control the virConnectPtr; meanwhile, chdir can be
done prior to forking (in fact, it's easier to report errors
on anything attempted before forking).

* tools/virt-login-shell.c (main): Single rather than double fork.
(virLoginShellFini): Delete, by inlining actions instead.

Signed-off-by: Eric Blake <eblake@redhat.com>
2014-03-03 12:40:32 -07:00
Eric Blake
25f87817ab virFork: simplify semantics
The old semantics of virFork() violates the priciple of good
usability: it requires the caller to check the pid argument
after use, *even when virFork returned -1*, in order to properly
abort a child process that failed setup done immediately after
fork() - that is, the caller must call _exit() in the child.
While uses in virfile.c did this correctly, uses in 'virsh
lxc-enter-namespace' and 'virt-login-shell' would happily return
from the calling function in both the child and the parent,
leading to very confusing results. [Thankfully, I found the
problem by inspection, and can't actually trigger the double
return on error without an LD_PRELOAD library.]

It is much better if the semantics of virFork are impossible
to abuse.  Looking at virFork(), the parent could only ever
return -1 with a non-negative pid if it misused pthread_sigmask,
but this never happens.  Up until this patch series, the child
could return -1 with non-negative pid if it fails to set up
signals correctly, but we recently fixed that to make the child
call _exit() at that point instead of forcing the caller to do
it.  Thus, the return value and contents of the pid argument are
now redundant (a -1 return now happens only for failure to fork,
a child 0 return only happens for a successful 0 pid, and a
parent 0 return only happens for a successful non-zero pid),
so we might as well return the pid directly rather than an
integer of whether it succeeded or failed; this is also good
from the interface design perspective as users are already
familiar with fork() semantics.

One last change in this patch: before returning the pid directly,
I found cases where using virProcessWait unconditionally on a
cleanup path of a virFork's -1 pid return would be nicer if there
were a way to avoid it overwriting an earlier message.  While
such paths are a bit harder to come by with my change to a direct
pid return, I decided to keep the virProcessWait change in this
patch.

* src/util/vircommand.h (virFork): Change signature.
* src/util/vircommand.c (virFork): Guarantee that child will only
return on success, to simplify callers.  Return pid rather than
status, now that the situations are always the same.
(virExec): Adjust caller, also avoid open-coding process death.
* src/util/virprocess.c (virProcessWait): Tweak semantics when pid
is -1.
(virProcessRunInMountNamespace): Adjust caller.
* src/util/virfile.c (virFileAccessibleAs, virFileOpenForked)
(virDirCreate): Likewise.
* tools/virt-login-shell.c (main): Likewise.
* tools/virsh-domain.c (cmdLxcEnterNamespace): Likewise.
* tests/commandtest.c (test23): Likewise.

Signed-off-by: Eric Blake <eblake@redhat.com>
2014-03-03 12:40:32 -07:00
Eric Blake
c72e76c3d9 util: make it easier to grab only regular process exit
Right now, a caller waiting for a child process either requires
the child to have status 0, or must use WIFEXITED() and friends
itself.  But in many cases, we want the middle ground of treating
fatal signals as an error, and directly accessing the normal exit
value without having to use WEXITSTATUS(), in order to easily
detect an expected non-zero exit status.  This adds the middle
ground to the low-level virProcessWait; the next patch will add
it to virCommand.

* src/util/virprocess.h (virProcessWait): Alter signature.
* src/util/virprocess.c (virProcessWait): Add parameter.
(virProcessRunInMountNamespace): Adjust caller.
* src/util/vircommand.c (virCommandWait): Likewise.
* src/util/virfile.c (virFileAccessibleAs): Likewise.
* src/lxc/lxc_container.c (lxcContainerHasReboot)
(lxcContainerAvailable): Likewise.
* daemon/libvirtd.c (daemonForkIntoBackground): Likewise.
* tools/virt-login-shell.c (main): Likewise.
* tools/virsh-domain.c (cmdLxcEnterNamespace): Likewise.
* tests/testutils.c (virtTestCaptureProgramOutput): Likewise.
* tests/commandtest.c (test23): Likewise.

Signed-off-by: Eric Blake <eblake@redhat.com>
2014-03-03 12:40:31 -07:00
Thorsten Behrens
721949059b maint: align whitespaces with project conventions. 2014-01-20 14:35:08 +01:00
Eric Blake
3d007cb5f8 virt-login-shell: fix regressions in behavior
Our fixes for CVE-2013-4400 were so effective at "fixing" bugs
in virt-login-shell that we ended up fixing it into a useless
do-nothing program.

Commit 3e2f27e1 picked the name LIBVIRT_SETUID_RPC_CLIENT for
the witness macro when we are doing secure compilation.  But
commit 9cd6a57d checked whether the name IN_VIRT_LOGIN_SHELL,
from an earlier version of the patch series, was defined; with
the net result that virt-login-shell invariably detected that
it was setuid and failed virInitialize.

Commit b7fcc799 closed all fds larger than stderr, but in the
wrong place.  Looking at the larger context, we mistakenly did
the close in between obtaining the set of namespace fds, then
actually using those fds to switch namespace, which means that
virt-login-shell will ALWAYS fail.

This is the minimal patch to fix the regressions, although
further patches are also worth having to clean up poor
semantics of the resulting program (for example, it is rude to
not pass on the exit status of the wrapped program back to the
invoking shell).

* tools/virt-login-shell.c (main): Don't close fds until after
namespace swap.
* src/libvirt.c (virGlobalInit): Use correct macro.

Signed-off-by: Eric Blake <eblake@redhat.com>
2014-01-09 15:05:04 -07:00
Eric Blake
7cc3a7189c virt-login-shell: clean up usage
I noticed a few odd things in 'virt-login-shell --help' output.

* tools/virt-login-shell.c (usage): At most one option accepted,
drop trailing colon.

Signed-off-by: Eric Blake <eblake@redhat.com>
2013-12-23 21:53:48 -07:00
Daniel P. Berrange
d665003da1 Set a sane $PATH for virt-login-shell
The virt-login-shell binary shouldn't need to execute programs
relying on $PATH, but just in case set a fixed $PATH value
of /bin:/usr/bin

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2013-10-21 14:03:52 +01:00
Daniel P. Berrange
b7fcc799ad Close all non-stdio FDs in virt-login-shell (CVE-2013-4400)
We don't want to inherit any FDs in the new namespace
except for the stdio FDs. Explicitly close them all,
just in case some do not have the close-on-exec flag
set.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2013-10-21 14:03:52 +01:00
Ruben Kerkhof
11cdc424d3 virt-login-shell: improve error message grammar
and wrap some long lines

Signed-off-by: Eric Blake <eblake@redhat.com>
2013-08-13 17:28:06 -06:00
Daniel P. Berrange
a396473494 Address missed feedback from review of virt-login-shell
Address a number of code, style and docs issues identified
in review of virt-login-shell after it was merged.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2013-08-13 14:06:01 +01:00
Daniel P. Berrange
ac692e3af2 Fix double-free and broken logic in virt-login-shell
The virLoginShellAllowedUser method must not free the 'groups'
parameter it is given, as that is owned by the caller.

The virLoginShellAllowedUser method should be checking
'!*ptr' (ie empty string) rather than '!ptr' (NULL string)
since the latter cannot be true.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2013-08-09 12:55:59 +01:00
Jim Fehlig
26b8a4dd23 build: fix compilation of virt-login-shell.c
virt-login-shell.c was failing to compile with

CC       virt_login_shell-virt-login-shell.o
virt-login-shell.c: In function 'main':
virt-login-shell.c:205:5: error: implicit declaration of function 'setlocale' [-Werror=implicit-function-declaration]
virt-login-shell.c:205:5: error: nested extern declaration of 'setlocale' [-Werror=nested-externs]
virt-login-shell.c:205:20: error: 'LC_ALL' undeclared (first use in this function)
2013-08-08 13:53:25 -06:00
Dan Walsh
54d69f540c Introduce a virt-login-shell binary
Add a virt-login-shell binary that can be set as a user's
shell, such that when they login, it causes them to enter
the LXC container with a name matching their user name.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2013-08-08 16:36:31 +01:00