Merge pull request #20305 from cyphar/fix-apparmor-version-checks

Fix apparmor version checks

contrib/apparmor/main.go

@@ -11,8 +11,7 @@ import (
 )
 
 type profileData struct {
-	MajorVersion int
-	MinorVersion int
+	Version int
 }
 
 func main() {
@@ -23,13 +22,12 @@ func main() {
 	// parse the arg
 	apparmorProfilePath := os.Args[1]
 
-	majorVersion, minorVersion, err := aaparser.GetVersion()
+	version, err := aaparser.GetVersion()
 	if err != nil {
 		log.Fatal(err)
 	}
 	data := profileData{
-		MajorVersion: majorVersion,
-		MinorVersion: minorVersion,
+		Version: version,
 	}
 	fmt.Printf("apparmor_parser is of version %+v\n", data)
 

contrib/apparmor/template.go

@@ -20,11 +20,11 @@ profile /usr/bin/docker (attach_disconnected, complain) {
 
   umount,
   pivot_root,
-{{if ge .MajorVersion 2}}{{if ge .MinorVersion 9}}
+{{if ge .Version 209000}}
   signal (receive) peer=@{profile_name},
   signal (receive) peer=unconfined,
   signal (send),
-{{end}}{{end}}
+{{end}}
   network,
   capability,
   owner /** rw,
@@ -46,12 +46,12 @@ profile /usr/bin/docker (attach_disconnected, complain) {
   /etc/ld.so.cache r,
   /etc/passwd r,
 
-{{if ge .MajorVersion 2}}{{if ge .MinorVersion 9}}
+{{if ge .Version 209000}}
   ptrace peer=@{profile_name},
   ptrace (read) peer=docker-default,
   deny ptrace (trace) peer=docker-default,
   deny ptrace peer=/usr/bin/docker///bin/ps,
-{{end}}{{end}}
+{{end}}
 
   /usr/lib/** rm,
   /lib/** rm,
@@ -72,11 +72,11 @@ profile /usr/bin/docker (attach_disconnected, complain) {
   /sbin/zfs rCx,
   /sbin/apparmor_parser rCx,
 
-{{if ge .MajorVersion 2}}{{if ge .MinorVersion 9}}
+{{if ge .Version 209000}}
   # Transitions
   change_profile -> docker-*,
   change_profile -> unconfined,
-{{end}}{{end}}
+{{end}}
 
   profile /bin/cat (complain) {
     /etc/ld.so.cache r,
@@ -98,10 +98,10 @@ profile /usr/bin/docker (attach_disconnected, complain) {
     /dev/null rw,
     /bin/ps mr,
 
-{{if ge .MajorVersion 2}}{{if ge .MinorVersion 9}}
+{{if ge .Version 209000}}
     # We don't need ptrace so we'll deny and ignore the error.
     deny ptrace (read, trace),
-{{end}}{{end}}
+{{end}}
 
     # Quiet dac_override denials
     deny capability dac_override,
@@ -119,15 +119,15 @@ profile /usr/bin/docker (attach_disconnected, complain) {
     /proc/tty/drivers r,
   }
   profile /sbin/iptables (complain) {
-{{if ge .MajorVersion 2}}{{if ge .MinorVersion 9}}
+{{if ge .Version 209000}}
     signal (receive) peer=/usr/bin/docker,
-{{end}}{{end}}
+{{end}}
     capability net_admin,
   }
   profile /sbin/auplink flags=(attach_disconnected, complain) {
-{{if ge .MajorVersion 2}}{{if ge .MinorVersion 9}}
+{{if ge .Version 209000}}
     signal (receive) peer=/usr/bin/docker,
-{{end}}{{end}}
+{{end}}
     capability sys_admin,
     capability dac_override,
 
@@ -146,9 +146,9 @@ profile /usr/bin/docker (attach_disconnected, complain) {
     /proc/[0-9]*/mounts rw,
   }
   profile /sbin/modprobe /bin/kmod (complain) {
-{{if ge .MajorVersion 2}}{{if ge .MinorVersion 9}}
+{{if ge .Version 209000}}
     signal (receive) peer=/usr/bin/docker,
-{{end}}{{end}}
+{{end}}
     capability sys_module,
     /etc/ld.so.cache r,
     /lib/** rm,
@@ -162,9 +162,9 @@ profile /usr/bin/docker (attach_disconnected, complain) {
   }
   # xz works via pipes, so we do not need access to the filesystem.
   profile /usr/bin/xz (complain) {
-{{if ge .MajorVersion 2}}{{if ge .MinorVersion 9}}
+{{if ge .Version 209000}}
     signal (receive) peer=/usr/bin/docker,
-{{end}}{{end}}
+{{end}}
     /etc/ld.so.cache r,
     /lib/** rm,
     /usr/bin/xz rm,

pkg/aaparser/aaparser.go

@@ -14,13 +14,13 @@ const (
 )
 
 // GetVersion returns the major and minor version of apparmor_parser.
-func GetVersion() (int, int, error) {
+func GetVersion() (int, error) {
 	output, err := cmd("", "--version")
 	if err != nil {
-		return -1, -1, err
+		return -1, err
 	}
 
-	return parseVersion(string(output))
+	return parseVersion(output)
 }
 
 // LoadProfile runs `apparmor_parser -r -W` on a specified apparmor profile to
@@ -47,30 +47,46 @@ func cmd(dir string, arg ...string) (string, error) {
 }
 
 // parseVersion takes the output from `apparmor_parser --version` and returns
-// the major and minor version for `apparor_parser`.
-func parseVersion(output string) (int, int, error) {
+// a representation of the {major, minor, patch} version as a single number of
+// the form MMmmPPP {major, minor, patch}.
+func parseVersion(output string) (int, error) {
 	// output is in the form of the following:
 	// AppArmor parser version 2.9.1
 	// Copyright (C) 1999-2008 Novell Inc.
 	// Copyright 2009-2012 Canonical Ltd.
+
 	lines := strings.SplitN(output, "\n", 2)
 	words := strings.Split(lines[0], " ")
 	version := words[len(words)-1]
 
 	// split by major minor version
 	v := strings.Split(version, ".")
-	if len(v) < 2 {
-		return -1, -1, fmt.Errorf("parsing major minor version failed for output: `%s`", output)
+	if len(v) == 0 || len(v) > 3 {
+		return -1, fmt.Errorf("parsing version failed for output: `%s`", output)
 	}
 
+	// Default the versions to 0.
+	var majorVersion, minorVersion, patchLevel int
+
 	majorVersion, err := strconv.Atoi(v[0])
 	if err != nil {
-		return -1, -1, err
+		return -1, err
 	}
-	minorVersion, err := strconv.Atoi(v[1])
-	if err != nil {
-		return -1, -1, err
+
+	if len(v) > 1 {
+		minorVersion, err = strconv.Atoi(v[1])
+		if err != nil {
+			return -1, err
+		}
+	}
+	if len(v) > 2 {
+		patchLevel, err = strconv.Atoi(v[2])
+		if err != nil {
+			return -1, err
+		}
 	}
 
-	return majorVersion, minorVersion, nil
+	// major*10^5 + minor*10^3 + patch*10^0
+	numericVersion := majorVersion*1e5 + minorVersion*1e3 + patchLevel
+	return numericVersion, nil
 }

pkg/aaparser/aaparser_test.go

@@ -5,9 +5,8 @@ import (
 )
 
 type versionExpected struct {
-	output string
-	major  int
-	minor  int
+	output  string
+	version int
 }
 
 func TestParseVersion(t *testing.T) {
@@ -18,8 +17,7 @@ Copyright (C) 1999-2008 Novell Inc.
 Copyright 2009-2012 Canonical Ltd.
 
 `,
-			major: 2,
-			minor: 10,
+			version: 210000,
 		},
 		{
 			output: `AppArmor parser version 2.8
@@ -27,8 +25,7 @@ Copyright (C) 1999-2008 Novell Inc.
 Copyright 2009-2012 Canonical Ltd.
 
 `,
-			major: 2,
-			minor: 8,
+			version: 208000,
 		},
 		{
 			output: `AppArmor parser version 2.20
@@ -36,8 +33,7 @@ Copyright (C) 1999-2008 Novell Inc.
 Copyright 2009-2012 Canonical Ltd.
 
 `,
-			major: 2,
-			minor: 20,
+			version: 220000,
 		},
 		{
 			output: `AppArmor parser version 2.05
@@ -45,21 +41,33 @@ Copyright (C) 1999-2008 Novell Inc.
 Copyright 2009-2012 Canonical Ltd.
 
 `,
-			major: 2,
-			minor: 5,
+			version: 205000,
+		},
+		{
+			output: `AppArmor parser version 2.9.95
+Copyright (C) 1999-2008 Novell Inc.
+Copyright 2009-2012 Canonical Ltd.
+
+`,
+			version: 209095,
+		},
+		{
+			output: `AppArmor parser version 3.14.159
+Copyright (C) 1999-2008 Novell Inc.
+Copyright 2009-2012 Canonical Ltd.
+
+`,
+			version: 314159,
 		},
 	}
 
 	for _, v := range versions {
-		major, minor, err := parseVersion(v.output)
+		version, err := parseVersion(v.output)
 		if err != nil {
 			t.Fatalf("expected error to be nil for %#v, got: %v", v, err)
 		}
-		if major != v.major {
-			t.Fatalf("expected major version to be %d, was %d, for: %#v\n", v.major, major, v)
-		}
-		if minor != v.minor {
-			t.Fatalf("expected minor version to be %d, was %d, for: %#v\n", v.minor, minor, v)
+		if version != v.version {
+			t.Fatalf("expected version to be %d, was %d, for: %#v\n", v.version, version, v)
 		}
 	}
 }

profiles/apparmor/apparmor.go

@@ -30,10 +30,8 @@ type profileData struct {
 	Imports []string
 	// InnerImports defines the apparmor functions to import in the profile.
 	InnerImports []string
-	// MajorVersion is the apparmor_parser major version.
-	MajorVersion int
-	// MinorVersion is the apparmor_parser minor version.
-	MinorVersion int
+	// Version is the {major, minor, patch} version of apparmor_parser as a single number.
+	Version int
 }
 
 // generateDefault creates an apparmor profile from ProfileData.

profiles/apparmor/template.go

@@ -38,13 +38,13 @@ profile {{.Name}} flags=(attach_disconnected,mediate_deleted) {
   deny /sys/firmware/efi/efivars/** rwklx,
   deny /sys/kernel/security/** rwklx,
 
-{{if ge .MajorVersion 2}}{{if ge .MinorVersion 8}}
+{{if ge .Version 208095}}
   # suppress ptrace denials when using 'docker ps' or using 'ps' inside a container
   ptrace (trace,read) peer=docker-default,
-{{end}}{{end}}
-{{if ge .MajorVersion 2}}{{if ge .MinorVersion 9}}
+{{end}}
+{{if ge .Version 209000}}
   # docker daemon confinement requires explict allow rule for signal
   signal (receive) set=(kill,term) peer={{.ExecPath}},
-{{end}}{{end}}
+{{end}}
 }
 `