All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] replace suid with capabilities, for example in busybox
@ 2015-12-09 19:14 Patrick Ohly
  2015-12-09 19:14 ` [PATCH 1/3] capabilities.bbclass: add file capabilities automatically Patrick Ohly
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Patrick Ohly @ 2015-12-09 19:14 UTC (permalink / raw
  To: openembedded-core

I started working on hardening a distro by replacing suid binaries
with executables that add only the necessary capabilities via file
capabilities.  It is understood that this is often still a path
towards privilege escalation (see
https://forums.grsecurity.net/viewtopic.php?f=7&t=2522&sid=c6fbcf62fd5d3472562540a7e608ce4e#p10271)
but as part of a defense-in-depth strategy it's still useful.

I'd like to get the first two patches reviewed and, if seen as useful,
merged into master.

The busybox_%.bbappend is just an example how this would be used. It's
not meant to be merged.

There's currently one caveat: the file capabilities do not get copied
into images. I see them under pseudo (with getcap, filecap and as
security.capability xattr with getfattr), but they do not get copied
into an ext4 image by mkfs.ext4.

Robert, I tried that with the patched e2fsprogs from meta-selinux. Is
that perhaps something you can look into as part of
https://bugzilla.yoctoproject.org/show_bug.cgi?id=8622 ?

Actually, I just noticed another problem with that e2fsprogs version:
with Smack enabled via meta-intel-iot-security/meta-security-smack,
/etc has under pseudo:
  # getfattr -d -m . rootfs/etc/
  # file: ../rootfs/etc/
  security.SMACK64="System::Shared"
  security.SMACK64TRANSMUTE="TRUE"

A loop-mounted ext4 image only has one xattr:
  security.SMACK64TRANSMUTE="TRUE"

The following changes since commit 192da885e92d3b163b9c4e6b8151c9ecc6062b14:

  build-appliance-image: Update to master head revision (2015-12-09 08:49:13 +0000)

are available in the git repository at:

  git://github.com/pohly/openembedded-core capabilities
  https://github.com/pohly/openembedded-core/tree/capabilities

Patrick Ohly (3):
  capabilities.bbclass: add file capabilities automatically
  busybox.inc: prepare for additional link files
  busybox_%.bbappend: run ping and traceroute with file capabilities

 meta/classes/capabilities.bbclass            | 58 ++++++++++++++++++++++++++++
 meta/recipes-core/busybox/busybox.inc        | 16 ++++----
 meta/recipes-core/busybox/busybox_%.bbappend | 41 ++++++++++++++++++++
 3 files changed, 108 insertions(+), 7 deletions(-)
 create mode 100644 meta/classes/capabilities.bbclass
 create mode 100644 meta/recipes-core/busybox/busybox_%.bbappend

-- 
2.1.4



^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 1/3] capabilities.bbclass: add file capabilities automatically
  2015-12-09 19:14 [PATCH 0/3] replace suid with capabilities, for example in busybox Patrick Ohly
@ 2015-12-09 19:14 ` Patrick Ohly
  2015-12-09 19:14 ` [PATCH 2/3] busybox.inc: prepare for additional link files Patrick Ohly
  2015-12-09 19:14 ` [PATCH 3/3] busybox_%.bbappend: run ping and traceroute with file capabilities Patrick Ohly
  2 siblings, 0 replies; 4+ messages in thread
From: Patrick Ohly @ 2015-12-09 19:14 UTC (permalink / raw
  To: openembedded-core

Setting file capabilities must be done in postinst scripts. This class
simplifies that by ensuring that the necessary tools are installed and
automatically generating the necessary postinst scripts based on the
CAPABILITIES_PACKAGES (list of packages which contain files with
special capabilities, ${PN} if not set) and CAPABILITIES_<pkg>
(definition of special files inside the <pkg> package).

Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
---
 meta/classes/capabilities.bbclass | 58 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)
 create mode 100644 meta/classes/capabilities.bbclass

diff --git a/meta/classes/capabilities.bbclass b/meta/classes/capabilities.bbclass
new file mode 100644
index 0000000..92c2a9a
--- /dev/null
+++ b/meta/classes/capabilities.bbclass
@@ -0,0 +1,58 @@
+# Simplifies setting file capabilities. Those have to go into
+# a postinst script because it is not guaranteed that the package
+# format preserves the security.capabilities xattr that holds
+# the file capabilities.
+#
+# Using this class also ensures that the necessary tools are
+# available.
+#
+# Example usage (for a hypothetical ping_%.bb[append]):
+#   inherit capabilities
+#   CAPABILITIES_PACKAGES = "${PN}-bin"
+#   CAPABILITIES_${PN}-bin = "${bindir}/ping=net_raw,net_admin ${bindir}/pong=net_broadcast"
+#
+# Copyright (C) 2015 Intel Corporation
+# Licensed under the MIT license
+
+# Dependency when install in cross-compile mode on build host.
+# Runtime DEPENDS_<pkg> needs to be created dynamically.
+DEPENDS += "libcap-ng-native"
+
+# Space-separated list of packages containing files with capabilities.
+CAPABILITIES_PACKAGES ??= "${PN}"
+
+# Space-separated list of files and their capabilities.
+# Capability list may be empty.
+# CAPABILITIES_${PN} = "/foo/bar=capability1,capability2 /abc/def="
+
+# Ensure that add_capabilities_postinsts gets re-run when the content
+# of CAPABILITIES_PACKAGES changes. CAPABILITIES_PACKAGES_<pkg> is added below.
+add_capabilities_postinsts[vardeps] = "CAPABILITIES_PACKAGES"
+python add_capabilities_postinsts() {
+    for pkg in d.getVar('CAPABILITIES_PACKAGES', True).split():
+        bb.note("adding capabilities postinst script to %s" % pkg)
+        postinst = d.getVar('pkg_postinst_%s' % pkg, True) or d.getVar('pkg_postinst', True) or ''
+        files = d.getVar('CAPABILITIES_' + pkg, True)
+        if files is None:
+            bb.fatal('%s listed in CAPABILITIES_PACKAGES but CAPABILITIES_%s is not set' % (pkg, pkg))
+        for entry in files.split():
+            filename, capabilities = entry.split('=', 1)
+            capabilities = capabilities.split(',')
+            if capabilities:
+                cmd = 'filecap $D%s %s' % (filename, ' '.join(capabilities))
+                # TODO: errors during do_rootfs do not show up in log.do_rootfs.
+                # This makes it easy to miss problems.
+                postinst += 'echo "{0}"; {0} || exit 1\n'.format(cmd)
+            d.setVar('pkg_postinst_%s' % pkg, postinst)
+}
+
+PACKAGEFUNCS =+ "add_capabilities_postinsts"
+
+python () {
+    capabilities_pkgs = d.getVar('CAPABILITIES_PACKAGES', True).split()
+    for pkg in capabilities_pkgs:
+        d.appendVar('RDEPENDS_' + pkg, ' libcap-ng-bin')
+
+    d.appendVarFlag('add_capabilities_postinsts', 'vardeps',
+                    ' ' + ' '.join(['CAPABILITIES_' + pkg for pkg in capabilities_pkgs]))
+}
-- 
2.1.4



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 2/3] busybox.inc: prepare for additional link files
  2015-12-09 19:14 [PATCH 0/3] replace suid with capabilities, for example in busybox Patrick Ohly
  2015-12-09 19:14 ` [PATCH 1/3] capabilities.bbclass: add file capabilities automatically Patrick Ohly
@ 2015-12-09 19:14 ` Patrick Ohly
  2015-12-09 19:14 ` [PATCH 3/3] busybox_%.bbappend: run ping and traceroute with file capabilities Patrick Ohly
  2 siblings, 0 replies; 4+ messages in thread
From: Patrick Ohly @ 2015-12-09 19:14 UTC (permalink / raw
  To: openembedded-core

Right now, do_package_prepend() supports putting all applets into one
binary or the suid/nosuid binaries.

It may also be useful to split busybox differently and then use more
fine-grained file capabilities instead of suid to control what each
applet is allowed to do, or use hardlinks and different file
capabilities on each copy.

This change supports this by replacing the hard-coded links file names
with something that operates on all files matching the busybox.links*
shell pattern. For each /etc/busybox.links<foobar> link file there has
to be a corresponding /bin/busybox<foobar>.

set_alternative_vars() now always gets called with full path of the
link file in the host file system (because that is what the caller
has) and, while at it, only checks for existance of the target once.

Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
---
 meta/recipes-core/busybox/busybox.inc | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/meta/recipes-core/busybox/busybox.inc b/meta/recipes-core/busybox/busybox.inc
index fba956e..65a17ee 100644
--- a/meta/recipes-core/busybox/busybox.inc
+++ b/meta/recipes-core/busybox/busybox.inc
@@ -331,7 +331,8 @@ python do_package_prepend () {
     dvar = d.getVar('D', True)
     pn = d.getVar('PN', True)
     def set_alternative_vars(links, target):
-        f = open('%s%s' % (dvar, links), 'r')
+        target_exists = os.path.exists('%s%s' % (dvar, target))
+        f = open(links, 'r')
         for alt_link_name in f:
             alt_link_name = alt_link_name.strip()
             alt_name = os.path.basename(alt_link_name)
@@ -340,16 +341,17 @@ python do_package_prepend () {
                 alt_name = 'lbracket'
             d.appendVar('ALTERNATIVE_%s' % (pn), ' ' + alt_name)
             d.setVarFlag('ALTERNATIVE_LINK_NAME', alt_name, alt_link_name)
-            if os.path.exists('%s%s' % (dvar, target)):
+            if target_exists:
                 d.setVarFlag('ALTERNATIVE_TARGET', alt_name, target)
         f.close()
         return
 
-    if os.path.exists('%s/etc/busybox.links' % (dvar)):
-        set_alternative_vars("${sysconfdir}/busybox.links", "${base_bindir}/busybox")
-    else:
-        set_alternative_vars("${sysconfdir}/busybox.links.nosuid", "${base_bindir}/busybox.nosuid")
-        set_alternative_vars("${sysconfdir}/busybox.links.suid", "${base_bindir}/busybox.suid")
+    import glob
+    base = dvar + "${sysconfdir}/busybox.links"
+    for links in glob.glob(base + "*"):
+        # Suffix might be empty when there is no split between busybox.suid and busybox.nosuid.
+        suffix = links[len(base):]
+        set_alternative_vars(links, "${base_bindir}/busybox" + suffix)
 }
 
 pkg_postinst_${PN} () {
-- 
2.1.4



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 3/3] busybox_%.bbappend: run ping and traceroute with file capabilities
  2015-12-09 19:14 [PATCH 0/3] replace suid with capabilities, for example in busybox Patrick Ohly
  2015-12-09 19:14 ` [PATCH 1/3] capabilities.bbclass: add file capabilities automatically Patrick Ohly
  2015-12-09 19:14 ` [PATCH 2/3] busybox.inc: prepare for additional link files Patrick Ohly
@ 2015-12-09 19:14 ` Patrick Ohly
  2 siblings, 0 replies; 4+ messages in thread
From: Patrick Ohly @ 2015-12-09 19:14 UTC (permalink / raw
  To: openembedded-core

ping, ping6 and traceroute are installed now so that when invoked by
normal users, the resulting process runs only with the new_raw
capability and not as root. This mitigates the effect when normal
invocations of these commands run into problems. A hardlink is used
to create the additional copy of the busybox binary, so the increase
in disk space is minimal.

However, a local attacker can still run these commands as root by
symlinking to the original busybox.suid. Fixing that would require
building busybox differently, which would cost more disk space.

Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
---
 meta/recipes-core/busybox/busybox_%.bbappend | 41 ++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)
 create mode 100644 meta/recipes-core/busybox/busybox_%.bbappend

diff --git a/meta/recipes-core/busybox/busybox_%.bbappend b/meta/recipes-core/busybox/busybox_%.bbappend
new file mode 100644
index 0000000..c27b0cd
--- /dev/null
+++ b/meta/recipes-core/busybox/busybox_%.bbappend
@@ -0,0 +1,41 @@
+inherit capabilities
+
+# This .bbappend  lowers privileges of certain commands from "runs as
+# root via suid" to "runs with a limited set of privileges via file
+# capabilities".
+#
+# The original list of symlinks to busybox.suid is, with (*) marking
+# commands which now can get executed with less privileges:
+#    /bin/ping (*)
+#    /bin/ping6 (*)
+#    /bin/login
+#    /usr/bin/passwd
+#    /bin/su
+#    /usr/bin/traceroute (*)
+#    /usr/bin/vock
+#
+# As it stands now, this change still leaves the "ping" and "traceroute"
+# code in the busybox.suid binary, where it can be executed as root by
+# a normal user by symlinking to it ("ln -s /bin/busybox.suid /tmp/ping;
+# /tmp/ping ...").
+#
+# To fix this, one would have to split up busybox even further, which
+# (somewhat) negates the space saving coming from implementing several
+# commands in the same binary.
+
+CAPABILITIES_${PN} = " \
+    ${base_bindir}/busybox.net_raw=net_raw \
+"
+
+do_install_append () {
+    ln ${D}/${base_bindir}/busybox.suid ${D}/${base_bindir}/busybox.net_raw
+    grep \
+       -e ping \
+       -e traceroute \
+       ${D}/${sysconfdir}/busybox.links.suid >${D}/${sysconfdir}/busybox.links.net_raw
+    grep -v \
+       -e ping \
+       -e traceroute \
+       ${D}/${sysconfdir}/busybox.links.suid >${D}/${sysconfdir}/busybox.links.suid.tmp
+    mv ${D}/${sysconfdir}/busybox.links.suid.tmp ${D}/${sysconfdir}/busybox.links.suid
+}
-- 
2.1.4



^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2015-12-09 18:14 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-12-09 19:14 [PATCH 0/3] replace suid with capabilities, for example in busybox Patrick Ohly
2015-12-09 19:14 ` [PATCH 1/3] capabilities.bbclass: add file capabilities automatically Patrick Ohly
2015-12-09 19:14 ` [PATCH 2/3] busybox.inc: prepare for additional link files Patrick Ohly
2015-12-09 19:14 ` [PATCH 3/3] busybox_%.bbappend: run ping and traceroute with file capabilities Patrick Ohly

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.