Skip to content

Commit 4bf7a84

Browse files
cypharcyphar
committed
apparmor: fix version checks to work properly
Using {{if major}}{{if minor}} doesn't work as expected when the major version changes. In addition, this didn't support patch levels (which is necessary in some cases when distributions ship apparmor weirdly). Signed-off-by: Aleksa Sarai <[email protected]>
1 parent 7d95325 commit 4bf7a84

6 files changed

Lines changed: 78 additions & 58 deletions

File tree

‎contrib/apparmor/main.go‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,7 @@ import (
1111
)
1212

1313
type profileData struct {
14-
MajorVersion int
15-
MinorVersion int
14+
Version int
1615
}
1716

1817
func main() {
@@ -23,13 +22,12 @@ func main() {
2322
// parse the arg
2423
apparmorProfilePath := os.Args[1]
2524

26-
majorVersion, minorVersion, err := aaparser.GetVersion()
25+
version, err := aaparser.GetVersion()
2726
if err != nil {
2827
log.Fatal(err)
2928
}
3029
data := profileData{
31-
MajorVersion: majorVersion,
32-
MinorVersion: minorVersion,
30+
Version: version,
3331
}
3432
fmt.Printf("apparmor_parser is of version %+v\n", data)
3533

‎contrib/apparmor/template.go‎

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,11 @@ profile /usr/bin/docker (attach_disconnected, complain) {
2020
2121
umount,
2222
pivot_root,
23-
{{if ge .MajorVersion 2}}{{if ge .MinorVersion 9}}
23+
{{if ge .Version 209000}}
2424
signal (receive) peer=@{profile_name},
2525
signal (receive) peer=unconfined,
2626
signal (send),
27-
{{end}}{{end}}
27+
{{end}}
2828
network,
2929
capability,
3030
owner /** rw,
@@ -46,12 +46,12 @@ profile /usr/bin/docker (attach_disconnected, complain) {
4646
/etc/ld.so.cache r,
4747
/etc/passwd r,
4848
49-
{{if ge .MajorVersion 2}}{{if ge .MinorVersion 9}}
49+
{{if ge .Version 209000}}
5050
ptrace peer=@{profile_name},
5151
ptrace (read) peer=docker-default,
5252
deny ptrace (trace) peer=docker-default,
5353
deny ptrace peer=/usr/bin/docker///bin/ps,
54-
{{end}}{{end}}
54+
{{end}}
5555
5656
/usr/lib/** rm,
5757
/lib/** rm,
@@ -72,11 +72,11 @@ profile /usr/bin/docker (attach_disconnected, complain) {
7272
/sbin/zfs rCx,
7373
/sbin/apparmor_parser rCx,
7474
75-
{{if ge .MajorVersion 2}}{{if ge .MinorVersion 9}}
75+
{{if ge .Version 209000}}
7676
# Transitions
7777
change_profile -> docker-*,
7878
change_profile -> unconfined,
79-
{{end}}{{end}}
79+
{{end}}
8080
8181
profile /bin/cat (complain) {
8282
/etc/ld.so.cache r,
@@ -98,10 +98,10 @@ profile /usr/bin/docker (attach_disconnected, complain) {
9898
/dev/null rw,
9999
/bin/ps mr,
100100
101-
{{if ge .MajorVersion 2}}{{if ge .MinorVersion 9}}
101+
{{if ge .Version 209000}}
102102
# We don't need ptrace so we'll deny and ignore the error.
103103
deny ptrace (read, trace),
104-
{{end}}{{end}}
104+
{{end}}
105105
106106
# Quiet dac_override denials
107107
deny capability dac_override,
@@ -119,15 +119,15 @@ profile /usr/bin/docker (attach_disconnected, complain) {
119119
/proc/tty/drivers r,
120120
}
121121
profile /sbin/iptables (complain) {
122-
{{if ge .MajorVersion 2}}{{if ge .MinorVersion 9}}
122+
{{if ge .Version 209000}}
123123
signal (receive) peer=/usr/bin/docker,
124-
{{end}}{{end}}
124+
{{end}}
125125
capability net_admin,
126126
}
127127
profile /sbin/auplink flags=(attach_disconnected, complain) {
128-
{{if ge .MajorVersion 2}}{{if ge .MinorVersion 9}}
128+
{{if ge .Version 209000}}
129129
signal (receive) peer=/usr/bin/docker,
130-
{{end}}{{end}}
130+
{{end}}
131131
capability sys_admin,
132132
capability dac_override,
133133
@@ -146,9 +146,9 @@ profile /usr/bin/docker (attach_disconnected, complain) {
146146
/proc/[0-9]*/mounts rw,
147147
}
148148
profile /sbin/modprobe /bin/kmod (complain) {
149-
{{if ge .MajorVersion 2}}{{if ge .MinorVersion 9}}
149+
{{if ge .Version 209000}}
150150
signal (receive) peer=/usr/bin/docker,
151-
{{end}}{{end}}
151+
{{end}}
152152
capability sys_module,
153153
/etc/ld.so.cache r,
154154
/lib/** rm,
@@ -162,9 +162,9 @@ profile /usr/bin/docker (attach_disconnected, complain) {
162162
}
163163
# xz works via pipes, so we do not need access to the filesystem.
164164
profile /usr/bin/xz (complain) {
165-
{{if ge .MajorVersion 2}}{{if ge .MinorVersion 9}}
165+
{{if ge .Version 209000}}
166166
signal (receive) peer=/usr/bin/docker,
167-
{{end}}{{end}}
167+
{{end}}
168168
/etc/ld.so.cache r,
169169
/lib/** rm,
170170
/usr/bin/xz rm,

‎pkg/aaparser/aaparser.go‎

Lines changed: 28 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,13 @@ const (
1414
)
1515

1616
// GetVersion returns the major and minor version of apparmor_parser.
17-
func GetVersion() (int, int, error) {
17+
func GetVersion() (int, error) {
1818
output, err := cmd("", "--version")
1919
if err != nil {
20-
return -1, -1, err
20+
return -1, err
2121
}
2222

23-
return parseVersion(string(output))
23+
return parseVersion(output)
2424
}
2525

2626
// LoadProfile runs `apparmor_parser -r -W` on a specified apparmor profile to
@@ -47,30 +47,46 @@ func cmd(dir string, arg ...string) (string, error) {
4747
}
4848

4949
// parseVersion takes the output from `apparmor_parser --version` and returns
50-
// the major and minor version for `apparor_parser`.
51-
func parseVersion(output string) (int, int, error) {
50+
// a representation of the {major, minor, patch} version as a single number of
51+
// the form MMmmPPP {major, minor, patch}.
52+
func parseVersion(output string) (int, error) {
5253
// output is in the form of the following:
5354
// AppArmor parser version 2.9.1
5455
// Copyright (C) 1999-2008 Novell Inc.
5556
// Copyright 2009-2012 Canonical Ltd.
57+
5658
lines := strings.SplitN(output, "\n", 2)
5759
words := strings.Split(lines[0], " ")
5860
version := words[len(words)-1]
5961

6062
// split by major minor version
6163
v := strings.Split(version, ".")
62-
if len(v) < 2 {
63-
return -1, -1, fmt.Errorf("parsing major minor version failed for output: `%s`", output)
64+
if len(v) == 0 || len(v) > 3 {
65+
return -1, fmt.Errorf("parsing version failed for output: `%s`", output)
6466
}
6567

68+
// Default the versions to 0.
69+
var majorVersion, minorVersion, patchLevel int
70+
6671
majorVersion, err := strconv.Atoi(v[0])
6772
if err != nil {
68-
return -1, -1, err
73+
return -1, err
6974
}
70-
minorVersion, err := strconv.Atoi(v[1])
71-
if err != nil {
72-
return -1, -1, err
75+
76+
if len(v) > 1 {
77+
minorVersion, err = strconv.Atoi(v[1])
78+
if err != nil {
79+
return -1, err
80+
}
81+
}
82+
if len(v) > 2 {
83+
patchLevel, err = strconv.Atoi(v[2])
84+
if err != nil {
85+
return -1, err
86+
}
7387
}
7488

75-
return majorVersion, minorVersion, nil
89+
// major*10^5 + minor*10^3 + patch*10^0
90+
numericVersion := majorVersion*1e5 + minorVersion*1e3 + patchLevel
91+
return numericVersion, nil
7692
}

‎pkg/aaparser/aaparser_test.go‎

Lines changed: 25 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,8 @@ import (
55
)
66

77
type versionExpected struct {
8-
output string
9-
major int
10-
minor int
8+
output string
9+
version int
1110
}
1211

1312
func TestParseVersion(t *testing.T) {
@@ -18,48 +17,57 @@ Copyright (C) 1999-2008 Novell Inc.
1817
Copyright 2009-2012 Canonical Ltd.
1918
2019
`,
21-
major: 2,
22-
minor: 10,
20+
version: 210000,
2321
},
2422
{
2523
output: `AppArmor parser version 2.8
2624
Copyright (C) 1999-2008 Novell Inc.
2725
Copyright 2009-2012 Canonical Ltd.
2826
2927
`,
30-
major: 2,
31-
minor: 8,
28+
version: 208000,
3229
},
3330
{
3431
output: `AppArmor parser version 2.20
3532
Copyright (C) 1999-2008 Novell Inc.
3633
Copyright 2009-2012 Canonical Ltd.
3734
3835
`,
39-
major: 2,
40-
minor: 20,
36+
version: 220000,
4137
},
4238
{
4339
output: `AppArmor parser version 2.05
4440
Copyright (C) 1999-2008 Novell Inc.
4541
Copyright 2009-2012 Canonical Ltd.
4642
4743
`,
48-
major: 2,
49-
minor: 5,
44+
version: 205000,
45+
},
46+
{
47+
output: `AppArmor parser version 2.9.95
48+
Copyright (C) 1999-2008 Novell Inc.
49+
Copyright 2009-2012 Canonical Ltd.
50+
51+
`,
52+
version: 209095,
53+
},
54+
{
55+
output: `AppArmor parser version 3.14.159
56+
Copyright (C) 1999-2008 Novell Inc.
57+
Copyright 2009-2012 Canonical Ltd.
58+
59+
`,
60+
version: 314159,
5061
},
5162
}
5263

5364
for _, v := range versions {
54-
major, minor, err := parseVersion(v.output)
65+
version, err := parseVersion(v.output)
5566
if err != nil {
5667
t.Fatalf("expected error to be nil for %#v, got: %v", v, err)
5768
}
58-
if major != v.major {
59-
t.Fatalf("expected major version to be %d, was %d, for: %#v\n", v.major, major, v)
60-
}
61-
if minor != v.minor {
62-
t.Fatalf("expected minor version to be %d, was %d, for: %#v\n", v.minor, minor, v)
69+
if version != v.version {
70+
t.Fatalf("expected version to be %d, was %d, for: %#v\n", v.version, version, v)
6371
}
6472
}
6573
}

‎profiles/apparmor/apparmor.go‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,8 @@ type profileData struct {
3030
Imports []string
3131
// InnerImports defines the apparmor functions to import in the profile.
3232
InnerImports []string
33-
// MajorVersion is the apparmor_parser major version.
34-
MajorVersion int
35-
// MinorVersion is the apparmor_parser minor version.
36-
MinorVersion int
33+
// Version is the {major, minor, patch} version of apparmor_parser as a single number.
34+
Version int
3735
}
3836

3937
// generateDefault creates an apparmor profile from ProfileData.

‎profiles/apparmor/template.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,13 +38,13 @@ profile {{.Name}} flags=(attach_disconnected,mediate_deleted) {
3838
deny /sys/firmware/efi/efivars/** rwklx,
3939
deny /sys/kernel/security/** rwklx,
4040
41-
{{if ge .MajorVersion 2}}{{if ge .MinorVersion 8}}
41+
{{if ge .Version 208000}}
4242
# suppress ptrace denials when using 'docker ps' or using 'ps' inside a container
4343
ptrace (trace,read) peer=docker-default,
44-
{{end}}{{end}}
45-
{{if ge .MajorVersion 2}}{{if ge .MinorVersion 9}}
44+
{{end}}
45+
{{if ge .Version 209000}}
4646
# docker daemon confinement requires explict allow rule for signal
4747
signal (receive) set=(kill,term) peer={{.ExecPath}},
48-
{{end}}{{end}}
48+
{{end}}
4949
}
5050
`

0 commit comments

Comments
 (0)