[PATCH] Drop Posix Capabilities

[PATCH] Drop Posix Capabilities

[Steve Grubb]

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 <sgrubb@redhat.com>


diff -urp bluez-4.54.orig/acinclude.m4 bluez-4.54/acinclude.m4
--- bluez-4.54.orig/acinclude.m4	2009-09-25 11:33:47.000000000 -0400
+++ bluez-4.54/acinclude.m4	2009-09-25 16:38:32.000000000 -0400
@@ -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
+])
diff -urp bluez-4.54.orig/configure.ac bluez-4.54/configure.ac
--- bluez-4.54.orig/configure.ac	2009-09-25 11:33:47.000000000 -0400
+++ bluez-4.54/configure.ac	2009-09-25 16:38:32.000000000 -0400
@@ -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)
diff -urp bluez-4.54.orig/Makefile.am bluez-4.54/Makefile.am
--- bluez-4.54.orig/Makefile.am	2009-09-25 11:33:47.000000000 -0400
+++ bluez-4.54/Makefile.am	2009-09-25 16:39:11.000000000 -0400
@@ -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
diff -urp bluez-4.54.orig/src/main.c bluez-4.54/src/main.c
--- bluez-4.54.orig/src/main.c	2009-09-25 11:33:47.000000000 -0400
+++ bluez-4.54/src/main.c	2009-09-25 16:38:32.000000000 -0400
@@ -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);

Re: [PATCH] Drop Posix Capabilities

[Marcel Holtmann]

Hi Steve,

> 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 <sgrubb@redhat.com>
> 
> 
> diff -urp bluez-4.54.orig/acinclude.m4 bluez-4.54/acinclude.m4
> --- bluez-4.54.orig/acinclude.m4	2009-09-25 11:33:47.000000000 -0400
> +++ bluez-4.54/acinclude.m4	2009-09-25 16:38:32.000000000 -0400
> @@ -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.

> +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

Re: [PATCH] Drop Posix Capabilities

[Steve Grubb]

On Friday 25 September 2009 05:35:21 pm Marcel Holtmann wrote:
> > diff -urp bluez-4.54.orig/acinclude.m4 bluez-4.54/acinclude.m4
> > --- bluez-4.54.orig/acinclude.m4      2009-09-25 11:33:47.000000000 -0400
> > +++ bluez-4.54/acinclude.m4   2009-09-25 16:38:32.000000000 -0400
> > @@ -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


> > +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

Re: [PATCH] Drop Posix Capabilities

[Marcel Holtmann]

Hi Steve,

> > > diff -urp bluez-4.54.orig/acinclude.m4 bluez-4.54/acinclude.m4
> > > --- bluez-4.54.orig/acinclude.m4      2009-09-25 11:33:47.000000000 -0400
> > > +++ bluez-4.54/acinclude.m4   2009-09-25 16:38:32.000000000 -0400
> > > @@ -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

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.

> > > +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

Re: [PATCH] Drop Posix Capabilities

[Steve Grubb]

On Sunday 27 September 2009 04:31:25 pm Marcel Holtmann wrote:
> > > > +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.

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 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);

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.

-Steve

Re: [PATCH] Drop Posix Capabilities

[Marcel Holtmann]

Hi Steve,

> > > > > +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.
> 
> 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?

> > 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);
> 
> 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

Re: [PATCH] Drop Posix Capabilities

[Steve Grubb]

On Monday 28 September 2009 07:40:53 pm Marcel Holtmann wrote:
> I quickly looked over the pc.in file and it looks good to me.

Thanks.

> 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.

 
> > > 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);
> > 
> > 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.

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


> 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

Re: [PATCH] Drop Posix Capabilities

[Marcel Holtmann]

Hi Steve,

> > 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 :)
 
> > > > 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);
> > > 
> > > 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.
> 
> Sure, I just wanted to point out that its a 1 line change in code if you ever 
> wanted to do this.
> 
> 
> > 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

Re: [PATCH] Drop Posix Capabilities

[Marcel Holtmann]

Hi Steve,

> 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