Change the definition of 'all' to be all named capabilities.

[This will be included in libcap-2.29.]

This change concerns the text formating functions:

  C:  cap_to_text(), cap_from_text()
 Go:  cap.FromText() and cap.Set.String()

Prior to this commit, "all" meant every bit of the capability vector
was raised - both named, and unnamed capabilities. Which following some
changes to the kernel (to clip the bounding set to only named values)
broke for libcap. I tried to work around it in 2008, but my workaround
was considered ugly and I now see was only partial...

This partial fix led to what I consider an outstanding kernel bug, and
'all' being unusable for process capabilities. So, this libcap change
is intended to undo my part of the error. While it gives tidier,
self-consistent output, it does introduce a backward incompatibility
in behavior...

After this commit 'all' means all the known/named capabilities only.
That is, as of today, Linux has 38 capabilities defined: 0 (cap_chown)
to 37 (cap_audit_read). So, today, 'all' means all 38 of these raised
and the remaining 26 'unnamed' capabilities lowered.

This has the following effect:

old_libcap_tools:

$ /sbin/getpcaps 1
Capabilities for `1': = cap_chown,cap_dac_override,cap_dac_read_search,cap_fowner,cap_fsetid,cap_kill,cap_setgid,cap_setuid,cap_setpcap,cap_linux_immutable,cap_net_bind_service,cap_net_broadcast,cap_net_admin,cap_net_raw,cap_ipc_lock,cap_ipc_owner,cap_sys_module,cap_sys_rawio,cap_sys_chroot,cap_sys_ptrace,cap_sys_pacct,cap_sys_admin,cap_sys_boot,cap_sys_nice,cap_sys_resource,cap_sys_time,cap_sys_tty_config,cap_mknod,cap_lease,cap_audit_write,cap_audit_control,cap_setfcap,cap_mac_override,cap_mac_admin,cap_syslog,cap_wake_alarm,cap_block_suspend,cap_audit_read+ep
$ echo exit > oldall
$ chmod +x oldall
$ sudo /sbin/setcap =ep oldall
$ /sbin/getcap oldall
oldall =ep

new_libcap_tools:

$ ./getpcaps 1
Capabilities for `1': =ep
$ echo exit > newall
$ chmod +x newall
$ sudo ./setcap =ep newall
$ ./getcap newall oldall
newall =ep
oldall =ep 38,39,40,41,42,43,44,45,46,47,48,49,50,51,52,53,54,55,56,57,58,59,60,61,62,63+ep

old_libcap_tools:

$ /sbin/getcap oldall newall
oldall =ep
newall = cap_chown,cap_dac_override,cap_dac_read_search,cap_fowner,cap_fsetid,cap_kill,cap_setgid,cap_setuid,cap_setpcap,cap_linux_immutable,cap_net_bind_service,cap_net_broadcast,cap_net_admin,cap_net_raw,cap_ipc_lock,cap_ipc_owner,cap_sys_module,cap_sys_rawio,cap_sys_chroot,cap_sys_ptrace,cap_sys_pacct,cap_sys_admin,cap_sys_boot,cap_sys_nice,cap_sys_resource,cap_sys_time,cap_sys_tty_config,cap_mknod,cap_lease,cap_audit_write,cap_audit_control,cap_setfcap,cap_mac_override,cap_mac_admin,cap_syslog,cap_wake_alarm,cap_block_suspend,cap_audit_read+ep

Cc: "Serge E. Hallyn" <serge@hallyn.com>
Cc: Michael Kerrisk <mtk.manpages@gmail.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: linux-security-module <linux-security-module@vger.kernel.org>
Cc: ksrot@redhat.com
Signed-off-by: Andrew G. Morgan <morgan@kernel.org>

5 files changed, 164 insertions, 34 deletions

diff --git a/cap/cap_test.go b/cap/cap_test.go
index e836fde..7b1f39f 100644
--- a/cap/cap_test.go
+++ b/cap/cap_test.go
@@ -5,6 +5,44 @@ import (
 	"testing"
 )
 
+func TestAllMask(t *testing.T) {
+	oldMask := maxValues
+	oldWords := words
+	defer func() {
+		maxValues = oldMask
+		words = oldWords
+	}()
+
+	maxValues = 35
+	words = 3
+
+	vs := []struct {
+		val   Value
+		index uint
+		bit   uint32
+		mask  uint32
+	}{
+		{val: CHOWN, index: 0, bit: 0x1, mask: ^uint32(0)},
+		{val: 38, index: 1, bit: (1 << 6), mask: 0x7},
+		{val: 34, index: 1, bit: (1 << 2), mask: 0x7},
+		{val: 65, index: 2, bit: (1 << 1), mask: 0},
+	}
+	for i, v := range vs {
+		index, bit, err := bitOf(Inheritable, v.val)
+		if err != nil {
+			t.Fatalf("[%d] %v(%d) - not bitOf: %v", i, v.val, v.val, err)
+		} else if index != v.index {
+			t.Errorf("[%d] %v(%d) - index: got=%d want=%d", i, v.val, v.val, index, v.index)
+		}
+		if bit != v.bit {
+			t.Errorf("[%d] %v(%d) - bit: got=%b want=%b", i, v.val, v.val, bit, v.bit)
+		}
+		if mask := allMask(index); mask != v.mask {
+			t.Errorf("[%d] %v(%d) - mask: got=%b want=%b", i, v.val, v.val, mask, v.mask)
+		}
+	}
+}
+
 func TestString(t *testing.T) {
 	a := CHOWN
 	if got, want := a.String(), "cap_chown"; got != want {
@@ -65,6 +103,13 @@ func same(a, b *Set) error {
 }
 
 func TestImportExport(t *testing.T) {
+	wantQ := "=ep cap_chown-e 63+ip"
+	if q, err := FromText(wantQ); err != nil {
+		t.Fatalf("failed to parse %q: %v", wantQ, err)
+	} else if gotQ := q.String(); gotQ != wantQ {
+		t.Fatalf("static test failed %q -> q -> %q", wantQ, gotQ)
+	}
+
 	// Sanity check empty import/export.
 	c := NewSet()
 	if ex, err := c.Export(); err != nil {
@@ -83,15 +128,15 @@ func TestImportExport(t *testing.T) {
 		v := Value(i % (maxValues + 3))
 		c.SetFlag(s, i&17 < 8, v)
 		if ex, err := c.Export(); err != nil {
-			t.Fatalf("[%d] failed to export empty set: %v", i, err)
+			t.Fatalf("[%d] failed to export (%q): %v", i, c, err)
 		} else if im, err := Import(ex); err != nil {
-			t.Fatalf("[%d] failed to import empty set: %v", i, err)
+			t.Fatalf("[%d] failed to import (%q) set: %v", i, c, err)
 		} else if got, want := im.String(), c.String(); got != want {
 			t.Fatalf("[%d] import != export: got=%q want=%q [%02x]", i, got, want, ex)
 		} else if parsed, err := FromText(got); err != nil {
 			t.Fatalf("[%d] failed to parse %q: %v", i, got, err)
 		} else if err := same(c, parsed); err != nil {
-			t.Fatalf("[%d] miscompare: %v", i, err)
+			t.Fatalf("[%d] miscompare (%q vs. %q): %v", i, got, parsed, err)
 		}
 	}
 }
diff --git a/cap/flags.go b/cap/flags.go
index 8e4fcd7..d19d0c2 100644
--- a/cap/flags.go
+++ b/cap/flags.go
@@ -90,7 +90,25 @@ func bitOf(vec Flag, val Value) (uint, uint32, error) {
 	return u / 32, uint32(1) << (u % 32), nil
 }
 
-// forceFlag sets all capability values of a flag vector to enable.
+// allMask returns the mask of valid bits in the all mask for index.
+func allMask(index uint) (mask uint32) {
+	if maxValues == 0 {
+		panic("uninitialized package")
+	}
+	base := 32 * uint(index)
+	if maxValues <= base {
+		return
+	}
+	if maxValues >= 32+base {
+		mask = ^mask
+		return
+	}
+	mask = uint32((uint64(1) << (maxValues % 32)) - 1)
+	return
+}
+
+// forceFlag sets 'all' capability values (supported by the kernel) of
+// a flag vector to enable.
 func (c *Set) forceFlag(vec Flag, enable bool) error {
 	if c == nil || len(c.flat) == 0 || vec > Inheritable {
 		return ErrBadSet
@@ -102,7 +120,7 @@ func (c *Set) forceFlag(vec Flag, enable bool) error {
 	c.mu.Lock()
 	defer c.mu.Unlock()
 	for i := range c.flat {
-		c.flat[i][vec] = m
+		c.flat[i][vec] = m & allMask(uint(i))
 	}
 	return nil
 }
diff --git a/cap/text.go b/cap/text.go
index b6117e5..117bedf 100644
--- a/cap/text.go
+++ b/cap/text.go
@@ -51,13 +51,13 @@ var combos = []string{"", "e", "p", "ep", "i", "ei", "ip", "eip"}
 func (c *Set) histo(m uint, bins []int, patterns []uint, from, limit Value) uint {
 	for v := from; v < limit; v++ {
 		b := uint(v & 31)
-		u, mask, err := bitOf(0, v)
+		u, bit, err := bitOf(0, v)
 		if err != nil {
 			break
 		}
-		x := uint((c.flat[u][Effective]&mask)>>b) * eBin
-		x |= uint((c.flat[u][Permitted]&mask)>>b) * pBin
-		x |= uint((c.flat[u][Inheritable]&mask)>>b) * iBin
+		x := uint((c.flat[u][Effective]&bit)>>b) * eBin
+		x |= uint((c.flat[u][Permitted]&bit)>>b) * pBin
+		x |= uint((c.flat[u][Inheritable]&bit)>>b) * iBin
 		bins[x]++
 		patterns[uint(v)] = x
 		if bins[m] <= bins[x] {
@@ -70,7 +70,7 @@ func (c *Set) histo(m uint, bins []int, patterns []uint, from, limit Value) uint
 // String converts a full capability Set into it canonical readable
 // string representation (which may contain spaces).
 func (c *Set) String() string {
-	if c == nil {
+	if c == nil || len(c.flat) == 0 {
 		return "<invalid>"
 	}
 	bins := make([]int, 8)
@@ -81,14 +81,10 @@ func (c *Set) String() string {
 
 	// Note, in order to have a *Set pointer, startUp.Do(cInit)
 	// must have been called which sets maxValues.
-	m := c.histo(0, bins, patterns, Value(maxValues), 32*Value(words))
-	// Background state is the most popular of the unnamed bits,
-	// this has the effect of tending to not list numerical values
-	// for unnamed capabilities in the generated text.
-	vs := []string{"=" + combos[m]}
+	m := c.histo(0, bins, patterns, 0, Value(maxValues))
 
-	// Extend the histogram with the remaining Value occurrences.
-	c.histo(m, bins, patterns, 0, Value(maxValues))
+	// Background state is the most popular of the named bits.
+	vs := []string{"=" + combos[m]}
 	for i := uint(8); i > 0; {
 		i--
 		if i == m || bins[i] == 0 {
@@ -108,6 +104,27 @@ func (c *Set) String() string {
 			vs = append(vs, strings.Join(list, ",")+"-"+combos[cf])
 		}
 	}
+
+	// The unnamed bits can only add to the above named ones since
+	// unnamed ones are always defaulted to lowered.
+	uBins := make([]int, 8)
+	uPatterns := make([]uint, 32*words)
+	c.histo(0, uBins, uPatterns, Value(maxValues), 32*Value(words))
+	for i := uint(8); i > 1; {
+		i--
+		if uBins[i] == 0 {
+			continue
+		}
+		var list []string
+		for j, p := range uPatterns {
+			if p != i {
+				continue
+			}
+			list = append(list, Value(j).String())
+		}
+		vs = append(vs, strings.Join(list, ",")+"+"+combos[i])
+	}
+
 	return strings.Join(vs, " ")
 }
 
@@ -157,13 +174,16 @@ func FromText(text string) (*Set, error) {
 			}
 		}
 		if sep == '=' {
-			keep := len(vs) == 0 // '=' means default to off except setting individual values.
+			// '=' means default to off for all named flags.
+			// '=ep' means default on for named e & p.
+			keep := len(vs) == 0
 			c.forceFlag(Effective, fE && keep)
 			c.forceFlag(Permitted, fP && keep)
 			c.forceFlag(Inheritable, fI && keep)
 			if keep {
 				continue
 			}
+
 		}
 		if fE {
 			c.SetFlag(Effective, sep != '-', vs...)
diff --git a/go/compare-cap.go b/go/compare-cap.go
index 2d04e24..e57e946 100644
--- a/go/compare-cap.go
+++ b/go/compare-cap.go
@@ -271,6 +271,16 @@ func main() {
 		log.Fatalf("c import from go yielded bad caps: got=%q want=%q", got, want)
 	}
 
+	// Validate that everyone agrees what all is:
+	want := "=ep"
+	all, err := cap.FromText("all=ep")
+	if err != nil {
+		log.Fatalf("unable to parse all=ep: %v", err)
+	}
+	if got := all.String(); got != want {
+		log.Fatalf("all decode failed in Go: got=%q, want=%q", got, want)
+	}
+
 	// Next, we attempt to manipulate some file capabilities on
 	// the running program.  These are optional, based on whether
 	// the current program is capable enough and do not involve
diff --git a/libcap/cap_text.c b/libcap/cap_text.c
index cb485a6..3b03977 100644
--- a/libcap/cap_text.c
+++ b/libcap/cap_text.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997-8,2007-8 Andrew G Morgan <morgan@kernel.org>
+ * Copyright (c) 1997-8,2007-8,2019 Andrew G Morgan <morgan@kernel.org>
  * Copyright (c) 1997 Andrew Main <zefram@dcs.warwick.ac.uk>
  *
  * This file deals with exchanging internal and textual
@@ -56,11 +56,22 @@ static char const *namcmp(char const *str, char const *nam)
     return str;
 }
 
+/*
+ * forceall forces all of the named capabilities to be assigned the
+ * masked value, and zeroed otherwise.
+ */
 static void forceall(__u32 *flat, __u32 value, unsigned blks)
 {
-    unsigned n;
-
-    for (n = blks; n--; flat[n] = value);
+    for (unsigned n = blks; n--; ) {
+	unsigned base = 32*n;
+	__u32 mask = 0;
+	if (__CAP_BITS >= base + 32) {
+	    mask = ~0;
+	} else if (__CAP_BITS > base) {
+	    mask = (unsigned) ((1ULL << (__CAP_BITS % 32)) - 1);
+	}
+	flat[n] = value & mask;
+    }
 
     return;
 }
@@ -149,7 +160,7 @@ cap_t cap_from_text(const char *str)
 	char op;
 	int flags = 0, listed=0;
 
-	forceall(list, 0, __CAP_BLKS);
+	memset(list, 0, sizeof(__u32)*__CAP_BLKS);
 
 	/* skip leading spaces */
 	while (isspace((unsigned char)*str))
@@ -366,8 +377,8 @@ char *cap_to_text(cap_t caps, ssize_t *length_p)
 
     memset(histo, 0, sizeof(histo));
 
-    /* default prevailing state to the upper - unnamed bits */
-    for (n = cap_maxbits-1; n > __CAP_BITS; n--)
+    /* default prevailing state to the named bits */
+    for (n = 0; n < __CAP_BITS; n++)
 	histo[getstateflags(caps, n)]++;
 
     /* find which combination of capability sets shares the most bits
@@ -378,12 +389,6 @@ char *cap_to_text(cap_t caps, ssize_t *length_p)
 	if (histo[t] >= histo[m])
 	    m = t;
 
-    /* capture remaining bits - selecting m from only the unnamed bits,
-       we maximize the likelihood that we won't see numeric capability
-       values in the text output. */
-    while (n--)
-	histo[getstateflags(caps, n)]++;
-
     /* blank is not a valid capability set */
     p = sprintf(buf, "=%s%s%s",
 		(m & LIBCAP_EFF) ? "e" : "",
@@ -397,9 +402,7 @@ char *cap_to_text(cap_t caps, ssize_t *length_p)
 	*p++ = ' ';
 	for (n = 0; n < cap_maxbits; n++) {
 	    if (getstateflags(caps, n) == t) {
-	        char *this_cap_name;
-
-	        this_cap_name = cap_to_name(n);
+	        char *this_cap_name = cap_to_name(n);
 	        if ((strlen(this_cap_name) + (p - buf)) > CAP_TEXT_SIZE) {
 		    cap_free(this_cap_name);
 		    errno = ERANGE;
@@ -429,6 +432,40 @@ char *cap_to_text(cap_t caps, ssize_t *length_p)
 	    return NULL;
 	}
     }
+
+    /* capture remaining unnamed bits - which must all be +. */
+    memset(histo, 0, sizeof(histo));
+    for (n = cap_maxbits-1; n >= __CAP_BITS; n--)
+	histo[getstateflags(caps, n)]++;
+
+    for (t = 8; t-- > 1; ) {
+	if (!histo[t]) {
+	    continue;
+	}
+	*p++ = ' ';
+	for (n = __CAP_BITS; n < cap_maxbits; n++) {
+	    if (getstateflags(caps, n) == t) {
+		char *this_cap_name = cap_to_name(n);
+	        if ((strlen(this_cap_name) + (p - buf)) > CAP_TEXT_SIZE) {
+		    cap_free(this_cap_name);
+		    errno = ERANGE;
+		    return NULL;
+	        }
+		p += sprintf(p, "%s,", this_cap_name);
+		cap_free(this_cap_name);
+	    }
+	}
+	p--;
+	p += sprintf(p, "+%s%s%s",
+		     (t & LIBCAP_EFF) ? "e" : "",
+		     (t & LIBCAP_INH) ? "i" : "",
+		     (t & LIBCAP_PER) ? "p" : "");
+	if (p - buf > CAP_TEXT_SIZE) {
+	    errno = ERANGE;
+	    return NULL;
+	}
+    }
+
     _cap_debug("%s", buf);
     if (length_p) {
 	*length_p = p - buf;