Fix EnforceMode, SetEnforceMode, and SecurityCheckContext

[kolyshkin]

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(), 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

Other improvements in this PR:

• TestSetEnforceMode: separate and fix for non-root
• simplify isProcHandle() to only return an error (and therefore simplify its usage)
• improve errors from isProcHandle (and thus readCon()/writeCon())
  to provide file name in case of fstatfs() error
• Remove SelinuxfsMagic constant, use one from x/sys/unix.

@rhatdan @mrunalp @stephensmalley PTAL

[kolyshkin]

What's missing is a test case for SecurityCheckContext -- I wish I knew enough to write it :-\

[rhatdan]

LGTM
@stephensmalley PTAL

[stephensmalley]

NB I'm not familiar with this code so take this with a grain of salt.
Wondering whether reuse of readCon/writeCon was appropriate here in the first place for selinuxfs operations or if you should have used different functions.
I guess you can't use bindings to the libselinux functions themselves?
FWIW, libselinux uses /proc/thread-self when available (on Linux >= 3.17) to access the attr/* files (e.g. /proc/thread-self/attr/current) and only falls back to /proc/self/task/tid if /proc/thread-self doesn't exist. That's a bit simpler.
SecurityCheckContext could be tested by calling it with the result of reading the current context (I think you called it CurrentLabel or something) and seeing that it succeeds and calling it with a known-invalid context like "this-is-not-syntactically-valid" and seeing that it fails.

[kolyshkin]

Wondering whether reuse of readCon/writeCon was appropriate here in the first place for selinuxfs operations or if you should have used different functions.

You're quite right, in fact there's no need to check for selinuxfs magic since it was checked already when we searched for selinuxfs mount point.

I've updated this PR to do just that.

SecurityCheckContext could be tested by ...

Thanks, I've added a test case.

FWIW, libselinux uses /proc/thread-self when available

I guess this is out of scope for this PR, so I'll create a separate one for that

[kolyshkin]

FWIW, libselinux uses /proc/thread-self when available

I guess this is out of scope for this PR, so I'll create a separate one for that

Here: #66

[mrunalp]

LGTM

[rhatdan]

@stephensmalley We wanted a pure Go library for handling SELinux functions. Using libselinux with this would have pulled in C Bindings which can cause issues. So years ago it was decided to write the functions required by selinux in native go.
LGTM

[kolyshkin]

Just found out this was a recent breakage, caused by #59

[dims]

@kolyshkin this change is causing a problem in kubernetes : https://prow.k8s.io/view/gcs/kubernetes-jenkins/pr-logs/pull/86975/pull-kubernetes-typecheck/1232997816310173697

ERROR(linux/386,linux/arm) vendor/github.com/opencontainers/selinux/go-selinux/selinux_linux.go:118:17: unix.SELINUX_MAGIC (untyped int constant 4185718668) overflows int32
exit status 1
!!! Type Check has failed. This may cause cross platform build failures.
!!! Please see https://git.k8s.io/kubernetes/test/typecheck for more information.

[thaJeztah]

@dims looks like 86a2c3f#diff-dce803d0d70ed84c964812cea9579bc4 removed the uint32 conversion, which was added in e7d810c for that reason

Probably good to add a comment when fixing; I can open a quick PR

[thaJeztah]

oh I see it's fixed on master through #71

[dims]

yep. thanks for checking :)