[libvirt] [PATCH 0/2] Fix permissions checks for virDomainGetTime/Hostname
These two APIs mistakenly allowed access to read-only users. This was was reported publically in a bug tracker, so I'm sending these patches without applying any embargo time. Daniel P. Berrangé (2): api: disallow virDomainGetHostname for read-only connections remote: enforce ACL write permission for getting guest time & hostname src/libvirt-domain.c | 2 ++ src/remote/remote_protocol.x | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) -- 2.20.1
The virDomainGetHostname API is fetching guest information and this may involve use of an untrusted guest agent. As such its use must be forbidden on a read-only connection to libvirt. Fixes CVE-2019-3886 Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/libvirt-domain.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index be5b1f6740..baf21824fe 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -11031,6 +11031,8 @@ virDomainGetHostname(virDomainPtr domain, unsigned int flags) virCheckDomainReturn(domain, NULL); conn = domain->conn; + virCheckReadOnlyGoto(domain->conn->flags, error); + if (conn->driver->domainGetHostname) { char *ret; ret = conn->driver->domainGetHostname(domain, flags); -- 2.20.1
On 4/3/19 8:00 AM, Daniel P. Berrangé wrote:
The virDomainGetHostname API is fetching guest information and this may involve use of an untrusted guest agent. As such its use must be forbidden on a read-only connection to libvirt.
Fixes CVE-2019-3886 Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/libvirt-domain.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index be5b1f6740..baf21824fe 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -11031,6 +11031,8 @@ virDomainGetHostname(virDomainPtr domain, unsigned int flags) virCheckDomainReturn(domain, NULL); conn = domain->conn;
+ virCheckReadOnlyGoto(domain->conn->flags, error); + if (conn->driver->domainGetHostname) { char *ret; ret = conn->driver->domainGetHostname(domain, flags);
Heh, I thought this and 2/2 were pushed already :-). Reviewed-by: Jim Fehlig <jfehlig@suse.com> Regards, Jim
Getting the guest time and hostname both require use of guest agent commands. These must not be allowed for read-only users, so the permissions check must validate "write" permission not "read". Fixes CVE-2019-3886 Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/remote/remote_protocol.x | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 74be4b37d0..11f44ee267 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -5513,7 +5513,7 @@ enum remote_procedure { /** * @generate: both - * @acl: domain:read + * @acl: domain:write */ REMOTE_PROC_DOMAIN_GET_HOSTNAME = 277, @@ -5908,7 +5908,7 @@ enum remote_procedure { /** * @generate: none - * @acl: domain:read + * @acl: domain:write */ REMOTE_PROC_DOMAIN_GET_TIME = 337, -- 2.20.1
On 4/3/19 8:00 AM, Daniel P. Berrangé wrote:
Getting the guest time and hostname both require use of guest agent commands. These must not be allowed for read-only users, so the permissions check must validate "write" permission not "read".
Fixes CVE-2019-3886 Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/remote/remote_protocol.x | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 74be4b37d0..11f44ee267 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -5513,7 +5513,7 @@ enum remote_procedure {
/** * @generate: both - * @acl: domain:read + * @acl: domain:write */ REMOTE_PROC_DOMAIN_GET_HOSTNAME = 277,
@@ -5908,7 +5908,7 @@ enum remote_procedure {
/** * @generate: none - * @acl: domain:read + * @acl: domain:write */ REMOTE_PROC_DOMAIN_GET_TIME = 337,
Reviewed-by: Jim Fehlig <jfehlig@suse.com> Regards, Jim
participants (2)
-
Daniel P. Berrangé -
Jim Fehlig