[PATCH] Whitelist libuuid clock file

[PATCH] Whitelist libuuid clock file

[Stanislav Brabec]

Return back ProtectSystem to strict, and enable access to
/var/lib/libuuid only.

Note: As LIBUUID_CLOCK_FILE does not use @localstatedir@, we use
/var here as well.

Signed-off-by: Ali Abdallah <ali.abdallah@suse.com>
Signed-off-by: Stanislav Brabec <sbrabec@suse.cz>
---
  misc-utils/uuidd.service.in | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/misc-utils/uuidd.service.in b/misc-utils/uuidd.service.in
index 065b4a194..e64ca59b5 100644
--- a/misc-utils/uuidd.service.in
+++ b/misc-utils/uuidd.service.in
@@ -8,6 +8,7 @@ ExecStart=@usrsbin_execdir@/uuidd --socket-activation
  Restart=no
  User=uuidd
  Group=uuidd
+ProtectSystem=strict
  ProtectHome=yes
  PrivateDevices=yes
  PrivateNetwork=yes
@@ -17,6 +18,7 @@ ProtectKernelModules=yes
  ProtectControlGroups=yes
  RestrictAddressFamilies=AF_UNIX
  MemoryDenyWriteExecute=yes
+ReadWritePaths=/var/lib/libuuid/
  SystemCallFilter=@default @file-system @basic-io @system-service 
@signal @io-event @network-io

  [Install]
-- 
2.34.1

-- 
Best Regards / S pozdravem,

Stanislav Brabec
software developer
---------------------------------------------------------------------
SUSE LINUX, s. r. o.                         e-mail: sbrabec@suse.com
Křižíkova 148/34 (Corso IIa)                    tel: +420 284 084 060
186 00 Praha 8-Karlín                          fax:  +420 284 084 001
Czech Republic                                    http://www.suse.cz/
PGP: 830B 40D5 9E05 35D8 5E27 6FA3 717C 209F A04F CD76

Re: [PATCH] Whitelist libuuid clock file

[Karel Zak]

On Tue, Jan 25, 2022 at 11:17:27AM +0100, Stanislav Brabec wrote:
> diff --git a/misc-utils/uuidd.service.in b/misc-utils/uuidd.service.in
> index 065b4a194..e64ca59b5 100644
> --- a/misc-utils/uuidd.service.in
> +++ b/misc-utils/uuidd.service.in
> @@ -8,6 +8,7 @@ ExecStart=@usrsbin_execdir@/uuidd --socket-activation
>  Restart=no
>  User=uuidd
>  Group=uuidd
> +ProtectSystem=strict
>  ProtectHome=yes
>  PrivateDevices=yes
>  PrivateNetwork=yes
> @@ -17,6 +18,7 @@ ProtectKernelModules=yes
>  ProtectControlGroups=yes
>  RestrictAddressFamilies=AF_UNIX
>  MemoryDenyWriteExecute=yes
> +ReadWritePaths=/var/lib/libuuid/

OK, seems better than my solution ;-) Thanks!

   Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: [PATCH] Whitelist libuuid clock file

[Stanislav Brabec]

Karel Zak wrote:
>
> OK, seems better than my solution ;-) Thanks!
Actually, I found a very exotic failure of uuidd accessing clock.txt, 
which is not yet covered:

ls -al /var/lib/libuuid/clock.txt
-rw-rw---- 1 root root 56 Jan 25 11:48 /var/lib/libuuid/clock.txt
i. e. root owned clock.txt

It happens only if more conditions are met:
- /var/lib/libuuid exists
- uuidd is not running nor socket activated
- uuigden --time is called as root

If this happens, /var/lib/libuuid/clock.txt is written as root:root by 
the code in libuuid/src/gen_uuid.c:get_clock(). When uuidd is started 
later, it is unable to use clock.txt.

It happens only in very special cases:
- util-linux is reinstalled from --without-uuidd to --with-uuidd
- util-linux is just being installed, and daemon nor socket activation 
are active yet
- start of uuidd fails for some exotic reason (out of memory, number of 
processes exhausted)

But if it happens, the problem stays forever.

So I think it should be addressed. But I am not sure how to fix it 
properly in the systemd service file.
ExecStartPre=-/usr/bin/chown uuidd:uuidd /var/lib/libuuid/clock.txt
does not work:
/usr/bin/chown: changing ownership of '/var/lib/libuuid/clock.txt': 
Operation not permitted

-- 
Best Regards / S pozdravem,

Stanislav Brabec
software developer
---------------------------------------------------------------------
SUSE LINUX, s. r. o.                         e-mail: sbrabec@suse.com
Křižíkova 148/34 (Corso IIa)                    tel: +420 284 084 060
186 00 Praha 8-Karlín                          fax:  +420 284 084 001
Czech Republic                                    http://www.suse.cz/
PGP: 830B 40D5 9E05 35D8 5E27 6FA3 717C 209F A04F CD76

Re: [PATCH] Whitelist libuuid clock file

[Karel Zak]

On Tue, Jan 25, 2022 at 12:37:09PM +0100, Stanislav Brabec wrote:
> Karel Zak wrote:
> > 
> > OK, seems better than my solution ;-) Thanks!
> Actually, I found a very exotic failure of uuidd accessing clock.txt, which
> is not yet covered:
> 
> ls -al /var/lib/libuuid/clock.txt
> -rw-rw---- 1 root root 56 Jan 25 11:48 /var/lib/libuuid/clock.txt
> i. e. root owned clock.txt
> 
> It happens only if more conditions are met:
> - /var/lib/libuuid exists
> - uuidd is not running nor socket activated
> - uuigden --time is called as root

And vice-versa, if you stop uuidd and start uuidgen as normal user you
get EACCES for /var/lib/libuuid/clock.txt, but this use-case is
probably not so important ...

> If this happens, /var/lib/libuuid/clock.txt is written as root:root by the
> code in libuuid/src/gen_uuid.c:get_clock(). When uuidd is started later, it
> is unable to use clock.txt.
...

> So I think it should be addressed. But I am not sure how to fix it properly
> in the systemd service file.
> ExecStartPre=-/usr/bin/chown uuidd:uuidd /var/lib/libuuid/clock.txt
> does not work:
> /usr/bin/chown: changing ownership of '/var/lib/libuuid/clock.txt':
> Operation not permitted

What about to create a user specific clock state file if we cannot access
the default one due to EACCES? Something like:

 state_fd = open(LIBUUID_CLOCK_FILE, O_RDWR|O_CREAT|O_CLOEXEC, 0660);
 if (state_fd < 0 &7 errno == EACCES) {
    snprintf(path, "%s-%d", LIBUUID_CLOCK_FILE, getuid());
    state_fd = open(path, O_RDWR|O_CREAT|O_CLOEXEC, 0660);
 } 

so for the bad use-case:

  /var/lib/libuuid/clock.txt
  /var/lib/libuuid/clock.txt-<uuidd_uid>

in theory it means two clock queues, but if uuidd is running then
libuuid/getuuid always uses it.

The important is that in this way we can be sure uuidd is always
successful, and I guess the sensitive use-case (SAP;-) is always about
uuidd. IMHO it's more robust then depend on systemd ExecStartPre (or
so).

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: [PATCH] Whitelist libuuid clock file

[Stanislav Brabec]

Karel Zak wrote:
> On Tue, Jan 25, 2022 at 12:37:09PM +0100, Stanislav Brabec wrote:
>> It happens only if more conditions are met:
>> - /var/lib/libuuid exists
>> - uuidd is not running nor socket activated
>> - uuigden --time is called as root
> And vice-versa, if you stop uuidd and start uuidgen as normal user you
> get EACCES for /var/lib/libuuid/clock.txt, but this use-case is
> probably not so important ...
"service uuidd stop" keeps socket activation enabled, so uuidgen 
launches uuidd again. No problem appears.

> What about to create a user specific clock state file if we cannot access
> the default one due to EACCES? Something like:
>
>   state_fd = open(LIBUUID_CLOCK_FILE, O_RDWR|O_CREAT|O_CLOEXEC, 0660);
>   if (state_fd < 0 &7 errno == EACCES) {
>      snprintf(path, "%s-%d", LIBUUID_CLOCK_FILE, getuid());
>      state_fd = open(path, O_RDWR|O_CREAT|O_CLOEXEC, 0660);
>   }
>
> so for the bad use-case:
>
>    /var/lib/libuuid/clock.txt
>    /var/lib/libuuid/clock.txt-<uuidd_uid>
It would work, but:
- It either needs pre-created files or world writable directory.
- It is a predictable name. In case of world writable directory, it is 
easy to perform denial of service.

> in theory it means two clock queues, but if uuidd is running then
> libuuid/getuuid always uses it.
Two clock queues are still better than no clock queue. And once uuidd 
starts, only clock.txt-uuidd will be in use. And it sysadmin decides to 
run system without uuidd, the it is still better than the previous state 
(only root or uuidd can use clock.txt)


Another ideas:
If uid is equal to "uuidd", use /var/lib/libuuid/clock.txt (or change to 
/var/lib/uuidd/clock.txt), else use home directory, e. g. 
~/.libuuid_clock.txt.

Or a separate service file: uuidd-clock.service with:
ExecStart=chown uuidd:uuidd /var/lib/libuuid/clock.txt
(That one would be still vulnerable to race condition attack. And it 
looks ugly.)

I like the home directory solution. It would never make things worse
> The important is that in this way we can be sure uuidd is always
> successful, and I guess the sensitive use-case (SAP;-) is always about
> uuidd. IMHO it's more robust then depend on systemd ExecStartPre (or
> so).
Sure, installations without uuidd can never guarantee absolute uniqueness.

-- 
Best Regards / S pozdravem,

Stanislav Brabec
software developer
---------------------------------------------------------------------
SUSE LINUX, s. r. o.                         e-mail: sbrabec@suse.com
Křižíkova 148/34 (Corso IIa)                    tel: +420 284 084 060
186 00 Praha 8-Karlín                          fax:  +420 284 084 001
Czech Republic                                    http://www.suse.cz/
PGP: 830B 40D5 9E05 35D8 5E27 6FA3 717C 209F A04F CD76

Re: [PATCH] Whitelist libuuid clock file

[Karel Zak]

On Wed, Jan 26, 2022 at 04:07:40PM +0100, Stanislav Brabec wrote:

> Another ideas:
> If uid is equal to "uuidd", use /var/lib/libuuid/clock.txt (or change to
> /var/lib/uuidd/clock.txt), else use home directory, e. g.
> ~/.libuuid_clock.txt.

This is nice idea. 

The file ~/.libuuid_clock.txt should be the default ;-), and we can  
extend __uuid_generate_time() to accept the clock file path as        
argument.  This function is called by uuidd, so the path will be fully
controlled by the daemon, then we can add --clock-state <path> to
overwrite /var/lib/uuidd/clock.txt.

This concept means that we do not have to care about uid at all and
the library (without uuidd) will not try to use any shared directory.

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com