Skip to content

Need to setup labeling of kernel keyrings. #2012

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.

Sign up for GitHub

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

Merged
mrunalp merged 1 commit into from
Mar 21, 2019
Merged

Need to setup labeling of kernel keyrings. #2012

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 4 additions & 0 deletions libcontainer/setns_init_linux.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ func (l *linuxSetnsInit) Init() error {
defer runtime.UnlockOSThread()

if !l.config.Config.NoNewKeyring {
if err := label.SetKeyLabel(l.config.ProcessLabel); err != nil {
return err
}
defer label.SetKeyLabel("")
// Do not inherit the parent's session keyring.
if _, err := keys.JoinSessionKeyring(l.getSessionRingName()); err != nil {
// Same justification as in standart_init_linux.go as to why we
Expand Down
4 changes: 4 additions & 0 deletions libcontainer/standard_init_linux.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,10 @@ func (l *linuxStandardInit) Init() error {
runtime.LockOSThread()
defer runtime.UnlockOSThread()
if !l.config.Config.NoNewKeyring {
if err := label.SetKeyLabel(l.config.ProcessLabel); err != nil {
Copy link
Copy Markdown
Member

@cyphar cyphar Mar 14, 2019

Choose a reason for hiding this comment

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

Just to double-check -- are you sure this needs to be done before we create a new session? (Is SetKeyLabel setting what the label will be for all future keys or the label for the current key?)

Copy link
Copy Markdown
Contributor Author

@rhatdan rhatdan Mar 14, 2019

Choose a reason for hiding this comment

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

Yes this says to the kernel, any new keyrings that get created by this process and its children should get created with the specified label.
When the new keyring gets created it gets created with the correct label.
Later on the defaulf SetKeyLabel("") tells the kernel to go back to the default labeling.

Copy link
Copy Markdown
Contributor Author

@rhatdan rhatdan Mar 14, 2019

Choose a reason for hiding this comment

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

BTW I have been testing this patch with the updated kernel, which allows sepate kernel keyrings for each new UserNamespace.

Copy link
Copy Markdown
Contributor

@mrunalp mrunalp Mar 14, 2019

Choose a reason for hiding this comment

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

I am not clear why we need to reset it back. Shouldn't it pass down to the container process?

Copy link
Copy Markdown
Contributor Author

@rhatdan rhatdan Mar 15, 2019

Choose a reason for hiding this comment

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

Yes it will, the reset is on defer. Not really important for runc, but if people vendor in this code, then we would want to make sure other code paths don't accidently set the label to this container.

Copy link
Copy Markdown
Member

@cyphar cyphar Mar 16, 2019

Choose a reason for hiding this comment

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

Yup, we've discussed the defer handling for SELinux in the past. Even though it doesn't really make sense for runc it avoids people vendoring it and shooting themselves in the foot (not that SELinux is the only thing that is a massive foot-gun in libcontainer).

return err
}
defer label.SetKeyLabel("")
ringname, keepperms, newperms := l.getSessionRingParams()

// Do not inherit the parent's session keyring.
Expand Down
2 changes: 1 addition & 1 deletion vendor.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ github.com/opencontainers/runtime-spec 29686dbc5559d93fb1ef402eeda3e35c38d75af4
# Core libcontainer functionality.
github.com/checkpoint-restore/go-criu v3.11
github.com/mrunalp/fileutils ed869b029674c0e9ce4c0dfa781405c2d9946d08
github.com/opencontainers/selinux v1.0.0-rc1
github.com/opencontainers/selinux v1.2
Copy link
Copy Markdown
Member

@thaJeztah thaJeztah Mar 13, 2019

Choose a reason for hiding this comment

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

(not directly related to this PR); I noticed that these tags miss a trailing .0, and because of that are following the SemVer syntax (which may complicate using them with Go mod)

Copy link
Copy Markdown
Contributor Author

@rhatdan rhatdan Mar 14, 2019

Choose a reason for hiding this comment

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

I can add another .0 if necessary. in the next version.

Copy link
Copy Markdown
Member

@thaJeztah thaJeztah Mar 14, 2019

Choose a reason for hiding this comment

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

Thanks! I noticed it earlier this week, and this PR made me remember that I saw that 🤗

github.com/seccomp/libseccomp-golang 84e90a91acea0f4e51e62bc1a75de18b1fc0790f
github.com/sirupsen/logrus a3f95b5c423586578a4e099b11a46c2479628cac
github.com/syndtr/gocapability db04d3cc01c8b54962a58ec7e491717d06cfcc16
Expand Down
11 changes: 11 additions & 0 deletions vendor/github.com/opencontainers/selinux/README.md
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

35 changes: 30 additions & 5 deletions vendor/github.com/opencontainers/selinux/go-selinux/label/label.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

105 changes: 94 additions & 11 deletions vendor/github.com/opencontainers/selinux/go-selinux/label/label_selinux.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading