Bug 1025068 (CVE-2017-2626)

This is about function IceGenerateMagicCookie() in iceauth.c. We don't even have an arc4random() variant in our codestreams:

SUSE:SLE-12:Update/libICE/libICE-1.0.8/src/iceauth.c
SUSE:SLE-11:Update/xorg-x11-libICE/libICE-1.0.4/src/iceauth.c
SUSE:SLE-10-SP3:Update/xorg-x11/xc/lib/ICE/iceauth.c

It's all based on gettimeofday().

There is no final patch available yet. In worst case we can read sensible random data from /dev/?random.

We will give an update when the final patch is available.

bugbot adjusting priority

is now public

Created attachment 715740 [details]
X41-2017-001.txt

X41-2017-001.txt

Weak Entropy Usage in Session Keys in libICE
============================================
Vulnerability Type: Other
Affected Products: libICE
Attack Type: Local
Impact: Escalation of Privileges        
Severity Rating: medium
Confirmed Affected Version: 1.0.9 and lower
Confirmed Patched Version:
Vector: local
CVE: CVE-2017-2626
CVSS Score: 7.1
CVSS Vector: CVSS:3.0/AV:L/AC:L/PR:N/UI:N/S:C/C:H/I:N/A:N

Summary and Impact
------------------

libICE depends on arc4random() as well to generate the session cookies,
thereby falling back to the same weak mechanism as libXdmcp:

        IceGenerateMagicCookie (
                int len
        )
        {
            char    *auth;
        #ifndef HAVE_ARC4RANDOM_BUF
            long    ldata[2];
            int     seed;
            int     value;
            int     i;
        #endif
        
            if ((auth = malloc (len + 1)) == NULL)
                return (NULL);
        
        #ifdef HAVE_ARC4RANDOM_BUF
            arc4random_buf(auth, len);
        #else
        #ifdef ITIMER_REAL
            {
                struct timeval  now;
                X_GETTIMEOFDAY (&now);
                ldata[0] = now.tv_sec;
                ldata[1] = now.tv_usec;
            }
        #else
            {
                long    time ();
                ldata[0] = time ((long *) 0);
                ldata[1] = getpid ();
            }
        #endif
            seed = (ldata[0]) + (ldata[1] << 16);
            srand (seed);
            for (i = 0; i < len; i++)
            {
                value = rand ();
                auth[i] = value & 0xff;
            }
        #endif
            auth[len] = '\0';
            return (auth);
        }

For this issue a PoC is available which takes 2-3 seconds to retrieve
the key:

https://www.x41-dsec.de/lab/sources/icetest.c

Workaround
----------

Compile against libbsd

Created attachment 715748 [details]
icetest.c

a reproducer posted by x41.

Meanwhile a fix came into git (libICE):

ff5e59f32255913bb1cdf51441b98c9107ae165b

This is an autogenerated message for OBS integration:
This bug (1025068) was mentioned in
https://build.opensuse.org/request/show/502905 Factory / libICE

Factory is done. See above.

I also prepared updated packages for sle12, sle11 and even sle10. But now I noticed, that it didn't make any sense. :-(

getentropy() needs glibc 2.25, which is currently only provided by factory. Leap 42.2/42.3 doesn't fullfill this requirement, let alone sle12, sle11, sle10. 

arc4random_buf() needs libbsd. Do we really want to add this requirement for the Leap products (42.2/42.3)? On sle12, sle11, sle10 we apparently ship no libbsd.

Matthias? Marcus?

Created attachment 729646 [details]
drop in replacement for getentropy() using getrandom()

Created attachment 729648 [details]
drop in replacement for getentropy() using /dev/urandom

Hi Stefan,

(In reply to sndirsch@suse.com from comment #9)
> getentropy() needs glibc 2.25, which is currently only provided by factory. Leap 42.2/42.3 doesn't fullfill this requirement, let alone sle12, sle11, sle10. 
> 
> arc4random_buf() needs libbsd. Do we really want to add this requirement for the Leap products (42.2/42.3)? On sle12, sle11, sle10 we apparently ship no libbsd.
> 
> Matthias? Marcus?

Given how easy this is to exploit I'd like to see this fixed in some way on
the older versions.

How about using a drop-in replacement for getentropy() on the versions that
miss it? I've provided examples:

- in attachment 729646 [details] using the getrandom() system call
- in attachment 729648 [details] using /dev/urandom, for versions that don't have the
  getrandom() system call.

If this works for you then we could to the same for the other bug 1025046 and
bug 1025084. The code should be reviewed closely by somebody else than me,
however, if we are going to use it.

Created attachment 729649 [details]
correct drop in replacement for getentropy() using getrandom()

Comment on attachment 729646 [details]
drop in replacement for getentropy() using getrandom()

#include <linux/random.h>
#include <sys/syscall.h>
#include <errno.h>

int getentropy(void *buffer, size_t length)
{
	int res;
	size_t filled = 0;

	if( length > 256 )
	{
		errno = EIO;
		return -1;
	}

	while( filled < length )
	{
		/*
		 * glibc does not contain a syscall wrapper for this in older
		 * versions
		 */
		res = syscall(SYS_getrandom, (char*)buffer + filled, length - filled, 0);

		if( res == -1 )
		{
			if( errno == EINTR )
				continue;

			return -1;
		}
		else if( res == 0 )
		{
			// no more bytes available? should not happen
			errno = EIO;
			return -1;
		}

		filled += res;
	}

	return 0;
}

Created attachment 729681 [details]
Additional patch for openSUSE 42.2 with using these drop-in replacements for getentropy()

Created attachment 729683 [details]
Complete patch for Leap 42.2 with those drop-in replacements for getentropy()

Apparently Leap 42.2 doesn't provide getrandom() either or I did something wrong. Can you confirm this, please?

[    7s] checking for arc4random_buf... no
[    7s] checking for getentropy... no
[    7s] checking for getrandom... no

mattzhias patch is not using getrandom itself, it uses syscall()

Which means what? How is this related to configure not finding getrandom() on Leap 42.2?

Or is it only possible to figure out, whether getrandom() is available during runtime? I can rewrite the patch, so the getrandom() replacement is tried. And if that fails, use the /dev/urandom replacement. I just need to know ...

(In reply to Stefan Dirsch from comment #19)
> Which means what? How is this related to configure not finding getrandom()
> on Leap 42.2?

The thing is that glibc only recently added a wrapper/declaration for the getrandom system call. It's been added to version 2.25:

https://sourceware.org/ml/libc-alpha/2017-02/msg00079.html

The system call was implemented in kernel for a much longer time, however:

https://github.com/torvalds/linux/commit/c6e9d6f388947
https://lwn.net/Articles/711013/

The configure check seems to look for the system call declaration, which is not present. The code I provided uses the syscall() approach to work around the missing declaration.

I don't know if there's a configure check to only check whether the system call is implemented in the kernel (would return ENOSYS if not so).

The problem is also stated here:

http://permalink.gmane.org/gmane.comp.lib.glibc.bugs/22297

We could change the drop-in replacement to check at runtime, whether the getrandom() system call works, and fall back to reading /dev/urandom in case ENOSYS is returned. I can adjust the code if and provide to you, if you need it.

(In reply to Matthias Gerstner from comment #21)
> We could change the drop-in replacement to check at runtime, whether the
> getrandom() system call works, and fall back to reading /dev/urandom in case
> ENOSYS is returned. I can adjust the code if and provide to you, if you need
> it.

That would be helpful and would make sense, I think. Of course I can just try both drop-in replacements as proposed in comment #20.

Created attachment 730002 [details]
Merge getrandom() and /dev/urandom usage in one function

How about this one? I guess I need a review here. ;-)

Created attachment 730193 [details]
runtime check for getrandom(), wrapper around both implementations

(In reply to sndirsch@suse.com from comment #23)
> Created attachment 730002 [details]
> Merge getrandom() and /dev/urandom usage in one function
> 
> How about this one? I guess I need a review here. ;-)

Sorry for the delay but I was sick for some days.

I'd rather keep the two approaches of using getrandom() / urandom clearly
separated, even if it means some duplicate code. It's easier to read and we
can also more easily compare it to similar wrappers in glibc, for example.

Also we need a check for the definition of SYS_getrandom.

Please find a new suggestion in attachment 730193 [details].

Created attachment 730205 [details]
Integrated previous patch by Matthias Gerstner

Added configure check for SYS_getrandom, which doesn't seem to work. :-( 

checking for arc4random_buf in -lbsd... no
checking for asprintf... yes
checking for arc4random_buf... no
checking for getentropy... no
checking for SYS_getrandom... no

At least not on openSUSE 42.2.

Renamed getentropy() wrapper to getentropy_emulate().

I'm afraid I haven't understood, what you've meant with

  "Also we need a check for the definition of SYS_getrandom"

:-(

Sorry, should have been more clear. I was referring to the

  #ifdef SYS_getrandom

that is already in the new code I suggested.

Ok. So no need for a configure check. ;-)

Created attachment 730212 [details]
Additional patch for openSUSE 42.2 with using drop-in replacements for getentropy()

Created attachment 730213 [details]
Complete patch for Leap 42.2 with that drop-in replacement for getentropy()

Created attachment 730361 [details]
adjusted PoC program

Let's not forget about the QA reproducer so we can check whether our patch really works. I'm attached an adjusted variant of the PoC program with which I was able to reproduce the issue. I've tested this on SLE-12 SP2.

For compilation you need to install:

  zypper in libICE-devel

Then build:

  gcc -oicetest2 icetest2.c -O2 -lICE

Remove old ICE connections and restart display manager (without this there seems to be a too high time jitter from boot for the PoC to work):

  rm /tmp/.ICE-unix/*
  systemctl restart xdm

Run the PoC:

  ./icetest2

*Without* the bugfix this should show:

> Connection opened
> Using time: 1498561424 212899

*After* the bugfix this should show:

> Failed

Thanks for the reproducer! I will test this later.

On sle10 O_CLOEXEC option for open() is not available yet. Also it isn''t supported before Kernel 2.6.23 and we still ship Kernel 2.6.16 with sle10.

So I've removed this option and close the FD before returning from the getentropy_urandom(...) function. Hope this makes sense.

(In reply to Stefan Dirsch from comment #33)
> Thanks for the reproducer! I will test this later.

Appears to work. Tested on Leap 42.3 with my current patches. :-)

Patches submitted for Leap 42.2, sle12, sle11 and sle10. Reassigning back to security team.

This is an autogenerated message for OBS integration:
This bug (1025068) was mentioned in
https://build.opensuse.org/request/show/506471 42.2 / libICE

(In reply to sndirsch@suse.com from comment #33)

> So I've removed this option and close the FD before returning from the
> getentropy_urandom(...) function. Hope this makes sense.

That's correct but also leads me to a bug in my code: The close() needs to be
added to all exit paths in all codestreams like this, otherwise there's a file
descriptor leak:

> int getentropy_urandom(void *buffer, size_t length)
> {
> 	int random_fd = -1;
> 	ssize_t res = -1;
> 	size_t filled = 0;
> 
> 	if( length > 256 )
> 	{
> 		errno = EIO;
> 		return -1;
> 	}
> 
> 	random_fd = open("/dev/urandom", O_RDONLY | O_CLOEXEC);
> 
> 	if( random_fd == -1 )
> 	{
> 		return -1;
> 	}
> 
> 	while( filled < length )
> 	{
> 		res = read(random_fd, (char*)buffer + filled, length - filled);
> 
> 		if( res == -1 )
> 		{
> 			// shouldn't actually happen acc. to man(4) random,
> 			// but you never know
> 			if( errno == EINTR )
> 				continue;
> 
> 			close(random_fd);
> 			return -1;
> 		}
> 		else if( res == 0 )
> 		{
> 			// no more bytes available? should not happen
> 			close(random_fd);
> 			errno = EIO;
> 			return -1;
> 		}
> 
> 		filled += res;
> 	}
> 
> 	close(random_fd);
> 	return 0;
> }

Can you please fix this, too? Sorry that this bug turned into such a monster
:-(

(In reply to Matthias Gerstner from comment #38)
> > So I've removed this option and close the FD before returning from the
> > getentropy_urandom(...) function. Hope this makes sense.
> 
> That's correct but also leads me to a bug in my code: The close() needs to be
> added to all exit paths in all codestreams like this, otherwise there's a
> file descriptor leak:
> Can you please fix this, too? 

done. :-)

> Sorry that this bug turned into such a monster > :-(

np

This is an autogenerated message for OBS integration:
This bug (1025068) was mentioned in
https://build.opensuse.org/request/show/506493 42.2 / libICE

An update workflow for this issue was started.
This issue was rated as moderate.
Please submit fixed packages until 2017-07-17.
When done, reassign the bug to security-team@suse.de.
https://swamp.suse.de/webswamp/wf/63741

Created attachment 730982 [details]
icetest2.c

QA REPRODUCER:

gcc -O2 -Wall -g -o icetest2 icetest2.c  -lICE
./icetest2

it should NOT report "Connection opened" like:
Trying connection local/jet.franken.de:@/tmp/.ICE-unix/2777
Connection opened
Using time: 1498981551 122280

openSUSE-SU-2017:1801-1: An update that fixes one vulnerability is now available.

Category: security (moderate)
Bug References: 1025068
CVE References: CVE-2017-2626
Sources used:
openSUSE Leap 42.2 (src):    libICE-1.0.9-5.3.1

When I am running the test case(icetest2) on sles11sp4. It seems it's falling in infinite loop and can never exit. The gnome-session daemon uses up 100% cpu.  Even after I kill the icetest2 process the gnome-session still uses 100% cpu. I have to reboot for gnome-session to go back to normal.

The result is the same regardless of whether I install the update or not.

The test case work well on sles12sp2 without the problem on #c46

Hmm. Even before security update? Have you read instructions of comment #32?

> [...]
> systemctl restart xdm

Since we did not have systemd on sle11 yet, this needs to be

  rcxdm restart

Just to be sure ...

(In reply to Stefan Dirsch from comment #48)
> Hmm. Even before security update? Have you read instructions of comment #32?

Yes, I followed the instructions of comment #32


> 
> > [...]
> > systemctl restart xdm
> 
> Since we did not have systemd on sle11 yet, this needs to be
> 
>   rcxdm restart
> 
> Just to be sure ...

Yes, rcxdm restart (or telinit 3, telinit 5)

Then I don't know either. It is my understanding that gnome-session is not running before one is doing a login, right?

I believe I was testing without login at all. I was logged in into the VM as root via ssh, removed the sockets below  /tmp/.ICE-unix, restarted display manager and then ran the reproducer withouth having access to any X display.

Matthias, can you help us here?

(In reply to Stefan Dirsch from comment #50)
> Then I don't know either. It is my understanding that gnome-session is not
> running before one is doing a login, right?
> 
> I believe I was testing without login at all. I was logged in into the VM as
> root via ssh, removed the sockets below  /tmp/.ICE-unix, restarted display
> manager and then ran the reproducer withouth having access to any X display.

Some local user must be logged in with a display manager like gdm that uses libICE for communication. Otherwise there should be no sockets in /tmp/.ICE-unix to brute force.
 
> Matthias, can you help us here?

Yeah I can reproduce the infinite loop in gnome-session on SLES-11-SP4 as well. gnome-session is the daemon that implements the other end of the libICE authentication in this scenario. After a long time of debugging it fell on my head. There's a file descriptor leak, probably in gnome-session or some X library on SLES-11-SP4. When you look at

  ls -l /proc/`pidof gnome-session`/fd

you'll see that 1023 file descriptors are opened by gnome-session and thus it can't accept() any more requests from the icetest2 PoC application. You'll also get a large .xsession-errors file in the logged in user's home directory with lines like this:

  _IceTransSocketUNIXAccept: accept() failed

All further communication of the gnome-session daemon then fails and the open gnome session is more or less broken, gnome-session wasting resources trying to communicate.

Long story short: There's also some local DoS in the gdm/X setup on SLES-11-SP4 allowing a local user to perform ~1024 unsuccessful calls to IceOpenConnection() and the gdm session will break.

I haven't found the exact location where the leak occurs. Will need to dig more into the sources.

So I've found the issue in gnome-session causing the file descriptor leak.
Basically this commit is missing:

https://github.com/GNOME/gnome-session/commit/b0dc999e0b45355314616321dbb6cb71e729fc9d

I've made a branch of gnome-session in

  home:mgerstner:branches:SUSE:SLE-11-SP1:Update/gnome-session

containing this fix. Using this variant of gnome-session the issue goes away.
For this issue we should open a bug against gnome-session maintainers, might
even we worth a CVE, because it's an easy local denial of service, but only in
old software.

However, even with fixed gnome-session, it seems the PoC is still not working
(failing). This might be due to a slightly different random data seed used in
SLE-11-SP4. I might investigate this further tomorrow.

Created attachment 731944 [details]
improved icetest PoC for SLES11

So I've been able to reproduce the PoC on SLES11-SP4 now. Using the gnome-session package from comment#52 and using this new attachment icetest3.c. This PoC worked reliably multiple times for me. The reason why the previous PoC was not working well was that on SLES11-SP4 the file system does not store nanosecond precision timestamps for the sockets in /tmp/.ICE-unix/*. Therefore the interval chosen by the previous PoC was not big enough. The new PoC cycles through all possible values that IceGenerateMagicCookie() can use. It doesn't take much longer to run.

Regarding the DoS issue in gnome-session on SLES11-SP4 I've submitted a CVE request and will post to oss-sec, security team will open a separate bug for that.

Please tell me when there are any other issues with testing this.

SUSE-SU-2017:1835-1: An update that fixes one vulnerability is now available.

Category: security (moderate)
Bug References: 1025068
CVE References: CVE-2017-2626
Sources used:
SUSE Linux Enterprise Software Development Kit 12-SP2 (src):    libICE-1.0.8-10.1
SUSE Linux Enterprise Server for Raspberry Pi 12-SP2 (src):    libICE-1.0.8-10.1
SUSE Linux Enterprise Server 12-SP2 (src):    libICE-1.0.8-10.1
SUSE Linux Enterprise Desktop 12-SP2 (src):    libICE-1.0.8-10.1

I created a branch for gnome-session from yours and tested the new reproducer with the gnome-session. It works well. 

A question: should we approve the sle11sp4 libice update now or wait until the fixed gnome-session update is available?

For the record: We've opened bug 1048274 for the gnome-session issue.

This is independent of the libICE issue handled in this bug. The gnome-session
bugfix is only required for testing the PoC for the libICE bug.

If the ICE issue is now fixed, you can approve it even before we have a gnome-session fix

SUSE-SU-2017:1848-1: An update that fixes one vulnerability is now available.

Category: security (moderate)
Bug References: 1025068
CVE References: CVE-2017-2626
Sources used:
SUSE Linux Enterprise Software Development Kit 11-SP4 (src):    xorg-x11-libICE-7.4-3.1
SUSE Linux Enterprise Server 11-SP4 (src):    xorg-x11-libICE-7.4-3.1
SUSE Linux Enterprise Debuginfo 11-SP4 (src):    xorg-x11-libICE-7.4-3.1

released

SUSE-SU-2018:0337-1: An update that fixes one vulnerability is now available.

Category: security (moderate)
Bug References: 1025068
CVE References: CVE-2017-2626
Sources used:
SUSE Linux Enterprise Software Development Kit 12-SP3 (src):    libICE-1.0.8-12.1
SUSE Linux Enterprise Server 12-SP3 (src):    libICE-1.0.8-12.1
SUSE Linux Enterprise Desktop 12-SP3 (src):    libICE-1.0.8-12.1