In the unlikely case that we were unable to set the new
identity, we would unref the old one even though it still
could be in the thread-local storage.
Fixes: c6825d8813
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This appears to be a copy-paste mistake from the check directly above.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The basic use case of VIR_IDENTITY_AUTORESTORE() is in
conjunction with virIdentityElevateCurrent(). What happens is
that virIdentityElevateCurrent() gets current identity (which
increases the refcounter of thread local virIdentity object) and
returns a pointer to it. Later, when the variable goes out of
scope the virIdentityRestoreHelper() is called which calls
virIdentitySetCurrent() over the old identity. But this means
that the refcounter is increased again.
Therefore, we have to explicitly decrease the refcounter by
calling g_object_unref().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
This is essentially a way to determine if the current identity
is that of another libvirt daemon.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
When talking to the secret driver, the callers inside libvirt daemons
need to be able to run with an elevated privileges that prove the API
calls are made by a libvirt daemon, not an end user application.
The virIdentityElevateCurrent method will take the current identity
and, if not already present, add the system token. The old current
identity is returned to the caller. With the VIR_IDENTITY_AUTORESTORE
annotation, the old current identity will be restored upon leaving
the codeblock scope.
... early work with regular privileges ...
if (something needing elevated privs) {
VIR_IDENTITY_AUTORESTORE virIdentity *oldident =
virIdentityElevateCurrent();
if (!oldident)
return -1;
... do something with elevated privileges ...
}
... later work with regular privileges ...
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
When creating the system identity set the system token. The system
token is currently stored in a local path
/var/run/libvirt/common/system.token
Obviously with only traditional UNIX DAC in effect, this is largely
security through obscurity, if the client is running at the same
privilege level as the daemon. It does, however, reliably distinguish
an unprivileged client from the system daemons.
With a MAC system like SELinux though, or possible use of containers,
access can be further restricted.
A possible future improvement for Linux would be to populate the
kernel keyring with a secret for libvirt daemons to share.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
We want a way to distinguish between calls from a libvirt daemon, and a
regular client application when both are running as the same user
account. This is not possible with the current set of attributes
recorded against an identity, as there is nothing that is common to all
of the modular libvirt daemons, while distinct to all other processes.
We thus introduce the idea of a system token, which is simply a random
hex string that is only known by the libvirt daemons, to be recorded
against the system identity.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Historically, we declared pointer type to our types:
typedef struct _virXXX virXXX;
typedef virXXX *virXXXPtr;
But usefulness of such declaration is questionable, at best.
Unfortunately, we can't drop every such declaration - we have to
carry some over, because they are part of public API (e.g.
virDomainPtr). But for internal types - we can do drop them and
use what every other C project uses 'virXXX *'.
This change was generated by a very ugly shell script that
generated sed script which was then called over each file in the
repository. For the shell script refer to the cover letter:
https://listman.redhat.com/archives/libvir-list/2021-March/msg00537.html
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Historically, we've used security_context_t for variables passed
to libselinux APIs. But almost 7 years ago, libselinux developers
admitted in their API that in fact, it's just a 'char *' type
[1]. Ever since then the APIs accept 'char *' instead, but they
kept the old alias just for API stability. Well, not anymore [2].
1: 9eb9c93275
2: 7a124ca275
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Converting from virObject to GObject is reasonably straightforward,
as illustrated by this patch for virIdentity
In the header file
- Remove
typedef struct _virIdentity virIdentity
- Add
#define VIR_TYPE_IDENTITY virIdentity_get_type ()
G_DECLARE_FINAL_TYPE (virIdentity, vir_identity, VIR, IDENTITY, GObject);
Which provides the typedef we just removed, and class
declaration boilerplate and various other constants/macros.
In the source file
- Change 'virObject parent' to 'GObject parent' in the struct
- Remove the virClass variable and its initializing call
- Add
G_DEFINE_TYPE(virIdentity, vir_identity, G_TYPE_OBJECT)
which declares the instance & class constructor functions
- Add an impl of the instance & class constructors
wiring up the finalize method to point to our dispose impl
In all files
- Replace VIR_AUTOUNREF(virIdentityPtr) with g_autoptr(virIdentity)
- Replace virObjectRef/Unref with g_object_ref/unref. Note
the latter functions do *NOT* accept a NULL object where as
libvirt's do. If you replace g_object_unref with g_clear_object
it is NULL safe, but also clears the pointer.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
To simplify the later conversion from virObject to GObject, introduce
the use of g_autoptr to the virIdentity implementnation and test suite.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Add ability to import/export all the parameters associated with an
identity, so that they can be exposed via the public API.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
We'll shortly be exposing the identity as virTypedParameter in the
public header, so it simplifies life to use that as the internal
representation too.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The virIdentity getters are unusual in that they return -1 to indicate
"not found" and don't report any error. Change them to return -1 for
real errors, 0 for not found, and 1 for success.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
It is simpler to remove this unused method than to rewrite it using
typed parameters in the next patch.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Only expose the type safe getters/setters to other code in preparation
for changing the internal storage of data.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Remove the "UNIX" tag from the names for user name, group name,
process ID and process time, since these attributes are all usable
for non-UNIX platforms like Windows.
User ID and group ID are left with a "UNIX" tag, since there's no
equivalent on Windows. The closest equivalent concept on Windows,
SID, is a struct containing a number of integer fields, which is
commonly represented in string format instead. This would require
a separate attribute, and is left for a future exercise, since
the daemons are not currently built on Windows anyway.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Missing semicolon at the end of macros can confuse some analyzers
(like cppcheck <filename>). VIR_ONCE_GLOBAL_INIT is almost
exclusively called without an ending semicolon, but let's
standardize on using one like the other macros.
Add a dummy struct definition at the end of the macro, so
the compiler will require callers to add a semicolon.
Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
By making use of GNU C's cleanup attribute handled by the
VIR_AUTOFREE macro for declaring scalar variables, majority
of the VIR_FREE calls can be dropped, which in turn leads to
getting rid of most of our cleanup sections.
Signed-off-by: Sukrit Bhatnagar <skrtbhtngr@gmail.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
So far we are repeating the following lines over and over:
if (!(virSomeObjectClass = virClassNew(virClassForObject(),
"virSomeObject",
sizeof(virSomeObject),
virSomeObjectDispose)))
return -1;
While this works, it is impossible to do some checking. Firstly,
the class name (the 2nd argument) doesn't match the name in the
code in all cases (the 3rd argument). Secondly, the current style
is needlessly verbose. This commit turns example into following:
if (!(VIR_CLASS_NEW(virSomeObject,
virClassForObject)))
return -1;
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
This initially started as a fix of some debug printing in
virCgroupDetect. However it turned out that other places suffer
from the similar problem. While dealing with pids, esp. in cases
where we cannot use pid_t for ABI stability reasons, we often
chose an unsigned integer type. This makes no sense as pid_t is
signed.
Also, new syntax-check rule is introduced so we won't repeat this
mistake.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Don't unref the old identity unless we set the new one correctly and
unref the new one on failure to set it so that we don't leak any
references or use invalid pointers.
Update virNetServerClientCreateIdentity and virIdentityGetSystem
to use the new typesafe APIs for setting identity attributes
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Per the documentation, is_selinux_enabled() returns -1 on error.
Account for this. Previously when -1 was being returned the condition
would still be true. I was noticing this because on my system that has
selinux disabled I was getting this in the libvirt.log every 5
seconds:
error : virIdentityGetSystem:173 : Unable to lookup SELinux process context: Invalid argument
With this patch applied, I no longer get these messages every 5
seconds. I am submitting this in case its deemed useful for inclusion.
Anyone have any comments on this change? This is a patch off current
master.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Any source file which calls the logging APIs now needs
to have a VIR_LOG_INIT("source.name") declaration at
the start of the file. This provides a static variable
of the virLogSource type.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
If SELinux is compiled into libvirt but it is disabled on the host,
libvirtd logs:
error : virIdentityGetSystem:173 : Unable to lookup SELinux process
context: Invalid argument
on each and every client connection.
Use is_selinux_enabled() to skip retrieval of the process's SELinux
context if SELinux is disabled.
Signed-off-by: Michael Chapman <mike@very.puzzling.org>
Most of the usage of getuid()/getgid() is in cases where we are
considering what privileges we have. As such the code should be
using the effective IDs, not real IDs.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The polkit access driver will want to use the process start
time field. This was already set for network identities, but
not for the system identity.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Future improvements to the polkit code will require access to
the numeric user ID, not merely user name.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
There are various methods named "virXXXXSecurityContext",
which are specific to SELinux. Rename them all to
"virXXXXSELinuxContext". They will still raise errors at
runtime if SELinux is not compiled in
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Some code mistakenly called virIdentityOnceInit directly
instead of virIdentityInitialize(). This meant that one-time
initializer was run many times with predictably bad results.
The virNetSocket & virIdentity classes accidentally got some
conditionals using HAVE_SELINUX instead of WITH_SELINUX.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
If no user identity is available, some operations may wish to
use the system identity. ie the identity of the current process
itself. Add an API to get such an identity.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
To allow any internal API to get the current identity, add APIs
to associate a virIdentityPtr with the current thread, via a
thread local
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Introduce a local object virIdentity for managing security
attributes used to form a client application's identity.
Instances of this object are intended to be used as if they
were immutable, once created & populated with attributes
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
