Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Screensaver lock by-pass via the virtual keyboard #354

Closed
ghost opened this issue Dec 28, 2020 · 50 comments
Closed

Screensaver lock by-pass via the virtual keyboard #354

ghost opened this issue Dec 28, 2020 · 50 comments

Comments

@ghost
Copy link

ghost commented Dec 28, 2020

 * Cinnamon version:  Cinnamon 4.6.7
 * Distribution: Fedora 32
 * Graphics hardware *and* driver used: 03:00.0 VGA compatible controller: Advanced Micro Devices, Inc. [AMD/ATI] Ellesmere [Radeon RX 470/480/570/570X/580/580X/590] (rev ef) using amdgpu
 * 32 or 64 bit: 64bit

Issue
Screensaver lock by-pass. It is possible to crash the screensaver and unlock the desktop via the virtual keyboard.

Steps to reproduce
Lock the system
Click on the virtual keyboard
Type at the real keyboard while typing at the virtual keyboard, both at the same time, as many keys as possible.

Expected behaviour
No crash.

Other information
A few weeks ago, my kids wanted to hack my linux desktop, so they typed and clicked everywhere, while I was standing behind them looking at them play... when the screensaver core dumped and they actually hacked their way in! wow, those little hackers... 🐈

I thought it was a unique incident, but they managed to do it a second time. So I'd consider this issue... reproducible... by kids 😄

I tried to recreate the crash on my own with no success, maybe because it required more than 4 little hands typing and using the mouse on the virtual keyboard.

Maybe not the best bug report, but I've seen the screenlock crash twice already with my own eyes, so its pretty real.

One last thing, after the desktop is unlocked, I can't re-lock it again, the screensaver process is pretty dead and requires me to open a shell and run 'cinnamon-screensaver' manually to get it working.

@leigh123linux leigh123linux transferred this issue from linuxmint/cinnamon Dec 28, 2020
@jbergknoff
Copy link

My kids came upon a similar cinnamon-screensaver segfault! I've emailed details of how to reproduce the problem to [email protected].

@clefebvre
Copy link
Member

clefebvre commented Jan 8, 2021

Quick update on this.

https://security-tracker.debian.org/tracker/CVE-2020-25712
https://ubuntu.com/security/CVE-2020-25712

We need to understand the xserver code a little bit more and check some of the later commits upstream to understand whether this is something to be fixed in xorg or caribou, and if it's to be fixed in xorg whether it's already fixed in their master version or not.

There's an update in Ubuntu 20.04 (Mint 20.x) which backports xorg 1.20.9 towards focal... that backport actually messed up and missed the CVE fix (which is present in the groovy version), so upgrading to this temporarily fixes this issue. When Ubuntu realize this mistake they'll backport the CVE fix again and this will break again.

Afaik this affects all distributions. On our side we could verify it affects Mint 20x and LMDE 4, and we're pretty sure it affects 19x and 18x as well.

@clefebvre
Copy link
Member

On-screen keyboard support was added to the screensaver in Cinnamon 4.2.

In Linux Mint it means the following releases are impacted:

image
image

@clefebvre
Copy link
Member

We have two different issues:

  • In all versions of Cinnamon, the on-screen keyboard (launched from the menu) runs within the Cinnamon process and uses libcaribou. Pressing ē crashes Cinnamon.
  • In versions of Cinnamon 4.2 and higher, there's a libcaribou OSK in the screensaver. Pressing ē there crashes the screensaver.

@clefebvre
Copy link
Member

Cinnamon crash reproduced in Mint 18.3.
Screensaver crash reproduced in Mint 19.3.
Screensaver crash reproduced in Mint 20 with xserver-common 1.20.8-2ubuntu2.2~18.04.4 (Ubuntu's 1.29 mistakenly forgot to include the CVE fix and overrid it, so as a side effect 1.29 fixes this issue in Mint 20.x right now but that's not an acceptable solution).
Screensaver crash reproduced in LMDE 4.

@ghost
Copy link
Author

ghost commented Jan 12, 2021

Is there a way to remove/uninstall the virtual keyboard?

@jbergknoff
Copy link

Oddly enough: since yesterday, we haven't been able to reproduce the crash here. Pressing ē on the on-screen keyboard, or changing the keyboard input language and pressing anything, both just result in characters being added to the password input. The only package change since my comment earlier in the issue was to upgrade NVIDIA drivers from 450.80.02 to 450.102.04. That did include an upgrade of xserver-xorg-video-nvidia-450 which sounds like it could be related, but I am skeptical. Pretty strange.

@clefebvre
Copy link
Member

@robo2bobo no. We'll most likely patch libcaribou here. This is a high priority bug for us, expect a fix ASAP.

@clefebvre
Copy link
Member

clefebvre commented Jan 12, 2021

@leigh123linux @eli-schwartz @willysr @Fantu this is a critical bug.

TLDR: Since the Xorg update to fix CVE-2020-25712, anybody can crash the screen lock and enter the session without a password. This affects all distributions running Cinnamon 4.2+. It also affects any software using libcaribou.

We're testing the following fix at the moment: https://gitlab.com/linuxmint/pins/mint/caribou/-/commit/c4bc79ddace3da812c529e631da5c04afde31705.

@eli-schwartz
Copy link
Contributor

What's the status of caribou upstream? Is gnome still maintaining it? Are they interested in help, or in merging patches needed by cinnamon? According to https://archlinux.org/packages/extra/x86_64/caribou/ the only current user in Arch is cinnamon.

@Fantu
Copy link
Contributor

Fantu commented Jan 12, 2021

As it affect all software using caribou is good to report in it ASAP (if not already done)
Edit: seems don't reported for now, or I'm wrong? https://gitlab.gnome.org/GNOME/caribou/-/issues
from apt-cache rdepend caribou is dep. of by gnome-core, gir1.2-caribou-1.0 is used by cinnamon, rdepend on libcaribou-dev and libcaribou0 return nothing so seems pratically used only by cinnamon (I don't know if there are other software out of debian repository that use it).

@clefebvre
Copy link
Member

We'll provide a merge request. This still needs patching in all distros though. Anyone wants to volunteer to create a CVE?

@clefebvre
Copy link
Member

@robo2bobo @jbergknoff here are patched packages for caribou:

patched_packages.zip

They should take care of the issue. We're hoping to have them tested tomorrow and pushed to repositories.

@clefebvre
Copy link
Member

Sorry these packages are no good, they don't contain our changes for some reason. I suspect the build isn't actually recompiling the vala code... pff...

@ghost
Copy link
Author

ghost commented Jan 13, 2021

Following the mantra "If it does not exist, there is nothing to exploit", it would be nice to be able to remove the virtual keyboard entirely.

@clefebvre
Copy link
Member

Indeed but we still need to fix the issue.

@leigh123linux
Copy link
Contributor

Sorry these packages are no good, they don't contain our changes for some reason. I suspect the build isn't actually recompiling the vala code... pff...

Did you run 'make clean'?

https://src.fedoraproject.org/rpms/caribou/blob/master/f/caribou.spec#_101

@clefebvre
Copy link
Member

It's specific to us @leigh123linux. We had the vala stamp files in the archive so valac wasn't regenerating the C code. It's fixed now.

I also fixed an issue noted by Michael in the Cinnamon OSK. I'm rebuilding packages for each release now and testing.

@clefebvre
Copy link
Member

Ok hopefully these should do the trick. mint-packages-v2.zip

@clefebvre
Copy link
Member

clefebvre commented Jan 13, 2021

Mint Tests

  • 19.3
  • 20.1
  • LMDE 4

@clefebvre
Copy link
Member

Fixed in Mint 19.x, Mint 20.x and LMDE 4.

image

@clefebvre
Copy link
Member

"CSR: Add a setting to disable OSK" added to the roadmap.

@clefebvre
Copy link
Member

Upstream merge request provided at https://gitlab.gnome.org/GNOME/caribou/-/merge_requests/3. It doesn't look like the project is still active though...

@clefebvre
Copy link
Member

If caribou is discontinued upstream we could consider taking over its maintenance. It's not used by GNOME anymore. That said if this happened, it wouldn't happen straight away, so for now I urge you to patch your distributions. It's done in Mint as of today, it needs to be done everywhere else as well. Patch asap if you can and/or spread the word to other maintainers.

Thank you.

@ericonr
Copy link

ericonr commented Jan 13, 2021

Hi! Since you closed the issue, you could consider pinning it so it gets some more visibility? Thanks for the fix!

@mtwebster mtwebster pinned this issue Jan 13, 2021
@mtwebster
Copy link
Member

done, thanks

@Firesphere
Copy link

This CVE should be attributed to the kids...

@Coeur-Noir
Copy link

@darkshram https://www.jwz.org/blog/2021/01/i-told-you-so-2021-edition/

@leigh123linux
Copy link
Contributor

leigh123linux commented Jan 18, 2021

cinnamon-screensaver had a complete rewrite many years ago 6794939

There is very little original gnome-screensaver/xsceensaver code left.
Perhaps he should stop his bogus code claims about Cinnamon-screensaver.

Mint-screensaver and Cinnamon-screensaver, being forks and descendants of Gnome-screensaver, have inherited this license violation and continue to perpetuate it. Every Linux distro is shipping this copyright- and license-infringing code.

I eagerly await hearing how they're going to make this right.

I would gladly meet the liar for a cage fight, winner takes all.
His other alternative is to submit a PR and correct the licence for the offending code.

@clefebvre
Copy link
Member

The MR was merged in libcaribou.

@clefebvre
Copy link
Member

clefebvre commented Jan 18, 2021

The two articles are cute and pretty accurate.

The blog post from JWZ is bitter, not constructive and contains some nonsense.

We were expecting it. You don't go telling people I told you so for 20 years and then not catch the opportunity to do it once more when it happens again. And it did happen again, yes, so enjoy the moment. Let's have one more I-told-you-so moment, if it can help us react even more and make things better for 5.0, let's embrace it.

If you are not running XScreenSaver on Linux, then it is safe to assume that your screen does not lock.

No. As mentioned above KDE has a distinct locker and so has light-locker, so no, XScreensaver isn't the only design which is safe from library/toolkit crashes.

You will recall that in 2004, which is now seventeen years ago, I wrote a document explaining why I made the design trade-offs that I did in XScreenSaver, and in that document I predicted this exact bug as my example of, "this is what will happen if you don't do it this way."

He did indeed and that design choice made a lot of sense. It provided a higher level of safety though he didn't address the needs people had.

JWZ' message of wisdom needs to be more pragmatic if it wants to be heard and taken seriously. If I tell you "don't go out of your house, you'll die" and come to your funerals 17 years later to tell your friends I had told you so, well, sure, I'll have a point but who cares? People want to play with knives, go out of their house, drive cars fast on highways and go across the street. Telling them it's inherently unsafe just misses the point if that's what they want to do. People want a pretty lock screen, they do. So let's work on that.

I wish JWZ had thought about a design that combined safety and a rich greeter, because at the time it would have provided a solution along with the warning. Instead the warning was lost because the provided solution did not address the need. And by the time we had solutions, the warning had been mostly dismissed because it wasn't pragmatic.

Looking at light-locker and KDE they seem to have gone further than JWZ's reflexion and provided a solution to the actual user's need while keeping the promise of safety.

When we first saw and shipped light-locker this didn't hit us, because we had already replaced xscreensaver with alternatives (gnome-scrensaver and mate-screensaver at the time), i.e. we had already accepted the security risk to address the need that was left vacant. By the time we saw the likes of light-locker, that warning was mostly forgotten about. It's true, and it's a pity.

When cinnamon-screensaver was written it was replacing gnome-screensaver, and again it didn't have that warning in mind because at the time we hadn't thought of doing what light-locker did, and doing what xscrensaver did (i.e. going toolkitless) simply wasn't acceptable.

And they went and made that happen.
Repeatedly.

True, they did, and we did right now. Because they had to. JWZ misses the point on this. You can't ask people to not do what they want to do and what they expect to be able to do. If they want to cross the road, you'll need to make it safe for them to do so. And you know what? It will never be as safe as NOT crossing the road. Having some wise guy telling them NEVER TO doesn't help, at all.

Every time this bug is re-introduced, someone pipes up and says something like, "So what, it was a bug, they've fixed it." That's really missing the point. The point is not that such a bug existed, but that such a bug was even possible. The real bug here is that the design of the system even permits this class of bug. It is unconscionable that someone designing a critical piece of security infrastructure would design the system in such a way that it does not fail safe.

I can see where JWZ is coming from. Though I'd like to point out GNOME rewrote their solution from scratch (I've no idea what design they used by the way), and so did we. I'm not sure these GNOME devs are the same as before and we certainly aren't. We are indeed making mistakes people did before us, and we did fix that bug pretty fast and patted ourselves on the back when it was done, but I don't think you can say we're satisfied and calling it "job done". This immediately makes us think as how we can prevent it from happening again, we have that separation of greeter/locker on our roadmap and it is very much planned to go ahead for 5.0.

An incendiary blog post and all the social media hype that goes with it will certainly help in making us care even more, but the mere fact that this happened in our code (this is OUR code right now, not just gnome-screensaver or something from upstream we'd just ship) and with our design is enough to make us want to review it.

Especially when I have given them nearly 30 years of prior art demonstrating how to do it right, and a two-decades-old document clearly explaining What Not To Do that coincidentally used this very bug as its illustrative strawman!

Xscreensaver didn't do it right. Not crossing the street isn't the safest way to cross the street.

He exposed an issue, he didn't give a solution. There is a need which is not addressed here, there is a danger which is, there is a solution which has been given by other projects, not xscreensaver. It will need to be properly audited, but to me light-locker and KDE seem to have the best solution at the moment both in terms of safety and in terms of features.

This same bug keeps cropping up in these other screen lockers for several reasons.

Writing security-critical code is hard. Most people can't do it.

Locking and authentication is an OS-level problem. And while X11 is at the heart of the OS of a Linux desktop computer, it was designed with no security to speak of, and so lockers have to run as normal, unprivileged, user-level applications. That makes the problem even harder.

This mistake of the X11 architecture can never, ever be fixed. X11 is too old, too ossified, and has too many quagmire-trapped stakeholders to ever make any meaningful changes to it again. That's why people keep trying to replace X11 -- and failing, because it's too entrenched. 

As always, these bugs are terrible because bad security is worse than no security. If you knew for a fact that your screen didn't lock, you would behave appropriately. Maybe you'd log out when you walked away. Maybe you wouldn't use that computer for certain things. But a security placebo makes you behave as if it's secure when in fact it is not.

I absolutely agree with all of this.

One of the infuriating parts of these recurring bugs is that the screen-locker part of XScreenSaver isn't even the fun part! I do not enjoy working on it. I never have. I added it in response to demand and necessity, not because it sounded like a good time.

I can understand this. I hate working on security myself, I think most of us do. We love doing cool things with technology not restrict ourselves because a few people abuse everything they can and ruin the party if we don't force ourselves to think of every little way they can use A or B against other people.

Sigh.

Xscreensaver has been a great project. We've been shipping it for a while and it made users happy at the time. It's also the codebase for its fork gnome-screensaver, which people have been using for years. So as a project we owe it a lot. The design choices its dev made were inspirational also because they explained the danger of relying on libs and toolkits in something like a locker, which had to be as crashsafe as possible. It failed in providing a solution to a need people had though while continuing to address that danger.

I'll go even further. What JWZ did in xscreensaver is the source to a key principle we use very often (although it's hard because people kinda push us the other way). We try to not implement features we don't need and not rely on libs/toolkits if we don't have to.

With that said, I have on message for JWZ. Don't be that guy. It's too easy to just tell people no to cross the street. Work with us on building that safest path. I would enjoy an audit of light-locker from you much more than a stupid I-told-u-so blog post. Don't be bitter, be part of the solution.

Mint-screensaver and Cinnamon-screensaver, being forks and descendants of Gnome-screensaver, have inherited this license violation and continue to perpetuate it. Every Linux distro is shipping this copyright- and license-infringing code.

  • mint-screensaver does not (and never did) exist.
  • cinnamon-screensaver is written from scratch.
  • gnome-screensaver is discontinued.
  • mate-screensaver was forked from gnome-screensaver and is still active, so maybe that one has a licensing issue?? I don't know.
  • xfce-screensaver was forked from mate-screensaver afaik... so maybe here as well. We use light-locker in Xfce so I'm not really sure.
  • Not every distro is shipping mate-screensaver or xfce-screensaver, no.
  • These copyright and license infringements have to be explained more in details. I'd suggest to contact interested parties (MATE and Xfce projects) directly.

I eagerly await hearing how they're going to make this right.

Writing a spiteful blog post about a non-related topic isn't the best way to get answers. How about contacting Xfce and MATE directly?

If you contact me JWZ, I tell you what I'd like. I'd like you to put your money where your mouth is and be as brilliant as you once were. I want you to use your expertise to audit the design of light-locker and the minimalistic locker KDE uses (https://github.com/KDE/kscreenlocker/blob/master/abstractlocker.cpp). I want you to come at us again in 6 months time when we've split our greeter away from our locker and get you to say "no, it's still not enough.. cause of A and B", or "ah yes, that's cool.. that's both a good looking greeter and a safe locker".

We also wear the distro hat and we also ship mate-screensaver and xfce-screensaver. If there is indeed a licensing issue, let's see how it unfolds with interested parties. We won't be judges in this, I'm sure you can have a talk with them. On our side we can continue to ship these screensavers or simply replace them with light-locker.

@Coeur-Noir
Copy link

@leigh123linux @clefebvre I just put the link to JWZ blog for reference, I had absolutely no opinion about it.

But your comments are highlighting.

@clefebvre
Copy link
Member

clefebvre commented Jan 18, 2021

Thanks, so are his. He does have a point and we do have work ahead of us. Life is like that. This server over there WILL get hacked one day. Somebody WILL die while crossing the street here one day. And our screensaver WILL crash AGAIN in the future, if it's not because of libcaribou it will be because of something else. We can make it so it does not open the session next time it DOES crash, and that's what we'll need to work on for Cinnamon 5.0.

@clefebvre
Copy link
Member

In terms of updates from other distributions, I'm aware of patches being worked on in Fedora, Void Linux, Gentoo and Debian. This is what's the most urgent right now. Libcaribou patches need go live in all distros. A CVE would make it easier to track this but we're still not sure whether the resolution lies within caribou, xorg or both. An xorg dev started looking into this.

@willysr
Copy link
Contributor

willysr commented Jan 18, 2021

it's patched in slackware-current already

@jbergknoff
Copy link

@clefebvre Thanks for the prompt fix for this issue. You responded to my bug report email in literally 1 or 2 minutes, recognized the severity, tracked down the root problem, and fixed it. 💯 Much appreciated.

ericonr added a commit to void-linux/void-packages that referenced this issue Jan 18, 2021
hazayan pushed a commit to hazayan/void-packages that referenced this issue Jan 19, 2021
ericonr added a commit to ericonr/void-packages that referenced this issue Jan 19, 2021
@bernd-wechner
Copy link

Wow, this sure made the news.

Enjoyed reading through this.

One curio of a question plagues me though.

It seems all to bold down to the stability (or not, crashing) of the screensaver?

Does security not suggest also a failsafe? Failsafe by definition meaning the problem is not the screensaver crashing at all (they can and will), but that its crashing leaves you logged in and empowered. Can a failsafe be implemented that when a locking screensaver is launched, a crash ensures a log out?

That would of course compromise unsaved work, but secure is secure and there's a priority to choose between secure against unwanted entry and secure against my own habit of leaving work unsaved long enough for a screensaver to start.

And is it even possible? I mean I can imagine a responsive and an apprehensive approach:

responsive: requires kernel support I imagine, akin to requesting a kill chain on crash. So a process can be started with a nominated kill chain and kernel if it spots that process crashing kills off the others in response. The higher and more complex the watchdog responding to such crashes is the more it becomes just a delegation of crash risk to the watchdog.

apprehensive: before launching the screensaver (or as part of the same action), the logged in session is in fact "hibernated" and logged out, to be restored only if the screensaver exits cleaning.

Anyhow, forgive my open musing in a busy thread. Just piqued my interest I guess.

@clefebvre
Copy link
Member

hi @bernd-wechner,

Absolutely. There are different ideas out there but that's one of them. I started talking privately with @mtwebster about killing Xorg on screensaver crash yesterday. He's not a fan of that approach, but it is indeed on the table.

What I like about it is that it doesn't require a locker (i.e. a minimalistic crash-safe window which prevents seeing or inputing into the session), and thus it doesn't require communication between the locker and the greeter. What I don't like about it is that it would only safeguard against crashes, but not against malfunctions...

Say the screensaver fails to cover the whole screen and leaves a part visible... say the screensaver fails to properly grab input... it wouldn't protect against that. A locker could do this better. Of course the more a locker does (in terms of reacting to monitors being connected, power and resolution changes.. inputs..) the more it can crash as well.

We might even need a combination of techniques, maybe a locker AND crashing Xorg if the locker itself ever crashes.

Another idea is to leverage the power of the WM. After all, window focus is attributed by the WM. It could be told to go into a locked state where no other windows are visible and no focus switches can happen. We don't like that idea because it's technically more complex and it would need a failsafe in case the WM itself crashes as well, but it is on the table as well.

We've got time for this anyway, we're not going to rush it, it will be done for 5.0 and there are many ideas.

What's for sure though is that we'll always regard a screensaver crash as a critical issue that needs fixing ASAP. As such, even if we have a failsafe/crasher/locker mechanism which prevents the issue from being a security issue, we'll still be in a state of mind where the screensaver (the greeter part people normally use) does not crash.

Typically here, imagine we had that mechanism in place, that libcairo issue would still have been critical, but it wouldn't have been a security issue, just a critical usability issue. It would have affected people for a short while until we sent a fix, but during that time it wouldn't have mattered whether people saw an ugly locker screen, or whether Xorg was killed without saving their work. The mechanism (or failsafe or whatever we call it) doesn't need to be pretty or even respectful of people's work. It would only come into play when something is very wrong and for a very short lap of time, and its only concern would be to secure the fact that people can't interact with the session.

@fossdd
Copy link

fossdd commented Jan 19, 2021

haha, good issue! 👍

Reached to one of the biggest tech news in germany: https://www.heise.de/news/l-f-Spielende-Kinder-hackten-Linux-Mint-5028653.html

@ItzSwirlz
Copy link

ItzSwirlz commented Jan 19, 2021

Tech I wish tech articles stopped saying it's patched; until it's patched upstream no distro can make stable release updates (at least Ubuntu can't).

I'm sure on GNOME irc there is a place to tell the caribou maintainers to merge the patch

Update: It's been merged

@frostworx
Copy link

Hehe, your "nice" bug now also made it to ycombinator.com.

@nschikora
Copy link

How I imagine devs trying to reproduce the bug.
ynetLKf

@df7cb
Copy link

df7cb commented Jan 20, 2021

Fwiw a similar bug got good old xlockmore removed from Debian back in 2005/2006:
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=318123
https://packages.qa.debian.org/x/xlockmore/news/20061119T233918Z.html

@micron
Copy link

micron commented Jan 20, 2021

Reminds me of https://www.securityfocus.com/archive/1/327837 :D

@linuxmint linuxmint locked as resolved and limited conversation to collaborators Jan 20, 2021
@mtwebster
Copy link
Member

This has stopped being productive, locking.

@clefebvre
Copy link
Member

Quick follow up on this. CSR 5.0 will include a locker window called fallback. This is a black window with instructions which looks similar to the window used in lightdm-locker. When CSR locks it launches the locker and the fallback windows in separate processes. When you log in, the locker kills the fallback.

The fallback window hides the session and grabs the input if the locker is no longer there. In the event of a CSR crash it prevents access to the session. It also has instructions on how to unlock from tty and to report the issue to us.

In the event of a fallback crash the locker notifies the user of the issue after a successful password entry. Both issues would therefore be visible to users so they can be reported to us.

@leigh123linux leigh123linux unpinned this issue Jan 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests