Store used MCS contexts on the filesystem #1742
Conversation
| @@ -270,19 +269,31 @@ func SelinuxGetEnforceMode() int { | |||
| } | |||
|
|
|||
| func mcsAdd(mcs string) error { | |||
| if mcsList[mcs] { | |||
| return fmt.Errorf("MCS Label already exists") | |||
| filename := fmt.Sprintf("%s/%s", "/run/rkt/", mcs) | |||
There was a problem hiding this comment.
Maybe add a helper for constructing this path and use it throughout?
| if os.IsExist(err) { | ||
| return fmt.Errorf("MCS Label already exists") | ||
| } else { | ||
| return fmt.Errorf("Unable to test MCS: %s", err) |
There was a problem hiding this comment.
Errors should be returned lower-cased. Also, some callers of this function are not checking them (maybe it's intended)
There was a problem hiding this comment.
We don't use any of the other callers of this. ReserveLabel() is fine with failure, CopyLabel() is inherently racy and so we'll be avoiding it in any case.
mjg59
force-pushed
the
maintain_contexts
branch
from
cec32d8
to
82e8775
Compare
| if mcsList[mcs] { | ||
| return fmt.Errorf("MCS Label already exists") | ||
| filename := mcsPath(mcs) | ||
| file, err := os.OpenFile(filename, os.O_CREATE|os.O_EXCL|os.O_RDONLY, 0644) |
There was a problem hiding this comment.
The directory /run/rkt seems to be created by metadata_service.go:unixListener() if the metadata service is used, but it might not exist otherwise.
Can you create the directory?
Actually, do all modern distributions set up a /run tmpfs? Or older distros like Centos 6.x (that we try to support, see #1443).
There was a problem hiding this comment.
We'll now create the directory if it doesn't exist.
|
other than alban's concerns, lgtm |
mjg59
force-pushed
the
maintain_contexts
branch
from
82e8775
to
840be78
Compare
In the absence of a long-running daemon, the current code that ensures that SELinux contexts aren't reused makes no sense - it's simply keeping a map of used contexts, which means independent instances of rkt aren't sharing this list. Keep the contexts in the filesystem instead in order to avoid this.
mjg59
force-pushed
the
maintain_contexts
branch
from
840be78
to
e1d3f4c
Compare
|
I think I've answered @alban's concerns |
|
LGTM |
Store used MCS contexts on the filesystem
In the absence of a long-running daemon, the current code that ensures that
SELinux contexts aren't reused makes no sense - it's simply keeping a map
of used contexts, which means independent instances of rkt aren't sharing
this list. Keep the contexts in the filesystem instead in order to avoid
this.