Re: Maybe we should get rid of ifuncs

[Florian Weimer <fweimer@redhat.com>]

* Zack Weinberg:

> I've been thinking about the XZ exploit (two versions of the compression
> library `liblzma` included Trojan horse code that injected a back
> door into sshd; see https://research.swtch.com/xz-timeline) and what
> it means for glibc, and what I've come to is we should reconsider the
> entire idea of ifuncs.
>
> The SSH protocol does not use XZ compression.  liblzma.so was loaded
> into sshd's address space because some Linux distributions patched
> sshd to use libsystemd, and some libsystemd functions (having to do
> with systemd's "journal" logging subsystem, IIUC) do use liblzma, but
> by itself that wouldn't have been enough to give the exploit control,
> because the patched sshd doesn't use any of those functions.  But
> these same Linux distributions also compile libsshd with -z now
> (ironically, as a hardening measure, together with -z relro) and that
> means the resolvers for all the ifuncs in *all* the loaded shared
> libraries will be invoked, early enough in process startup that the
> PLT and GOT are still writable.  The XZ exploit used an ifunc resolver
> to rewrite a whole bunch of PLT entries, intercepting both calls
> within sshd proper, and calls from sshd to libcrypto.so
> (i.e. OpenSSL's general-purpose cryptography library).

GOT rewriting wasn't required.  OpenSSL itself has support for hooking
the relevant functions:

  <https://openssl.org/docs/man3.0/man3/RSA_set_method.html>

For some of the link orders I've seen, plain ELF symbol interposition
would have worked as well.  We don't know if such a thing gets detected
in practice.

> Ifuncs were already a problem -- resolvers are arbitrary application
> code that gets called from deep within the guts of the dynamic loader,
> possibly while internal locks are held (I don't know for sure).

Internal locks are held.  That also happens for ELF constructors (but is
perhaps fixable there).

> In -z now mode, they are called not just before the core C library is
> fully initialized, but before symbol resolution is complete, meaning
> that they can't necessarily make *any* function calls; we've had any
> number of bug reports about this.

We are getting closer to be able to fully initialize libc before IFUNC
resolvers run.  It should be a relatively short patch (a few dozen
lines), at least for Linux.  Since commit 78ca44da0160a0b442f ("elf:
Relocate libc.so early during startup and dlmopen (bug 31083)") we
already relocate libc out of order.

Beyond libc, there are still issues around symbol interposition (or
underlinking) and execution of IFUNC resolvers implemented in
yet-to-be-relocated shared objects.

In the other direction, I think it would be valuable to offer a mode
where we run ELF constructors when .data.rel.ro is still writable.

(In general, I'd be worried to chase last month's problem.)

> As far as I know, the only legitimate (non-malicious) use case anyone
> wants for ifuncs is to allow a library to select one of several
> implementations of a single function, based on the characteristics of
> the CPU -- such as how glibc itself selects the best available
> implementation of `memcpy` for the CPU.  It seems to me that we ought
> to be able to come up with a completely declarative mechanism for this
> use case.

Selection rules for string functions can be quite complicated, depending
not just on advertised CPU capabilities but also on CPU models (and
preferences derived from that).  For each new selection criteria, we'd
have to update glibc to implement it before it becomes usable by
applications.

I'm not sure how valuable it is to prevent code execution at the
relocation stage when a few microseconds later, the ELF constructor is
invoked, which by definition contains arbitrary code.

> And, in -z relro -z now mode, it would mean that no application code
> could run before the PLT and GOT are made read-only, closing the path
> that the XZ trojan used to hook itself into sshd.

It's possible to revert RELRO.  We do that for static dlopen on some
architectures (something that could be avoided with early libc
initialization described above).

Thanks,
Florian