Repository: redhat-openshift-ecosystem/openshift-preflight
Pull request #211 (2021-09-07 12:59:34)

Move Runs As Root to Crane Engine
This moves the Runs as Root check to user the Crane engine.

Signed-off-by: Brad P. Crochet <brad@redhat.com>


Patch:
diff --git a/certification/engine/engine.go b/certification/engine/engine.go
index d2b633d..8e4cdc6 100644
--- a/certification/engine/engine.go
+++ b/certification/engine/engine.go
@@ -124,7 +124,7 @@ func queryNewChecks(checkName string) certification.Check {
 }
 
 // Register all checks
-var runAsNonRootCheck certification.Check = &shell.RunAsNonRootCheck{}
+var deprecatedRunAsNonRootCheck certification.Check = &shell.RunAsNonRootCheck{}
 var deprecatedUnderLayerMaxCheck certification.Check = &shell.UnderLayerMaxCheck{}
 var deprecatedHasRequiredLabelCheck certification.Check = &shell.HasRequiredLabelsCheck{}
 var basedOnUbiCheck certification.Check = &shell.BaseOnUBICheck{}
@@ -154,6 +154,7 @@ var scorecardOlmSuiteCheck certification.Check = operatorpol.NewScorecardOlmSuit
 var maxLayersCheck certification.Check = &containerpol.MaxLayersCheck{}
 var hasNoProhibitedCheck certification.Check = &containerpol.HasNoProhibitedPackagesCheck{}
 var hasRequiredLabelsCheck certification.Check = &containerpol.HasRequiredLabelsCheck{}
+var runAsRootCheck certification.Check = &containerpol.RunAsNonRootCheck{}
 
 var operatorPolicy = map[string]certification.Check{
 	operatorPkgNameIsUniqueCheck.Name(): operatorPkgNameIsUniqueCheck,
@@ -169,10 +170,11 @@ var containerPolicy = map[string]certification.Check{
 	hasLicenseCheck.Name():        hasLicenseCheck,
 	hasNoProhibitedCheck.Name():   hasNoProhibitedCheck,
 	hasRequiredLabelsCheck.Name(): hasRequiredLabelsCheck,
+	runAsRootCheck.Name():         runAsRootCheck,
 }
 
 var oldContainerPolicy = map[string]certification.Check{
-	runAsNonRootCheck.Name():               runAsNonRootCheck,
+	deprecatedRunAsNonRootCheck.Name():     deprecatedRunAsNonRootCheck,
 	deprecatedUnderLayerMaxCheck.Name():    deprecatedUnderLayerMaxCheck,
 	deprecatedHasRequiredLabelCheck.Name(): deprecatedHasRequiredLabelCheck,
 	basedOnUbiCheck.Name():                 basedOnUbiCheck,
diff --git a/certification/internal/policy/container/runs_as_nonroot.go b/certification/internal/policy/container/runs_as_nonroot.go
new file mode 100644
index 0000000..d938600
--- /dev/null
+++ b/certification/internal/policy/container/runs_as_nonroot.go
@@ -0,0 +1,64 @@
+package container
+
+import (
+	cranev1 "github.com/google/go-containerregistry/pkg/v1"
+	"github.com/redhat-openshift-ecosystem/openshift-preflight/certification"
+	log "github.com/sirupsen/logrus"
+)
+
+// RunAsNonRootCheck evaluates the image to determine that the runtime UID is not 0,
+// which correlates to the root user.
+type RunAsNonRootCheck struct{}
+
+func (p *RunAsNonRootCheck) Validate(imgRef certification.ImageReference) (bool, error) {
+	user, err := p.getDataToValidate(imgRef.ImageInfo)
+	if err != nil {
+		return false, err
+	}
+
+	return p.validate(user)
+}
+
+func (p *RunAsNonRootCheck) getDataToValidate(image cranev1.Image) (string, error) {
+	configFile, err := image.ConfigFile()
+	if err != nil {
+		log.Error("could not retrieve ConfigFile from Image")
+		return "", err
+	}
+	return configFile.Config.User, nil
+}
+
+func (p *RunAsNonRootCheck) validate(user string) (bool, error) {
+	if user == "" {
+		log.Debug("detected empty user. Presumed to be running as root")
+		return false, nil
+	}
+
+	if user == "0" || user == "root" {
+		log.Debugf("detected user specified as root: %s", user)
+		return false, nil
+	}
+
+	log.Debug("User specified that was not root")
+	return true, nil
+}
+
+func (p *RunAsNonRootCheck) Name() string {
+	return "RunAsNonRoot"
+}
+
+func (p *RunAsNonRootCheck) Metadata() certification.Metadata {
+	return certification.Metadata{
+		Description:      "Checking if container runs as the root user because a container that does not specify a non-root user will fail the automatic certification, and will be subject to a manual review before the container can be approved for publication",
+		Level:            "best",
+		KnowledgeBaseURL: "https://connect.redhat.com/zones/containers/container-certification-policy-guide",
+		CheckURL:         "https://connect.redhat.com/zones/containers/container-certification-policy-guide",
+	}
+}
+
+func (p *RunAsNonRootCheck) Help() certification.HelpText {
+	return certification.HelpText{
+		Message:    "Check RunAsNonRoot encountered an error. Please review the preflight.log file for more information.",
+		Suggestion: "Indicate a specific USER in the dockerfile or containerfile",
+	}
+}


Test patch:
diff --git a/certification/internal/policy/container/runs_as_nonroot_test.go b/certification/internal/policy/container/runs_as_nonroot_test.go
new file mode 100644
index 0000000..31360a7
--- /dev/null
+++ b/certification/internal/policy/container/runs_as_nonroot_test.go
@@ -0,0 +1,98 @@
+package container
+
+import (
+	cranev1 "github.com/google/go-containerregistry/pkg/v1"
+	fakecranev1 "github.com/google/go-containerregistry/pkg/v1/fake"
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+	"github.com/redhat-openshift-ecosystem/openshift-preflight/certification"
+)
+
+func userConfigFile(user string) (*cranev1.ConfigFile, error) {
+	return &cranev1.ConfigFile{
+		Config: cranev1.Config{
+			User: user,
+		},
+	}, nil
+}
+
+func configFileWithEmptyUser() (*cranev1.ConfigFile, error) {
+	return userConfigFile("")
+}
+
+func configFileWithGoodUser() (*cranev1.ConfigFile, error) {
+	return userConfigFile("1000")
+}
+
+func configFileWithRootUid() (*cranev1.ConfigFile, error) {
+	return userConfigFile("0")
+}
+
+func configFileWithRootUsername() (*cranev1.ConfigFile, error) {
+	return userConfigFile("root")
+}
+
+var _ = Describe("RunAsNonRoot", func() {
+	var (
+		runAsNonRoot RunAsNonRootCheck
+		imageRef     certification.ImageReference
+	)
+
+	BeforeEach(func() {
+		fakeImage := fakecranev1.FakeImage{
+			ConfigFileStub: configFileWithGoodUser,
+		}
+		imageRef.ImageInfo = &fakeImage
+	})
+
+	Describe("Checking manifest user is not root", func() {
+		Context("When manifest user is not root", func() {
+			It("should pass Validate", func() {
+				ok, err := runAsNonRoot.Validate(imageRef)
+				Expect(err).ToNot(HaveOccurred())
+				Expect(ok).To(BeTrue())
+			})
+		})
+	})
+	Describe("Checking manifest user is root", func() {
+		Context("When manifest user is empty", func() {
+			BeforeEach(func() {
+				fakeImage := fakecranev1.FakeImage{
+					ConfigFileStub: configFileWithEmptyUser,
+				}
+				imageRef.ImageInfo = &fakeImage
+			})
+			It("should not pass Validate", func() {
+				ok, err := runAsNonRoot.Validate(imageRef)
+				Expect(err).ToNot(HaveOccurred())
+				Expect(ok).To(BeFalse())
+			})
+		})
+		Context("When manifest user is string root", func() {
+			BeforeEach(func() {
+				fakeImage := fakecranev1.FakeImage{
+					ConfigFileStub: configFileWithRootUsername,
+				}
+				imageRef.ImageInfo = &fakeImage
+			})
+			It("should not pass Validate", func() {
+				ok, err := runAsNonRoot.Validate(imageRef)
+				Expect(err).ToNot(HaveOccurred())
+				Expect(ok).To(BeFalse())
+			})
+		})
+		Context("When manifest user is UID 0", func() {
+			BeforeEach(func() {
+				fakeImage := fakecranev1.FakeImage{
+					ConfigFileStub: configFileWithRootUid,
+				}
+				imageRef.ImageInfo = &fakeImage
+			})
+			It("should not pass Validate", func() {
+				ok, err := runAsNonRoot.Validate(imageRef)
+				Expect(err).ToNot(HaveOccurred())
+				Expect(ok).To(BeFalse())
+			})
+		})
+	})
+})