From 86a2c3f3bd5df7c6f96c072b51d515c29f04a143 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 18 Feb 2020 15:29:44 -0800 Subject: [PATCH 1/4] Remove SelinuxfsMagic Use the value from x/sys/unix directly. While at it, remove buf.Type typecast, since we're comparing it with an (untyped) const, so it should not bring any warnings on e.g. 32-bit ARM. Signed-off-by: Kir Kolyshkin --- go-selinux/selinux_linux.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/go-selinux/selinux_linux.go b/go-selinux/selinux_linux.go index 90d65c1..ea5ee09 100644 --- a/go-selinux/selinux_linux.go +++ b/go-selinux/selinux_linux.go @@ -37,7 +37,6 @@ const ( selinuxTag = "SELINUX" xattrNameSelinux = "security.selinux" stRdOnly = 0x01 - selinuxfsMagic = 0xf97cff8c ) type selinuxState struct { @@ -118,7 +117,8 @@ func verifySELinuxfsMount(mnt string) bool { } return false } - if uint32(buf.Type) != uint32(selinuxfsMagic) { + + if buf.Type != unix.SELINUX_MAGIC { return false } if (buf.Flags & stRdOnly) != 0 { From aa2b9552c9762a795f1c9e11b8179eda89e024cf Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 18 Feb 2020 16:17:29 -0800 Subject: [PATCH 2/4] TestSetEnforceMode: separate and fix for non-root The part of TestSELinux test case fails: > selinux_linux_test.go:72: enforcing selinux failed: open /sys/fs/selinux/enforce: permission denied The problem is, one needs to be root to open this file rw. Let's move this code to a separate test case, adding a Skip() if run under non-root. That way, we will: * not fail tests if run under non-root; * see that this test case was NOT run. Signed-off-by: Kir Kolyshkin --- go-selinux/selinux_linux_test.go | 33 ++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/go-selinux/selinux_linux_test.go b/go-selinux/selinux_linux_test.go index 866ecc3..ab1feba 100644 --- a/go-selinux/selinux_linux_test.go +++ b/go-selinux/selinux_linux_test.go @@ -53,9 +53,6 @@ func TestSELinux(t *testing.T) { t.Log(plabel) t.Log(flabel) ReleaseLabel(plabel) - t.Log("Enforcing Mode", EnforceMode()) - mode := DefaultEnforceMode() - t.Log("Default Enforce Mode ", mode) plabel, flabel = ContainerLabels() t.Log(plabel) @@ -67,15 +64,6 @@ func TestSELinux(t *testing.T) { t.Log(flabel) ReleaseLabel(plabel) - defer SetEnforceMode(mode) - if err := SetEnforceMode(Enforcing); err != nil { - t.Fatalf("enforcing selinux failed: %v", err) - } - if err := SetEnforceMode(Permissive); err != nil { - t.Fatalf("setting selinux mode to permissive failed: %v", err) - } - SetEnforceMode(mode) - pid := os.Getpid() t.Logf("PID:%d MCS:%s\n", pid, intToMcs(pid, 1023)) err = SetFSCreateLabel("unconfined_u:unconfined_r:unconfined_t:s0") @@ -95,6 +83,27 @@ func TestSELinux(t *testing.T) { t.Log(PidLabel(1)) } +func TestSetEnforceMode(t *testing.T) { + if !GetEnabled() { + t.Skip("SELinux not enabled, skipping.") + } + if os.Geteuid() != 0 { + t.Skip("root required, skipping") + } + + t.Log("Enforcing Mode:", EnforceMode()) + mode := DefaultEnforceMode() + t.Log("Default Enforce Mode:", mode) + defer SetEnforceMode(mode) + + if err := SetEnforceMode(Enforcing); err != nil { + t.Fatalf("setting selinux mode to enforcing failed: %v", err) + } + if err := SetEnforceMode(Permissive); err != nil { + t.Fatalf("setting selinux mode to permissive failed: %v", err) + } +} + func TestCanonicalizeContext(t *testing.T) { if !GetEnabled() { t.Skip("SELinux not enabled, skipping.") From a843350825b615a52988c8acb164ac745e428a76 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 18 Feb 2020 16:22:20 -0800 Subject: [PATCH 3/4] Fix [Set]EnforceMode and SecurityCheckContext While working on minor improvements to tests, I found out that SetEnforceMode() is not working even for the root user: > --- FAIL: TestSetEnforceMode (0.00s) > selinux_linux_test.go:94: Enforcing Mode: -1 > selinux_linux_test.go:96: Default Enforce Mode: 1 > selinux_linux_test.go:100: setting selinux mode to enforcing failed: /sys/fs/selinux/enforce not on procfs If you look closer, you'll see that EnforceMode() is not working either, since it returns -1. The problem is, these functions, as well as SecurityCheckContext(), are using readCon() and writeCon() methods, which require the files being read/written to be on `procfs`. In these cases, though, the files are on `selinuxfs`, and the filesystem check is not really required since we already checked that during selinuxfs mount point search. So, just use ioutil.ReadFile/WriteFile here. While at it - convert code to use path.Join() instead of fmt.Sprintf() - add a test case for SecurityCheckContext Signed-off-by: Kir Kolyshkin --- go-selinux/selinux_linux.go | 12 ++++++------ go-selinux/selinux_linux_test.go | 25 +++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 6 deletions(-) diff --git a/go-selinux/selinux_linux.go b/go-selinux/selinux_linux.go index ea5ee09..ceb1671 100644 --- a/go-selinux/selinux_linux.go +++ b/go-selinux/selinux_linux.go @@ -11,6 +11,7 @@ import ( "io" "io/ioutil" "os" + "path" "path/filepath" "regexp" "strconv" @@ -505,19 +506,18 @@ func ReserveLabel(label string) { } func selinuxEnforcePath() string { - return fmt.Sprintf("%s/enforce", getSelinuxMountPoint()) + return path.Join(getSelinuxMountPoint(), "enforce") } // EnforceMode returns the current SELinux mode Enforcing, Permissive, Disabled func EnforceMode() int { var enforce int - enforceS, err := readCon(selinuxEnforcePath()) + enforceB, err := ioutil.ReadFile(selinuxEnforcePath()) if err != nil { return -1 } - - enforce, err = strconv.Atoi(string(enforceS)) + enforce, err = strconv.Atoi(string(enforceB)) if err != nil { return -1 } @@ -529,7 +529,7 @@ SetEnforceMode sets the current SELinux mode Enforcing, Permissive. Disabled is not valid, since this needs to be set at boot time. */ func SetEnforceMode(mode int) error { - return writeCon(selinuxEnforcePath(), fmt.Sprintf("%d", mode)) + return ioutil.WriteFile(selinuxEnforcePath(), []byte(strconv.Itoa(mode)), 0644) } /* @@ -711,7 +711,7 @@ exit: // SecurityCheckContext validates that the SELinux label is understood by the kernel func SecurityCheckContext(val string) error { - return writeCon(fmt.Sprintf("%s/context", getSelinuxMountPoint()), val) + return ioutil.WriteFile(path.Join(getSelinuxMountPoint(), "context"), []byte(val), 0644) } /* diff --git a/go-selinux/selinux_linux_test.go b/go-selinux/selinux_linux_test.go index ab1feba..945acf3 100644 --- a/go-selinux/selinux_linux_test.go +++ b/go-selinux/selinux_linux_test.go @@ -175,3 +175,28 @@ func TestFindSELinuxfsInMountinfo(t *testing.T) { } } } + +func TestSecurityCheckContext(t *testing.T) { + if !GetEnabled() { + t.Skip("SELinux not enabled, skipping.") + } + + // check with valid context + context, err := CurrentLabel() + if err != nil { + t.Fatalf("CurrentLabel() error: %v", err) + } + if context != "" { + t.Logf("SecurityCheckContext(%q)", context) + err = SecurityCheckContext(context) + if err != nil { + t.Errorf("SecurityCheckContext(%q) error: %v", context, err) + } + } + + context = "not-syntactically-valid" + err = SecurityCheckContext(context) + if err == nil { + t.Errorf("SecurityCheckContext(%q) succeeded, expected to fail", context) + } +} From 0d4b6a22580a1719a56a10eb7e09b5358db6ad73 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Wed, 19 Feb 2020 10:35:11 -0800 Subject: [PATCH 4/4] isProcHandle: simplify usage, improve diagnostics Simplify isProcHandle() to only return an error, making it easier to use. Improve errors from isProcHandle (and so from readCon/writeCon) to provide the file name in case of fstatfs() error. Signed-off-by: Kir Kolyshkin --- go-selinux/selinux_linux.go | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/go-selinux/selinux_linux.go b/go-selinux/selinux_linux.go index ceb1671..38a001e 100644 --- a/go-selinux/selinux_linux.go +++ b/go-selinux/selinux_linux.go @@ -255,10 +255,17 @@ func getSELinuxPolicyRoot() string { return filepath.Join(selinuxDir, readConfig(selinuxTypeTag)) } -func isProcHandle(fh *os.File) (bool, error) { +func isProcHandle(fh *os.File) error { var buf unix.Statfs_t err := unix.Fstatfs(int(fh.Fd()), &buf) - return buf.Type == unix.PROC_SUPER_MAGIC, err + if err != nil { + return fmt.Errorf("statfs(%q) failed: %v", fh.Name(), err) + } + if buf.Type != unix.PROC_SUPER_MAGIC { + return fmt.Errorf("file %q is not on procfs", fh.Name()) + } + + return nil } func readCon(fpath string) (string, error) { @@ -272,10 +279,8 @@ func readCon(fpath string) (string, error) { } defer in.Close() - if ok, err := isProcHandle(in); err != nil { + if err := isProcHandle(in); err != nil { return "", err - } else if !ok { - return "", fmt.Errorf("%s not on procfs", fpath) } var retval string @@ -347,7 +352,7 @@ func ExecLabel() (string, error) { return readCon(fmt.Sprintf("/proc/self/task/%d/attr/exec", syscall.Gettid())) } -func writeCon(fpath string, val string) error { +func writeCon(fpath, val string) error { if fpath == "" { return ErrEmptyPath } @@ -363,10 +368,8 @@ func writeCon(fpath string, val string) error { } defer out.Close() - if ok, err := isProcHandle(out); err != nil { + if err := isProcHandle(out); err != nil { return err - } else if !ok { - return fmt.Errorf("%s not on procfs", fpath) } if val != "" {