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

[XFCE] locked xscreensaver displays sensitive notifications #2026

Open
mfc opened this Issue May 24, 2016 · 14 comments

Comments

Projects
None yet
3 participants
@mfc
Member

mfc commented May 24, 2016

Qubes OS version (e.g., R3.1):

R3.1

Affected TemplateVMs (e.g., fedora-23, if applicable):

xscreensaver on XFCE


Expected behavior:

sensitive notifications are not displayed when the screen is locked.

Actual behavior:

sensitive notifications are displayed when the screen is locked.

Steps to reproduce the behavior:

Use Signal Desktop, receive a message. The message will be pushed through xscreensaver and display to the user even if they have locked the screen.


Related issues:

#888
#963
#1917

@mfc

This comment has been minimized.

Show comment
Hide comment
@mfc

mfc May 24, 2016

Member

just checked KDE lock screen and it does not leak these same notifications.

Member

mfc commented May 24, 2016

just checked KDE lock screen and it does not leak these same notifications.

@mfc mfc changed the title from locked xscreensaver displays sensitive notifications to locked xscreensaver displays sensitive notifications (XFCE) May 24, 2016

@mfc mfc changed the title from locked xscreensaver displays sensitive notifications (XFCE) to [XFCE] locked xscreensaver displays sensitive notifications May 24, 2016

@mfc

This comment has been minimized.

Show comment
Hide comment
@mfc

mfc Oct 19, 2016

Member

also a note that split-gpg notifications of private key access are pushed through the lock screen. This is on R3.2.

Member

mfc commented Oct 19, 2016

also a note that split-gpg notifications of private key access are pushed through the lock screen. This is on R3.2.

@andrewdavidwong

This comment has been minimized.

Show comment
Hide comment
@andrewdavidwong

andrewdavidwong Dec 24, 2016

Member

This would be (hopefully) fixed by changing the default screen locker to physlock (#1917).

Member

andrewdavidwong commented Dec 24, 2016

This would be (hopefully) fixed by changing the default screen locker to physlock (#1917).

@jpouellet jpouellet referenced this issue in nvesely/qubes-physlock-systemd-scripts Apr 28, 2017

Open

Physlock has no -u #1

@cfcs

This comment has been minimized.

Show comment
Hide comment
@cfcs

cfcs Feb 12, 2018

@mfc KDE has the same problems, as outlined by @nvesely in #963 (comment)

(also, this bug is still a problem in Q3.2)

cfcs commented Feb 12, 2018

@mfc KDE has the same problems, as outlined by @nvesely in #963 (comment)

(also, this bug is still a problem in Q3.2)

@andrewdavidwong

This comment has been minimized.

Show comment
Hide comment
@andrewdavidwong

andrewdavidwong Feb 12, 2018

Member

I've found that this is not a Qubes-specific issue. I see popup notifications display through xscreensaver all the time on multiple physically distinct machines running an unrelated Linux distro. In light of this, @marmarek, should this issue be closed as notourbug?

Member

andrewdavidwong commented Feb 12, 2018

I've found that this is not a Qubes-specific issue. I see popup notifications display through xscreensaver all the time on multiple physically distinct machines running an unrelated Linux distro. In light of this, @marmarek, should this issue be closed as notourbug?

@cfcs

This comment has been minimized.

Show comment
Hide comment
@cfcs

cfcs Feb 12, 2018

@andrewdavidwong It's a bug that we expose our users to by the choice of software we include, and by choice, since there are solutions available that does not have these security flaws (things leaking, bypassing authentication when the application crashes). Notably physlock, which we have been discussing for years in these Github threads now.

Saying that it's notourbug is IMHO equal to saying meltdown / spectre is not our bug, so lets ignore it, and if that is the strategy that Qubes wants to go with, we could erase most of the Qubes security bulletins already, since even if they affect Qubes users, very few of them are in software maintained by the Qubes team.

cfcs commented Feb 12, 2018

@andrewdavidwong It's a bug that we expose our users to by the choice of software we include, and by choice, since there are solutions available that does not have these security flaws (things leaking, bypassing authentication when the application crashes). Notably physlock, which we have been discussing for years in these Github threads now.

Saying that it's notourbug is IMHO equal to saying meltdown / spectre is not our bug, so lets ignore it, and if that is the strategy that Qubes wants to go with, we could erase most of the Qubes security bulletins already, since even if they affect Qubes users, very few of them are in software maintained by the Qubes team.

@andrewdavidwong

This comment has been minimized.

Show comment
Hide comment
@andrewdavidwong

andrewdavidwong Feb 13, 2018

Member

It's a bug that we expose our users to by the choice of software we include, and by choice, since there are solutions available that does not have these security flaws (things leaking, bypassing authentication when the application crashes). Notably physlock, which we have been discussing for years in these Github threads now.

It's not entirely "by choice." If we could wave a magic wand and switch to physlock with everything working perfectly, I'm sure we would've done so years ago. But switching is not easy, and doing so carries opportunity costs.

Saying that it's notourbug is IMHO equal to saying meltdown / spectre is not our bug, so lets ignore it,

No, it's not. Closing this issue as notourbug does not entail that we'll ignore the screeenlocker topic in general. To be precise, closing #2026 does not entail closing #1917. They're two distinct issues. If #2026 is genuinely a bug in an upstream project's software and not ours, and if our developers cannot or will not fix that bug for the other project, then closing #2026 as notourbug is probably the correct thing to do. It doesn't follow that we're going to ignore the underlying issue or that we don't care about it. On the contrary, if it looks like the upstream project isn't going to fix #2026 (and we can't or won't do it for them), that might be all the more reason to increase the priority of #1917.

and if that is the strategy that Qubes wants to go with, we could erase most of the Qubes security bulletins already, since even if they affect Qubes users, very few of them are in software maintained by the Qubes team.

No. We issue QSBs to notify users of security-critical bugs and how to patch them (among other reasons). Given the nature of Qubes, most bugs (e.g., web browser bugs) are not security critical for Qubes, so we don't publish QSBs about them. As you know, most QSBs are about Xen bugs, because Xen is security critical. The fact that the Qubes team does not maintain Xen (the main project) is irrelevant to whether we issue QSBs for security-critical Xen bugs that affect Qubes.

Member

andrewdavidwong commented Feb 13, 2018

It's a bug that we expose our users to by the choice of software we include, and by choice, since there are solutions available that does not have these security flaws (things leaking, bypassing authentication when the application crashes). Notably physlock, which we have been discussing for years in these Github threads now.

It's not entirely "by choice." If we could wave a magic wand and switch to physlock with everything working perfectly, I'm sure we would've done so years ago. But switching is not easy, and doing so carries opportunity costs.

Saying that it's notourbug is IMHO equal to saying meltdown / spectre is not our bug, so lets ignore it,

No, it's not. Closing this issue as notourbug does not entail that we'll ignore the screeenlocker topic in general. To be precise, closing #2026 does not entail closing #1917. They're two distinct issues. If #2026 is genuinely a bug in an upstream project's software and not ours, and if our developers cannot or will not fix that bug for the other project, then closing #2026 as notourbug is probably the correct thing to do. It doesn't follow that we're going to ignore the underlying issue or that we don't care about it. On the contrary, if it looks like the upstream project isn't going to fix #2026 (and we can't or won't do it for them), that might be all the more reason to increase the priority of #1917.

and if that is the strategy that Qubes wants to go with, we could erase most of the Qubes security bulletins already, since even if they affect Qubes users, very few of them are in software maintained by the Qubes team.

No. We issue QSBs to notify users of security-critical bugs and how to patch them (among other reasons). Given the nature of Qubes, most bugs (e.g., web browser bugs) are not security critical for Qubes, so we don't publish QSBs about them. As you know, most QSBs are about Xen bugs, because Xen is security critical. The fact that the Qubes team does not maintain Xen (the main project) is irrelevant to whether we issue QSBs for security-critical Xen bugs that affect Qubes.

@cfcs

This comment has been minimized.

Show comment
Hide comment
@cfcs

cfcs Feb 13, 2018

  • Re: "by choice": Maybe that was a poor/provocative choice of words, I'm sorry. I appreciate that every reconfiguration carries cost, and it's a question of priorities. I personally think it's a somewhat important aspect of a distribution that aims to provide people with a secure desktop, but if a working screen locker is not a priority that the Qubes team shares, and the only security-critical component worth issuing QSBs about is in fact Xen, I feel it would be more honest to be up front about that, or just removing xscreensaver entirely and let people who cares about being able to safely lock their screen seek out their own solution.
    At least this stance comes as a surprise to me, and I think it will be surprising to people who currently use Qubes in shared office spaces, in transit, or anywhere their equipment may conceivably be stolen or momentarily accessed by other people.

It is my understanding that your argument for refraining from noticing people about bugs in browsers, or the recent LibreOffice exploit (that still works?) is that they expose AppVMs, but not dom0, to danger, and you don't care about what runs in people's AppVMs. The screen locker runs in dom0 and protects the whole system, so I don't see how that argument intuitively extends to mean that screenlocking is not security-critical.

  • Re: Is it it a bug? As detailed by upstream in the FAQ this is not so much a bug in XScreenSaver as it is a requirement that was overlooked in the design of X11 (and X.org) decades ago, and neglected in the favor of backwards compatibility ever since. This means #2026 will not be fixed upstream since it cannot be fixed.

XScreenSaver in particular has had a few "exciting" bugs that allowed bypassing the lock screen:

But this is not exclusive to XScreenSaver (see #963 for the exact same issue in KDE's screenlocker, also on Qubes; and here @marmarek also reports that it's broken); building an X11 screensaver application is a design flaw in itself. Originally screensavers were meant to - save the screen from burning a permanent image, not to act as a security-critical component. The latter part was bolted onto the solutions as an afterthought, and it kind of shows.

  • notourbug: When upstream can't fix a security flaw, and you won't fix it, closing the github issue that collects information about it removes visibility of the bug, which I feel is detrimental to the user experience. If a security-focused software distribution that I am using ships with known security flaws, I would like to know about them so that they can be weighed into my workflow. If people were made aware that their screen content would leak, they could choose not to use Qubes in settings where other people may see their (locked) screen (or install physlock or seek help to mitigate the problem themselves, like installing physlock or similar using the instructions provided in the github issue thread).

cfcs commented Feb 13, 2018

  • Re: "by choice": Maybe that was a poor/provocative choice of words, I'm sorry. I appreciate that every reconfiguration carries cost, and it's a question of priorities. I personally think it's a somewhat important aspect of a distribution that aims to provide people with a secure desktop, but if a working screen locker is not a priority that the Qubes team shares, and the only security-critical component worth issuing QSBs about is in fact Xen, I feel it would be more honest to be up front about that, or just removing xscreensaver entirely and let people who cares about being able to safely lock their screen seek out their own solution.
    At least this stance comes as a surprise to me, and I think it will be surprising to people who currently use Qubes in shared office spaces, in transit, or anywhere their equipment may conceivably be stolen or momentarily accessed by other people.

It is my understanding that your argument for refraining from noticing people about bugs in browsers, or the recent LibreOffice exploit (that still works?) is that they expose AppVMs, but not dom0, to danger, and you don't care about what runs in people's AppVMs. The screen locker runs in dom0 and protects the whole system, so I don't see how that argument intuitively extends to mean that screenlocking is not security-critical.

  • Re: Is it it a bug? As detailed by upstream in the FAQ this is not so much a bug in XScreenSaver as it is a requirement that was overlooked in the design of X11 (and X.org) decades ago, and neglected in the favor of backwards compatibility ever since. This means #2026 will not be fixed upstream since it cannot be fixed.

XScreenSaver in particular has had a few "exciting" bugs that allowed bypassing the lock screen:

But this is not exclusive to XScreenSaver (see #963 for the exact same issue in KDE's screenlocker, also on Qubes; and here @marmarek also reports that it's broken); building an X11 screensaver application is a design flaw in itself. Originally screensavers were meant to - save the screen from burning a permanent image, not to act as a security-critical component. The latter part was bolted onto the solutions as an afterthought, and it kind of shows.

  • notourbug: When upstream can't fix a security flaw, and you won't fix it, closing the github issue that collects information about it removes visibility of the bug, which I feel is detrimental to the user experience. If a security-focused software distribution that I am using ships with known security flaws, I would like to know about them so that they can be weighed into my workflow. If people were made aware that their screen content would leak, they could choose not to use Qubes in settings where other people may see their (locked) screen (or install physlock or seek help to mitigate the problem themselves, like installing physlock or similar using the instructions provided in the github issue thread).
@cfcs

This comment has been minimized.

Show comment
Hide comment
@cfcs

cfcs Feb 13, 2018

BTW, found this other related thread (now closed for some reason) where @rootkovska and @marmarek also discuss screen locking: #2120

cfcs commented Feb 13, 2018

BTW, found this other related thread (now closed for some reason) where @rootkovska and @marmarek also discuss screen locking: #2120

@cfcs

This comment has been minimized.

Show comment
Hide comment
@cfcs

cfcs Feb 13, 2018

Was closing the tabs from yesterday when I spotted this quote from the author of XScreenSaver regarding his apparent frustration with the situation (which is that you cannot write a secure X11-based screensaver):

There's little that I can do to make the screen locker secure so long as the kernel and X11 developers are actively working against security. The strength of the lock on your front door doesn't matter much so long as someone else in the house insists on leaving a key under the welcome mat.

I really don't want to sound too bitter or accusatory, please don't read any of this as an attack. I'm just a little weary of having discussed this in Github threads for 3 years now, and it seems like it is very unintuitive to people that xscreensaver, KDE's locker program etc are essentially toys, not security mechanisms (hence all the links and quotes to support this claim).

cfcs commented Feb 13, 2018

Was closing the tabs from yesterday when I spotted this quote from the author of XScreenSaver regarding his apparent frustration with the situation (which is that you cannot write a secure X11-based screensaver):

There's little that I can do to make the screen locker secure so long as the kernel and X11 developers are actively working against security. The strength of the lock on your front door doesn't matter much so long as someone else in the house insists on leaving a key under the welcome mat.

I really don't want to sound too bitter or accusatory, please don't read any of this as an attack. I'm just a little weary of having discussed this in Github threads for 3 years now, and it seems like it is very unintuitive to people that xscreensaver, KDE's locker program etc are essentially toys, not security mechanisms (hence all the links and quotes to support this claim).

@andrewdavidwong

This comment has been minimized.

Show comment
Hide comment
@andrewdavidwong

andrewdavidwong Feb 14, 2018

Member

I personally think it's a somewhat important aspect of a distribution that aims to provide people with a secure desktop, but if a working screen locker is not a priority that the Qubes team shares, and the only security-critical component worth issuing QSBs about is in fact Xen, I feel it would be more honest to be up front about that, or just removing xscreensaver entirely and let people who cares about being able to safely lock their screen seek out their own solution.

A secure screen locker is a high priority for us. We care about the security of Qubes OS as a whole, not just Xen. It just happens to be the case that many QSBs have been about Xen because many of the security-critical bugs that have affected Qubes have been in Xen.

When upstream can't fix a security flaw, and you won't fix it, closing the github issue that collects information about it removes visibility of the bug, which I feel is detrimental to the user experience. If a security-focused software distribution that I am using ships with known security flaws, I would like to know about them so that they can be weighed into my workflow. If people were made aware that their screen content would leak, they could choose not to use Qubes in settings where other people may see their (locked) screen (or install physlock or seek help to mitigate the problem themselves, like installing physlock or similar using the instructions provided in the github issue thread).

It sounds like the proper place to communicate such information is in the documentation, release notes, or in the installer. GitHub issues are meant for tracking issues, not for notifying users about things. If the crux of this issue is that users need to be informed about it, then it should be turned into a documentation issue (or similar), and closed once the documentation (or announcement or whatever) is done. The fact that people should be aware of something is not a good argument for leaving a GitHub issue about that thing open, since many users never see the issue tracker, and those who do can easily search through both open and closed issues. More importantly, leaving issues open when they aren't actionable makes the entire issue tracking system less efficient, which hinders our developers from doing their work.

Member

andrewdavidwong commented Feb 14, 2018

I personally think it's a somewhat important aspect of a distribution that aims to provide people with a secure desktop, but if a working screen locker is not a priority that the Qubes team shares, and the only security-critical component worth issuing QSBs about is in fact Xen, I feel it would be more honest to be up front about that, or just removing xscreensaver entirely and let people who cares about being able to safely lock their screen seek out their own solution.

A secure screen locker is a high priority for us. We care about the security of Qubes OS as a whole, not just Xen. It just happens to be the case that many QSBs have been about Xen because many of the security-critical bugs that have affected Qubes have been in Xen.

When upstream can't fix a security flaw, and you won't fix it, closing the github issue that collects information about it removes visibility of the bug, which I feel is detrimental to the user experience. If a security-focused software distribution that I am using ships with known security flaws, I would like to know about them so that they can be weighed into my workflow. If people were made aware that their screen content would leak, they could choose not to use Qubes in settings where other people may see their (locked) screen (or install physlock or seek help to mitigate the problem themselves, like installing physlock or similar using the instructions provided in the github issue thread).

It sounds like the proper place to communicate such information is in the documentation, release notes, or in the installer. GitHub issues are meant for tracking issues, not for notifying users about things. If the crux of this issue is that users need to be informed about it, then it should be turned into a documentation issue (or similar), and closed once the documentation (or announcement or whatever) is done. The fact that people should be aware of something is not a good argument for leaving a GitHub issue about that thing open, since many users never see the issue tracker, and those who do can easily search through both open and closed issues. More importantly, leaving issues open when they aren't actionable makes the entire issue tracking system less efficient, which hinders our developers from doing their work.

@cfcs

This comment has been minimized.

Show comment
Hide comment
@cfcs

cfcs Feb 14, 2018

@andrewdavidwong I think it would be very good if this was detailed in the documentation (I guess release notes are more for information specific to a certain release), and/or in the installer indeed.
But right now this is not mentioned anywhere else, which is why I'm concerned that by closing the issue we would lose the only place where users can currently learn about the issue.
Information dissemination aside, this is an issue that affects current versions of Qubes, and therefore I do think it belongs in the issue tracker. It is a technical issue to which concrete solutions have been proposed, so I would prefer the fix being implemented in Qubes rather than shipping affected releases and informing that user that it comes pre-installed with security vulnerabilities. The latter is a perfectly valid approach to dealing with security vulnerabilities that are impossible to solve, but where solutions are available I find it suboptimal.

It is an actionable issue; the fix is simple: Use physlock instead.

cfcs commented Feb 14, 2018

@andrewdavidwong I think it would be very good if this was detailed in the documentation (I guess release notes are more for information specific to a certain release), and/or in the installer indeed.
But right now this is not mentioned anywhere else, which is why I'm concerned that by closing the issue we would lose the only place where users can currently learn about the issue.
Information dissemination aside, this is an issue that affects current versions of Qubes, and therefore I do think it belongs in the issue tracker. It is a technical issue to which concrete solutions have been proposed, so I would prefer the fix being implemented in Qubes rather than shipping affected releases and informing that user that it comes pre-installed with security vulnerabilities. The latter is a perfectly valid approach to dealing with security vulnerabilities that are impossible to solve, but where solutions are available I find it suboptimal.

It is an actionable issue; the fix is simple: Use physlock instead.

@andrewdavidwong

This comment has been minimized.

Show comment
Hide comment
@andrewdavidwong

andrewdavidwong Feb 15, 2018

Member

I think it would be very good if this was detailed in the documentation (I guess release notes are more for information specific to a certain release), and/or in the installer indeed.
But right now this is not mentioned anywhere else, which is why I'm concerned that by closing the issue we would lose the only place where users can currently learn about the issue.
Information dissemination aside, this is an issue that affects current versions of Qubes, and therefore I do think it belongs in the issue tracker.

Please read my comment above again. Specifically:

"If the crux of this issue is that users need to be informed about it, then it should be turned into a documentation issue (or similar), and closed once the documentation (or announcement or whatever) is done."

It is a technical issue to which concrete solutions have been proposed, so I would prefer the fix being implemented in Qubes rather than shipping affected releases and informing that user that it comes pre-installed with security vulnerabilities. The latter is a perfectly valid approach to dealing with security vulnerabilities that are impossible to solve, but where solutions are available I find it suboptimal.

It is an actionable issue; the fix is simple: Use physlock instead.

That's precisely what #1917 is about.

Member

andrewdavidwong commented Feb 15, 2018

I think it would be very good if this was detailed in the documentation (I guess release notes are more for information specific to a certain release), and/or in the installer indeed.
But right now this is not mentioned anywhere else, which is why I'm concerned that by closing the issue we would lose the only place where users can currently learn about the issue.
Information dissemination aside, this is an issue that affects current versions of Qubes, and therefore I do think it belongs in the issue tracker.

Please read my comment above again. Specifically:

"If the crux of this issue is that users need to be informed about it, then it should be turned into a documentation issue (or similar), and closed once the documentation (or announcement or whatever) is done."

It is a technical issue to which concrete solutions have been proposed, so I would prefer the fix being implemented in Qubes rather than shipping affected releases and informing that user that it comes pre-installed with security vulnerabilities. The latter is a perfectly valid approach to dealing with security vulnerabilities that are impossible to solve, but where solutions are available I find it suboptimal.

It is an actionable issue; the fix is simple: Use physlock instead.

That's precisely what #1917 is about.

@cfcs

This comment has been minimized.

Show comment
Hide comment
@cfcs

cfcs Feb 15, 2018

@andrewdavidwong Ah, sorry: This thread is about the glitching, and #1917 is about the fix.
Then I agree with you. :-)

cfcs commented Feb 15, 2018

@andrewdavidwong Ah, sorry: This thread is about the glitching, and #1917 is about the fix.
Then I agree with you. :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment