Security Audit Request for Screenlocker Branch

[Oswald Buddenhagen]

On Tue, Oct 11, 2011 at 06:30:40PM +0200, Martin Gräßlin wrote:
> On Tuesday 11 October 2011 17:34:10 Oswald Buddenhagen wrote:
> > on a more serious note, [h]ow do you handle the lock grace time?
>
> this is actually not affected by the changes. Dim Display and turning off the 
> screen are decoupled from the screen locking.
>
that's not a response to my question. the old lock engine offers the
option to start a saver which only after a few seconds requires a password
to make it go away.

> > On Tue, Oct 11, 2011 at 04:00:17PM +0200, Martin Gräßlin wrote:
> > > KWin has currently one reproducable crash listed in bko.
> > 
> > irrelevant. there will always be new crashes, also outside kwin's
> > control.
>
> irrelevant for all implementations - also the existing ones. If we want to be 
> secure against crashes introduced by dependencies we have to write something 
> which does not link anything except Xlib.
>
it's not so much about the number of dependencies, but the number of
code path exercised.

> Of course KWin is a more complex application than others, but given
> what we need in a screen locker the difference becomes marginal IMHO.
>
yes. one should consider decoupling the greeter from the core engine.

> > > I myself have never run into a situation where KWin did not restart
> > > [...]
> > 
> > even if it restarts, you still have a non-trivial racing window.
> > additionally, it probably allows a waiting app (some popup) to grab
> > input and thus make the subsequent re-lock fail.
>
> ok, this is a concern I have not yet considered. Any ideas how we could handle 
> such a situation?
>
by not crashing in the first place. seriously. think about it.

> Having the locker outside of the compositor brings back the issues which I 
> wanted to solve in the first place:
> * windows can get on top of the screen locker
>
no, because by "tagging" the screensaver window(s) you tell kwin not to
render other windows.

> * screen is not correctly blanked
>
i'm not sure what you mean, but i suppose the above reply applies here,
too.

> * how to tell KWin about a screen locker window in a secure way?
>
it doesn't have to be secure, because it's the session that protects
itself from outsiders. you can just use X properties and it's fine. in
fact, it would be even backwards compatible with less capable window
managers.

> * that is one of the reasons why the lock screen work was started:
> having a solution for both X and Wayland
>
given how fundamentally different the architectures are, it's clear that
you'll have two different code paths. i don't see why you must
encapsulate that fully inside kwin only, especially if it would be
detrimental to the legacy support (which is going to stay the state of
the art for years to come, as you noted yourself).