Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Instance: Allow nosymfollow mount flag for container apparmor profile #13681

Merged
merged 2 commits into from
Jul 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions lxd/apparmor/apparmor.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,22 @@ func parserSupports(sysOS *sys.OS, feature string) (bool, error) {
return ver.Compare(minVer) >= 0, nil
}

if feature == "mount_nosymfollow" {
sysOS.AppArmorFeatures.Lock()
defer sysOS.AppArmorFeatures.Unlock()
supported, ok := sysOS.AppArmorFeatures.Map[feature]
if !ok {
supported, err = FeatureCheck(sysOS, feature)
if err != nil {
return false, nil
}

sysOS.AppArmorFeatures.Map[feature] = supported
}

return supported, nil
}

return false, nil
}

Expand Down
75 changes: 75 additions & 0 deletions lxd/apparmor/feature_check.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
package apparmor

import (
"fmt"
"os"
"path/filepath"
"strings"
"text/template"

"github.com/google/uuid"

"github.com/canonical/lxd/lxd/sys"
"github.com/canonical/lxd/shared"
)

var featureCheckProfileTpl = template.Must(template.New("featureCheckProfile").Parse(`
profile "{{ .name }}" flags=(attach_disconnected,mediate_deleted) {

{{- if eq .feature "mount_nosymfollow" }}
mount options=(nosymfollow) /,
{{- end }}

}
`))

// FeatureCheck tries to generate feature check profile and process it with apparmor_parser.
func FeatureCheck(sysOS *sys.OS, feature string) (bool, error) {
randomUUID := uuid.New().String()
name := fmt.Sprintf("<%s-%s>", randomUUID, feature)
profileName := profileName("featurecheck", name)
profilePath := filepath.Join(aaPath, "profiles", profileName)
content, err := os.ReadFile(profilePath)
if err != nil && !os.IsNotExist(err) {
return false, err
}

updated, err := featureCheckProfile(profileName, feature)
if err != nil {
return false, err
}

if string(content) != string(updated) {
err = os.WriteFile(profilePath, []byte(updated), 0600)
if err != nil {
return false, err
}
}

defer func() {
_ = deleteProfile(sysOS, profileName, profileName)
}()

err = parseProfile(sysOS, profileName)
if err != nil {
return false, nil
}

return true, nil
}

// featureCheckProfile generates the AppArmor profile.
func featureCheckProfile(profileName string, feature string) (string, error) {
// Render the profile.
sb := &strings.Builder{}
err := featureCheckProfileTpl.Execute(sb, map[string]any{
"name": profileName,
"snap": shared.InSnap(),
"feature": feature,
})
if err != nil {
return "", err
}

return sb.String(), nil
}
26 changes: 16 additions & 10 deletions lxd/apparmor/instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,18 +154,24 @@ func instanceProfile(sysOS *sys.OS, inst instance) (string, error) {
}

// Render the profile.
var sb *strings.Builder = &strings.Builder{}
sb := &strings.Builder{}
if inst.Type() == instancetype.Container {
mountNosymfollowSupported, err := parserSupports(sysOS, "mount_nosymfollow")
if err != nil {
return "", err
}

err = lxcProfileTpl.Execute(sb, map[string]any{
"feature_cgns": sysOS.CGInfo.Namespacing,
"feature_cgroup2": sysOS.CGInfo.Layout == cgroup.CgroupsUnified || sysOS.CGInfo.Layout == cgroup.CgroupsHybrid,
"feature_stacking": sysOS.AppArmorStacking && !sysOS.AppArmorStacked,
"feature_unix": unixSupported,
"name": InstanceProfileName(inst),
"namespace": InstanceNamespaceName(inst),
"nesting": shared.IsTrue(inst.ExpandedConfig()["security.nesting"]),
"raw": rawContent,
"unprivileged": shared.IsFalseOrEmpty(inst.ExpandedConfig()["security.privileged"]) || sysOS.RunningInUserNS,
"feature_cgns": sysOS.CGInfo.Namespacing,
"feature_cgroup2": sysOS.CGInfo.Layout == cgroup.CgroupsUnified || sysOS.CGInfo.Layout == cgroup.CgroupsHybrid,
"feature_stacking": sysOS.AppArmorStacking && !sysOS.AppArmorStacked,
"feature_unix": unixSupported,
"feature_mount_nosymfollow": mountNosymfollowSupported,
"name": InstanceProfileName(inst),
"namespace": InstanceNamespaceName(inst),
"nesting": shared.IsTrue(inst.ExpandedConfig()["security.nesting"]),
"raw": rawContent,
"unprivileged": shared.IsFalseOrEmpty(inst.ExpandedConfig()["security.privileged"]) || sysOS.RunningInUserNS,
})
if err != nil {
return "", err
Expand Down
20 changes: 20 additions & 0 deletions lxd/apparmor/instance_lxc.go
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,26 @@ profile "{{ .name }}" flags=(attach_disconnected,mediate_deleted) {
mount options=(ro,remount,bind,nosuid,noexec,strictatime) /sy[^s]*{,/**},
mount options=(ro,remount,bind,nosuid,noexec,strictatime) /sys?*{,/**},

{{- if .feature_mount_nosymfollow }}
# see https://github.com/canonical/lxd/pull/12698
mount options=(ro,remount,bind,nosuid,noexec,nodev,nosymfollow) /[^spd]*{,/**},
mount options=(ro,remount,bind,nosuid,noexec,nodev,nosymfollow) /d[^e]*{,/**},
mount options=(ro,remount,bind,nosuid,noexec,nodev,nosymfollow) /de[^v]*{,/**},
mount options=(ro,remount,bind,nosuid,noexec,nodev,nosymfollow) /dev/.[^l]*{,/**},
mount options=(ro,remount,bind,nosuid,noexec,nodev,nosymfollow) /dev/.l[^x]*{,/**},
mount options=(ro,remount,bind,nosuid,noexec,nodev,nosymfollow) /dev/.lx[^c]*{,/**},
mount options=(ro,remount,bind,nosuid,noexec,nodev,nosymfollow) /dev/.lxc?*{,/**},
mount options=(ro,remount,bind,nosuid,noexec,nodev,nosymfollow) /dev/[^.]*{,/**},
mount options=(ro,remount,bind,nosuid,noexec,nodev,nosymfollow) /dev?*{,/**},
mount options=(ro,remount,bind,nosuid,noexec,nodev,nosymfollow) /p[^r]*{,/**},
mount options=(ro,remount,bind,nosuid,noexec,nodev,nosymfollow) /pr[^o]*{,/**},
mount options=(ro,remount,bind,nosuid,noexec,nodev,nosymfollow) /pro[^c]*{,/**},
mount options=(ro,remount,bind,nosuid,noexec,nodev,nosymfollow) /proc?*{,/**},
mount options=(ro,remount,bind,nosuid,noexec,nodev,nosymfollow) /s[^y]*{,/**},
mount options=(ro,remount,bind,nosuid,noexec,nodev,nosymfollow) /sy[^s]*{,/**},
mount options=(ro,remount,bind,nosuid,noexec,nodev,nosymfollow) /sys?*{,/**},
{{- end }}

# Allow bind-mounts of anything except /proc, /sys and /dev/.lxc
mount options=(rw,bind) /[^spd]*{,/**},
mount options=(rw,bind) /d[^e]*{,/**},
Expand Down
2 changes: 2 additions & 0 deletions lxd/sys/apparmor.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,8 @@ func (s *OS) initAppArmor() []cluster.Warning {
s.AppArmorConfined = true
}

s.AppArmorFeatures.Map = map[string]bool{}

return dbWarnings
}

Expand Down
7 changes: 7 additions & 0 deletions lxd/sys/os.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,12 @@ type InotifyInfo struct {
Targets map[string]*InotifyTargetInfo
}

// AppArmorFeaturesInfo records the AppArmor features availability.
type AppArmorFeaturesInfo struct {
sync.Mutex
Map map[string]bool
}

// OS is a high-level facade for accessing all operating-system
// level functionality that LXD uses.
type OS struct {
Expand Down Expand Up @@ -69,6 +75,7 @@ type OS struct {
AppArmorConfined bool
AppArmorStacked bool
AppArmorStacking bool
AppArmorFeatures AppArmorFeaturesInfo
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we make AppArmor a field with its own struct and internal mutex and then Features a map type, so we access it using OS.AppArmor.Features, rather than OS.AppArmorFeatures.Map which seems rather unusual (to have a field named after a data type) or OS.AppArmorFeatures.Features (which stutters)?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'd need to move the other AppArmor* fields into this new sub-struct.


// Cgroup features
CGInfo cgroup.Info
Expand Down
Loading