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@xxxxxxxxxx> Michal
