Skip to content

apparmor: allow readby and tracedby#39121

Merged
thaJeztah merged 1 commit intomoby:masterfrom
goldwynr:master
Jun 11, 2019
Merged

apparmor: allow readby and tracedby#39121
thaJeztah merged 1 commit intomoby:masterfrom
goldwynr:master

Conversation

@goldwynr
Copy link
Copy Markdown
Contributor

@goldwynr goldwynr commented Apr 22, 2019

Fixes audit errors such as:

type=AVC msg=audit(1550236803.810:143):
apparmor="DENIED" operation="ptrace" profile="docker-default"
pid=3181 comm="ps" requested_mask="readby" denied_mask="readby"
peer="docker-default"

audit(1550236375.918:3): apparmor="DENIED" operation="ptrace"
profile="docker-default" pid=2267 comm="ps"
requested_mask="tracedby" denied_mask="tracedby"
peer="docker-default"

Signed-off-by: Goldwyn Rodrigues [email protected]

- What I did

- How I did it

- How to verify it

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

@goldwynr
apparmor: allow readby and tracedby
b364552 
Fixes audit errors such as:

type=AVC msg=audit(1550236803.810:143):
apparmor="DENIED" operation="ptrace" profile="docker-default"
pid=3181 comm="ps" requested_mask="readby" denied_mask="readby"
peer="docker-default"

audit(1550236375.918:3): apparmor="DENIED" operation="ptrace"
profile="docker-default" pid=2267 comm="ps"
requested_mask="tracedby" denied_mask="tracedby"
peer="docker-default"

Signed-off-by: Goldwyn Rodrigues <[email protected]>
@goldwynr goldwynr mentioned this pull request Apr 22, 2019
Closed
@thaJeztah
Copy link
Copy Markdown
Member

thaJeztah commented Apr 23, 2019

ping @justincormack @cyphar PTAL

@cyphar
Copy link
Copy Markdown
Contributor

cyphar commented Apr 24, 2019

Seems reasonable.

justincormack
justincormack reviewed Apr 26, 2019
View reviewed changes
Comment thread profiles/apparmor/template.go
{{if ge .Version 208095}}
# suppress ptrace denials when using 'docker ps' or using 'ps' inside a container
ptrace (trace,read) peer={{.Name}},
ptrace (trace,read,tracedby,readby) peer={{.Name}},
Copy link
Copy Markdown
Contributor

@justincormack justincormack Apr 26, 2019

Choose a reason for hiding this comment

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

I think maybe the comment should be changed on the line above - these are I think now needed as we allow ptrace which we did not for seccomp reasons before kernel 4.8, and I don't think you would get these just from ps.

@Lekensteyn
Copy link
Copy Markdown

Lekensteyn commented May 10, 2019

Had a coworker running into the same problem when running ls -l /proc/$pid/fd:

audit[22984]: AVC apparmor="DENIED" operation="ptrace" profile="docker-default" pid=22984 comm="ls" requested_mask="readby" denied_mask="readby" peer="docker-default"

Ubuntu 18.04 with a 4.18.0-17-generic kernel and the following Docker version:

Client:
 Version:           18.09.5
 API version:       1.38 (downgraded from 1.39)
 Go version:        go1.10.8
 Git commit:        e8ff056
 Built:             Thu Apr 11 04:43:57 2019
 OS/Arch:           linux/amd64
 Experimental:      false

Server:
 Engine:
  Version:          18.06.1-ce
  API version:      1.38 (minimum version 1.12)
  Go version:       go1.10.4
  Git commit:       e68fc7a
  Built:            Tue May  7 17:57:34 2019
  OS/Arch:          linux/amd64
  Experimental:     false

Patching the default profile with this tracedby,readby option (and testing it with apparmor_profile -r < new_profile.txt to replace the old profile) ensured that ls -l /proc/$pid/fd, ss -tlpn, lsof -iTCP, etc. work again and show the process name.

The readby option alone should also be sufficient to permit reading procfs, see also https://gitlab.com/apparmor/apparmor/wikis/TechnicalDoc_Proc_and_ptrace#ptrace-rules-for-proc

Documentation for the tracedby and readby settings can be found in the apparmor.d manual:
https://gitlab.com/apparmor/apparmor/blob/master/parser/apparmor.d.pod#L933

@thaJeztah
Copy link
Copy Markdown
Member

thaJeztah commented May 10, 2019

ping @goldwynr could you have a look at the review comment?

@justincormack any other issues with this patch, other than the comment?

@goldwynr
Copy link
Copy Markdown
Contributor Author

goldwynr commented May 11, 2019

@justincormack Without this patch the "denied "readby" messages are shown for docker ps, though the command succeeds. So, the comment still holds good.

@goldwynr
Copy link
Copy Markdown
Contributor Author

goldwynr commented Jun 10, 2019

ping! could the maintainers review the changes?

thaJeztah
thaJeztah approved these changes Jun 11, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

@thaJeztah thaJeztah merged commit 9e763de into moby:master Jun 11, 2019
@thaJeztah thaJeztah added this to the 20.03.0 milestone Apr 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@thaJeztah thaJeztah thaJeztah approved these changes

+1 more reviewer

@justincormack justincormack justincormack approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@vdemeester vdemeester

Labels

area/security/apparmor status/2-code-review

Projects

None yet

Milestone

20.10.0

Development

Successfully merging this pull request may close these issues.

8 participants

@goldwynr @thaJeztah @cyphar @Lekensteyn @justincormack @ziqianggeoffreychen @vdemeester @GordonTheTurtle