Repository: JonathonReinhart/staticx (language: python)

Issue:
Fix broken nssfix build on newer glibc
Previously, `libnssfix.so` would [unconditionally link against](https://github.com/JonathonReinhart/staticx/blob/fe8c1e8de9b0ef7201ca9dcfcfeef5e311ce26f5/libnssfix/SConscript#L57) `libnss_files` and `libnss_dns`, as those are the only two libraries ("services" in NSS parlance)  specified in our [`nsswitch.conf`](https://github.com/JonathonReinhart/staticx/blob/main/libnssfix/nsswitch.conf).

As of GLIBC 2.34, the `files` and `dns` NSS "modules" are built-in to libc (https://github.com/bminor/glibc/commit/6212bb67f4695962748a5981e1b9fea105af74f6 and https://github.com/bminor/glibc/commit/e1fcf21474c5b522fdad4ac0191d5dcc3271dba6). While `libnss_files.so.2` and `libnss_dns.so.2` still exist, they are empty stubs (bminor/glibc@9ed48feed8c268e98baf00f3608d85dafb8215f3), and there are no symlinks without `.2`, which causes a link failure for us.

This PR adds a configure step to determine which NSS modules are built-in, by simply trying to link against them. Modules which are built-in are excluded from the link when building `libnssfix.so`. Because SCons/scons#4373 silently causes a false negative, we add another basic conftest called `BasicCheckLib()`.

Finally, we update the CI configuration to build under *both* Ubuntu 20.04 and 22.04. This required a creative update to the `build` job matrix, to  exclude the right Python version from the deadsnakes/action setup.

Fixes #245
Fix broken nssfix build on newer glibc
Previously, `libnssfix.so` would [unconditionally link against](https://github.com/JonathonReinhart/staticx/blob/fe8c1e8de9b0ef7201ca9dcfcfeef5e311ce26f5/libnssfix/SConscript#L57) `libnss_files` and `libnss_dns`, as those are the only two libraries ("services" in NSS parlance)  specified in our [`nsswitch.conf`](https://github.com/JonathonReinhart/staticx/blob/main/libnssfix/nsswitch.conf).

As of GLIBC 2.34, the `files` and `dns` NSS "modules" are built-in to libc (https://github.com/bminor/glibc/commit/6212bb67f4695962748a5981e1b9fea105af74f6 and https://github.com/bminor/glibc/commit/e1fcf21474c5b522fdad4ac0191d5dcc3271dba6). While `libnss_files.so.2` and `libnss_dns.so.2` still exist, they are empty stubs (bminor/glibc@9ed48feed8c268e98baf00f3608d85dafb8215f3), and there are no symlinks without `.2`, which causes a link failure for us.

This PR adds a configure step to determine which NSS modules are built-in, by simply trying to link against them. Modules which are built-in are excluded from the link when building `libnssfix.so`. Because SCons/scons#4373 silently causes a false negative, we add another basic conftest called `BasicCheckLib()`.

Finally, we update the CI configuration to build under *both* Ubuntu 20.04 and 22.04. This required a creative update to the `build` job matrix, to  exclude the right Python version from the deadsnakes/action setup.

Fixes #245


Discussion / hints:
It dawned on me that this probably doesn't really fix the problem. Yes, it fixes building staticx under new GLIBC>=2.34, but I think it breaks things  in the opposite way when using that staticx (w/ libnssfix built against GLIBC>=2.34) on a system with older GLIBC < 2.34:
- Staticx built on GLIBC >= 2.34:
  - `libnss_files` and `libnss_dns` builtin and thus not present
  - `libnssfix.so` *not* linked against them
- Staticx runs to bundle an app linked against GLIBC < 2.34:
  - glibc hook detects glibc app and injects `libnssfix` dependency
  - glibc hook runs ldd on `libnssfix.so` and finds no other deps.
  - `libnss_files` and `libnss_dns` are *not* included in the archive.
- Staticx-ified app runs and calls `getXXbyYY()`
  - Bundled libc loads `libnss_files.so` from the host system 🐛 



This is from a process I was stepping through (`test/nss-isolated/build/app.sx`) with GDB:

```gdb
(gdb) where
#0  _nss_files_getpwuid_r (uid=1000, result=0x7ffff7fc2840 <resbuf>, buffer=0x55555555ba70 "", buflen=1024,
    errnop=0x7ffff7dee6c0) at nss_files/files-XXX.c:77
#1  0x00007ffff7eb7e23 in __getpwuid_r (uid=uid@entry=1000, resbuf=resbuf@entry=0x7ffff7fc2840 <resbuf>,
    buffer=0x55555555ba70 "", buflen=buflen@entry=1024, result=result@entry=0x7fffffffe200) at ../nss/getXXbyYY_r.c:315
#2  0x00007ffff7eb7513 in getpwuid (uid=1000) at ../nss/getXXbyYY.c:134
#3  0x0000555555555384 in test_passwd ()
#4  0x00005555555555fb in run_tests ()
#5  0x00005555555559b1 in main ()
```

```console
$ awk '{ print $6 }' /proc/19393/maps | sort -u

[heap]
[stack]
/tmp/staticx-gLhGjp/app
/tmp/staticx-gLhGjp/ld-2.31.so
/tmp/staticx-gLhGjp/libc-2.31.so
/tmp/staticx-gLhGjp/libnssfix.so
/usr/lib/x86_64-linux-gnu/libnss_files-2.31.so
[vdso]
[vvar]
```

I was wondering why none of my tests caught this. Particularly when testing locally (Debian 11, glibc 2.31) with `staticx 0.14.0` preview.

The problem is that I  didn't have `TEST_DOCKER_IMAGE` set, so the test that is designed to catch this did not run.



```console
$ TEST_DOCKER_IMAGE=centos:5  ./run_test.sh
...
Running test: parent
Error: Failed to look up user for uid=0
```
It dawned on me that this probably doesn't really fix the problem. Yes, it fixes building staticx under new GLIBC>=2.34, but I think it breaks things  in the opposite way when using that staticx (w/ libnssfix built against GLIBC>=2.34) on a system with older GLIBC < 2.34:
- Staticx built on GLIBC >= 2.34:
  - `libnss_files` and `libnss_dns` builtin and thus not present
  - `libnssfix.so` *not* linked against them
- Staticx runs to bundle an app linked against GLIBC < 2.34:
  - glibc hook detects glibc app and injects `libnssfix` dependency
  - glibc hook runs ldd on `libnssfix.so` and finds no other deps.
  - `libnss_files` and `libnss_dns` are *not* included in the archive.
- Staticx-ified app runs and calls `getXXbyYY()`
  - Bundled libc loads `libnss_files.so` from the host system 🐛 



This is from a process I was stepping through (`test/nss-isolated/build/app.sx`) with GDB:

```gdb
(gdb) where
#0  _nss_files_getpwuid_r (uid=1000, result=0x7ffff7fc2840 <resbuf>, buffer=0x55555555ba70 "", buflen=1024,
    errnop=0x7ffff7dee6c0) at nss_files/files-XXX.c:77
#1  0x00007ffff7eb7e23 in __getpwuid_r (uid=uid@entry=1000, resbuf=resbuf@entry=0x7ffff7fc2840 <resbuf>,
    buffer=0x55555555ba70 "", buflen=buflen@entry=1024, result=result@entry=0x7fffffffe200) at ../nss/getXXbyYY_r.c:315
#2  0x00007ffff7eb7513 in getpwuid (uid=1000) at ../nss/getXXbyYY.c:134
#3  0x0000555555555384 in test_passwd ()
#4  0x00005555555555fb in run_tests ()
#5  0x00005555555559b1 in main ()
```

```console
$ awk '{ print $6 }' /proc/19393/maps | sort -u

[heap]
[stack]
/tmp/staticx-gLhGjp/app
/tmp/staticx-gLhGjp/ld-2.31.so
/tmp/staticx-gLhGjp/libc-2.31.so
/tmp/staticx-gLhGjp/libnssfix.so
/usr/lib/x86_64-linux-gnu/libnss_files-2.31.so
[vdso]
[vvar]
```

I was wondering why none of my tests caught this. Particularly when testing locally (Debian 11, glibc 2.31) with `staticx 0.14.0` preview.

The problem is that I  didn't have `TEST_DOCKER_IMAGE` set, so the test that is designed to catch this did not run.



```console
$ TEST_DOCKER_IMAGE=centos:5  ./run_test.sh
...
Running test: parent
Error: Failed to look up user for uid=0
```

Fix patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index c0a1a89..03e76da 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,11 @@
 All notable changes to this project will be documented in this file.
 This project adheres to [Semantic Versioning](https://semver.org/).
 
+## [Unreleased]
+### Fixed
+- Reverted invalid fix ([#255]) for libnssfix link failure ([#262])
+
+
 ## [0.14.0] - 2023-07-10
 ### Fixed
 - Fixed issue causing libnssfix link failure when building on GLIBC 2.34 ([#255])
@@ -339,3 +344,4 @@ Initial release
 [#238]: https://github.com/JonathonReinhart/staticx/pull/238
 [#247]: https://github.com/JonathonReinhart/staticx/pull/247
 [#255]: https://github.com/JonathonReinhart/staticx/pull/255
+[#262]: https://github.com/JonathonReinhart/staticx/pull/262
diff --git a/SConstruct b/SConstruct
index f3d4325..44bb3b2 100644
--- a/SConstruct
+++ b/SConstruct
@@ -66,18 +66,6 @@ conf = base_env.Configure(custom_tests=custom_tests)
 # Does our default libc have NSS?
 conf.env["HAS_NSS"] = conf.CheckNSS()
 
-# Starting in GLIBC 2.34, some NSS modules (files, dns) can be built-in to
-# libc.so. Determine which ones are builtin. See #245.
-conf.env["NSS_MODS_BUILTIN"] = []
-if conf.env["HAS_NSS"]:
-    maybe_builtin_nss_modules = ["files", "dns"]
-    for mod in maybe_builtin_nss_modules:
-        libname = "libnss_" + mod
-        # It the library doesn't exist, assume it is built-in.
-        if not conf.BasicCheckLib(libname):
-            conf.env.Append(NSS_MODS_BUILTIN=libname)
-    print("Builtin NSS modules:", conf.env["NSS_MODS_BUILTIN"])
-
 base_env = conf.Finish()
 
 ################################################################################
diff --git a/libnssfix/SConscript b/libnssfix/SConscript
index 7d4912f..08f2fa6 100644
--- a/libnssfix/SConscript
+++ b/libnssfix/SConscript
@@ -22,16 +22,6 @@ env.NsswitchConfH(
     source = nsswitch_conf,
 )
 
-
-# Link against the configured NSS module libraries.
-# These aren't needed directly for the library itself, but this ensures
-# that they will be available for nss at runtime (via dlopen), and
-# allows their paths to be easily discovered via ldd(1).
-nss_libs = set(env.GetNsswitchLibs(nsswitch_conf))
-
-# Exclude those already built-in to glibc.
-nss_libs -= set(env["NSS_MODS_BUILTIN"])
-
 # Build libnssfix.so
 libnssfix = env.SharedLibrary(
     target = 'nssfix',
@@ -60,7 +50,12 @@ libnssfix = env.SharedLibrary(
     ],
     LIBS = [
         libc_stub,
-    ] + list(nss_libs),
+
+        # These aren't needed directly for the library itself, but this ensures
+        # that they will be available for nss at runtime (via dlopen), and
+        # allows their paths to be easily discovered via ldd(1).
+        env.GetNsswitchLibs(nsswitch_conf),
+    ],
 )
 
 Return('libnssfix')


Test patch:
diff --git a/.github/workflows/build-test.yml b/.github/workflows/build-test.yml
index 58fa030..d82a86d 100644
--- a/.github/workflows/build-test.yml
+++ b/.github/workflows/build-test.yml
@@ -14,8 +14,6 @@ jobs:
         os:
           - version: "ubuntu-20.04"
             native-python: "3.8"
-          - version: "ubuntu-22.04"
-            native-python: "3.10"
         python-version:
           - "3.7"
           - "3.8"
diff --git a/site_scons/conftest.py b/site_scons/conftest.py
index fb139dd..dc09809 100644
--- a/site_scons/conftest.py
+++ b/site_scons/conftest.py
@@ -14,26 +14,6 @@ def CheckNSS(context):
     return ok
 
 
-def BasicCheckLib(context, libname):
-    """Check for a C library.
-
-    Similar to built-in CheckLib but works around
-    https://github.com/SCons/scons/issues/4373.
-    """
-    context.Message("Checking for library %s... " % libname)
-
-    oldLIBS = context.AppendLIBS([libname])
-    ok = context.TryLink(
-        "int main(void) { return 0; }",
-        ".c"
-    )
-    context.SetLIBS(oldLIBS)
-
-    context.Result(ok)
-    return ok
-
-
 custom_tests = dict(
     CheckNSS = CheckNSS,
-    BasicCheckLib = BasicCheckLib,
 )