[libvirt PATCH v3 00/10] make internal only secrets work with split daemons

[Michal Prívozník]

On 5/12/21 3:33 PM, 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.
> 
> Changed in v3...
> 
> Properly mock the new APIs in test suite
> 
> Changed in v2...
> 
> We can't set the elevated identity only when opening the virConnect
> for the secret driver. This works for modular daemons, as the identity
> is passed to the virsecretd at time of opening and thus applies to
> the later virSecretGetValue call on that connection.
> 
> For monolithic daemon, the identity present at virConnectOpen is
> irrelevant. The virSecretGetValue call will just directly query
> the current thread's identity.
> 
> IOW, to work in both deployment scenarios we need to have the
> elevated identity set across both virConnectOpen and virSecretGetValue
> 
> Daniel P. Berrangé (10):
>   util: add virRandomToken API
>   util: introduce concept of a system token into identities
>   util: generate a persistent system token
>   util: set system token for system identity
>   util: add API for copying identity objects
>   util: helper to temporary elevate privileges of the current identity
>   src: add API to determine if current identity is a system identity
>   src: set identity when opening secondary drivers
>   src: elevate current identity privilege when fetching secret
>   secret: rework handling of private secrets
> 
>  src/driver-secret.h                        |   9 +-
>  src/driver.c                               |  27 +++
>  src/libvirt-secret.c                       |   2 +-
>  src/libvirt_private.syms                   |   8 +
>  src/libxl/libxl_conf.c                     |   5 +
>  src/qemu/qemu_domain.c                     |  11 +-
>  src/qemu/qemu_tpm.c                        |   5 +
>  src/remote/remote_driver.c                 |   8 +-
>  src/secret/secret_driver.c                 |  34 ++-
>  src/storage/storage_backend_iscsi.c        |   5 +
>  src/storage/storage_backend_iscsi_direct.c |   5 +
>  src/storage/storage_backend_rbd.c          |   5 +
>  src/storage/storage_util.c                 |   5 +
>  src/util/viridentity.c                     | 244 ++++++++++++++++++++-
>  src/util/viridentity.h                     |  11 +
>  src/util/viridentitypriv.h                 |  30 +++
>  src/util/virrandom.c                       |  18 ++
>  src/util/virrandom.h                       |   1 +
>  src/util/virsecret.c                       |   3 +-
>  tests/qemuxml2argvmock.c                   |   9 +
>  tests/qemuxml2argvtest.c                   |   9 +-
>  tests/viridentitytest.c                    |  11 +-
>  22 files changed, 435 insertions(+), 30 deletions(-)
>  create mode 100644 src/util/viridentitypriv.h
> 

Reviewed-by: Michal Privoznik <mprivozn at redhat.com>

Michal