Hi,

On 2019-02-15 18:37, Thorsten Glaser wrote:
> Hi,
> 
> >at first sight I'm not a huge fan of that. LD_PRELOAD and setuid stuff is
> >always a bit tricky, because abusing setuid files (and libraries here) might
> >mean privilege escalation. At lot of attacks in the past just abused setuid
> >binaries to do bad stuff in order to gain root privilege.
> 
> that’s unfortunately true.
> 
> >I'm unsure if and how it can be used with eatmydata, but considering the
> 
> Not sure, I’d think not?
> 
> >Maybe Aurelien and Florian (on team@ but CC:ed just in case) have some input
> >on this too? It might be worth asking opinions on oss-sec as well.
 
[ snip ]

> That means that, yes, it catches keystrokes, but, again, it only
> applies when manually invoked (by adding to LD_PRELOAD), and to

I guess we do not agree on the threat model here. The problem I am
worried about with setting the setuid bit on libeatmydata.so is actually
that it can be used by users to attack existing setuid binaries my
changing the behavior of the glibc. I can imagine two main
possibilities:

1) Using eatmydata, some functions behaves differently. It can be bugs
in eatmydata or it can be by design because the purpose of eatmydata is
to make the function do nothing. Let me give examples after a quick code
review:
 - fsync(), fdatasync(), msync() and sync_file_range() reset errno to 0.
   This never happens in the glibc. This is clearly a bug in eatmydata.
 - fsync() and fdatasync() always succeed when used with eatmydata. In
   the glibc cases, it fails if fd is not a valid file descriptor or if 
   fd is bound to a special file (e.g., a pipe, FIFO, or socket) which
   does not support synchronization. POSIX clearly says that it SHALL
   fail in that case. I agree that using fsync() or fdatasync() to check
   if fd is a valid file descriptor is plainly silly, but I am not sure
   no such code exists.
 - The same way the msync() implementation in eatmydata does not fail in
   some cases, for example if the memory range is not mapped or if the 
   flags are not correct.
 - The same way the sync_file_range() implementation in eatmydata does
   not fail with a wrong fd or if fd refers to something other than a
   regular file, a block device, or a directory. Note that this function
   is linux/glibc specific, so that might less problematic.

2) Using eatmydata, the files are not synced on the disk or the
timestamps are not updated. That is I am not sure of the consequences,
but imagine a setuid program doing something like:
  - writing a config file
  - fdatasync()
  - restart a daemon to use that new config file (can be a simple kill
    -SIGHUP)
In that case the daemon might fail to restart because the config file is
not fully written.

All the above are purely hypothetical cases and I do not have a good
view of which setuid binaries are available on a standard system. Anyway
I would not encourage to set the setuid bit on libeatmydata.so until at
least most of the issues from 1) are fixed. This can be done for example
by running the glibc testsuite with eatmydata and fixing the issues. I
haven't tried recently, but it used to fail, and in the past many people
have reported bugs considering that the bugs were on the glibc side.

Note also that for some use cases, a tmpfs filesystem is a much better
than preloading libeatmydata.so. This is for example used for the build
daemons. It should also probably be used for the autopktests instead of
eatmydata.

Aurelien

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurel...@aurel32.net                 http://www.aurel32.net

Attachment: signature.asc
Description: PGP signature

Reply via email to