[libvirt PATCH 0/9] make internal only secrets work with split daemons

[Daniel P. Berrangé]

I'm not going to apply this yet as I realized that I have broken
internal secrets with monolithic libvirtd instead :-(

On Tue, May 04, 2021 at 06:43:41PM +0100, Daniel P. Berrangé wrote:
> If you define a secret with private="yes", then libvirt won't let any
> client query the secret value after it is set. Only other libvirt
> drivers inside the daemon can query it by passing a special internal
> only flag to the virSecretGetValue API. The remote driver/daemon
> refuses to let this internal flag go over the wire preventing normal
> clients from using it
> 
> This doesn't work with the split daemons because the virSecretGetValue
> API done by virqemud / virtstoraged has to go over the wire to reach
> the virsecretd.
> 
> We need to come up with an alternative way to "prove" that the caller
> of virSecretGetValue is a libvirt daemon, as opposed to a general
> libvirt client.
> 
> Note with if only traditional POSIX DAC permissions are in effect
> then we could consider it pointless trying to restrict access to
> clients running the same user/group as the libvirt daemon. We ought
> to take into account that the client might be confined by SELinux
> though, so the "private secret" concept isn't entirely pointless.
> 
> Thus doing a simple uid of client == uid of daemon check is a bit
> too weak. The UID check might also not fly if the modular daemons
> are run inside containers with user namespaces, as the container
> for virtsecretd and virtqemud might have different user mappings
> in theory.
> 
> This series adds a concept of a "token" which is known only to the
> libvirt daemons. The first daemon to use it writes a random hex
> string to /var/run/libvirt/common/system.token. Other daemons can
> read and compare this. Unless a MAC system is present this is still
> largely security theatre, but that's not really worse than the
> historical behaviour.
> 
> When an API call is made the virIdentity by default reflects the
> identity of the UNIX process that initiated it.
> 
> When connecting to virtproxyd, the client apps' identity is forwarded
> to the next virtNNNNd daemon.
> 
> When libvirt drivers, however, initiate an API call we never set any
> identity. With monolithic libvirtd, they'd inherit the current client
> identity automagically since it was all in the same thread local. With
> modular daemons the othe driver would see the identity of the other
> libvirt daemon which is bad as this gives elevated privileges in the
> ACL check.
> 
> Thus we fix the code which drivers use to open a connection to other
> daemons, such that it applies the current caller's identity. It does
> this using an "elevated" identity though, which means, we have added
> in the system token.  Thus the virtsecretd daemon getting the call
> virSecretGetValue sees the virIdentity reflecting the client
> application which originally called the virDomainCreate() API, but
> with the system token set. Thus virsecretd can see that the
> virSecretGetValue was invoked by another daemon, not a libvirt
> client app.
> 
> Daniel P. Berrangé (9):
>   util: add virRandomToken API
>   util: introduce concept of a system token into identities
>   util: generate a persistent system token
>   src: set system token for system identity
>   util: add API for copying identity objects
>   util: add method for getting the current identity with system token
>   src: add API to determine if current identity is a system identity
>   secret: rework handling of private secrets
>   src: set identity when opening secondary drivers
> 
>  src/driver-secret.h        |   9 +-
>  src/driver.c               |  27 +++++
>  src/libvirt-secret.c       |   2 +-
>  src/libvirt_private.syms   |   5 +
>  src/remote/remote_driver.c |   8 +-
>  src/secret/secret_driver.c |  34 ++++--
>  src/util/viridentity.c     | 230 +++++++++++++++++++++++++++++++++++++
>  src/util/viridentity.h     |   7 ++
>  src/util/virrandom.c       |  18 +++
>  src/util/virrandom.h       |   1 +
>  src/util/virsecret.c       |   3 +-
>  tests/qemuxml2argvtest.c   |   3 +-
>  12 files changed, 320 insertions(+), 27 deletions(-)
> 
> -- 
> 2.31.1
> 
> 

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|