Do not force any selinux context on volumeDir

docker should be able to handle all of this correctly without the two problems in this patch. It hard codes svirt_sandbox_file_t, which is actually policy specific, even if basically everyone uses our reference policy. It means that things under that directory could (if they can break out of their mount namespace) see this directory. docker doesn't require this anymore. So remove this custom hack.
jthiatt · Jun 20, 2017 · 88e4c71 · 88e4c71
1 parent df9c0ed
commit 88e4c71
Show file tree

Hide file tree

Showing 7 changed files with 11 additions and 101 deletions.
diff --git a/docs/debugging-openshift.md b/docs/debugging-openshift.md
@@ -66,20 +66,6 @@ If this shows up in your build logs, restart docker and then resubmit a build:
     $ sudo systemctl restart docker
     $ oc start-build --from-build=<your build identifier>
 
-Another item seen stems from how OpenShift operates in a SELinux environment.  The SELinux policy requires that host directories that are bind mounted have the svirt_sandbox_file_t label.  Generally
-this simply happens for you under the covers, but there is a growing list of user operations which hamper the registry deployment to the point where the svrt_sandbox_file_t label ends up missing, and you can see
-various authentication or push failures.  One example, when initiating a build:
-
-     Failed to push image: Error pushing to registry: Server error: unexpected 500 response status trying to initiate upload of test/origin-ruby-sample
-
-And when inspecting the Docker registry, you will see messages like this:
-
-    173.17.42.1 - - [03/Jun/2015:13:26:19 +0000] "POST /v2/test/origin-ruby-sample/blobs/uploads/ HTTP/1.1" 500 203 "" "docker/1.6.0 go/go1.4.2 kernel/3.17.4-301.fc21.x86_64 os/linux arch/amd64"
-
-When this sequence occurs, without needing to restart Docker nor OpenShift, you can work around it by running the following command:
-
-     $ sudo chcon -R -t svirt_sandbox_file_t < path to >/openshift.local.volumes
-
 Docker Registry
 ---------------
 

diff --git a/hack/build-in-docker.sh b/hack/build-in-docker.sh
@@ -12,16 +12,6 @@ STARTTIME=$(date +%s)
 source "$(dirname "${BASH_SOURCE}")/lib/init.sh"
 origin_path="src/github.com/openshift/origin"
 
-# TODO: Remove this check and fix the docker command instead after the
-#       following PR is merged: https://github.com/docker/docker/pull/5910
-#       should be done in docker 1.6.
-if [ -d /sys/fs/selinux ]; then
-    if ! ls --context "$(absolute_path $OS_ROOT)" | grep --quiet svirt_sandbox_file_t; then
-        os::text::print_red "Warning: SELinux labels are not set correctly; run chcon -Rt svirt_sandbox_file_t $(absolute_path $OS_ROOT)"
-        exit 1
-    fi
-fi
-
-docker run -e "OWNER_GROUP=$UID:$GROUPS" --rm -v "$(absolute_path $OS_ROOT):/go/${origin_path}" openshift/origin-release /usr/bin/openshift-origin-build.sh $@
+docker run -e "OWNER_GROUP=$UID:$GROUPS" --rm -v "$(absolute_path $OS_ROOT):/go/${origin_path}:z" openshift/origin-release /usr/bin/openshift-origin-build.sh $@
 
 ret=$?; ENDTIME=$(date +%s); echo "$0 took $(($ENDTIME - $STARTTIME)) seconds"; exit "$ret"
diff --git a/pkg/cmd/server/kubernetes/node/node.go b/pkg/cmd/server/kubernetes/node/node.go
@@ -6,7 +6,6 @@ import (
 	"net"
 	"net/url"
 	"os"
-	"os/exec"
 	"path/filepath"
 	"time"
 
@@ -54,17 +53,6 @@ type commandExecutor interface {
 	Run(command string, args ...string) error
 }
 
-type defaultCommandExecutor struct{}
-
-func (ce defaultCommandExecutor) LookPath(executable string) (string, error) {
-	return exec.LookPath(executable)
-}
-
-func (ce defaultCommandExecutor) Run(command string, args ...string) error {
-	c := exec.Command(command, args...)
-	return c.Run()
-}
-
 const minimumDockerAPIVersionWithPullByID = "1.18"
 
 // EnsureKubeletAccess performs a number of test operations that the Kubelet requires to properly function.
@@ -181,14 +169,14 @@ func (c *NodeConfig) HandleDockerError(message string) {
 // an absolute path and create the directory if it does not exist. Will exit if
 // an error is encountered.
 func (c *NodeConfig) EnsureVolumeDir() {
-	if volumeDir, err := c.initializeVolumeDir(&defaultCommandExecutor{}, c.VolumeDir); err != nil {
+	if volumeDir, err := c.initializeVolumeDir(c.VolumeDir); err != nil {
 		glog.Fatal(err)
 	} else {
 		c.VolumeDir = volumeDir
 	}
 }
 
-func (c *NodeConfig) initializeVolumeDir(ce commandExecutor, path string) (string, error) {
+func (c *NodeConfig) initializeVolumeDir(path string) (string, error) {
 	rootDirectory, err := filepath.Abs(path)
 	if err != nil {
 		return "", fmt.Errorf("Error converting volume directory to an absolute path: %v", err)
@@ -199,14 +187,6 @@ func (c *NodeConfig) initializeVolumeDir(ce commandExecutor, path string) (strin
 			return "", fmt.Errorf("Couldn't create kubelet volume root directory '%s': %s", rootDirectory, err)
 		}
 	}
-	// always try to chcon, in case the volume dir existed prior to the node starting
-	if chconPath, err := ce.LookPath("chcon"); err != nil {
-		glog.V(2).Infof("Couldn't locate 'chcon' to set the kubelet volume root directory SELinux context: %s", err)
-	} else {
-		if err := ce.Run(chconPath, "-t", "svirt_sandbox_file_t", rootDirectory); err != nil {
-			glog.Warningf("Error running 'chcon' to set the kubelet volume root directory SELinux context: %s", err)
-		}
-	}
 	return rootDirectory, nil
 }
 

diff --git a/pkg/cmd/server/kubernetes/node/node_test.go b/pkg/cmd/server/kubernetes/node/node_test.go
@@ -1,34 +1,13 @@
 package node
 
 import (
-	"errors"
 	"io/ioutil"
 	"os"
 	"path"
 )
 
 import "testing"
 
-type fakeCommandExecutor struct {
-	commandFound bool
-	commandErr   error
-	runCalled    bool
-	lookCalled   bool
-}
-
-func (f *fakeCommandExecutor) LookPath(path string) (string, error) {
-	f.lookCalled = true
-	if f.commandFound {
-		return path, nil
-	}
-	return "", errors.New("not found")
-}
-
-func (f *fakeCommandExecutor) Run(command string, args ...string) error {
-	f.runCalled = true
-	return f.commandErr
-}
-
 func TestInitializeVolumeDir(t *testing.T) {
 	parentDir, err := ioutil.TempDir("", "")
 	if err != nil {
@@ -42,22 +21,13 @@ func TestInitializeVolumeDir(t *testing.T) {
 	volumeDir := path.Join(parentDir, "somedir")
 
 	testCases := map[string]struct {
-		chconFound       bool
-		chconRunErr      error
 		dirAlreadyExists bool
 	}{
-		"no chcon":                  {chconFound: false},
-		"have chcon":                {chconFound: true},
-		"chcon error":               {chconFound: true, chconRunErr: errors.New("e")},
-		"volume dir already exists": {chconFound: true, dirAlreadyExists: true},
+		"volume dir does not exist": {dirAlreadyExists: false},
+		"volume dir already exists": {dirAlreadyExists: true},
 	}
 
 	for name, testCase := range testCases {
-		ce := &fakeCommandExecutor{
-			commandFound: testCase.chconFound,
-			commandErr:   testCase.chconRunErr,
-		}
-
 		if testCase.dirAlreadyExists {
 			if err := os.MkdirAll(volumeDir, 0750); err != nil {
 				t.Fatalf("%s: error creating volume dir: %v", name, err)
@@ -69,17 +39,8 @@ func TestInitializeVolumeDir(t *testing.T) {
 		}
 
 		nc := &NodeConfig{VolumeDir: volumeDir}
-		path, err := nc.initializeVolumeDir(ce, nc.VolumeDir)
+		path, err := nc.initializeVolumeDir(nc.VolumeDir)
 
-		if !ce.lookCalled {
-			t.Errorf("%s: expected look for chcon", name)
-		}
-		if !testCase.chconFound && ce.runCalled {
-			t.Errorf("%s: unexpected run after chcon not found", name)
-		}
-		if testCase.chconFound && !ce.runCalled {
-			t.Errorf("%s: expected chcon run", name)
-		}
 		if err != nil {
 			t.Errorf("%s: unexpected err: %s", name, err)
 		}

diff --git a/pkg/diagnostics/cluster/registry.go b/pkg/diagnostics/cluster/registry.go
@@ -108,10 +108,6 @@ ownership/permissions.
 For volume permission problems please consult the Persistent Storage section
 of the Administrator's Guide.
 
-In the case of SELinux this may be resolved on the node by running:
-
-    sudo chcon -R -t svirt_sandbox_file_t [PATH_TO]/openshift.local.volumes
-
 %s`
 
 	clRegNoEP = `

diff --git a/test/extended/setup.sh b/test/extended/setup.sh
@@ -82,13 +82,10 @@ function os::test::extended::setup () {
 		SKIP_NODE=1
 	fi
 
-	# when selinux is enforcing, the volume dir selinux label needs to be
-	# svirt_sandbox_file_t
-	#
-	# TODO: fix the selinux policy to either allow openshift_var_lib_dir_t
-	# or to default the volume dir to svirt_sandbox_file_t.
+	# make sure the volume dir has the same label as we would apply to the default VOLUME_DIR
 	if selinuxenabled; then
-		${sudo} chcon -t svirt_sandbox_file_t ${VOLUME_DIR}
+		local label=$(matchpathcon -m dir /var/lib/openshift/openshift.local.volumes)
+		${sudo} chcon "${label}" ${VOLUME_DIR}
 	fi
 	CONFIG_VERSION=""
 	if [[ -n "${API_SERVER_VERSION:-}" ]]; then
@@ -240,4 +237,4 @@ function os::test::extended::merge_junit () {
 	rm "${TEST_REPORT_DIR}"/*.xml
 	mv "${output}" "${TEST_REPORT_DIR}/junit.xml"
 }
-readonly -f os::test::extended::merge_junit
+readonly -f os::test::extended::merge_junit
diff --git a/test/extended/util/framework.go b/test/extended/util/framework.go
@@ -1182,7 +1182,7 @@ func SetupHostPathVolumes(c kcoreclient.PersistentVolumeInterface, prefix, capac
 			return volumes, err
 		}
 		if _, err = exec.LookPath("chcon"); err == nil {
-			err := exec.Command("chcon", "-t", "svirt_sandbox_file_t", dir).Run()
+			err := exec.Command("chcon", "-t", "container_file_t", dir).Run()
 			if err != nil {
 				return volumes, err
 			}