Support default profile for apparmor

Currently you can not apply an ApparmorProfile if you specify
--privileged.  This patch will allow both to be specified
simultaniosly.

By default Apparmor should be disabled if the user
specifies --privileged, but if the user specifies --security apparmor:PROFILE,
with --privileged, we should do both.

Added e2e run_apparmor_test.go

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>

cmd/podman/common/create.go

@@ -405,7 +405,7 @@ func GetCreateFlags(cf *ContainerCLIOpts) *pflag.FlagSet {
 	)
 	createFlags.StringArrayVar(
 		&cf.SecurityOpt,
-		"security-opt", containerConfig.SecurityOptions(),
+		"security-opt", []string{},
 		"Security Options",
 	)
 	createFlags.String(

cmd/podman/common/specgen.go

@@ -512,10 +512,8 @@ func FillOutSpecGen(s *specgen.SpecGenerator, c *ContainerCLIOpts, args []string
 				s.ContainerSecurityConfig.SelinuxOpts = append(s.ContainerSecurityConfig.SelinuxOpts, con[1])
 				s.Annotations[define.InspectAnnotationLabel] = strings.Join(s.ContainerSecurityConfig.SelinuxOpts, ",label=")
 			case "apparmor":
-				if !c.Privileged {
-					s.ContainerSecurityConfig.ApparmorProfile = con[1]
-					s.Annotations[define.InspectAnnotationApparmor] = con[1]
-				}
+				s.ContainerSecurityConfig.ApparmorProfile = con[1]
+				s.Annotations[define.InspectAnnotationApparmor] = con[1]
 			case "seccomp":
 				s.SeccompProfilePath = con[1]
 				s.Annotations[define.InspectAnnotationSeccomp] = con[1]

contrib/cirrus/setup_environment.sh

@@ -41,7 +41,6 @@ case "${OS_RELEASE_ID}" in
     ubuntu)
 	    apt-get update
 	    apt-get install -y containers-common
-	    sed -ie 's/^\(# \)\?apparmor_profile =.*/apparmor_profile = ""/' /etc/containers/containers.conf
 	    if [[ "$OS_RELEASE_VER" == "19" ]]; then
 		    apt-get purge -y --auto-remove golang*
 		    apt-get install -y golang-1.13

libpod/container_internal_linux.go

@@ -250,7 +250,7 @@ func (c *Container) generateSpec(ctx context.Context) (*spec.Spec, error) {
 	}
 
 	// Apply AppArmor checks and load the default profile if needed.
-	if !c.config.Privileged {
+	if len(c.config.Spec.Process.ApparmorProfile) > 0 {
 		updatedProfile, err := apparmor.CheckProfileAndLoadDefault(c.config.Spec.Process.ApparmorProfile)
 		if err != nil {
 			return nil, err

pkg/specgen/container_validate.go

@@ -81,10 +81,6 @@ func (s *SpecGenerator) Validate() error {
 	if len(s.CapAdd) > 0 && s.Privileged {
 		return exclusiveOptions("CapAdd", "privileged")
 	}
-	// apparmor and privileged are exclusive
-	if len(s.ApparmorProfile) > 0 && s.Privileged {
-		return exclusiveOptions("AppArmorProfile", "privileged")
-	}
 	// userns and idmappings conflict
 	if s.UserNS.IsPrivate() && s.IDMappings == nil {
 		return errors.Wrap(ErrInvalidSpecConfig, "IDMappings are required when not creating a User namespace")

pkg/specgen/generate/oci.go

@@ -285,13 +285,6 @@ func SpecGenToOCI(ctx context.Context, s *specgen.SpecGenerator, rt *libpod.Runt
 		}
 	}
 
-	// SECURITY OPTS
-	g.SetProcessNoNewPrivileges(s.NoNewPrivileges)
-
-	if !s.Privileged {
-		g.SetProcessApparmorProfile(s.ApparmorProfile)
-	}
-
 	BlockAccessToKernelFilesystems(s.Privileged, s.PidNS.IsHost(), &g)
 
 	for name, val := range s.Env {

pkg/specgen/generate/security.go

@@ -3,6 +3,7 @@ package generate
 import (
 	"strings"
 
+	"github.com/containers/common/pkg/apparmor"
 	"github.com/containers/common/pkg/capabilities"
 	"github.com/containers/common/pkg/config"
 	"github.com/containers/libpod/v2/libpod"
@@ -56,6 +57,28 @@ func setLabelOpts(s *specgen.SpecGenerator, runtime *libpod.Runtime, pidConfig s
 	return nil
 }
 
+func setupApparmor(s *specgen.SpecGenerator, rtc *config.Config, g *generate.Generator) error {
+	hasProfile := len(s.ApparmorProfile) > 0
+	if !apparmor.IsEnabled() {
+		if hasProfile {
+			return errors.Errorf("Apparmor profile %q specified, but Apparmor is not enabled on this system", s.ApparmorProfile)
+		}
+		return nil
+	}
+	// If privileged and caller did not specify apparmor profiles return
+	if s.Privileged && !hasProfile {
+		return nil
+	}
+	if !hasProfile {
+		s.ApparmorProfile = rtc.Containers.ApparmorProfile
+	}
+	if len(s.ApparmorProfile) > 0 {
+		g.SetProcessApparmorProfile(s.ApparmorProfile)
+	}
+
+	return nil
+}
+
 func securityConfigureGenerator(s *specgen.SpecGenerator, g *generate.Generator, newImage *image.Image, rtc *config.Config) error {
 	var (
 		caplist []string
@@ -105,6 +128,13 @@ func securityConfigureGenerator(s *specgen.SpecGenerator, g *generate.Generator,
 			}
 		}
 	}
+
+	g.SetProcessNoNewPrivileges(s.NoNewPrivileges)
+
+	if err := setupApparmor(s, rtc, g); err != nil {
+		return err
+	}
+
 	configSpec := g.Config
 	configSpec.Process.Capabilities.Bounding = caplist
 

test/e2e/run_apparmor_test.go

@@ -0,0 +1,158 @@
+// +build !remote
+
+package integration
+
+import (
+	"fmt"
+	"io/ioutil"
+	"os"
+	"path/filepath"
+
+	"github.com/containers/common/pkg/apparmor"
+	. "github.com/containers/libpod/v2/test/utils"
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+)
+
+func skipIfAppArmorEnabled() {
+	if apparmor.IsEnabled() {
+		Skip("Apparmor is enabled")
+	}
+}
+func skipIfAppArmorDisabled() {
+	if !apparmor.IsEnabled() {
+		Skip("Apparmor is not enabled")
+	}
+}
+
+var _ = Describe("Podman run", func() {
+	var (
+		tempdir    string
+		err        error
+		podmanTest *PodmanTestIntegration
+	)
+
+	BeforeEach(func() {
+		tempdir, err = CreateTempDirInTempDir()
+		if err != nil {
+			os.Exit(1)
+		}
+		podmanTest = PodmanTestCreate(tempdir)
+		podmanTest.Setup()
+		podmanTest.SeedImages()
+	})
+
+	AfterEach(func() {
+		podmanTest.Cleanup()
+		f := CurrentGinkgoTestDescription()
+		processTestResult(f)
+
+	})
+
+	It("podman run apparmor default", func() {
+		skipIfAppArmorDisabled()
+		session := podmanTest.Podman([]string{"create", ALPINE, "ls"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+
+		cid := session.OutputToString()
+		// Verify that apparmor.Profile is being set
+		inspect := podmanTest.InspectContainer(cid)
+		Expect(inspect[0].AppArmorProfile).To(Equal(apparmor.Profile))
+	})
+
+	It("podman run no apparmor --privileged", func() {
+		skipIfAppArmorDisabled()
+		session := podmanTest.Podman([]string{"create", "--privileged", ALPINE, "ls"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+
+		cid := session.OutputToString()
+		// Verify that apparmor.Profile is being set
+		inspect := podmanTest.InspectContainer(cid)
+		Expect(inspect[0].AppArmorProfile).To(Equal(""))
+	})
+
+	It("podman run no apparmor --security-opt=apparmor.Profile --privileged", func() {
+		skipIfAppArmorDisabled()
+		session := podmanTest.Podman([]string{"create", "--security-opt", fmt.Sprintf("apparmor=%s", apparmor.Profile), "--privileged", ALPINE, "ls"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+
+		cid := session.OutputToString()
+		// Verify that apparmor.Profile is being set
+		inspect := podmanTest.InspectContainer(cid)
+		Expect(inspect[0].AppArmorProfile).To(Equal(apparmor.Profile))
+	})
+
+	It("podman run apparmor aa-test-profile", func() {
+		skipIfAppArmorDisabled()
+		aaProfile := `
+#include <tunables/global>
+profile aa-test-profile flags=(attach_disconnected,mediate_deleted) {
+  #include <abstractions/base>
+  deny mount,
+  deny /sys/[^f]*/** wklx,
+  deny /sys/f[^s]*/** wklx,
+  deny /sys/fs/[^c]*/** wklx,
+  deny /sys/fs/c[^g]*/** wklx,
+  deny /sys/fs/cg[^r]*/** wklx,
+  deny /sys/firmware/efi/efivars/** rwklx,
+  deny /sys/kernel/security/** rwklx,
+}
+`
+		aaFile := filepath.Join(os.TempDir(), "aaFile")
+		Expect(ioutil.WriteFile(aaFile, []byte(aaProfile), 0755)).To(BeNil())
+		parse := SystemExec("apparmor_parser", []string{"-Kr", aaFile})
+		Expect(parse.ExitCode()).To(Equal(0))
+
+		session := podmanTest.Podman([]string{"create", "--security-opt", "apparmor=aa-test-profile", "ls"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+
+		cid := session.OutputToString()
+		// Verify that apparmor.Profile is being set
+		inspect := podmanTest.InspectContainer(cid)
+		Expect(inspect[0].AppArmorProfile).To(Equal("aa-test-profile"))
+	})
+
+	It("podman run apparmor invalid", func() {
+		skipIfAppArmorDisabled()
+		session := podmanTest.Podman([]string{"run", "--security-opt", "apparmor=invalid", ALPINE, "ls"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).ToNot(Equal(0))
+	})
+
+	It("podman run apparmor unconfined", func() {
+		skipIfAppArmorDisabled()
+		session := podmanTest.Podman([]string{"create", "--security-opt", "apparmor=unconfined", ALPINE, "ls"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+
+		cid := session.OutputToString()
+		// Verify that apparmor.Profile is being set
+		inspect := podmanTest.InspectContainer(cid)
+		Expect(inspect[0].AppArmorProfile).To(Equal("unconfined"))
+	})
+
+	It("podman run apparmor disabled --security-opt apparmor fails", func() {
+		skipIfAppArmorEnabled()
+		// Should fail if user specifies apparmor on disabled system
+		session := podmanTest.Podman([]string{"create", "--security-opt", fmt.Sprintf("apparmor=%s", apparmor.Profile), ALPINE, "ls"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).ToNot(Equal(0))
+	})
+
+	It("podman run apparmor disabled no default", func() {
+		skipIfAppArmorEnabled()
+		// Should succeed if user specifies apparmor on disabled system
+		session := podmanTest.Podman([]string{"create", ALPINE, "ls"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+
+		cid := session.OutputToString()
+		// Verify that apparmor.Profile is being set
+		inspect := podmanTest.InspectContainer(cid)
+		Expect(inspect[0].AppArmorProfile).To(Equal(""))
+	})
+})