Skip to content

Don't inject private libraries into system applications

Stefan Metzmacher requested to merge samba-team/devel/samba:metze-static into master

Bug: https://bugzilla.samba.org/show_bug.cgi?id=14780

Over the last month I got more and more reports, that it's not possible to use a custom Samba version on systems with sssd being installed, which depends on some specific samba libraries installed in the system.

One major problem is that the custom libnss_winbind.so.2 depends on the libreplace-samba4.so of the custom build and also injects an RPATH into the running process. When sssd uses any nss library call it will get this, when it then tries to load some of its plugins via dlopen(), e.g.

ldd /usr/lib64/sssd/libsss_ad.so| grep samba

   libsamba-util.so.0 => /lib64/libsamba-util.so.0
   libreplace-samba4.so => /usr/lib64/samba/libreplace-samba4.so
   libsamba-security-samba4.so => /usr/lib64/samba/libsamba-security-samba4.so
   libsamba-errors.so.1 => /lib64/libsamba-errors.so.1
   libsamba-debug-samba4.so => /usr/lib64/samba/libsamba-debug-samba4.so
   libgenrand-samba4.so => /usr/lib64/samba/libgenrand-samba4.so
   libsocket-blocking-samba4.so => /usr/lib64/samba/libsocket-blocking-samba4.so
   libtime-basic-samba4.so => /usr/lib64/samba/libtime-basic-samba4.so
   libsys-rw-samba4.so => /usr/lib64/samba/libsys-rw-samba4.so
   libiov-buf-samba4.so => /usr/lib64/samba/libiov-buf-samba4.so

When that loads dlopen() will fail as a soname libreplace-samba4.so is already loaded, but the symbol version within the other one don't match, as the contain the exact version, e.g. replace_dummy@@SAMBA_4.13.3.

This is just an example and similar things can happen in all situations where we provide libraries, which are potentially injected into every process of the running system. These should only depend on libc.so and related basic system libraries in order to avoid the problem.

We have the following libraries, which are in the that category:

- libnss_winbind.so.2
- libnss_wins.so.2
- pam_winbind.so
- winbind_krb5_locator.so
- async_dns_krb5_locator.so

The rules of library loading are really complex and symbol versioning is not enough to solve it, only the combination of unique soname and unique symbol version suffix seem to solve the problem, but injecting an RPATH is still a problem.

In order to solve the problem I experimented with adding SAMBA_SUBSYSTEM() definitions with 'hide_symbols=True' in order to do some static linking of selected components, e.g.

   bld.SAMBA_SUBSYSTEM('replace-hidden',
                       source=REPLACE_SOURCE,
                       group='base_libraries',
                       hide_symbols=True,
                       deps='dl attr' + extra_libs)

It's relatively simple to get to the point where the following are completely static:

- libnss_winbind.so.2
- libnss_wins.so.2
- pam_winbind.so
- winbind_krb5_locator.so

But 'async_dns_krb5_locator.so' links in almost everything! And the krb5 plugins are always loaded without any configuration, every .so in a special pass are loaded with dlopen(), by every application using kerberos, so we load a lot of samba libraries into them.

I think we should disable async_dns_krb5_locator.so by default, or at least install it into a path that's not reachable by libkrb5.so. But it would still allow the admin to create a symlink to activate it. As a longterm solution we may want to change async_dns_krb5_locator.so to use a helper process with posix_spawn() instead of doing everything within the process.

Checklist

  • Commits have Signed-off-by: with name/author being identical to the commit author
  • (optional) This MR is just one part towards a larger feature.
  • (optional, if backport required) Bugzilla bug filed and BUG: tag added
  • Test suite updated with functionality tests
  • Test suite updated with negative tests
  • Documentation updated
  • CI timeout is 3h or higher (see Settings/CICD/General pipelines/ Timeout)

Reviewer's checklist:

  • There is a test suite reasonably covering new functionality or modifications
  • Function naming, parameters, return values, types, etc., are consistent and according to README.Coding.md
  • This feature/change has adequate documentation added
  • No obvious mistakes in the code
Edited by Stefan Metzmacher

Merge request reports