[libvirt] [PATCH] virlog: determine the hostname on startup CVE-2018-XXX
From: Lubomir Rintel <lkundrak@v3.sk> 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(&hoststr, "hostname: %s", hostname) < 0) { - VIR_FREE(hostname); + if (virAsprintfQuiet(&hoststr, "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
+-- On Mon, 5 Feb 2018, Daniel P. Berrangé wrote --+ | From: Lubomir Rintel <lkundrak@v3.sk> | | 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
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 <lkundrak@v3.sk> | | 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 :|
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 <lkundrak@v3.sk> | | 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 0x00007f6e714b57e7 in futex_wait (private=0, expected=1, futex_word=0x7f6e71f2fb80 <virLogOnceControl>) at ../sysdeps/unix/sysv/linux/futex-internal.h:61 #1 futex_wait_simple (private=0, expected=1, futex_word=0x7f6e71f2fb80 <virLogOnceControl>) at ../sysdeps/nptl/futex-internal.h:135 #2 __pthread_once_slow (once_control=0x7f6e71f2fb80 <virLogOnceControl>, init_routine=0x7f6e71d09949 <virLogOnce>) at pthread_once.c:105 #3 0x00007f6e71d16e7d in virOnce (once=0x7f6e71f2fb80 <virLogOnceControl>, init=0x7f6e71d09949 <virLogOnce>) at util/virthread.c:48 #4 0x00007f6e71d0997c in virLogInitialize () at util/virlog.c:285 #5 0x00007f6e71d0a09a in virLogVMessage (source=0x7f6e71f2f130 <virLogSelf>, 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 0x00007f6e71d09ffd in virLogMessage (source=0x7f6e71f2f130 <virLogSelf>, 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 0x00007f6e71d0db22 in virObjectNew (klass=0x56348c51c500) at util/virobject.c:254 #8 0x00007f6e71d0dbf1 in virObjectLockableNew (klass=0x56348c51c500) at util/virobject.c:272 #9 0x00007f6e71d0d3e5 in virMacMapNew (file=0x56348c520e70 "/var/lib/libvirt/dnsmasq//virbr0.macs") at util/virmacmap.c:319 #10 0x00007f6e71cdc50a in findLease (name=0x7ffe45fac1d0 "burns", af=0, address=0x7ffe45fab980, naddress=0x7ffe45fab988, found=0x7ffe45fab973, errnop=0x7ffe45fabb88) at nss/libvirt_nss.c:301 #11 0x00007f6e71cdcc56 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 0x00007f6e724631fc in gaih_inet (name=name@entry=0x7ffe45fac1d0 "burns", service=<optimized out>, req=req@entry=0x7ffe45fac1a0, pai=pai@entry=0x7ffe45fabc98, naddrs=naddrs@entry=0x7ffe45fabc94, tmpbuf=tmpbuf@entry=0x7ffe45fabd00) at ../sysdeps/posix/getaddrinfo.c:825 #13 0x00007f6e72464697 in __GI_getaddrinfo (name=<optimized out>, service=<optimized out>, hints=0x7ffe45fac1a0, pai=0x7ffe45fac198) at ../sysdeps/posix/getaddrinfo.c:2370 #14 0x00007f6e71d19e81 in virGetHostnameImpl (quiet=true) at util/virutil.c:717 #15 0x00007f6e71d1a057 in virGetHostnameQuiet () at util/virutil.c:759 #16 0x00007f6e71d09936 in virLogOnceInit () at util/virlog.c:279 #17 0x00007f6e71d09952 in virLogOnce () at util/virlog.c:285 #18 0x00007f6e714b5829 in __pthread_once_slow (once_control=0x7f6e71f2fb80 <virLogOnceControl>, init_routine=0x7f6e71d09949 <virLogOnce>) at pthread_once.c:116 #19 0x00007f6e71d16e7d in virOnce (once=0x7f6e71f2fb80 <virLogOnceControl>, init=0x7f6e71d09949 <virLogOnce>) at util/virthread.c:48 #20 0x00007f6e71d0997c in virLogInitialize () at util/virlog.c:285 #21 0x00007f6e71d0a09a in virLogVMessage (source=0x7f6e71f2f130 <virLogSelf>, 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 0x00007f6e71d09ffd in virLogMessage (source=0x7f6e71f2f130 <virLogSelf>, 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 0x00007f6e71d0db22 in virObjectNew (klass=0x56348c51c500) at util/virobject.c:254 #24 0x00007f6e71d0dbf1 in virObjectLockableNew (klass=0x56348c51c500) at util/virobject.c:272 #25 0x00007f6e71d0d3e5 in virMacMapNew (file=0x56348c51c430 "/var/lib/libvirt/dnsmasq//virbr0.macs") at util/virmacmap.c:319 #26 0x00007f6e71cdc50a in findLease (name=0x7ffe45faddc5 "libvirt.org", af=10, address=0x7ffe45fac6c0, naddress=0x7ffe45fac6c8, found=0x7ffe45fac6b3, errnop=0x7f6e7290a698) at nss/libvirt_nss.c:301 #27 0x00007f6e71cdc839 in _nss_libvirt_gethostbyname3_r (name=0x7ffe45faddc5 "libvirt.org", af=10, result=0x7f6e7272f200 <resbuf>, buffer=0x56348c511000 "& ", buflen=1024, errnop=0x7f6e7290a698, herrnop=0x7ffe45fac85c, ttlp=0x0, canonp=0x0) at nss/libvirt_nss.c:412 #28 0x00007f6e71cdc724 in _nss_libvirt_gethostbyname2_r (name=0x7ffe45faddc5 "libvirt.org", af=10, result=0x7f6e7272f200 <resbuf>, buffer=0x56348c511000 "& ", buflen=1024, errnop=0x7f6e7290a698, herrnop=0x7ffe45fac85c) at nss/libvirt_nss.c:372 #29 0x00007f6e7248f72f in __gethostbyname2_r (name=name@entry=0x7ffe45faddc5 "libvirt.org", af=af@entry=10, resbuf=resbuf@entry=0x7f6e7272f200 <resbuf>, buffer=<optimized out>, buflen=1024, result=result@entry=0x7ffe45fac860, h_errnop=0x7ffe45fac85c) at ../nss/getXXbyYY_r.c:314 #30 0x00007f6e7248f494 in gethostbyname2 (name=0x7ffe45faddc5 "libvirt.org", af=10) at ../nss/getXXbyYY.c:116 #31 0x000056348c30c36d in hosts_keys (number=<optimized out>, key=<optimized out>) at getent.c:317 #32 0x000056348c30b7d2 in main (argc=<optimized out>, argv=0x7ffe45faca28) at getent.c:955 So in frames 28 an 27 libvirt nss module is consulted. However, this means that virLog*() is issued from one of many internal APIs the NSS plugin uses. So it causes virLogInitialize() to be called. However, after this patch was pushed in we try to resolve the hostname in virGetHostnameImpl() which leads to NSS plugin and thus completes the circle and deadlocks. I'm not quite sure how to solve this issue. Ideally, the NSS plugin wouldn't use logging at all (and it isn't directly just indirectly) but is there a way to not compile in virlog.c? We can turn off DEBUG but that's not going to be enough (as you can see in the stack trace). Michal
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 <lkundrak@v3.sk> | | 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 0x00007f6e714b57e7 in futex_wait (private=0, expected=1, futex_word=0x7f6e71f2fb80 <virLogOnceControl>) at ../sysdeps/unix/sysv/linux/futex-internal.h:61 #1 futex_wait_simple (private=0, expected=1, futex_word=0x7f6e71f2fb80 <virLogOnceControl>) at ../sysdeps/nptl/futex-internal.h:135 #2 __pthread_once_slow (once_control=0x7f6e71f2fb80 <virLogOnceControl>, init_routine=0x7f6e71d09949 <virLogOnce>) at pthread_once.c:105 #3 0x00007f6e71d16e7d in virOnce (once=0x7f6e71f2fb80 <virLogOnceControl>, init=0x7f6e71d09949 <virLogOnce>) at util/virthread.c:48 #4 0x00007f6e71d0997c in virLogInitialize () at util/virlog.c:285 #5 0x00007f6e71d0a09a in virLogVMessage (source=0x7f6e71f2f130 <virLogSelf>, 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 0x00007f6e71d09ffd in virLogMessage (source=0x7f6e71f2f130 <virLogSelf>, 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 0x00007f6e71d0db22 in virObjectNew (klass=0x56348c51c500) at util/virobject.c:254 #8 0x00007f6e71d0dbf1 in virObjectLockableNew (klass=0x56348c51c500) at util/virobject.c:272 #9 0x00007f6e71d0d3e5 in virMacMapNew (file=0x56348c520e70 "/var/lib/libvirt/dnsmasq//virbr0.macs") at util/virmacmap.c:319 #10 0x00007f6e71cdc50a in findLease (name=0x7ffe45fac1d0 "burns", af=0, address=0x7ffe45fab980, naddress=0x7ffe45fab988, found=0x7ffe45fab973, errnop=0x7ffe45fabb88) at nss/libvirt_nss.c:301 #11 0x00007f6e71cdcc56 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 0x00007f6e724631fc in gaih_inet (name=name@entry=0x7ffe45fac1d0 "burns", service=<optimized out>, req=req@entry=0x7ffe45fac1a0, pai=pai@entry=0x7ffe45fabc98, naddrs=naddrs@entry=0x7ffe45fabc94, tmpbuf=tmpbuf@entry=0x7ffe45fabd00) at ../sysdeps/posix/getaddrinfo.c:825 #13 0x00007f6e72464697 in __GI_getaddrinfo (name=<optimized out>, service=<optimized out>, hints=0x7ffe45fac1a0, pai=0x7ffe45fac198) at ../sysdeps/posix/getaddrinfo.c:2370 #14 0x00007f6e71d19e81 in virGetHostnameImpl (quiet=true) at util/virutil.c:717 #15 0x00007f6e71d1a057 in virGetHostnameQuiet () at util/virutil.c:759 #16 0x00007f6e71d09936 in virLogOnceInit () at util/virlog.c:279 #17 0x00007f6e71d09952 in virLogOnce () at util/virlog.c:285 #18 0x00007f6e714b5829 in __pthread_once_slow (once_control=0x7f6e71f2fb80 <virLogOnceControl>, init_routine=0x7f6e71d09949 <virLogOnce>) at pthread_once.c:116 #19 0x00007f6e71d16e7d in virOnce (once=0x7f6e71f2fb80 <virLogOnceControl>, init=0x7f6e71d09949 <virLogOnce>) at util/virthread.c:48 #20 0x00007f6e71d0997c in virLogInitialize () at util/virlog.c:285 #21 0x00007f6e71d0a09a in virLogVMessage (source=0x7f6e71f2f130 <virLogSelf>, 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 0x00007f6e71d09ffd in virLogMessage (source=0x7f6e71f2f130 <virLogSelf>, 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 0x00007f6e71d0db22 in virObjectNew (klass=0x56348c51c500) at util/virobject.c:254 #24 0x00007f6e71d0dbf1 in virObjectLockableNew (klass=0x56348c51c500) at util/virobject.c:272 #25 0x00007f6e71d0d3e5 in virMacMapNew (file=0x56348c51c430 "/var/lib/libvirt/dnsmasq//virbr0.macs") at util/virmacmap.c:319 #26 0x00007f6e71cdc50a in findLease (name=0x7ffe45faddc5 "libvirt.org", af=10, address=0x7ffe45fac6c0, naddress=0x7ffe45fac6c8, found=0x7ffe45fac6b3, errnop=0x7f6e7290a698) at nss/libvirt_nss.c:301 #27 0x00007f6e71cdc839 in _nss_libvirt_gethostbyname3_r (name=0x7ffe45faddc5 "libvirt.org", af=10, result=0x7f6e7272f200 <resbuf>, buffer=0x56348c511000 "& ", buflen=1024, errnop=0x7f6e7290a698, herrnop=0x7ffe45fac85c, ttlp=0x0, canonp=0x0) at nss/libvirt_nss.c:412 #28 0x00007f6e71cdc724 in _nss_libvirt_gethostbyname2_r (name=0x7ffe45faddc5 "libvirt.org", af=10, result=0x7f6e7272f200 <resbuf>, buffer=0x56348c511000 "& ", buflen=1024, errnop=0x7f6e7290a698, herrnop=0x7ffe45fac85c) at nss/libvirt_nss.c:372 #29 0x00007f6e7248f72f in __gethostbyname2_r (name=name@entry=0x7ffe45faddc5 "libvirt.org", af=af@entry=10, resbuf=resbuf@entry=0x7f6e7272f200 <resbuf>, buffer=<optimized out>, buflen=1024, result=result@entry=0x7ffe45fac860, h_errnop=0x7ffe45fac85c) at ../nss/getXXbyYY_r.c:314 #30 0x00007f6e7248f494 in gethostbyname2 (name=0x7ffe45faddc5 "libvirt.org", af=10) at ../nss/getXXbyYY.c:116 #31 0x000056348c30c36d in hosts_keys (number=<optimized out>, key=<optimized out>) at getent.c:317 #32 0x000056348c30b7d2 in main (argc=<optimized out>, argv=0x7ffe45faca28) at getent.c:955
So in frames 28 an 27 libvirt nss module is consulted. However, this means that virLog*() is issued from one of many internal APIs the NSS plugin uses. So it causes virLogInitialize() to be called. However, after this patch was pushed in we try to resolve the hostname in virGetHostnameImpl() which leads to NSS plugin and thus completes the circle and deadlocks.
I'm not quite sure how to solve this issue. Ideally, the NSS plugin wouldn't use logging at all (and it isn't directly just indirectly) but is there a way to not compile in virlog.c? We can turn off DEBUG but that's not going to be enough (as you can see in the stack trace).
I think we should just call hostname() directly in virLogInitialize. The other stuff virGetHostname() does is not really relevant for the usage we have in logging. Also, the virLogOnce call ought to be done outside the logging lock, since the pthread once stuff already guarantees serialization and single execution without needing the caller to provide a mutex. 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 :|
participants (3)
-
Daniel P. Berrangé -
Michal Privoznik -
P J P