The following pull request was submitted through Github. It can be accessed and reviewed at: https://github.com/lxc/lxd/pull/7664
This e-mail was sent by the LXC bot, direct replies will not reach the author unless they happen to be subscribed to this list. === Description (from pull-request) ===
From 2b2ee9dd33452f41db38b30fb05a6a2c636413be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Graber?= <[email protected]> Date: Thu, 16 Jul 2020 14:39:14 -0400 Subject: [PATCH 1/2] lxd/sys: Create apparmor/seccomp paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Stéphane Graber <[email protected]> --- lxd/sys/fs.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lxd/sys/fs.go b/lxd/sys/fs.go index 88297d5b60..8c000ddcd7 100644 --- a/lxd/sys/fs.go +++ b/lxd/sys/fs.go @@ -55,6 +55,10 @@ func (s *OS) initDirs() error { {s.LogDir, 0700}, {filepath.Join(s.VarDir, "networks"), 0711}, {filepath.Join(s.VarDir, "security"), 0700}, + {filepath.Join(s.VarDir, "security", "apparmor"), 0700}, + {filepath.Join(s.VarDir, "security", "apparmor", "cache"), 0700}, + {filepath.Join(s.VarDir, "security", "apparmor", "profiles"), 0700}, + {filepath.Join(s.VarDir, "security", "seccomp"), 0700}, {filepath.Join(s.VarDir, "shmounts"), 0711}, // snapshots is 0700 as liblxc does not need to access this. {filepath.Join(s.VarDir, "snapshots"), 0700}, From 1b9d3152532646756a1a80b74680705753cba4e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Graber?= <[email protected]> Date: Thu, 16 Jul 2020 14:39:41 -0400 Subject: [PATCH 2/2] lxd/apparmor: Split and rename instance functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Stéphane Graber <[email protected]> --- lxd/apparmor/apparmor.go | 281 ++++++------------ lxd/apparmor/instance.go | 175 +++++++++++ .../{container.go => instance_lxc.go} | 2 +- lxd/instance/drivers/driver_lxc.go | 20 +- 4 files changed, 281 insertions(+), 197 deletions(-) create mode 100644 lxd/apparmor/instance.go rename lxd/apparmor/{container.go => instance_lxc.go} (99%) diff --git a/lxd/apparmor/apparmor.go b/lxd/apparmor/apparmor.go index 7598ba1211..e277358567 100644 --- a/lxd/apparmor/apparmor.go +++ b/lxd/apparmor/apparmor.go @@ -1,21 +1,16 @@ package apparmor import ( - "crypto/sha256" "fmt" - "io" - "io/ioutil" "os" - "path" + "path/filepath" "strings" - "github.com/lxc/lxd/lxd/project" + "github.com/pkg/errors" + "github.com/lxc/lxd/lxd/state" "github.com/lxc/lxd/shared" - "github.com/lxc/lxd/shared/logger" "github.com/lxc/lxd/shared/version" - - log "github.com/lxc/lxd/shared/log15" ) const ( @@ -26,252 +21,139 @@ const ( var aaPath = shared.VarPath("security", "apparmor") -type instance interface { - Project() string - Name() string - IsNesting() bool - IsPrivileged() bool - ExpandedConfig() map[string]string -} - -func mkApparmorName(name string) string { - if len(name)+7 >= 253 { - hash := sha256.New() - io.WriteString(hash, name) - return fmt.Sprintf("%x", hash.Sum(nil)) - } - - return name -} - -// Namespace returns the instance's apparmor namespace. -func Namespace(c instance) string { - /* / is not allowed in apparmor namespace names; let's also trim the - * leading / so it doesn't look like "-var-lib-lxd" - */ - lxddir := strings.Replace(strings.Trim(shared.VarPath(""), "/"), "/", "-", -1) - lxddir = mkApparmorName(lxddir) - name := project.Instance(c.Project(), c.Name()) - return fmt.Sprintf("lxd-%s_<%s>", name, lxddir) -} - -// ProfileFull returns the instance's apparmor profile. -func ProfileFull(c instance) string { - lxddir := shared.VarPath("") - lxddir = mkApparmorName(lxddir) - name := project.Instance(c.Project(), c.Name()) - return fmt.Sprintf("lxd-%s_<%s>", name, lxddir) -} - -func profileShort(c instance) string { - name := project.Instance(c.Project(), c.Name()) - return fmt.Sprintf("lxd-%s", name) -} - -// profileContent generates the apparmor profile template from the given container. -// This includes the stock lxc includes as well as stuff from raw.apparmor. -func profileContent(state *state.State, c instance) (string, error) { - // Prepare raw.apparmor. - rawContent := "" - rawApparmor, ok := c.ExpandedConfig()["raw.apparmor"] - if ok { - for _, line := range strings.Split(strings.Trim(rawApparmor, "\n"), "\n") { - rawContent += fmt.Sprintf(" %s\n", line) - } - } - - // Render the profile. - var sb *strings.Builder = &strings.Builder{} - err := containerProfile.Execute(sb, map[string]interface{}{ - "feature_unix": parserSupports("unix"), - "feature_cgns": shared.PathExists("/proc/self/ns/cgroup"), - "feature_stacking": state.OS.AppArmorStacking && !state.OS.AppArmorStacked, - "namespace": Namespace(c), - "nesting": c.IsNesting(), - "name": ProfileFull(c), - "unprivileged": !c.IsPrivileged() || state.OS.RunningInUserNS, - "raw": rawContent, - }) - if err != nil { - return "", err - } - - return sb.String(), nil -} - -func runApparmor(state *state.State, command string, c instance) error { +// runApparmor runs the relevant AppArmor command. +func runApparmor(state *state.State, command string, name string) error { if !state.OS.AppArmorAvailable { return nil } - output, err := shared.RunCommand("apparmor_parser", []string{ + _, err := shared.RunCommand("apparmor_parser", []string{ fmt.Sprintf("-%sWL", command), - path.Join(aaPath, "cache"), - path.Join(aaPath, "profiles", profileShort(c)), + filepath.Join(aaPath, "cache"), + filepath.Join(aaPath, "profiles", name), }...) if err != nil { - logger.Error("Running apparmor", - log.Ctx{"action": command, "output": output, "err": err}) + return err } - return err + return nil } -func getCacheDir() string { - basePath := path.Join(aaPath, "cache") - - ver, err := getVersion() - if err != nil { - logger.Errorf("Unable to get AppArmor version: %v", err) - return basePath - } - - // Multiple policy cache directories were only added in v2.13. - minVer, err := version.NewDottedVersion("2.13") - if err != nil { - logger.Errorf("Unable to parse AppArmor version 2.13: %v", err) - return basePath - } - - if ver.Compare(minVer) < 0 { - return basePath - } - - output, err := shared.RunCommand("apparmor_parser", "-L", basePath, "--print-cache-dir") - if err != nil { - logger.Errorf("Unable to get AppArmor cache directory: %v", err) - return basePath +// createNamespace creates a new AppArmor namespace. +func createNamespace(state *state.State, name string) error { + if !state.OS.AppArmorAvailable { + return nil } - return strings.TrimSpace(output) -} - -func mkApparmorNamespace(state *state.State, c instance, namespace string) error { if !state.OS.AppArmorStacking || state.OS.AppArmorStacked { return nil } - p := path.Join("/sys/kernel/security/apparmor/policy/namespaces", namespace) - if err := os.Mkdir(p, 0755); !os.IsExist(err) { + p := filepath.Join("/sys/kernel/security/apparmor/policy/namespaces", name) + err := os.Mkdir(p, 0755) + if err != nil && !os.IsExist(err) { return err } return nil } -// LoadProfile ensures that the instances's policy is loaded into the kernel so the it can boot. -func LoadProfile(state *state.State, c instance) error { - if !state.OS.AppArmorAdmin { +// deleteNamespace destroys an AppArmor namespace. +func deleteNamespace(state *state.State, name string) error { + if !state.OS.AppArmorAvailable { return nil } - err := mkApparmorNamespace(state, c, Namespace(c)) - if err != nil { - return err + if !state.OS.AppArmorStacking || state.OS.AppArmorStacked { + return nil } - /* In order to avoid forcing a profile parse (potentially slow) on - * every container start, let's use apparmor's binary policy cache, - * which checks mtime of the files to figure out if the policy needs to - * be regenerated. - * - * Since it uses mtimes, we shouldn't just always write out our local - * apparmor template; instead we should check to see whether the - * template is the same as ours. If it isn't we should write our - * version out so that the new changes are reflected and we definitely - * force a recompile. - */ - profile := path.Join(aaPath, "profiles", profileShort(c)) - content, err := ioutil.ReadFile(profile) + p := filepath.Join("/sys/kernel/security/apparmor/policy/namespaces", name) + err := os.Remove(p) if err != nil && !os.IsNotExist(err) { return err } - updated, err := profileContent(state, c) - if err != nil { - return err - } - - if string(content) != string(updated) { - err = os.MkdirAll(path.Join(aaPath, "cache"), 0700) - if err != nil { - return err - } - - err = os.MkdirAll(path.Join(aaPath, "profiles"), 0700) - if err != nil { - return err - } + return nil +} - err = ioutil.WriteFile(profile, []byte(updated), 0600) - if err != nil { - return err - } +// parseProfile parses the profile without loading it into the kernel. +func parseProfile(state *state.State, name string) error { + if !state.OS.AppArmorAvailable { + return nil } - return runApparmor(state, cmdLoad, c) + return runApparmor(state, cmdParse, name) } -// Destroy ensures that the instances's policy namespace is unloaded to free kernel memory. -// This does not delete the policy from disk or cache. -func Destroy(state *state.State, c instance) error { +// loadProfile loads the AppArmor profile into the kernel. +func loadProfile(state *state.State, name string) error { if !state.OS.AppArmorAdmin { return nil } - if state.OS.AppArmorStacking && !state.OS.AppArmorStacked { - p := path.Join("/sys/kernel/security/apparmor/policy/namespaces", Namespace(c)) - if err := os.Remove(p); err != nil { - logger.Error("Error removing apparmor namespace", log.Ctx{"err": err, "ns": p}) - } - } - - return runApparmor(state, cmdUnload, c) + return runApparmor(state, cmdLoad, name) } -// ParseProfile parses the profile without loading it into the kernel. -func ParseProfile(state *state.State, c instance) error { +// unloadProfile removes the profile from the kernel. +func unloadProfile(state *state.State, name string) error { if !state.OS.AppArmorAvailable { return nil } - return runApparmor(state, cmdParse, c) + return runApparmor(state, cmdUnload, name) } -// DeleteProfile removes the policy from cache/disk. -func DeleteProfile(state *state.State, c instance) { +// deleteProfile unloads and delete profile and cache for a profile. +func deleteProfile(state *state.State, name string) error { if !state.OS.AppArmorAdmin { - return + return nil + } + + cacheDir, err := getCacheDir() + if err != nil { + return err + } + + err = unloadProfile(state, name) + if err != nil { + return err + } + + err = os.Remove(filepath.Join(cacheDir, name)) + if err != nil && !os.IsNotExist(err) { + return errors.Wrapf(err, "Failed to remove: %s", filepath.Join(cacheDir, name)) } - /* It's ok if these deletes fail: if the container was never started, - * we'll have never written a profile or cached it. - */ - os.Remove(path.Join(getCacheDir(), profileShort(c))) - os.Remove(path.Join(aaPath, "profiles", profileShort(c))) + err = os.Remove(filepath.Join(aaPath, "profiles", name)) + if err != nil && !os.IsNotExist(err) { + return errors.Wrapf(err, "Failed to remove: %s", filepath.Join(aaPath, "profiles", name)) + } + + return nil } -func parserSupports(feature string) bool { +// parserSupports checks if the parser supports a particular feature. +func parserSupports(feature string) (bool, error) { ver, err := getVersion() if err != nil { - logger.Errorf("Unable to get AppArmor version: %v", err) - return false + return false, err } if feature == "unix" { minVer, err := version.NewDottedVersion("2.10.95") if err != nil { - logger.Errorf("Unable to parse AppArmor version 2.10.95: %v", err) - return false + return false, err } - return ver.Compare(minVer) >= 0 + return ver.Compare(minVer) >= 0, nil } - return false + return false, nil } +// getVersion reads and parses the AppArmor version. func getVersion() (*version.DottedVersion, error) { out, err := shared.RunCommand("apparmor_parser", "--version") if err != nil { @@ -281,3 +163,30 @@ func getVersion() (*version.DottedVersion, error) { fields := strings.Fields(strings.Split(out, "\n")[0]) return version.NewDottedVersion(fields[len(fields)-1]) } + +// getCacheDir returns the applicable AppArmor cache directory. +func getCacheDir() (string, error) { + basePath := filepath.Join(aaPath, "cache") + + ver, err := getVersion() + if err != nil { + return "", err + } + + // Multiple policy cache directories were only added in v2.13. + minVer, err := version.NewDottedVersion("2.13") + if err != nil { + return "", err + } + + if ver.Compare(minVer) < 0 { + return basePath, nil + } + + output, err := shared.RunCommand("apparmor_parser", "-L", basePath, "--print-cache-dir") + if err != nil { + return "", err + } + + return strings.TrimSpace(output), nil +} diff --git a/lxd/apparmor/instance.go b/lxd/apparmor/instance.go new file mode 100644 index 0000000000..9bdd93fb62 --- /dev/null +++ b/lxd/apparmor/instance.go @@ -0,0 +1,175 @@ +package apparmor + +import ( + "crypto/sha256" + "fmt" + "io" + "io/ioutil" + "os" + "path/filepath" + "strings" + + "github.com/lxc/lxd/lxd/project" + "github.com/lxc/lxd/lxd/state" + "github.com/lxc/lxd/shared" +) + +// Internal copy of the instance interface. +type instance interface { + Project() string + Name() string + IsNesting() bool + IsPrivileged() bool + ExpandedConfig() map[string]string +} + +// InstanceProfileName returns the instance's AppArmor profile name. +func InstanceProfileName(inst instance) string { + path := shared.VarPath("") + name := fmt.Sprintf("%s_<%s>", project.Instance(inst.Project(), inst.Name()), path) + + // Max length in AppArmor is 253 chars. + if len(name)+4 >= 253 { + hash := sha256.New() + io.WriteString(hash, name) + name = fmt.Sprintf("%x", hash.Sum(nil)) + } + + return fmt.Sprintf("lxd-%s", name) +} + +// InstanceNamespaceName returns the instance's AppArmor namespace. +func InstanceNamespaceName(inst instance) string { + // Unlike in profile names, / isn't an allowed character so replace with a -. + path := strings.Replace(strings.Trim(shared.VarPath(""), "/"), "/", "-", -1) + name := fmt.Sprintf("%s_<%s>", project.Instance(inst.Project(), inst.Name()), path) + + // Max length in AppArmor is 253 chars. + if len(name)+4 >= 253 { + hash := sha256.New() + io.WriteString(hash, name) + name = fmt.Sprintf("%x", hash.Sum(nil)) + } + + return fmt.Sprintf("lxd-%s", name) +} + +// instanceProfileFilename returns the name of the on-disk profile name. +func instanceProfileFilename(inst instance) string { + name := project.Instance(inst.Project(), inst.Name()) + + // Max length in AppArmor is 253 chars. + if len(name)+4 >= 253 { + hash := sha256.New() + io.WriteString(hash, name) + name = fmt.Sprintf("%x", hash.Sum(nil)) + } + + return fmt.Sprintf("lxd-%s", name) +} + +// InstanceLoad ensures that the instances's policy is loaded into the kernel so the it can boot. +func InstanceLoad(state *state.State, inst instance) error { + err := createNamespace(state, InstanceNamespaceName(inst)) + if err != nil { + return err + } + + /* In order to avoid forcing a profile parse (potentially slow) on + * every container start, let's use AppArmor's binary policy cache, + * which checks mtime of the files to figure out if the policy needs to + * be regenerated. + * + * Since it uses mtimes, we shouldn't just always write out our local + * AppArmor template; instead we should check to see whether the + * template is the same as ours. If it isn't we should write our + * version out so that the new changes are reflected and we definitely + * force a recompile. + */ + profile := filepath.Join(aaPath, "profiles", instanceProfileFilename(inst)) + content, err := ioutil.ReadFile(profile) + if err != nil && !os.IsNotExist(err) { + return err + } + + updated, err := instanceProfile(state, inst) + if err != nil { + return err + } + + if string(content) != string(updated) { + err = ioutil.WriteFile(profile, []byte(updated), 0600) + if err != nil { + return err + } + } + + err = loadProfile(state, instanceProfileFilename(inst)) + if err != nil { + return err + } + + return nil +} + +// InstanceUnload ensures that the instances's policy namespace is unloaded to free kernel memory. +// This does not delete the policy from disk or cache. +func InstanceUnload(state *state.State, inst instance) error { + err := deleteNamespace(state, InstanceNamespaceName(inst)) + if err != nil { + return err + } + + err = unloadProfile(state, instanceProfileFilename(inst)) + if err != nil { + return err + } + + return nil +} + +// InstanceParse validates the instance profile. +func InstanceParse(state *state.State, inst instance) error { + return parseProfile(state, instanceProfileFilename(inst)) +} + +// InstanceDelete removes the policy from cache/disk. +func InstanceDelete(state *state.State, inst instance) error { + return deleteProfile(state, instanceProfileFilename(inst)) +} + +// instanceProfile generates the AppArmor profile template from the given instance. +func instanceProfile(state *state.State, inst instance) (string, error) { + // Prepare raw.apparmor. + rawContent := "" + rawApparmor, ok := inst.ExpandedConfig()["raw.apparmor"] + if ok { + for _, line := range strings.Split(strings.Trim(rawApparmor, "\n"), "\n") { + rawContent += fmt.Sprintf(" %s\n", line) + } + } + + // Check for features. + unixSupported, err := parserSupports("unix") + if err != nil { + return "", err + } + + // Render the profile. + var sb *strings.Builder = &strings.Builder{} + err = lxcProfile.Execute(sb, map[string]interface{}{ + "feature_unix": unixSupported, + "feature_cgns": shared.PathExists("/proc/self/ns/cgroup"), + "feature_stacking": state.OS.AppArmorStacking && !state.OS.AppArmorStacked, + "namespace": InstanceNamespaceName(inst), + "nesting": inst.IsNesting(), + "name": InstanceProfileName(inst), + "unprivileged": !inst.IsPrivileged() || state.OS.RunningInUserNS, + "raw": rawContent, + }) + if err != nil { + return "", err + } + + return sb.String(), nil +} diff --git a/lxd/apparmor/container.go b/lxd/apparmor/instance_lxc.go similarity index 99% rename from lxd/apparmor/container.go rename to lxd/apparmor/instance_lxc.go index 02a08da532..4d6c423b54 100644 --- a/lxd/apparmor/container.go +++ b/lxd/apparmor/instance_lxc.go @@ -4,7 +4,7 @@ import ( "text/template" ) -var containerProfile = template.Must(template.New("containerProfile").Parse(`#include <tunables/global> +var lxcProfile = template.Must(template.New("lxcProfile").Parse(`#include <tunables/global> profile "{{ .name }}" flags=(attach_disconnected,mediate_deleted) { ### Base profile capability, diff --git a/lxd/instance/drivers/driver_lxc.go b/lxd/instance/drivers/driver_lxc.go index b5eac46db7..d0a5d59ead 100644 --- a/lxd/instance/drivers/driver_lxc.go +++ b/lxd/instance/drivers/driver_lxc.go @@ -977,7 +977,7 @@ func (c *lxc) initLXC(config bool) error { } } else { // If not currently confined, use the container's profile - profile := apparmor.ProfileFull(c) + profile := apparmor.InstanceProfileName(c) /* In the nesting case, we want to enable the inside * LXD to load its profile. Unprivileged containers can @@ -987,7 +987,7 @@ func (c *lxc) initLXC(config bool) error { * profile. */ if c.state.OS.AppArmorStacking && !c.state.OS.AppArmorStacked { - profile = fmt.Sprintf("%s//&:%s:", profile, apparmor.Namespace(c)) + profile = fmt.Sprintf("%s//&:%s:", profile, apparmor.InstanceNamespaceName(c)) } err := lxcSetConfigItem(cc, "lxc.apparmor.profile", profile) @@ -2474,7 +2474,7 @@ func (c *lxc) onStart(_ map[string]string) error { } // Load the container AppArmor profile - err = apparmor.LoadProfile(c.state, c) + err = apparmor.InstanceLoad(c.state, c) if err != nil { if ourStart { c.unmount() @@ -2488,7 +2488,7 @@ func (c *lxc) onStart(_ map[string]string) error { // Run any template that needs running err = c.templateApplyNow(c.localConfig[key]) if err != nil { - apparmor.Destroy(c.state, c) + apparmor.InstanceUnload(c.state, c) if ourStart { c.unmount() } @@ -2498,7 +2498,7 @@ func (c *lxc) onStart(_ map[string]string) error { // Remove the volatile key from the DB err := c.state.Cluster.DeleteInstanceConfigKey(c.id, key) if err != nil { - apparmor.Destroy(c.state, c) + apparmor.InstanceUnload(c.state, c) if ourStart { c.unmount() } @@ -2508,7 +2508,7 @@ func (c *lxc) onStart(_ map[string]string) error { err = c.templateApplyNow("start") if err != nil { - apparmor.Destroy(c.state, c) + apparmor.InstanceUnload(c.state, c) if ourStart { c.unmount() } @@ -2856,7 +2856,7 @@ func (c *lxc) onStop(args map[string]string) error { c.IsRunning() // Unload the apparmor profile - err = apparmor.Destroy(c.state, c) + err = apparmor.InstanceUnload(c.state, c) if err != nil { logger.Error("Failed to destroy apparmor namespace", log.Ctx{"container": c.Name(), "err": err}) } @@ -3441,7 +3441,7 @@ func (c *lxc) cleanup() { c.removeDiskDevices() // Remove the security profiles - apparmor.DeleteProfile(c.state, c) + apparmor.InstanceDelete(c.state, c) seccomp.DeleteProfile(c) // Remove the devices path @@ -4010,7 +4010,7 @@ func (c *lxc) Update(args db.InstanceArgs, userRequested bool) error { // If apparmor changed, re-validate the apparmor profile if shared.StringInSlice("raw.apparmor", changedConfig) || shared.StringInSlice("security.nesting", changedConfig) { - err = apparmor.ParseProfile(c.state, c) + err = apparmor.InstanceParse(c.state, c) if err != nil { return errors.Wrap(err, "Parse AppArmor profile") } @@ -4082,7 +4082,7 @@ func (c *lxc) Update(args db.InstanceArgs, userRequested bool) error { if key == "raw.apparmor" || key == "security.nesting" { // Update the AppArmor profile - err = apparmor.LoadProfile(c.state, c) + err = apparmor.InstanceLoad(c.state, c) if err != nil { return err }
_______________________________________________ lxc-devel mailing list [email protected] http://lists.linuxcontainers.org/listinfo/lxc-devel
