Review Request 121429: Use out-of-band communication between ksld and greeter

[Martin Gräßlin]

> On Dec. 15, 2014, 11:45 p.m., Àlex Fiestas wrote:
> > Code looks good. 
> > 
> > Could you perhaps add an integration test for this? Since we are "abstracted" by the socket it should be possible. If it is too much work feel free to push it.
> 
> Martin Gräßlin wrote:
>     what do you want the integration test to test? I certainly can start the daemon but I'm not sure what it would give us as the only way to return from it requires a password. And that's what the test application in tests already does.
> 
> Àlex Fiestas wrote:
>     Well, this patch adds a lot of new logic that can be tested, since it does not have unit test (and doing them in ksmserver migh prove difficult) we can test the code with an integraiton test.
>     
>     I see lots of socket logic
>     I see logic in addAllowedWindow
>     And also the biggest method setKsldSocket which has 2 lambdas that (afaik) can't be tested in any other way.

The point is I don't see how to do an integration test for it. If we pull up everything the screen is locked, like locked. It needs a damn password to be entered to get unlocked. I just don't see how this could be integration tested.

If you see how to integration test it please provide the code for it.


- Martin


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


On Dec. 15, 2014, 10:29 a.m., Martin Gräßlin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121429/
> -----------------------------------------------------------
> 
> (Updated Dec. 15, 2014, 10:29 a.m.)
> 
> 
> Review request for Plasma, Àlex Fiestas and David Edmundson.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> -------
> 
> The screenlocker_greet needs to tell the parent ksld process which
> windows it created. Ksld sends input events to these windows. So
> far this was based on an X property on the window. Unfortunately
> ksld didn't validate whether the windows tagged with this property
> belong to the screenlocker_greet process it started.
> 
> With this change the communication for announcing windows is moved
> away from the X11 protocol and instead a custom Wayland protocol is
> used.
> 
> Ksld starts a KWaylandServer when the greet process gets started. It
> creates anonymous unix sockets for the connection and passes one
> filedescriptor to the started greeter process.
> 
> The check for the X property is removed in ksld and instead only
> windows ids passed through the Wayland socket connection are
> accepted.
> 
> 
> Diffs
> -----
> 
>   ksmserver/screenlocker/ksldapp.cpp 22698ce37e9d4be17126111b3ded8133f7c3baa6 
>   ksmserver/screenlocker/lockwindow.h 9938d201269c89a24c9c0bd6275aa5f731bb5535 
>   ksmserver/screenlocker/lockwindow.cpp 3aa963a59e21636862f5ca59e220bbea3bd41ff9 
>   ksmserver/screenlocker/protocols/ksld.xml PRE-CREATION 
>   ksmserver/screenlocker/waylandserver.h PRE-CREATION 
>   ksmserver/screenlocker/waylandserver.cpp PRE-CREATION 
>   ksmserver/screenlocker/greeter/greeterapp.h b92b13b63365a9026dba5d71b772dcd8c9ee3d3b 
>   ksmserver/screenlocker/greeter/greeterapp.cpp 30d1821bdba38028959f3457e900a1b32e628192 
>   ksmserver/screenlocker/greeter/main.cpp 12e570107d0cba851b8978131d730b27924529bb 
>   ksmserver/screenlocker/ksldapp.h 095424c9845c134aa156917aeb6c8ddf31e8d25a 
>   CMakeLists.txt c6d89c14b05f5639937aee5692d305fa2faed974 
>   ksmserver/screenlocker/CMakeLists.txt 5378a10df2be70cee95b5612c23046eae639f610 
>   ksmserver/screenlocker/greeter/CMakeLists.txt 10c473488f08354096f68784b9240392a444af5f 
> 
> Diff: https://git.reviewboard.kde.org/r/121429/diff/
> 
> 
> Testing
> -------
> 
> Running ksmserver with the patch. Lock/unlock working, my exploit is failing.
> 
> 
> Thanks,
> 
> Martin Gräßlin
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20141218/3b44fb64/attachment.html>