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

Cleanup privilege dropping/raising in pam_fscrypt #103

Merged
merged 3 commits into from Aug 23, 2018

Conversation

Projects
None yet
1 participant
@josephlr
Copy link
Member

commented Aug 23, 2018

Fixes #77 and CVE-2018-6558

This PR changes how privilege dropping/raising is done throughout fscrypt. It makes sure that all changes are exactly reversed to prevent issues where user's would have their group membership messed up after login. This lead to a security issue where users could end up a member of the root group (GID=0), resulting in privilege escalation.

josephlr added some commits Aug 22, 2018

Ensure setting user privileges is reversible
This change makes sure after dropping then elevating privileges for a
process, the euid, guid, and groups are all the same as they were
originally. This significantly simplifies the privilege logic.

This fixes CVE-2018-6558, which allowed an unprivleged user to gain
membership in the root group (gid 0) due to the groups not being
properly reset in the process.
Ensure keyring privilege changes are reversible
This change makes sure that, when we set the ruid and euid in order to
get the user keyring linked into the current process keyring, we will
always be able to reverse these changes (using a suid of 0).

This fixes an issue where "su <user>" would result in a system error
when called by an unprivileged user. It also explains exactly how and
why we are making these privilege changes.

@josephlr josephlr merged commit 6ba94e2 into master Aug 23, 2018

2 of 4 checks passed

coverage/coveralls Coverage decreased (-0.2%) to 32.452%
Details
continuous-integration/travis-ci/push The Travis CI build is in progress
Details
cla/google All necessary CLAs are signed
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@josephlr josephlr deleted the pam branch Aug 23, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.