Review Request 126524: Fixed custom wallpaper being not respected by sddm.
David Edmundson
david at davidedmundson.co.uk
Mon Dec 28 18:49:08 UTC 2015
> On Dec. 27, 2015, 4:37 p.m., David Edmundson wrote:
> > sddmauthhelper.cpp, line 74
> > <https://git.reviewboard.kde.org/r/126524/diff/1/?file=426085#file426085line74>
> >
> > Ideally we should check the *calling* user can read this file, as you technically have a security bug.
> >
> > Otherwise I could just select /etc/shadow as my background and suddenly it's available world readable.
> >
> > A distro/admin could theoretically set polkit up to allow any users to change the SDDM wallpaper. though TBH it'd never happen.
> >
> > Polkit-Qt has that information available. KAuth does not seem to publicly.
>
> Joshua Noack wrote:
> Not sure if I understand you correctly...
> In addition to the KAuth dialog where the user needs to authenticate, a check if the user can read the file should be added?
> Shouldn't the file chooser restrict the user here in the first place?
>
> David Edmundson wrote:
> it needs to happen inside the helper, not on the client side.
>
> From a terminal I can manually start the sddm helper saying I want to set /etc/shadow as the background.
>
> The helper will then check if the user is permitted to set the SDDM wallpaper in policykit, which is up to the sysadmin's policy
> If that returns true, the helper will procede to copy the file as root.
>
> That's a security hole as it would potentially allow me to read any file.
>
> Joshua Noack wrote:
> Okay, I looked up policykit and don't see any rule in /usr/share/polkit-1/ for it.
> So... should I create a new rule (like org.kde.sddm.policy) ? But judging from your words, it seems to be already possible?
>
> If you could tell me where the sysadmin sets his policies I would be grateful. :p
We already have a policy.
See org.kde.kcontrol.kcmsddm.policy
our defaults are:
<defaults>
<allow_inactive>no</allow_inactive>
<allow_active>auth_admin_keep</allow_active>
</defaults>
which is good.
However, we can't predict that every setup and every distro will have the same setup.
They could change it to be
<allow_active>yes</allow_active>
and then we have a problem.
- David
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126524/#review90132
-----------------------------------------------------------
On Dec. 28, 2015, 3:44 p.m., Joshua Noack wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126524/
> -----------------------------------------------------------
>
> (Updated Dec. 28, 2015, 3:44 p.m.)
>
>
> Review request for Plasma and David Edmundson.
>
>
> Repository: sddm-kcm
>
>
> Description
> -------
>
> For some reason sddm cannot handle absolute file paths to wallpapers
> and also needs the wallpaper to be readable by others.
>
> This is fixed by copying the wallpaper to the root directory of the
> selected theme.
>
> On save the sddmauthhelper copies the background from the absolute path
> into the theme directory and sets the "background" key of the
> theme.user.conf to the copied file. If previously a different background was
> set it is removed beforehand.
>
>
> Diffs
> -----
>
> sddmauthhelper.cpp 648b24c77e7570641d454fca9d121709a622bc36
> src/themeconfig.cpp bdd6dd29fd8eb052e2f2b2239b0c46ebbebec88c
>
> Diff: https://git.reviewboard.kde.org/r/126524/diff/
>
>
> Testing
> -------
>
> Copies and removes backgrounds as intended.
> The wallpaper is shown in sddm.
>
>
> Thanks,
>
> Joshua Noack
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20151228/ae0bd55e/attachment-0001.html>
More information about the Plasma-devel
mailing list