On 5/4/21 7:43 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.
>
> 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(-)
>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Michal