Require user to authenticate when trying to change lock screen settings
AbandonedPublic

Authored by graesslin on Jan 12 2016, 12:52 PM.

Details

Summary

The idea behind this change is to make it impossible for a malicious
process to just disable the lock screen. To achieve that modifying the
lock screen settings are denied for the normal user. When the user wants
to modify the settings a KAuth helper is required.

The security is done by setting the config file to immutable. That way
a non-root process is no longer able to modify the file. Neither write
to it, nor rename it or delete it.

The KAuth helper removes the immutable flag, writes the changes and
sets the immutable flag again. If the file doesn't exist, it creates
it, changes ownership to the owning user and performs the normal
action.

The KAuth helper performs various tasks to ensure the security:

  • validates that neither config directory, nor the config file are symlinks
  • validates that config directory and config file are owned by the user
  • only writes the known values

Diff Detail

Repository
R133 KScreenLocker
Branch
authorize-config-changes
Lint
No Linters Available
Unit
No Unit Test Coverage
graesslin updated this revision to Diff 1898.Jan 12 2016, 12:52 PM
graesslin retitled this revision from to Require user to authenticate when trying to change lock screen settings.
graesslin updated this object.
graesslin edited the test plan for this revision. (Show Details)
graesslin added reviewers: bshah, davidedmundson.
graesslin added a subscriber: plasma-devel.

Another required change will be to mark the config file as immutable on startup if it isn't already. I haven't included this yet as I want to get the basic concept reviewed first.

auth-helper/kscreenlockerauthhelper.cpp
34

As this is a linux-ism we need to make it ifdef and decide what to do if it's not provided. My suggestion would be to not use KAuth at all

This breaks every user's backup script by having root files in the user's home. So I am very much not happy with this idea at all.
Especially as it acheives very little anyway, if you have a malicious app on your system - why on Earth does it want to modify your lock screen settings when it has access to everything the user has already?

We want to sandbox apps that might misbehave from the user, not elevate user processes above the user.

In fact with this, you can now never remove a user. That's definitely a blocker

When set, prevents, even the superuser, from erasing or changing the contents of the file.

auth-helper/kscreenlockerauthhelper.cpp
94

Unintuitively, you're better off opening the file before you do the security checks.

Otherwise we have a race condition where I can launch this action and then swap the file for a symlink. With a scripted million attempts, it might work.

mak added a subscriber: mak.Jan 12 2016, 6:34 PM

This breaks every user's backup script by having root files in the user's home. So I am very much not happy with this idea at all.
Especially as it acheives very little anyway, if you have a malicious app on your system - why on Earth does it want to modify your lock screen settings when it has access to everything the user has already?

We want to sandbox apps that might misbehave from the user, not elevate user processes above the user.

I must agree with David on this, generally having root own files in /home is a terrible idea.
One solution that might work is to move the whole configuration file out of home and into /etc/kde/plasma-screenlocker/<username-or-uid>/config and have the KCM write to that file and have the screenlocker read information from there. It's still a hack, but I think it's a better one than having rood fiddle with stuff in /home.
On the general usecase, I think it really adds just marginal additional security, and personally I would ignore this particular attack vector with the same reasoning @davidedmundson already outlined. On the other hand though, every bit of additional security might be a good thing, and a fully-sandboxed world won't happen on the Linux desktop within the next years, so if a good solution can be found, we should use it.

I must agree with David on this, generally having root own files in /home is a terrible idea.

Note it's not actually root owned. It gets set back to being owned by the user but with "chattr -i" set by root.

colomar edited edge metadata.Jan 12 2016, 7:58 PM

I must say that I am not happy with this solution, either.
I agree with David: If there is a malicious process running, the lock screen is likely the least problem the user has to worry about. Why switch off lock screen and wait until I have unobserved physical access to a computer when I can just give myself remote access while the user is at their machine?
If we ask the password for changing any user-specific setting that could have a potential security implication (the lock screen is certainly not the only setting where a malicious application could cause a security problem), configuring your system will become quite tedious.

I disagree on the point that if a malicious process is already running the lock screen is the least to worry about. It's one of the items to worry about and I'm trying to fix them all. It's just the first I picked.

Why is this one important: because it doesn't need a malicious process. It just needs access to the file system, you don't need to run a program. How to get access to the file system: drive-by download vulnerability is enough. So yeah that's something we need to fix.

I'm fine with moving the config to /etc, I thought it's particular good to leave it on /home, but I understand the backup/restore problem.

A good rule is that if you want to protect against a security issue, you must first explain why the user is not already screwed.
If a malicious app is running and can disable the screen locker (or not), the security state transition is from "the user is completely screwed" to "the user is completely screwed". If you have a rogue process running, it has almost infinite ways to grab the user's information and whatnot. Disabling the screen locker is not the problem at that point.

On X11 I agree, there is no point in this change. On Wayland there is, because on Wayland attacking the lock screen is the best way to gain user's password.

graesslin abandoned this revision.Feb 9 2016, 10:07 AM