Make label.Relabel safer.

- Check if Selinux is enabled before relabeling. This is a bug.
- Make exclusion detection constant time. Kinda buggy too, imo.
- Do not depend on a magic string to create a new Selinux context.

Signed-off-by: David Calavera <david.calavera@gmail.com>

libcontainer/label/label.go

@@ -29,7 +29,7 @@ func SetFileCreateLabel(fileLabel string) error {
 	return nil
 }
 
-func Relabel(path string, fileLabel string, relabel string) error {
+func Relabel(path string, fileLabel string, shared bool) error {
 	return nil
 }
 
@@ -59,3 +59,13 @@ func DupSecOpt(src string) []string {
 func DisableSecOpt() []string {
 	return nil
 }
+
+// Validate checks that the label does not include unexpected options
+func Validate(label string) error {
+	return nil
+}
+
+// IsShared checks that the label includes a "shared" mark
+func IsShared(label string) bool {
+	return false
+}

libcontainer/label/label_selinux.go

@@ -9,6 +9,8 @@ import (
 	"github.com/opencontainers/runc/libcontainer/selinux"
 )
 
+var ErrIncompatibleLabel = fmt.Errorf("Bad SELinux option z and Z can not be used together")
+
 // InitLabels returns the process label and file labels to be used within
 // the container.  A list of options can be passed into this function to alter
 // the labels.  The labels returned will include a random MCS String, that is
@@ -95,28 +97,24 @@ func SetFileCreateLabel(fileLabel string) error {
 	return nil
 }
 
-// Change the label of path to the filelabel string.  If the relabel string
-// is "z", relabel will change the MCS label to s0.  This will allow all
-// containers to share the content.  If the relabel string is a "Z" then
-// the MCS label should continue to be used.  SELinux will use this field
-// to make sure the content can not be shared by other containes.
-func Relabel(path string, fileLabel string, relabel string) error {
-	exclude_path := []string{"/", "/usr", "/etc"}
+// Change the label of path to the filelabel string.
+// It changes the MCS label to s0 if shared is true.
+// This will allow all containers to share the content.
+func Relabel(path string, fileLabel string, shared bool) error {
+	if !selinux.SelinuxEnabled() {
+		return nil
+	}
+
 	if fileLabel == "" {
 		return nil
 	}
-	if !strings.ContainsAny(relabel, "zZ") {
-		return nil
+
+	exclude_paths := map[string]bool{"/": true, "/usr": true, "/etc": true}
+	if exclude_paths[path] {
+		return fmt.Errorf("Relabeling of %s is not allowed", path)
 	}
-	for _, p := range exclude_path {
-		if path == p {
-			return fmt.Errorf("Relabeling of %s is not allowed", path)
-		}
-	}
-	if strings.Contains(relabel, "z") && strings.Contains(relabel, "Z") {
-		return fmt.Errorf("Bad SELinux option z and Z can not be used together")
-	}
-	if strings.Contains(relabel, "z") {
+
+	if shared {
 		c := selinux.NewContext(fileLabel)
 		c["level"] = "s0"
 		fileLabel = c.Get()
@@ -161,3 +159,16 @@ func DupSecOpt(src string) []string {
 func DisableSecOpt() []string {
 	return selinux.DisableSecOpt()
 }
+
+// Validate checks that the label does not include unexpected options
+func Validate(label string) error {
+	if strings.Contains(label, "z") && strings.Contains(label, "Z") {
+		return ErrIncompatibleLabel
+	}
+	return nil
+}
+
+// IsShared checks that the label includes a "shared" mark
+func IsShared(label string) bool {
+	return strings.Contains(label, "z")
+}

libcontainer/label/label_selinux_test.go

@@ -90,28 +90,50 @@ func TestDuplicateLabel(t *testing.T) {
 func TestRelabel(t *testing.T) {
 	testdir := "/tmp/test"
 	label := "system_u:system_r:svirt_sandbox_file_t:s0:c1,c2"
-	if err := Relabel(testdir, "", "z"); err != nil {
+	if err := Relabel(testdir, "", true); err != nil {
 		t.Fatal("Relabel with no label failed: %v", err)
 	}
-	if err := Relabel(testdir, label, ""); err != nil {
-		t.Fatal("Relabel with no relabel field failed: %v", err)
-	}
-	if err := Relabel(testdir, label, "z"); err != nil {
+	if err := Relabel(testdir, label, true); err != nil {
 		t.Fatal("Relabel shared failed: %v", err)
 	}
-	if err := Relabel(testdir, label, "Z"); err != nil {
+	if err := Relabel(testdir, label, false); err != nil {
 		t.Fatal("Relabel unshared failed: %v", err)
 	}
-	if err := Relabel(testdir, label, "zZ"); err == nil {
-		t.Fatal("Relabel with shared and unshared succeeded")
-	}
-	if err := Relabel("/etc", label, "zZ"); err == nil {
+	if err := Relabel("/etc", label, false); err == nil {
 		t.Fatal("Relabel /etc succeeded")
 	}
-	if err := Relabel("/", label, ""); err == nil {
+	if err := Relabel("/", label, false); err == nil {
 		t.Fatal("Relabel / succeeded")
 	}
-	if err := Relabel("/usr", label, "Z"); err == nil {
+	if err := Relabel("/usr", label, false); err == nil {
 		t.Fatal("Relabel /usr succeeded")
 	}
 }
+
+func TestValidate(t *testing.T) {
+	if err := Validate("zZ"); err != ErrIncompatibleLabel {
+		t.Fatalf("Expected incompatible error, got %v", err)
+	}
+	if err := Validate("Z"); err != nil {
+		t.Fatal(err)
+	}
+	if err := Validate("z"); err != nil {
+		t.Fatal(err)
+	}
+	if err := Validate(""); err != nil {
+		t.Fatal(err)
+	}
+}
+
+func TestIsShared(t *testing.T) {
+	if shared := IsShared("Z"); shared {
+		t.Fatal("Expected label `Z` to not be shared, got %v", shared)
+	}
+	if shared := IsShared("z"); !shared {
+		t.Fatal("Expected label `z` to be shared, got %v", shared)
+	}
+	if shared := IsShared("Zz"); !shared {
+		t.Fatal("Expected label `Zz` to be shared, got %v", shared)
+	}
+
+}

libcontainer/rootfs_linux.go

@@ -160,7 +160,11 @@ func mountToRootfs(m *configs.Mount, rootfs, mountLabel string) error {
 			}
 		}
 		if m.Relabel != "" {
-			if err := label.Relabel(m.Source, mountLabel, m.Relabel); err != nil {
+			if err := label.Validate(m.Relabel); err != nil {
+				return err
+			}
+			shared := label.IsShared(m.Relabel)
+			if err := label.Relabel(m.Source, mountLabel, shared); err != nil {
 				return err
 			}
 		}