pkg/apparmor: use a pipe instead of a tmp file

Use a pipe instead of a temporary file to load the apparmor profile.
This change has a measurable speed improvement for apparmor users.

Signed-off-by: Valentin Rothberg <vrothberg@suse.com>

Closes: #1262
Approved by: mheon

pkg/apparmor/apparmor_linux.go

@@ -6,7 +6,6 @@ import (
 	"bufio"
 	"fmt"
 	"io"
-	"io/ioutil"
 	"os"
 	"os/exec"
 	"path"
@@ -69,28 +68,30 @@ func macroExists(m string) bool {
 	return err == nil
 }
 
-// InstallDefault generates a default profile in a temp directory determined by
-// os.TempDir(), then loads the profile into the kernel using 'apparmor_parser'.
+// InstallDefault generates a default profile and loads it into the kernel
+// using 'apparmor_parser'.
 func InstallDefault(name string) error {
 	p := profileData{
 		Name: name,
 	}
 
-	// Install to a temporary directory.
-	f, err := ioutil.TempFile("", name)
+	cmd := exec.Command("apparmor_parser", "-Kr")
+	pipe, err := cmd.StdinPipe()
 	if err != nil {
 		return err
 	}
-	profilePath := f.Name()
-
-	defer f.Close()
-	defer os.Remove(profilePath)
-
-	if err := p.generateDefault(f); err != nil {
+	if err := cmd.Start(); err != nil {
+		pipe.Close()
+		return err
+	}
+	if err := p.generateDefault(pipe); err != nil {
+		pipe.Close()
+		cmd.Wait()
 		return err
 	}
 
-	return loadProfile(profilePath)
+	pipe.Close()
+	return cmd.Wait()
 }
 
 // IsLoaded checks if a profile with the given name has been loaded into the
@@ -135,14 +136,6 @@ func execAAParser(dir string, args ...string) (string, error) {
 	return string(output), nil
 }
 
-// loadProfile runs `apparmor_parser -Kr` on a specified apparmor profile to
-// replace the profile. The `-K` is necessary to make sure that apparmor_parser
-// doesn't try to write to a read-only filesystem.
-func loadProfile(profilePath string) error {
-	_, err := execAAParser("", "-Kr", profilePath)
-	return err
-}
-
 // getAAParserVersion returns the major and minor version of apparmor_parser.
 func getAAParserVersion() (int, error) {
 	output, err := execAAParser("", "--version")

pkg/apparmor/apparmor_linux_test.go

@@ -3,6 +3,7 @@
 package apparmor
 
 import (
+	"os"
 	"testing"
 )
 
@@ -76,3 +77,53 @@ Copyright 2009-2012 Canonical Ltd.
 		}
 	}
 }
+
+func TestInstallDefault(t *testing.T) {
+	profile := "libpod-default-testing"
+	aapath := "/sys/kernel/security/apparmor/"
+
+	if _, err := os.Stat(aapath); err != nil {
+		t.Skip("AppArmor isn't available in this environment")
+	}
+
+	// removes `profile`
+	removeProfile := func() error {
+		path := aapath + ".remove"
+
+		f, err := os.OpenFile(path, os.O_APPEND|os.O_WRONLY, os.ModeAppend)
+		if err != nil {
+			return err
+		}
+		defer f.Close()
+
+		_, err = f.WriteString(profile)
+		return err
+	}
+
+	// makes sure `profile` is loaded according to `state`
+	checkLoaded := func(state bool) {
+		loaded, err := IsLoaded(profile)
+		if err != nil {
+			t.Fatalf("Error searching AppArmor profile '%s': %v", profile, err)
+		}
+		if state != loaded {
+			if state {
+				t.Fatalf("AppArmor profile '%s' isn't loaded but should", profile)
+			} else {
+				t.Fatalf("AppArmor profile '%s' is loaded but shouldn't", profile)
+			}
+		}
+	}
+
+	// test installing the profile
+	if err := InstallDefault(profile); err != nil {
+		t.Fatalf("Couldn't install AppArmor profile '%s': %v", profile, err)
+	}
+	checkLoaded(true)
+
+	// remove the profile and check again
+	if err := removeProfile(); err != nil {
+		t.Fatalf("Couldn't remove AppArmor profile '%s': %v", profile, err)
+	}
+	checkLoaded(false)
+}