Re: Review Request 117157: Unlock session via DBus

2014-03-30 Thread Kirill Elagin


> On March 29, 2014, 12:05 p.m., Martin Gräßlin wrote:
> > I also have problems imagining what a use case for this is and I consider 
> > this as a security issue. It basically means that the session can get 
> > unlocked without going through authentication.
> 
> Kirill Elagin wrote:
> You have to authenticate anyway to access the DBus session bus.
> 
> Martin Gräßlin wrote:
> and running applications? It would allow $evilsecretservice to unlock the 
> screen when $agent needs to use the system after remote installing some 
> software. Since Snowden this doesn't sound so far fetched any more 
> (unfortunately).
> 
> Thomas Lübking wrote:
> you need access to a random shell of that user (what does not imply you 
> actively logged into it), but can expose another session that pot. allows 
> access to other logins (mail, webservices, even banking)
> 
> this should (by default) no way be possible. any way to circumvent 
> authentication to this very session should be guarded by a special 
> "knowwhatido" key or require active authentication (ie. passing the pass hash 
> via dbus - what's insecure enough by itself)
> 
> Kirill Elagin wrote:
> If you've installed your evil software you already have a thousand of 
> ways of accessing the system.
> My point is: if you've got privileges to issue this DBus call, you 
> already have privileges to bypass the lockscreen. This is just a _sane_ way 
> of doing so, because if you're an $evilagent you don't care how many lines of 
> shell code it will take you to bypass the lockscreen, but if you are the 
> user, it starts to matter.
> 
> Martin Gräßlin wrote:
> no, the lockscreen is secure. If you are logged in at a tty there is no 
> way to unlock the screen - the only way to bypass the lock is to kill 
> ksmserver which results in the session being killed.
> 
> Kirill Elagin wrote:
> It seems to me that it's not secure actually. If you have a look at the 
> bug I mentioned, you'll see a one-liner that unlocks the screen, right?
> Unfortunately I'm not really familiar with KDE internals, but I don't see 
> any way to avoid this. (I should point out that, still, I don't see this as a 
> security issue;
> once an $evilagent got your shell, you already lost, because now he can 
> _modify memory_ of any of your processes, including ksmserver.)
> 
> But if you still don't agree… well, will it be acceptable to have an 
> option in `kscreensaverrc` or `ksmserverrc` that triggers this behaviour?
> 
> Thomas Lübking wrote:
> > It seems to me that it's not secure actually.
> As pointed out in the very first comment, i consider this a serious bug 
> and security issue - yes.
> 
> FTR:
> There's a difference between the ability to poke memory on the one hand 
> and have a nice GUI access to your money accounts, an open ssl session or 
> root shell of a running system.
> 
> Accessing random shells/client conections of the current session is the 
> pot. privilegue escalation here, open to an attacker how could eg. not 
> manipulate the software stack.
> 
> Martin Gräßlin wrote:
> > you'll see a one-liner that unlocks the screen, right?
> 
> this shouldn't be, and is clearly a bug. Will be the first thing I look 
> into on Monday.
> 
> > will it be acceptable to have an option in `kscreensaverrc` or 
> `ksmserverrc` that triggers this behaviour?
> 
> That doesn't increase security. If you want such a functionality it must 
> go into logind IMHO.
> 
> Thomas Lübking wrote:
> > Will be the first thing I look into on Monday.
> *cough* https://git.reviewboard.kde.org/r/117166/
> 
> un/locking depending on HW dongles (bluetooth, USB) is certainly a nice 
> feature, but requires some sort of internal support (where you'd just 
> configure the HW id to trigger this)
> 
> Unlocking via a dbus command is imo very problematic.
> If we require a password, the user would be trapped into having it in his 
> shell history or invited to store it (plaintext) on disk.
> If such tool would be required, it could work by having the user solve a 
> "captcha" before reading the PW from stdin (to prevent automization)
> 
> $ kscreenlocker unlock
> 9*8+3?
> > 75
> Password?
> 
> $

When I was using pam_usb to unlock KDE with a USB drive I had to do two things 
to login/unlock: insert my USB and hit Enter.

This suggests a way to go with HW dongles: we could expose a method “Hit enter” 
via DBus. 

Re: Review Request 117166: remove /MainApplication object from screenlocker greeter interface

2014-03-29 Thread Kirill Elagin


> On March 29, 2014, 8:39 p.m., Kirill Elagin wrote:
> > Why not simply add a parameter to KApplication constructor?
> 
> Thomas Lübking wrote:
> Being?
> I'm not aware of such parameter, kdelibs is semi-frozen and the 
> requirement is also pretty special to add such feature to KApplication, yesno?

I've checked KApplication code and it seems that it creates a DBus service 
unconditionally. Which is pretty weird.
Is that true that _every single_ application wants to be exposed via DBus? I 
guess, no. I wouldn't call this requirement “special” at all.

I'm not sure about your freezing rules, but adding a parameter with a 
compatible default value shouldn't be a big deal, right?


- Kirill


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117166/#review54562
---


On March 29, 2014, 8:32 p.m., Thomas Lübking wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/117166/
> ---
> 
> (Updated March 29, 2014, 8:32 p.m.)
> 
> 
> Review request for kde-workspace, Martin Gräßlin and Kirill Elagin.
> 
> 
> Repository: kde-workspace
> 
> 
> Description
> ---
> 
> Turned out it's possible to kquitapp the greeter w/o having to provide the 
> desktop.
> whatever is the final resolution to bug #314989 resp. review #117157, 
> ignorantly exposing the /MainApplication object on this process is certainly 
> a bug. (I considered turning it into a QApplication, but that would have 
> turned a HUGE patch and also the KDebug interface might be a benefit)
> 
> As the issue exists since 4.10, i don't think it's necessary to press this 
> into 4.11.8 (and break the "workaround" in bug #314989, which then can be 
> reasonably resolved before 4.11.9)
> 
> 
> Diffs
> -
> 
>   ksmserver/screenlocker/greeter/greeterapp.cpp c5e2f85 
> 
> Diff: https://git.reviewboard.kde.org/r/117166/diff/
> 
> 
> Testing
> ---
> 
> locked screen, checked dbus interface of the greeter - MainApplication is 
> gone.
> 
> 
> Thanks,
> 
> Thomas Lübking
> 
>



Re: Review Request 117166: remove /MainApplication object from screenlocker greeter interface

2014-03-29 Thread Kirill Elagin

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117166/#review54562
---


Why not simply add a parameter to KApplication constructor?

- Kirill Elagin


On March 29, 2014, 8:32 p.m., Thomas Lübking wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/117166/
> ---
> 
> (Updated March 29, 2014, 8:32 p.m.)
> 
> 
> Review request for kde-workspace, Martin Gräßlin and Kirill Elagin.
> 
> 
> Repository: kde-workspace
> 
> 
> Description
> ---
> 
> Turned out it's possible to kquitapp the greeter w/o having to provide the 
> desktop.
> whatever is the final resolution to bug #314989 resp. review #117157, 
> ignorantly exposing the /MainApplication object on this process is certainly 
> a bug. (I considered turning it into a QApplication, but that would have 
> turned a HUGE patch and also the KDebug interface might be a benefit)
> 
> As the issue exists since 4.10, i don't think it's necessary to press this 
> into 4.11.8 (and break the "workaround" in bug #314989, which then can be 
> reasonably resolved before 4.11.9)
> 
> 
> Diffs
> -
> 
>   ksmserver/screenlocker/greeter/greeterapp.cpp c5e2f85 
> 
> Diff: https://git.reviewboard.kde.org/r/117166/diff/
> 
> 
> Testing
> ---
> 
> locked screen, checked dbus interface of the greeter - MainApplication is 
> gone.
> 
> 
> Thanks,
> 
> Thomas Lübking
> 
>



Re: Review Request 117157: Unlock session via DBus

2014-03-29 Thread Kirill Elagin


> On March 29, 2014, 12:05 p.m., Martin Gräßlin wrote:
> > I also have problems imagining what a use case for this is and I consider 
> > this as a security issue. It basically means that the session can get 
> > unlocked without going through authentication.
> 
> Kirill Elagin wrote:
> You have to authenticate anyway to access the DBus session bus.
> 
> Martin Gräßlin wrote:
> and running applications? It would allow $evilsecretservice to unlock the 
> screen when $agent needs to use the system after remote installing some 
> software. Since Snowden this doesn't sound so far fetched any more 
> (unfortunately).
> 
> Thomas Lübking wrote:
> you need access to a random shell of that user (what does not imply you 
> actively logged into it), but can expose another session that pot. allows 
> access to other logins (mail, webservices, even banking)
> 
> this should (by default) no way be possible. any way to circumvent 
> authentication to this very session should be guarded by a special 
> "knowwhatido" key or require active authentication (ie. passing the pass hash 
> via dbus - what's insecure enough by itself)
> 
> Kirill Elagin wrote:
> If you've installed your evil software you already have a thousand of 
> ways of accessing the system.
> My point is: if you've got privileges to issue this DBus call, you 
> already have privileges to bypass the lockscreen. This is just a _sane_ way 
> of doing so, because if you're an $evilagent you don't care how many lines of 
> shell code it will take you to bypass the lockscreen, but if you are the 
> user, it starts to matter.
> 
> Martin Gräßlin wrote:
> no, the lockscreen is secure. If you are logged in at a tty there is no 
> way to unlock the screen - the only way to bypass the lock is to kill 
> ksmserver which results in the session being killed.

It seems to me that it's not secure actually. If you have a look at the bug I 
mentioned, you'll see a one-liner that unlocks the screen, right?
Unfortunately I'm not really familiar with KDE internals, but I don't see any 
way to avoid this. (I should point out that, still, I don't see this as a 
security issue;
once an $evilagent got your shell, you already lost, because now he can _modify 
memory_ of any of your processes, including ksmserver.)

But if you still don't agree… well, will it be acceptable to have an option in 
`kscreensaverrc` or `ksmserverrc` that triggers this behaviour?


- Kirill


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117157/#review54538
---


On March 29, 2014, 11:58 a.m., Kirill Elagin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/117157/
> ---
> 
> (Updated March 29, 2014, 11:58 a.m.)
> 
> 
> Review request for kde-workspace.
> 
> 
> Bugs: 314989
> http://bugs.kde.org/show_bug.cgi?id=314989
> 
> 
> Repository: kde-workspace
> 
> 
> Description
> ---
> 
> Unlock session via DBus
> 
> Make org.freedesktop.ScreenSaver.SetActive(false) unlock session.
> 
> 
> Diffs
> -
> 
>   plasma-workspace/ksmserver/screenlocker/interface.cpp 
> ecb30a37b1a207cf9dab8c53b1b879108a99a45b 
>   plasma-workspace/ksmserver/screenlocker/ksldapp.h 
> b292b62f4df073fff31bcbfd0e39f4c4fe04c92d 
>   plasma-workspace/ksmserver/screenlocker/ksldapp.cpp 
> f2e5262524447e8ae1df1fbf6543297c3be3e6b8 
> 
> Diff: https://git.reviewboard.kde.org/r/117157/diff/
> 
> 
> Testing
> ---
> 
> I've tested this with KDE 4.11.5 which I'm currently running.
> Rebasing to master was completely trivial; I've looked through the code and I 
> believe all the assumptions I made are still valid in master.
> 
> 
> Thanks,
> 
> Kirill Elagin
> 
>



Re: Review Request 117157: Unlock session via DBus

2014-03-29 Thread Kirill Elagin


> On March 29, 2014, 12:05 p.m., Martin Gräßlin wrote:
> > I also have problems imagining what a use case for this is and I consider 
> > this as a security issue. It basically means that the session can get 
> > unlocked without going through authentication.
> 
> Kirill Elagin wrote:
> You have to authenticate anyway to access the DBus session bus.
> 
> Martin Gräßlin wrote:
> and running applications? It would allow $evilsecretservice to unlock the 
> screen when $agent needs to use the system after remote installing some 
> software. Since Snowden this doesn't sound so far fetched any more 
> (unfortunately).
> 
> Thomas Lübking wrote:
> you need access to a random shell of that user (what does not imply you 
> actively logged into it), but can expose another session that pot. allows 
> access to other logins (mail, webservices, even banking)
> 
> this should (by default) no way be possible. any way to circumvent 
> authentication to this very session should be guarded by a special 
> "knowwhatido" key or require active authentication (ie. passing the pass hash 
> via dbus - what's insecure enough by itself)

If you've installed your evil software you already have a thousand of ways of 
accessing the system.
My point is: if you've got privileges to issue this DBus call, you already have 
privileges to bypass the lockscreen. This is just a _sane_ way of doing so, 
because if you're an $evilagent you don't care how many lines of shell code it 
will take you to bypass the lockscreen, but if you are the user, it starts to 
matter.


- Kirill


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117157/#review54538
---


On March 29, 2014, 11:58 a.m., Kirill Elagin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/117157/
> ---
> 
> (Updated March 29, 2014, 11:58 a.m.)
> 
> 
> Review request for kde-workspace.
> 
> 
> Bugs: 314989
> http://bugs.kde.org/show_bug.cgi?id=314989
> 
> 
> Repository: kde-workspace
> 
> 
> Description
> ---
> 
> Unlock session via DBus
> 
> Make org.freedesktop.ScreenSaver.SetActive(false) unlock session.
> 
> 
> Diffs
> -
> 
>   plasma-workspace/ksmserver/screenlocker/interface.cpp 
> ecb30a37b1a207cf9dab8c53b1b879108a99a45b 
>   plasma-workspace/ksmserver/screenlocker/ksldapp.h 
> b292b62f4df073fff31bcbfd0e39f4c4fe04c92d 
>   plasma-workspace/ksmserver/screenlocker/ksldapp.cpp 
> f2e5262524447e8ae1df1fbf6543297c3be3e6b8 
> 
> Diff: https://git.reviewboard.kde.org/r/117157/diff/
> 
> 
> Testing
> -------
> 
> I've tested this with KDE 4.11.5 which I'm currently running.
> Rebasing to master was completely trivial; I've looked through the code and I 
> believe all the assumptions I made are still valid in master.
> 
> 
> Thanks,
> 
> Kirill Elagin
> 
>



Re: Review Request 117157: Unlock session via DBus

2014-03-29 Thread Kirill Elagin


> On March 29, 2014, 12:05 p.m., Martin Gräßlin wrote:
> > I also have problems imagining what a use case for this is and I consider 
> > this as a security issue. It basically means that the session can get 
> > unlocked without going through authentication.

You have to authenticate anyway to access the DBus session bus.


- Kirill


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117157/#review54538
---


On March 29, 2014, 11:58 a.m., Kirill Elagin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/117157/
> ---
> 
> (Updated March 29, 2014, 11:58 a.m.)
> 
> 
> Review request for kde-workspace.
> 
> 
> Bugs: 314989
> http://bugs.kde.org/show_bug.cgi?id=314989
> 
> 
> Repository: kde-workspace
> 
> 
> Description
> ---
> 
> Unlock session via DBus
> 
> Make org.freedesktop.ScreenSaver.SetActive(false) unlock session.
> 
> 
> Diffs
> -
> 
>   plasma-workspace/ksmserver/screenlocker/interface.cpp 
> ecb30a37b1a207cf9dab8c53b1b879108a99a45b 
>   plasma-workspace/ksmserver/screenlocker/ksldapp.h 
> b292b62f4df073fff31bcbfd0e39f4c4fe04c92d 
>   plasma-workspace/ksmserver/screenlocker/ksldapp.cpp 
> f2e5262524447e8ae1df1fbf6543297c3be3e6b8 
> 
> Diff: https://git.reviewboard.kde.org/r/117157/diff/
> 
> 
> Testing
> ---
> 
> I've tested this with KDE 4.11.5 which I'm currently running.
> Rebasing to master was completely trivial; I've looked through the code and I 
> believe all the assumptions I made are still valid in master.
> 
> 
> Thanks,
> 
> Kirill Elagin
> 
>



Re: Review Request 117157: Unlock session via DBus

2014-03-29 Thread Kirill Elagin


> On March 29, 2014, 12:02 p.m., Thomas Lübking wrote:
> > what is the valid (read: not malicious) usecase for this?
> > 
> > i'd rather say that if quitting the greeter to exit the lock w/o password, 
> > that should be fixed to *not* exit the lock w/o password provision.

There are some usecases mentioned in the bug I referenced.

I can add another one: imagine that you are hosting, say, a programming 
competition (like ACM ICPC). You've got a room of PCs, they are locked before 
the contest.
When the contest starts you want to unlock them all simultaneously instead of 
having contestants enter passwords by hands.


- Kirill


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117157/#review54536
---


On March 29, 2014, 11:58 a.m., Kirill Elagin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/117157/
> ---
> 
> (Updated March 29, 2014, 11:58 a.m.)
> 
> 
> Review request for kde-workspace.
> 
> 
> Bugs: 314989
> http://bugs.kde.org/show_bug.cgi?id=314989
> 
> 
> Repository: kde-workspace
> 
> 
> Description
> ---
> 
> Unlock session via DBus
> 
> Make org.freedesktop.ScreenSaver.SetActive(false) unlock session.
> 
> 
> Diffs
> -
> 
>   plasma-workspace/ksmserver/screenlocker/interface.cpp 
> ecb30a37b1a207cf9dab8c53b1b879108a99a45b 
>   plasma-workspace/ksmserver/screenlocker/ksldapp.h 
> b292b62f4df073fff31bcbfd0e39f4c4fe04c92d 
>   plasma-workspace/ksmserver/screenlocker/ksldapp.cpp 
> f2e5262524447e8ae1df1fbf6543297c3be3e6b8 
> 
> Diff: https://git.reviewboard.kde.org/r/117157/diff/
> 
> 
> Testing
> ---
> 
> I've tested this with KDE 4.11.5 which I'm currently running.
> Rebasing to master was completely trivial; I've looked through the code and I 
> believe all the assumptions I made are still valid in master.
> 
> 
> Thanks,
> 
> Kirill Elagin
> 
>



Re: Review Request 117157: Unlock session via DBus

2014-03-29 Thread Kirill Elagin

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117157/
---

(Updated March 29, 2014, 11:58 a.m.)


Review request for kde-workspace.


Changes
---

How I tested it.


Bugs: 314989
http://bugs.kde.org/show_bug.cgi?id=314989


Repository: kde-workspace


Description
---

Unlock session via DBus

Make org.freedesktop.ScreenSaver.SetActive(false) unlock session.


Diffs
-

  plasma-workspace/ksmserver/screenlocker/interface.cpp 
ecb30a37b1a207cf9dab8c53b1b879108a99a45b 
  plasma-workspace/ksmserver/screenlocker/ksldapp.h 
b292b62f4df073fff31bcbfd0e39f4c4fe04c92d 
  plasma-workspace/ksmserver/screenlocker/ksldapp.cpp 
f2e5262524447e8ae1df1fbf6543297c3be3e6b8 

Diff: https://git.reviewboard.kde.org/r/117157/diff/


Testing (updated)
---

I've tested this with KDE 4.11.5 which I'm currently running.
Rebasing to master was completely trivial; I've looked through the code and I 
believe all the assumptions I made are still valid in master.


Thanks,

Kirill Elagin



Review Request 117157: Unlock session via DBus

2014-03-29 Thread Kirill Elagin

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117157/
---

Review request for kde-workspace.


Bugs: 314989
http://bugs.kde.org/show_bug.cgi?id=314989


Repository: kde-workspace


Description
---

Unlock session via DBus

Make org.freedesktop.ScreenSaver.SetActive(false) unlock session.


Diffs
-

  plasma-workspace/ksmserver/screenlocker/interface.cpp 
ecb30a37b1a207cf9dab8c53b1b879108a99a45b 
  plasma-workspace/ksmserver/screenlocker/ksldapp.h 
b292b62f4df073fff31bcbfd0e39f4c4fe04c92d 
  plasma-workspace/ksmserver/screenlocker/ksldapp.cpp 
f2e5262524447e8ae1df1fbf6543297c3be3e6b8 

Diff: https://git.reviewboard.kde.org/r/117157/diff/


Testing
---


Thanks,

Kirill Elagin