Re: [libvirt] [PATCH] virlog: determine the hostname on startup CVE-2018-XXX

2018-02-12 Thread Daniel P . Berrangé
On Sat, Feb 10, 2018 at 07:06:22AM +0100, Michal Privoznik wrote:
> On 02/07/2018 02:13 PM, Daniel P. Berrangé wrote:
> > On Wed, Feb 07, 2018 at 09:58:21AM +0530, P J P wrote:
> >> +-- On Mon, 5 Feb 2018, Daniel P. Berrangé wrote --+
> >> | From: Lubomir Rintel 
> >> | 
> >> | At later point it might not be possible or even safe to use 
> >> getaddrinfo(). It
> >> | can in turn result in a load of NSS module.
> >> | 
> >> | Notably, on a LXC container startup we may find ourselves with the guest
> >> | filesystem already having replaced the host one. Loading a NSS module
> >> | from the guest tree could allow a malicous guest to escape the
> >> | confinement of its container environment because libvirt will not yet
> >> | have locked it down.
> >> | ---
> >> | 
> >> | NB, we're still awaiting CVE allocation before pushing to git
> >>
> >> 'CVE-2018-6764' has been assigned to this issue by Mitre.
> > 
> > Thanks, I have pushed this patch now
> > 
> 
> Actually, I think we need to revisit this patch. It makes
> gethostbyname() deadlock if libvirt NSS plugin is enabled. Here's the
> stack trace of what's going on (I've ran `getent hosts libvirt.org`):
> 
> #0  0x7f6e714b57e7 in futex_wait (private=0, expected=1, 
> futex_word=0x7f6e71f2fb80 ) at 
> ../sysdeps/unix/sysv/linux/futex-internal.h:61
> #1  futex_wait_simple (private=0, expected=1, futex_word=0x7f6e71f2fb80 
> ) at ../sysdeps/nptl/futex-internal.h:135
> #2  __pthread_once_slow (once_control=0x7f6e71f2fb80 , 
> init_routine=0x7f6e71d09949 ) at pthread_once.c:105
> #3  0x7f6e71d16e7d in virOnce (once=0x7f6e71f2fb80 , 
> init=0x7f6e71d09949 ) at util/virthread.c:48
> #4  0x7f6e71d0997c in virLogInitialize () at util/virlog.c:285
> #5  0x7f6e71d0a09a in virLogVMessage (source=0x7f6e71f2f130 , 
> priority=VIR_LOG_INFO, filename=0x7f6e71d2184e "util/virobject.c", 
> linenr=254, funcname=0x7f6e71d21a58 <__func__.7842> "virObjectNew", 
> metadata=0x0,
> fmt=0x7f6e71d218d8 "OBJECT_NEW: obj=%p classname=%s", 
> vargs=0x7ffe45fab6e0) at util/virlog.c:582
> #6  0x7f6e71d09ffd in virLogMessage (source=0x7f6e71f2f130 , 
> priority=VIR_LOG_INFO, filename=0x7f6e71d2184e "util/virobject.c", 
> linenr=254, funcname=0x7f6e71d21a58 <__func__.7842> "virObjectNew", 
> metadata=0x0,
> fmt=0x7f6e71d218d8 "OBJECT_NEW: obj=%p classname=%s") at util/virlog.c:542
> #7  0x7f6e71d0db22 in virObjectNew (klass=0x56348c51c500) at 
> util/virobject.c:254
> #8  0x7f6e71d0dbf1 in virObjectLockableNew (klass=0x56348c51c500) at 
> util/virobject.c:272
> #9  0x7f6e71d0d3e5 in virMacMapNew (file=0x56348c520e70 
> "/var/lib/libvirt/dnsmasq//virbr0.macs") at util/virmacmap.c:319
> #10 0x7f6e71cdc50a in findLease (name=0x7ffe45fac1d0 "burns", af=0, 
> address=0x7ffe45fab980, naddress=0x7ffe45fab988, found=0x7ffe45fab973, 
> errnop=0x7ffe45fabb88) at nss/libvirt_nss.c:301
> #11 0x7f6e71cdcc56 in _nss_libvirt_gethostbyname4_r (name=0x7ffe45fac1d0 
> "burns", pat=0x7ffe45fabb98, buffer=0x7ffe45fabd10 "& ", buflen=1024, 
> errnop=0x7ffe45fabb88, herrnop=0x7ffe45fabbb0, ttlp=0x0) at 
> nss/libvirt_nss.c:522
> #12 0x7f6e724631fc in gaih_inet (name=name@entry=0x7ffe45fac1d0 "burns", 
> service=, req=req@entry=0x7ffe45fac1a0, 
> pai=pai@entry=0x7ffe45fabc98, naddrs=naddrs@entry=0x7ffe45fabc94, 
> tmpbuf=tmpbuf@entry=0x7ffe45fabd00)
> at ../sysdeps/posix/getaddrinfo.c:825
> #13 0x7f6e72464697 in __GI_getaddrinfo (name=, 
> service=, hints=0x7ffe45fac1a0, pai=0x7ffe45fac198) at 
> ../sysdeps/posix/getaddrinfo.c:2370
> #14 0x7f6e71d19e81 in virGetHostnameImpl (quiet=true) at 
> util/virutil.c:717
> #15 0x7f6e71d1a057 in virGetHostnameQuiet () at util/virutil.c:759
> #16 0x7f6e71d09936 in virLogOnceInit () at util/virlog.c:279
> #17 0x7f6e71d09952 in virLogOnce () at util/virlog.c:285
> #18 0x7f6e714b5829 in __pthread_once_slow (once_control=0x7f6e71f2fb80 
> , init_routine=0x7f6e71d09949 ) at 
> pthread_once.c:116
> #19 0x7f6e71d16e7d in virOnce (once=0x7f6e71f2fb80 , 
> init=0x7f6e71d09949 ) at util/virthread.c:48
> #20 0x7f6e71d0997c in virLogInitialize () at util/virlog.c:285
> #21 0x7f6e71d0a09a in virLogVMessage (source=0x7f6e71f2f130 , 
> priority=VIR_LOG_INFO, filename=0x7f6e71d2184e "util/virobject.c", 
> linenr=254, funcname=0x7f6e71d21a58 <__func__.7842> "virObjectNew", 
> metadata=0x0,
> fmt=0x7f6e71d218d8 "OBJECT_NEW: obj=%p classname=%s", 
> vargs=0x7ffe45fac410) at util/virlog.c:582
> #22 0x7f6e71d09ffd in virLogMessage (source=0x7f6e71f2f130 , 
> priority=VIR_LOG_INFO, filename=0x7f6e71d2184e "util/virobject.c", 
> linenr=254, funcname=0x7f6e71d21a58 <__func__.7842> "virObjectNew", 
> metadata=0x0,
> fmt=0x7f6e71d218d8 "OBJECT_NEW: obj=%p classname=%s") at util/virlog.c:542
> #23 0x7f6e71d0db22 in virObjectNew (klass=0x56348c51c500) at 
> util/virobject.c:254
> #24 0x7f6e71d0dbf1 in virObjectLockableNew (klass=0x56348c51c500) at 
> util/virobject.c:272
> #25 

Re: [libvirt] [PATCH] virlog: determine the hostname on startup CVE-2018-XXX

2018-02-09 Thread Michal Privoznik
On 02/07/2018 02:13 PM, Daniel P. Berrangé wrote:
> On Wed, Feb 07, 2018 at 09:58:21AM +0530, P J P wrote:
>> +-- On Mon, 5 Feb 2018, Daniel P. Berrangé wrote --+
>> | From: Lubomir Rintel 
>> | 
>> | At later point it might not be possible or even safe to use getaddrinfo(). 
>> It
>> | can in turn result in a load of NSS module.
>> | 
>> | Notably, on a LXC container startup we may find ourselves with the guest
>> | filesystem already having replaced the host one. Loading a NSS module
>> | from the guest tree could allow a malicous guest to escape the
>> | confinement of its container environment because libvirt will not yet
>> | have locked it down.
>> | ---
>> | 
>> | NB, we're still awaiting CVE allocation before pushing to git
>>
>> 'CVE-2018-6764' has been assigned to this issue by Mitre.
> 
> Thanks, I have pushed this patch now
> 

Actually, I think we need to revisit this patch. It makes
gethostbyname() deadlock if libvirt NSS plugin is enabled. Here's the
stack trace of what's going on (I've ran `getent hosts libvirt.org`):

#0  0x7f6e714b57e7 in futex_wait (private=0, expected=1, 
futex_word=0x7f6e71f2fb80 ) at 
../sysdeps/unix/sysv/linux/futex-internal.h:61
#1  futex_wait_simple (private=0, expected=1, futex_word=0x7f6e71f2fb80 
) at ../sysdeps/nptl/futex-internal.h:135
#2  __pthread_once_slow (once_control=0x7f6e71f2fb80 , 
init_routine=0x7f6e71d09949 ) at pthread_once.c:105
#3  0x7f6e71d16e7d in virOnce (once=0x7f6e71f2fb80 , 
init=0x7f6e71d09949 ) at util/virthread.c:48
#4  0x7f6e71d0997c in virLogInitialize () at util/virlog.c:285
#5  0x7f6e71d0a09a in virLogVMessage (source=0x7f6e71f2f130 , 
priority=VIR_LOG_INFO, filename=0x7f6e71d2184e "util/virobject.c", linenr=254, 
funcname=0x7f6e71d21a58 <__func__.7842> "virObjectNew", metadata=0x0,
fmt=0x7f6e71d218d8 "OBJECT_NEW: obj=%p classname=%s", vargs=0x7ffe45fab6e0) 
at util/virlog.c:582
#6  0x7f6e71d09ffd in virLogMessage (source=0x7f6e71f2f130 , 
priority=VIR_LOG_INFO, filename=0x7f6e71d2184e "util/virobject.c", linenr=254, 
funcname=0x7f6e71d21a58 <__func__.7842> "virObjectNew", metadata=0x0,
fmt=0x7f6e71d218d8 "OBJECT_NEW: obj=%p classname=%s") at util/virlog.c:542
#7  0x7f6e71d0db22 in virObjectNew (klass=0x56348c51c500) at 
util/virobject.c:254
#8  0x7f6e71d0dbf1 in virObjectLockableNew (klass=0x56348c51c500) at 
util/virobject.c:272
#9  0x7f6e71d0d3e5 in virMacMapNew (file=0x56348c520e70 
"/var/lib/libvirt/dnsmasq//virbr0.macs") at util/virmacmap.c:319
#10 0x7f6e71cdc50a in findLease (name=0x7ffe45fac1d0 "burns", af=0, 
address=0x7ffe45fab980, naddress=0x7ffe45fab988, found=0x7ffe45fab973, 
errnop=0x7ffe45fabb88) at nss/libvirt_nss.c:301
#11 0x7f6e71cdcc56 in _nss_libvirt_gethostbyname4_r (name=0x7ffe45fac1d0 
"burns", pat=0x7ffe45fabb98, buffer=0x7ffe45fabd10 "& ", buflen=1024, 
errnop=0x7ffe45fabb88, herrnop=0x7ffe45fabbb0, ttlp=0x0) at 
nss/libvirt_nss.c:522
#12 0x7f6e724631fc in gaih_inet (name=name@entry=0x7ffe45fac1d0 "burns", 
service=, req=req@entry=0x7ffe45fac1a0, 
pai=pai@entry=0x7ffe45fabc98, naddrs=naddrs@entry=0x7ffe45fabc94, 
tmpbuf=tmpbuf@entry=0x7ffe45fabd00)
at ../sysdeps/posix/getaddrinfo.c:825
#13 0x7f6e72464697 in __GI_getaddrinfo (name=, 
service=, hints=0x7ffe45fac1a0, pai=0x7ffe45fac198) at 
../sysdeps/posix/getaddrinfo.c:2370
#14 0x7f6e71d19e81 in virGetHostnameImpl (quiet=true) at util/virutil.c:717
#15 0x7f6e71d1a057 in virGetHostnameQuiet () at util/virutil.c:759
#16 0x7f6e71d09936 in virLogOnceInit () at util/virlog.c:279
#17 0x7f6e71d09952 in virLogOnce () at util/virlog.c:285
#18 0x7f6e714b5829 in __pthread_once_slow (once_control=0x7f6e71f2fb80 
, init_routine=0x7f6e71d09949 ) at 
pthread_once.c:116
#19 0x7f6e71d16e7d in virOnce (once=0x7f6e71f2fb80 , 
init=0x7f6e71d09949 ) at util/virthread.c:48
#20 0x7f6e71d0997c in virLogInitialize () at util/virlog.c:285
#21 0x7f6e71d0a09a in virLogVMessage (source=0x7f6e71f2f130 , 
priority=VIR_LOG_INFO, filename=0x7f6e71d2184e "util/virobject.c", linenr=254, 
funcname=0x7f6e71d21a58 <__func__.7842> "virObjectNew", metadata=0x0,
fmt=0x7f6e71d218d8 "OBJECT_NEW: obj=%p classname=%s", vargs=0x7ffe45fac410) 
at util/virlog.c:582
#22 0x7f6e71d09ffd in virLogMessage (source=0x7f6e71f2f130 , 
priority=VIR_LOG_INFO, filename=0x7f6e71d2184e "util/virobject.c", linenr=254, 
funcname=0x7f6e71d21a58 <__func__.7842> "virObjectNew", metadata=0x0,
fmt=0x7f6e71d218d8 "OBJECT_NEW: obj=%p classname=%s") at util/virlog.c:542
#23 0x7f6e71d0db22 in virObjectNew (klass=0x56348c51c500) at 
util/virobject.c:254
#24 0x7f6e71d0dbf1 in virObjectLockableNew (klass=0x56348c51c500) at 
util/virobject.c:272
#25 0x7f6e71d0d3e5 in virMacMapNew (file=0x56348c51c430 
"/var/lib/libvirt/dnsmasq//virbr0.macs") at util/virmacmap.c:319
#26 0x7f6e71cdc50a in findLease (name=0x7ffe45faddc5 "libvirt.org", af=10, 
address=0x7ffe45fac6c0, naddress=0x7ffe45fac6c8, 

Re: [libvirt] [PATCH] virlog: determine the hostname on startup CVE-2018-XXX

2018-02-07 Thread P J P
+-- On Mon, 5 Feb 2018, Daniel P. Berrangé wrote --+
| From: Lubomir Rintel 
| 
| At later point it might not be possible or even safe to use getaddrinfo(). It
| can in turn result in a load of NSS module.
| 
| Notably, on a LXC container startup we may find ourselves with the guest
| filesystem already having replaced the host one. Loading a NSS module
| from the guest tree could allow a malicous guest to escape the
| confinement of its container environment because libvirt will not yet
| have locked it down.
| ---
| 
| NB, we're still awaiting CVE allocation before pushing to git

'CVE-2018-6764' has been assigned to this issue by Mitre.

Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] virlog: determine the hostname on startup CVE-2018-XXX

2018-02-07 Thread Daniel P . Berrangé
On Wed, Feb 07, 2018 at 09:58:21AM +0530, P J P wrote:
> +-- On Mon, 5 Feb 2018, Daniel P. Berrangé wrote --+
> | From: Lubomir Rintel 
> | 
> | At later point it might not be possible or even safe to use getaddrinfo(). 
> It
> | can in turn result in a load of NSS module.
> | 
> | Notably, on a LXC container startup we may find ourselves with the guest
> | filesystem already having replaced the host one. Loading a NSS module
> | from the guest tree could allow a malicous guest to escape the
> | confinement of its container environment because libvirt will not yet
> | have locked it down.
> | ---
> | 
> | NB, we're still awaiting CVE allocation before pushing to git
> 
> 'CVE-2018-6764' has been assigned to this issue by Mitre.

Thanks, I have pushed this patch now


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 :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] virlog: determine the hostname on startup CVE-2018-XXX

2018-02-05 Thread Daniel P . Berrangé
From: Lubomir Rintel 

At later point it might not be possible or even safe to use getaddrinfo(). It
can in turn result in a load of NSS module.

Notably, on a LXC container startup we may find ourselves with the guest
filesystem already having replaced the host one. Loading a NSS module
from the guest tree could allow a malicous guest to escape the
confinement of its container environment because libvirt will not yet
have locked it down.
---

NB, we're still awaiting CVE allocation before pushing to git

 src/util/virlog.c | 14 +-
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/src/util/virlog.c b/src/util/virlog.c
index 68439b9194..9105337ce6 100644
--- a/src/util/virlog.c
+++ b/src/util/virlog.c
@@ -64,6 +64,7 @@
 VIR_LOG_INIT("util.log");
 
 static regex_t *virLogRegex;
+static char *virLogHostname;
 
 
 #define VIR_LOG_DATE_REGEX "[0-9]{4}-[0-9]{2}-[0-9]{2}"
@@ -271,6 +272,12 @@ virLogOnceInit(void)
 VIR_FREE(virLogRegex);
 }
 
+/* We get and remember the hostname early, because at later time
+ * it might not be possible to load NSS modules via getaddrinfo()
+ * (e.g. at container startup the host filesystem will not be
+ * accessible anymore. */
+virLogHostname = virGetHostnameQuiet();
+
 virLogUnlock();
 return 0;
 }
@@ -466,17 +473,14 @@ static int
 virLogHostnameString(char **rawmsg,
  char **msg)
 {
-char *hostname = virGetHostnameQuiet();
 char *hoststr;
 
-if (!hostname)
+if (!virLogHostname)
 return -1;
 
-if (virAsprintfQuiet(, "hostname: %s", hostname) < 0) {
-VIR_FREE(hostname);
+if (virAsprintfQuiet(, "hostname: %s", virLogHostname) < 0) {
 return -1;
 }
-VIR_FREE(hostname);
 
 if (virLogFormatString(msg, 0, NULL, VIR_LOG_INFO, hoststr) < 0) {
 VIR_FREE(hoststr);
-- 
2.14.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list