Drop Posix Capabilities

PATCH In review Steve Grubb 2 participants 9 messages Sep 25, 2009 – Oct 2, 2009

Subsystems (2)

Bluetooth drivers Kbuild

lore

Share & tools

lore

Steve Grubb

author

2009-09-25 20:47 UTC #0 lore raw

PATCH 1/1 Drop Posix Capabilities

Hello,

The following patch against the 4.54 codebase drops posix capabilities after startup so that the bluetooth daemon is less of a threat to the system should there be any way to compromise it. The retained capabilities was compared to selinux policy to make sure that its roughly the same. It uses the libcap-ng library which allows patches for dropping capabilities to be much smaller.

Signed-off-by Steve Grubb <[email protected]>

acinclude.m4 +33 -0

diff -urp bluez-4.54.orig/acinclude.m4 bluez-4.54/acinclude.m4
@@ -352,3 +352,36 @@ AC_DEFUN([AC_ARG_BLUEZ], [
 	AM_CONDITIONAL(CONFIGFILES, test "${configfiles_enable}" = "yes")
 	AM_CONDITIONAL(CABLE, test "${cable_enable}" = "yes" && test "${cable_found}" = "yes")
 ])
+
+AC_DEFUN([LIBCAP_NG_PATH],
+[
+	AC_ARG_WITH(libcap-ng,
+	[ --with-libcap-ng=[auto/yes/no]  Add Libcap-ng support
+		[default=auto]],, with_libcap_ng=auto)
+
+	if test x$with_libcap_ng = xno ; then
+		have_libcap_ng=no;
+	else
+		# Start by checking for header file
+		AC_CHECK_HEADER(cap-ng.h, capng_headers=yes, capng_headers=no)
+
+		# See if we have libcap-ng library
+		AC_CHECK_LIB(cap-ng, capng_clear, CAPNG_LDADD=-lcap-ng,)
+
+		# Check results are usable
+		if test x$with_libcap_ng = xyes -a x$CAPNG_LDADD = x ; then
+			AC_MSG_ERROR(libcap-ng support was requested and the library was not found)
+		fi
+		if test x$CAPNG_LDADD != x -a $capng_headers = no ; then
+			AC_MSG_ERROR(libcap-ng libraries found but headers are missing)
+		fi
+	fi
+	AC_SUBST(CAPNG_LDADD)
+	AC_MSG_CHECKING(whether to use libcap-ng)
+	if test x$CAPNG_LDADD != x ; then
+		AC_DEFINE(HAVE_LIBCAP_NG,1,[libcap-ng support])
+		AC_MSG_RESULT(yes)
+	else
+		AC_MSG_RESULT(no)
+	fi
+])

configure.ac +1 -0

diff -urp bluez-4.54.orig/configure.ac bluez-4.54/configure.ac
@@ -45,6 +45,7 @@ AC_PATH_NETLINK
 AC_PATH_SNDFILE
 
 AC_ARG_BLUEZ
+LIBCAP_NG_PATH
 
 AC_OUTPUT(Makefile scripts/bluetooth.rules doc/version.xml
 					src/bluetoothd.8 bluez.pc)

Makefile.am +2 -1

diff -urp bluez-4.54.orig/Makefile.am bluez-4.54/Makefile.am
@@ -200,7 +200,8 @@ src_bluetoothd_SOURCES = $(gdbus_sources
 			src/device.h src/device.c \
 			src/dbus-common.c src/dbus-common.h \
 			src/dbus-hci.h src/dbus-hci.c
-src_bluetoothd_LDADD = lib/libbluetooth.la @GLIB_LIBS@ @DBUS_LIBS@ -ldl
+src_bluetoothd_LDADD = lib/libbluetooth.la @GLIB_LIBS@ @DBUS_LIBS@ \
+				@CAPNG_LDADD@ -ldl
 src_bluetoothd_LDFLAGS = -Wl,--export-dynamic \
 					-Wl,--version-script=src/bluetooth.ver
 src_bluetoothd_DEPENDENCIES = src/bluetooth.ver lib/libbluetooth.la

src/main.c +11 -0

diff -urp bluez-4.54.orig/src/main.c bluez-4.54/src/main.c
@@ -55,6 +55,9 @@
 #include "dbus-common.h"
 #include "agent.h"
 #include "manager.h"
+#ifdef HAVE_LIBCAP_NG
+#include <cap-ng.h>
+#endif
 
 #define LAST_ADAPTER_EXIT_TIMEOUT 30

@@ -343,6 +346,14 @@ int main(int argc, char *argv[])
 	GKeyFile *config;
 
 	init_defaults();
+#ifdef HAVE_LIBCAP_NG
+	/* Drop capabilities */
+	capng_clear(CAPNG_SELECT_BOTH);
+	capng_updatev(CAPNG_ADD, CAPNG_EFFECTIVE|CAPNG_PERMITTED,
+			CAP_NET_BIND_SERVICE, CAP_NET_ADMIN, CAP_NET_RAW,
+			CAP_IPC_LOCK, -1);
+	capng_apply(CAPNG_SELECT_BOTH);
+#endif
 
 	context = g_option_context_new(NULL);
 	g_option_context_add_main_entries(context, options, NULL);

Marcel Holtmann

maintainer

2009-09-25 21:35 UTC #1 lore raw

Hi Steve,

Quoted 14 lines

The following patch against the 4.54 codebase drops posix capabilities after startup so that the bluetooth daemon is less of a threat to the system should there be any way to compromise it. The retained capabilities was compared to selinux policy to make sure that its roughly the same. It uses the libcap-ng library which allows patches for dropping capabilities to be much smaller.
Signed-off-by Steve Grubb <[email protected]>
diff -urp bluez-4.54.orig/acinclude.m4 bluez-4.54/acinclude.m4

acinclude.m4 AC_DEFUN([AC_ARG_BLUEZ], [ · line 354

Quoted 3 lines

acinclude.m4 +0 -0

@@ -352,3 +352,36 @@ AC_DEFUN([AC_ARG_BLUEZ], [
 	AM_CONDITIONAL(CONFIGFILES, test "${configfiles_enable}" = "yes")
 	AM_CONDITIONAL(CABLE, test "${cable_enable}" = "yes" && test "${cable_found}" = "yes")
 ])

please base the patch against upstream and not the Fedora package.

acinclude.m4 AC_DEFUN([AC_ARG_BLUEZ], [ · line 361

Quoted 6 lines

acinclude.m4 +6 -0

+AC_DEFUN([LIBCAP_NG_PATH],
+[
+	AC_ARG_WITH(libcap-ng,
+	[ --with-libcap-ng=[auto/yes/no]  Add Libcap-ng support
+		[default=auto]],, with_libcap_ng=auto)
+

I mention this to Bastien already. I want pkgconfig support here and not some old style aclocal magic.

Regards

Marcel

Steve Grubb

author

2009-09-26 14:29 UTC #2 lore raw

Quoted 9 lines · Marcel Holtmann

acinclude.m4 2009-09-25 16:38:32.000000000 -0400 AC_DEFUN([AC_ARG_BLUEZ], [ · line 354
Quoted 7 lines
diff -urp bluez-4.54.orig/acinclude.m4 bluez-4.54/acinclude.m4
acinclude.m4 2009-09-25 16:38:32.000000000 -0400 renamed from acinclude.m4 2009-09-25 11:33:47.000000000 -0400 +0 -0
@@ -352,3 +352,36 @@ AC_DEFUN([AC_ARG_BLUEZ], [
      AM_CONDITIONAL(CONFIGFILES, test "${configfiles_enable}" = "yes")
      AM_CONDITIONAL(CABLE, test "${cable_enable}" = "yes" && test
 "${cable_found}" = "yes") ])
please base the patch against upstream and not the Fedora package.

I just retried against the 4.54 tarball and it applies, but with some fuzz.

patch -p1 < ../bluez-4.54-capability.patch patching file acinclude.m4
Hunk #1 succeeded at 336 with fuzz 2 (offset -16 lines). patching file configure.ac
Hunk #1 succeeded at 44 (offset -1 lines). patching file Makefile.am
Hunk #1 succeeded at 191 (offset -9 lines). patching file src/main.c

Quoted 9 lines

acinclude.m4 2009-09-25 16:38:32.000000000 -0400 AC_DEFUN([AC_ARG_BLUEZ], [ · line 360
Quoted 6 lines
acinclude.m4 2009-09-25 16:38:32.000000000 -0400 renamed from acinclude.m4 2009-09-25 11:33:47.000000000 -0400 +6 -0
+AC_DEFUN([LIBCAP_NG_PATH],
+[
+     AC_ARG_WITH(libcap-ng,
+     [ --with-libcap-ng=[auto/yes/no]  Add Libcap-ng support
+             [default=auto]],, with_libcap_ng=auto)
+
I mention this to Bastien already. I want pkgconfig support here and not some old style aclocal magic.

libcap-ng itself does not provide any pkgconfig support. I'll check on this.

-Steve

Marcel Holtmann

maintainer

2009-09-27 20:31 UTC #3 lore raw

Hi Steve,

Quoted 12 lines

Quoted 9 lines
acinclude.m4 2009-09-25 16:38:32.000000000 -0400 AC_DEFUN([AC_ARG_BLUEZ], [ · line 354
Quoted 7 lines
diff -urp bluez-4.54.orig/acinclude.m4 bluez-4.54/acinclude.m4
acinclude.m4 2009-09-25 16:38:32.000000000 -0400 renamed from acinclude.m4 2009-09-25 11:33:47.000000000 -0400 +0 -0
@@ -352,3 +352,36 @@ AC_DEFUN([AC_ARG_BLUEZ], [
      AM_CONDITIONAL(CONFIGFILES, test "${configfiles_enable}" = "yes")
      AM_CONDITIONAL(CABLE, test "${cable_enable}" = "yes" && test
 "${cable_found}" = "yes") ])
please base the patch against upstream and not the Fedora package.
I just retried against the 4.54 tarball and it applies, but with some fuzz.

Quoted 8 lines

patch -p1 < ../bluez-4.54-capability.patch patching file acinclude.m4
Hunk #1 succeeded at 336 with fuzz 2 (offset -16 lines). patching file configure.ac
Hunk #1 succeeded at 44 (offset -1 lines). patching file Makefile.am
Hunk #1 succeeded at 191 (offset -9 lines). patching file src/main.c

and git am will not like fuzz that much. So I prefer to have patches against the upstream GIT repository and not the Fedora tree.

Quoted 11 lines

Quoted 9 lines
acinclude.m4 2009-09-25 16:38:32.000000000 -0400 AC_DEFUN([AC_ARG_BLUEZ], [ · line 360
Quoted 6 lines
acinclude.m4 2009-09-25 16:38:32.000000000 -0400 renamed from acinclude.m4 2009-09-25 11:33:47.000000000 -0400 +6 -0
+AC_DEFUN([LIBCAP_NG_PATH],
+[
+     AC_ARG_WITH(libcap-ng,
+     [ --with-libcap-ng=[auto/yes/no]  Add Libcap-ng support
+             [default=auto]],, with_libcap_ng=auto)
+
I mention this to Bastien already. I want pkgconfig support here and not some old style aclocal magic.
libcap-ng itself does not provide any pkgconfig support. I'll check on this.

According to the bug you guys have open for this, you try to fix this upstream in libcap-ng. So please push a pkgconfig file to libcap-ng and then I am more than happy to accept a patch for this.

I like to have capability dropping in bluetoothd, but I do wanna do it with a proper upstream project. And not having pkgconfig support in library is just not cutting it anymore. Since the project seems to be maintained by you, it should be easy to fix ;)

Regards

Marcel

Steve Grubb

author

2009-09-28 21:31 UTC #4 lore raw

Quoted 10 lines · Marcel Holtmann

Quoted 10 lines
Quoted 9 lines
acinclude.m4 AC_DEFUN([AC_ARG_BLUEZ], [ · line 361
Quoted 6 lines
acinclude.m4 +6 -0
+AC_DEFUN([LIBCAP_NG_PATH],
+[
+     AC_ARG_WITH(libcap-ng,
+     [ --with-libcap-ng=[auto/yes/no]  Add Libcap-ng support
+             [default=auto]],, with_libcap_ng=auto)
+
I mention this to Bastien already. I want pkgconfig support here and not some old style aclocal magic.

Quoted 6 lines

Quoted 2 lines
libcap-ng itself does not provide any pkgconfig support. I'll check on this.
According to the bug you guys have open for this, you try to fix this upstream in libcap-ng. So please push a pkgconfig file to libcap-ng and then I am more than happy to accept a patch for this.

The bug report only said pkg-config support might be nice, but it was not specified as a requirement. Anyways, I made a new release of libcap-ng today with a pkg-config file added. There are no other user of that file, so please let me know if something is wrong with it.

http://people.redhat.com/sgrubb/libcap-ng/libcap-ng-0.6.2.tar.gz

Quoted 2 lines

I like to have capability dropping in bluetoothd, but I do wanna do it with a proper upstream project.

one other thing I thought I would point out. The patch I sent can make it easy to run the bluetooth daemon as non-root user. If we switch this line:

capng_apply(CAPNG_SELECT_BOTH);

capng_change_id(uid, gid, CAPNG_DROP_SUPP_GRP | CAPNG_CLEAR_BOUNDING);

then the job is easier. Of course you would likely need to fixup file permissions in places, but in theory a non-root bluetooth daemon is possible with a 1 line change in the patch. You would probably want to add error handling and a way to specify the uid/gid, too.

-Steve

Marcel Holtmann

maintainer

2009-09-28 23:40 UTC #5 lore raw

Hi Steve,

Quoted 17 lines

Quoted 10 lines
Quoted 10 lines
Quoted 9 lines
acinclude.m4 AC_DEFUN([AC_ARG_BLUEZ], [ · line 361
Quoted 6 lines
acinclude.m4 +6 -0
+AC_DEFUN([LIBCAP_NG_PATH],
+[
+     AC_ARG_WITH(libcap-ng,
+     [ --with-libcap-ng=[auto/yes/no]  Add Libcap-ng support
+             [default=auto]],, with_libcap_ng=auto)
+
I mention this to Bastien already. I want pkgconfig support here and not some old style aclocal magic.
Quoted 6 lines
Quoted 2 lines
libcap-ng itself does not provide any pkgconfig support. I'll check on this.
According to the bug you guys have open for this, you try to fix this upstream in libcap-ng. So please push a pkgconfig file to libcap-ng and then I am more than happy to accept a patch for this.

Quoted 6 lines

The bug report only said pkg-config support might be nice, but it was not specified as a requirement. Anyways, I made a new release of libcap-ng today with a pkg-config file added. There are no other user of that file, so please let me know if something is wrong with it.
http://people.redhat.com/sgrubb/libcap-ng/libcap-ng-0.6.2.tar.gz

I might have mentioned it only to Bastien, but for me it is a hard requirement since fiddling around with m4 files is just stupid and actually pretty much painful. We are in the 21st century and should do everything nicely with pkg-config :)

I quickly looked over the pc.in file and it looks good to me. Thanks for adding this. Another question that comes to my mind. Where do you have the GIT tree for libcap-ng so we can track the development?

Quoted 8 lines

Quoted 2 lines
I like to have capability dropping in bluetoothd, but I do wanna do it with a proper upstream project.
one other thing I thought I would point out. The patch I sent can make it easy to run the bluetooth daemon as non-root user. If we switch this line:
capng_apply(CAPNG_SELECT_BOTH);

Quoted 8 lines

to
capng_change_id(uid, gid, CAPNG_DROP_SUPP_GRP | CAPNG_CLEAR_BOUNDING);
then the job is easier. Of course you would likely need to fixup file permissions in places, but in theory a non-root bluetooth daemon is possible with a 1 line change in the patch. You would probably want to add error handling and a way to specify the uid/gid, too.

I am not really sold on the non-root daemon idea and there might be hidden problems where this will not work out. However I don't mind trying at some point, but there are other things to sort out first. We should postpone this for the 5.x series.

Please re-send the original patch using pkg-config so I can go ahead an apply it. Even if Rawhide is not carrying the updated libcap-ng package.

Regards

Marcel

Steve Grubb

author

2009-09-29 13:00 UTC #6 lore raw

Quoted 1 line · Marcel Holtmann

I quickly looked over the pc.in file and it looks good to me.

Thanks.

Quoted 2 lines

Thanks foradding this. Another question that comes to my mind. Where do you have the GIT tree for libcap-ng so we can track the development?

I've never needed to use git - so no git tree. The project has a home page
here: http://people.redhat.com/sgrubb/libcap-ng/
and I announce package updates at freshmeat.net. The libcap-ng package is stable and I would not have made a release yesterday if it weren't for needing to a pc file. I don't forsee much development in libcap-ng unless there are updates in the kernel that I need to take into account. IOW, all planned features are complete and I'm not tracking any bugs.

Quoted 17 lines

Quoted 8 lines
Quoted 2 lines
I like to have capability dropping in bluetoothd, but I do wanna do it with a proper upstream project.
one other thing I thought I would point out. The patch I sent can make it easy to run the bluetooth daemon as non-root user. If we switch this line:
capng_apply(CAPNG_SELECT_BOTH);
Quoted 8 lines
to
capng_change_id(uid, gid, CAPNG_DROP_SUPP_GRP | CAPNG_CLEAR_BOUNDING);
then the job is easier. Of course you would likely need to fixup file permissions in places, but in theory a non-root bluetooth daemon is possible with a 1 line change in the patch. You would probably want to add error handling and a way to specify the uid/gid, too.

Quoted 4 lines

I am not really sold on the non-root daemon idea and there might be hidden problems where this will not work out. However I don't mind trying at some point, but there are other things to sort out first. We should postpone this for the 5.x series.

Sure, I just wanted to point out that its a 1 line change in code if you ever wanted to do this.

Quoted 2 lines

Please re-send the original patch using pkg-config so I can go ahead an apply it. Even if Rawhide is not carrying the updated libcap-ng package.

OK, as soon as I figure out pkg-config. M4 is easier. :)

-Steve

Marcel Holtmann

maintainer

2009-09-29 18:03 UTC #7 lore raw

Hi Steve,

Quoted 10 lines

Quoted 2 lines
Thanks foradding this. Another question that comes to my mind. Where do you have the GIT tree for libcap-ng so we can track the development?
I've never needed to use git - so no git tree. The project has a home page
here: http://people.redhat.com/sgrubb/libcap-ng/
and I announce package updates at freshmeat.net. The libcap-ng package is stable and I would not have made a release yesterday if it weren't for needing to a pc file. I don't forsee much development in libcap-ng unless there are updates in the kernel that I need to take into account. IOW, all planned features are complete and I'm not tracking any bugs.

from a security point of it is nice if you have a source code repository with version tracking. Only thinking about your security team if something might happen :)

Quoted 22 lines

Quoted 17 lines
Quoted 8 lines
Quoted 2 lines
I like to have capability dropping in bluetoothd, but I do wanna do it with a proper upstream project.
one other thing I thought I would point out. The patch I sent can make it easy to run the bluetooth daemon as non-root user. If we switch this line:
capng_apply(CAPNG_SELECT_BOTH);
Quoted 8 lines
to
capng_change_id(uid, gid, CAPNG_DROP_SUPP_GRP | CAPNG_CLEAR_BOUNDING);
then the job is easier. Of course you would likely need to fixup file permissions in places, but in theory a non-root bluetooth daemon is possible with a 1 line change in the patch. You would probably want to add error handling and a way to specify the uid/gid, too.
Quoted 4 lines
I am not really sold on the non-root daemon idea and there might be hidden problems where this will not work out. However I don't mind trying at some point, but there are other things to sort out first. We should postpone this for the 5.x series.

Quoted 8 lines

Sure, I just wanted to point out that its a 1 line change in code if you ever wanted to do this.
Quoted 2 lines
Please re-send the original patch using pkg-config so I can go ahead an apply it. Even if Rawhide is not carrying the updated libcap-ng package.
OK, as soon as I figure out pkg-config. M4 is easier. :)

But the number of people who can read M4 getting less and less. I can read it, but then nobody else inside the team will have a real clue. Just send me a patch without the pkg-config magic and I can add that for you after I applied the patch.

Regards

Marcel

Marcel Holtmann

maintainer

2009-10-02 09:46 UTC #8 lore raw

In reply to Steve Grubb

Hi Steve,

Quoted 6 lines

The following patch against the 4.54 codebase drops posix capabilities after startup so that the bluetooth daemon is less of a threat to the system should there be any way to compromise it. The retained capabilities was compared to selinux policy to make sure that its roughly the same. It uses the libcap-ng library which allows patches for dropping capabilities to be much smaller.

so I went through the patch and applied it with using pkg-config support. So if you would update libcap-ng in Fedora 11, I would be able to test it. Otherwise it has to wait until I actually switch to the next release ;)

Regards

Marcel