2009-09-25 20:47:15

by Steve Grubb

[permalink] [raw]
Subject: [PATCH] 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]>


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 @[email protected] @[email protected] -ldl
+src_bluetoothd_LDADD = lib/libbluetooth.la @[email protected] @[email protected] \
+ @[email protected] -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);


2009-09-29 18:03:46

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH] Drop Posix Capabilities

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



2009-09-29 13:00:50

by Steve Grubb

[permalink] [raw]
Subject: Re: [PATCH] Drop Posix Capabilities

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

2009-09-28 23:40:53

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH] Drop Posix Capabilities

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



2009-09-28 21:31:00

by Steve Grubb

[permalink] [raw]
Subject: Re: [PATCH] Drop Posix Capabilities

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

2009-09-27 20:31:25

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH] Drop Posix Capabilities

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




2009-09-26 14:29:06

by Steve Grubb

[permalink] [raw]
Subject: Re: [PATCH] Drop Posix Capabilities

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

2009-09-25 21:35:21

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH] Drop Posix Capabilities

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 <[email protected]>
>
>
> 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



2009-10-02 09:46:51

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH] Drop Posix Capabilities

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