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

core/exec: Restore SmackProcessLabel setting #7378

Merged
merged 1 commit into from Nov 21, 2017
Merged

core/exec: Restore SmackProcessLabel setting #7378

merged 1 commit into from Nov 21, 2017

Conversation

jobol
Copy link
Contributor

@jobol jobol commented Nov 17, 2017

Smack LSM needs the capability CAP_MAC_ADMIN to allow
setting of the current Smack exec label. Consequently,
dropping capabilities must be done after changing the
current exec label.

This is only related to Smack LSM. But for clarity and
regularity, all setting of security context moved before
dropping capabilities.

See Issue #7108

@walyong
Copy link
Contributor

walyong commented Nov 18, 2017

LGTM, but systemd does not take Change-Id and Signe-off-by. Remove them.

@jobol
Copy link
Contributor Author

jobol commented Nov 20, 2017

Done: I removed the 2 lines and rebased on master

@poettering
Copy link
Member

I am not sure we should move more than the SMACK stuff. I mean, the SMACK stuff broke because we didn't really understand the effects of moving it down in the first place because SMACK is not tested regularly upstream (and isn't part of the CI either). Now, the other MACs aren't tested like that either, hence we should learn from that and not touch them for now, otherwise we might break them too...

Hence, let#s leave Selinux/AA stuff where it is, and only move SMACK up

@poettering poettering added the reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks label Nov 20, 2017
Smack LSM needs the capability CAP_MAC_ADMIN to allow
setting of the current Smack exec label. Consequently,
dropping capabilities must be done after changing the
current exec label.

This is only related to Smack LSM. But for clarity and
regularity, all setting of security context moved before
dropping capabilities.

See Issue 7108
@keszybz keszybz added good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed and removed reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks labels Nov 20, 2017
@keszybz
Copy link
Member

keszybz commented Nov 20, 2017

I'll restart s390x.

@poettering poettering merged commit 37ac274 into systemd:master Nov 21, 2017
@KumarShrawan
Copy link

KumarShrawan commented Nov 21, 2017

The below snip from service file was working earlier (Systemd229), recently moved to systemd234 . Now it throws error "failed to set SMACK process label: Operation not permitted".

Is this the root cause ?

PermissionsStartOnly=true
User=user1
Group=group1
SmackProcessLabel=xyz
CapabilityBoundingSet=

@jobol
Copy link
Contributor Author

jobol commented Nov 21, 2017

@KumarShrawan I guess yes and this patch is supposed to fix the error

feedback welcome obviously

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed smack
Development

Successfully merging this pull request may close these issues.

None yet

5 participants