selinux: insufficient M4 detection with building static binaries

selinux: insufficient M4 detection with building static binaries

[Assaf Gordon]

Hi,

While exploring build coreutils as static binary
( https://lists.gnu.org/r/coreutils/2019-04/msg00001.html )
I noticed that gnulib's selinux detection is incomplete.

Details:
The m4/selinux-selinux.m4 files checks for 'setfilecon' function like 
so: "AC_SEARCH_LIBS([setfilecon], [selinux], ..."
https://git.savannah.gnu.org/cgit/gnulib.git/tree/m4/selinux-selinux-h.m4#n56

This function can be linked statically,

But cp,mv and install also use "matchpathcon_init_prefix",
which can't to be linked statically (unless selinux was built
for static linking?), and so linking fails.

To reproduce:

   --- se-good.c ---
   extern char setfilecon();
   int main(){return setfilecon();}

   --- se-bad.c ---
   extern char matchpathcon_init_prefix();
   int main(){return matchpathcon_init_prefix();}


   $ gcc -o 1 -static se-good.c -lselinux && echo ok
   ok

   $ gcc -o 1 -static se-bad.c -lselinux
 
/usr/lib/gcc/x86_64-linux-gnu/6/../../../x86_64-linux-gnu/libselinux.a(regex.o): 
In function `regex_writef':
(.text+0x7b): undefined reference to `pcre_fullinfo'
 
/usr/lib/gcc/x86_64-linux-gnu/6/../../../x86_64-linux-gnu/libselinux.a(regex.o): 
In function `regex_writef':
(.text+0xef): undefined reference to `pcre_fullinfo'
   ...
 
/usr/lib/gcc/x86_64-linux-gnu/6/../../../x86_64-linux-gnu/libselinux.a(load_policy.o): 
In function `selinux_mkload_policy':
(.text+0x7cc): undefined reference to `sepol_policy_kern_vers_max'
 
/usr/lib/gcc/x86_64-linux-gnu/6/../../../x86_64-linux-gnu/libselinux.a(load_policy.o): 
In function `selinux_mkload_policy':
(.text+0x7d5): undefined reference to `sepol_policy_kern_vers_min'
   collect2: error: ld returned 1 exit status

And so when building static binaries, SELinux is detected as available,
but linking cp/mv/install fails with the above errors.

---

I see that coreutil's m4/jm-macros.m4 does contain special
checks for "matchpathcon_init_prefix":
https://git.savannah.gnu.org/cgit/coreutils.git/tree/m4/jm-macros.m4#n51

Perhaps it used to be that "matchpathcon_init_prefix" was optional
when building with selinux?

It seems that now it is required.

---

tweaking m4/selinux combinations is beyond my comfort zone...
the following hack at least avoids the issue by detecting that
linking with "matchpathcon_init_prefix" fails, thus automatically
disabling SELinux for static builds:

---
diff --git a/m4/selinux-selinux-h.m4 b/m4/selinux-selinux-h.m4
index 8bbbf0535..a35ce6cf0 100644
--- a/m4/selinux-selinux-h.m4
+++ b/m4/selinux-selinux-h.m4
@@ -56,12 +56,13 @@ AC_DEFUN([gl_LIBSELINUX],
      AC_SEARCH_LIBS([setfilecon], [selinux],
                     [test "$ac_cv_search_setfilecon" = "none required" ||
                      LIB_SELINUX=$ac_cv_search_setfilecon])
+    AC_CHECK_LIB([selinux], [matchpathcon_init_prefix], [], [])
      LIBS=$gl_save_LIBS
    fi
    AC_SUBST([LIB_SELINUX])

    # Warn if SELinux is found but libselinux is absent;
-  if test "$ac_cv_search_setfilecon" = no; then
+  if test "$ac_cv_search_setfilecon" = no || test 
"$ac_cv_lib_selinux_matchpathcon_init_prefix" = no ; then
      if test "$host" = "$build" && test -d /selinux; then
        AC_MSG_WARN([This system supports SELinux but libselinux is 
missing.])
        AC_MSG_WARN([AC_PACKAGE_NAME will be compiled without SELinux 
support.])
---



regards,
  - assaf

Re: selinux: insufficient M4 detection with building static binaries

[Bruno Haible]

Hi Assaf,

>    --- se-good.c ---
>    extern char setfilecon();
>    int main(){return setfilecon();}
> 
>    --- se-bad.c ---
>    extern char matchpathcon_init_prefix();
>    int main(){return matchpathcon_init_prefix();}
> 
> 
>    $ gcc -o 1 -static se-good.c -lselinux && echo ok
>    ok
> 
>    $ gcc -o 1 -static se-bad.c -lselinux

When a shared library depends on other shared libraries, you don't have to
specify the dependencies; ELF cares about that. But when you are linking
statically, you need the dependencies. An installed .la file would provide
this information, but unfortunately the distros don't ship a libselinux.la
file. So you have to put the knowledge about the dependencies into the
.m4 file.

In this case:

$ ldd /lib/x86_64-linux-gnu/libselinux.so.1
        linux-vdso.so.1 =>  (0x00007ffda95ee000)
        libpcre.so.3 => /lib/x86_64-linux-gnu/libpcre.so.3 (0x00007f9f1d00a000)
        libdl.so.2 => /lib/x86_64-linux-gnu/libdl.so.2 (0x00007f9f1ce06000)
        libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007f9f1ca3c000)
        /lib64/ld-linux-x86-64.so.2 (0x00007f9f1d49c000)
        libpthread.so.0 => /lib/x86_64-linux-gnu/libpthread.so.0 (0x00007f9f1c81f000)

So, you see, it depends on -lpcre and -lpthread.

$ gcc -o 1 -static se-bad.c -lselinux
/usr/lib/gcc/x86_64-linux-gnu/5/../../../x86_64-linux-gnu/libselinux.a(label_file.o): In function `closef':
(.text+0x136): undefined reference to `pcre_free'
...

pcre_free is contained in libpcre.

$ gcc -o 1 -static se-bad.c -lselinux -lpcre
/usr/lib/gcc/x86_64-linux-gnu/5/../../../x86_64-linux-gnu/libselinux.a(load_policy.o): In function `selinux_mkload_policy':
(.text+0x11f): undefined reference to `sepol_policy_kern_vers_max'
...

sepol_policy_kern_vers_max is contained in libsepol.

$ gcc -o 1 -static se-bad.c -lselinux -lpcre -lsepol
/usr/lib/gcc/x86_64-linux-gnu/5/../../../x86_64-linux-gnu/libpcre.a(libpcre_la-pcre_jit_compile.o): In function `sljit_generate_code':
(.text+0x6d1): undefined reference to `pthread_mutex_lock'
...

pthread_mutex_lock is contained in libpthread.

$ gcc -o 1 -static se-bad.c -lselinux -lpcre -lsepol -lpthread
/usr/lib/gcc/x86_64-linux-gnu/5/../../../x86_64-linux-gnu/libselinux.a(seusers.o): In function `getseuserbyname':
(.text+0x574): warning: Using 'getgrouplist' in statically linked applications requires at runtime the shared libraries from the glibc version used for linking
/usr/lib/gcc/x86_64-linux-gnu/5/../../../x86_64-linux-gnu/libselinux.a(seusers.o): In function `getseuserbyname':
(.text+0x546): warning: Using 'getgrnam_r' in statically linked applications requires at runtime the shared libraries from the glibc version used for linking
/usr/lib/gcc/x86_64-linux-gnu/5/../../../x86_64-linux-gnu/libselinux.a(seusers.o): In function `getseuserbyname':
(.text+0x433): warning: Using 'getpwnam_r' in statically linked applications requires at runtime the shared libraries from the glibc version used for linking
$ echo $?
0

So, these are the libraries that are needed. Apparently libselinux is
built in such a way that it needs libdl in the shared build, but libsepol
in the static build.

> Perhaps it used to be that "matchpathcon_init_prefix" was optional
> when building with selinux?
> 
> It seems that now it is required.

matchpathcon_init_prefix is declared in gnulib's lib/se-selinux.in.h
since the beginning. It's not a later addition.

> tweaking m4/selinux combinations is beyond my comfort zone...
> the following hack at least avoids the issue by detecting that
> linking with "matchpathcon_init_prefix" fails, thus automatically
> disabling SELinux for static builds:
> 
> ---
> diff --git a/m4/selinux-selinux-h.m4 b/m4/selinux-selinux-h.m4
> index 8bbbf0535..a35ce6cf0 100644
> --- a/m4/selinux-selinux-h.m4
> +++ b/m4/selinux-selinux-h.m4
> @@ -56,12 +56,13 @@ AC_DEFUN([gl_LIBSELINUX],
>       AC_SEARCH_LIBS([setfilecon], [selinux],
>                      [test "$ac_cv_search_setfilecon" = "none required" ||
>                       LIB_SELINUX=$ac_cv_search_setfilecon])
> +    AC_CHECK_LIB([selinux], [matchpathcon_init_prefix], [], [])
>       LIBS=$gl_save_LIBS
>     fi
>     AC_SUBST([LIB_SELINUX])
> 
>     # Warn if SELinux is found but libselinux is absent;
> -  if test "$ac_cv_search_setfilecon" = no; then
> +  if test "$ac_cv_search_setfilecon" = no || test 
> "$ac_cv_lib_selinux_matchpathcon_init_prefix" = no ; then
>       if test "$host" = "$build" && test -d /selinux; then
>         AC_MSG_WARN([This system supports SELinux but libselinux is 
> missing.])
>         AC_MSG_WARN([AC_PACKAGE_NAME will be compiled without SELinux 
> support.])
> ---

It would be better to instead set LIB_SELINUX to
"-lselinux -lpcre -lsepol -lpthread" (in the static case).

Bruno

Re: selinux: insufficient M4 detection with building static binaries

[Assaf Gordon]

Hello Bruno,

Thanks for investigating further and the clear explanation.

On 2019-04-07 5:08 a.m., Bruno Haible wrote:
[....]
> So, these are the libraries that are needed. Apparently libselinux is
> built in such a way that it needs libdl in the shared build, but libsepol
> in the static build.

Indeed, I now see they use dlopen/dlsym when building as a shared
library, but direct linking when static:

https://github.com/SELinuxProject/selinux/blob/5d59284381193d5a6806e7fa310ed4c4064d225d/libselinux/src/load_policy.c#L56

[...]


> It would be better to instead set LIB_SELINUX to
> "-lselinux -lpcre -lsepol -lpthread" (in the static case).

This is a better solution, sadly I do not know how to
implement this in M4 (e.g. detecting a static build,
then deducing the required static libraries...).


regards,
  - assaf

Re: selinux: insufficient M4 detection with building static binaries

[Bruno Haible]

Hi Assaf,

> > It would be better to instead set LIB_SELINUX to
> > "-lselinux -lpcre -lsepol -lpthread" (in the static case).
> 
> This is a better solution, sadly I do not know how to
> implement this in M4 (e.g. detecting a static build,
> then deducing the required static libraries...).

I would not try do implement this by detecting a static build.
I would try to do it by first trying to link only with '-lselinux',
and if that fails, try with '-lselinux -lpcre -lsepol -lpthread'.
Basically, replace

    AC_SEARCH_LIBS([setfilecon], [selinux],
                   [test "$ac_cv_search_setfilecon" = "none required" ||
                    LIB_SELINUX=$ac_cv_search_setfilecon])

with two AC_LINK_IFELSE invocations. And with the function
'matchpathcon_init_prefix' instead of 'setfilecon', of course.

Bruno