We already depend on libgdm in several places. Do we need to remove those, too?

Until we drop fallback mode, we'll have gnome-screensaver, but falling back forever is pretty unappealing.  You have to understand that GNOME gets a lot of code savings and integration by having gdm reuse gnome-shell.

We could at least say "Lock screen not supported" or something I guess.

Jasper, no there's no problem depending on libgdm, just the issue of the GDM daemon not actually being there. The solution might involve libdgm indicating to GNOME Shell that GDM is not present.

A possible fallback solution is to do a PAM authentication from the gnome-shell process (i.e. the way gnome-screensaver does it now). In that way you can use the same UI but use an alternate authentication mechanism. I don't know how hard that would be to add/support.

(In reply to comment #3)
> Jasper, no there's no problem depending on libgdm, just the issue of the GDM
> daemon not actually being there. The solution might involve libdgm indicating
> to GNOME Shell that GDM is not present.
> 
> A possible fallback solution is to do a PAM authentication from the gnome-shell
> process (i.e. the way gnome-screensaver does it now). In that way you can use
> the same UI but use an alternate authentication mechanism. I don't know how
> hard that would be to add/support.

That would basically involve copying half of the gdm codebase in-tree.

Putting as 3.6 blocker. Not to force a particular solution, but to track & ensure GNOME doesn't break under a different DM than GDM.

So, I don't think we want random things around the stack running random PAM modules to do a reauth with. gnome-screensaver just did that. It didn't run as root, either, so it looks like it would just break with random PAM modules that require it.

Ray is against a setuid helper to do the reauth, as that makes us more vulnerable to attacks. A root helper, like GDM/LightDM, should start the PAM authentication process in all cases. That makes the gnome-screensaver reauth approach out of the question, if I understand things correctly.

Does it make sense to make both LightDM and GDM export a common DBus interface that would start a reauth session? libgdm could use that, and that would work for both LightDM and GDM.

There are at least 7 different ?dm variants in Debian/Ubuntu and there are sure to be more out there. They implement the functionality that was required from a DM since the early days of X. If Gnome can not work with say xdm without changes on the xdm side, then that is a bug of Gnome.

I understand that it is cooler, more modern and safer from the architecture point of view if the DM is the only component handling user authentication, but you must detect at runtime if:

1) there is a DM being used at all (startx and some methods of running terminal sessions do not run any DM at all and are still used quite widely for good reasons);

2) if the DM that is being used supports the 're-auth' feature and/or 'user switch' feature;

If not, then at the very least you must disable those features so as not to break user systems. But the sane would be to implement an alternate code path that works without a supportive DM, just like it did before. For code perspective, it is likely that it would be the simplest to push the relevant code from GDM to libgdm and then call it from there in the Shell.

Sure, it is suboptimal, but you can not depend on the DM being there to catch you.

Note that by dropping "screensaver in session", we also drop support for XDCMP deployments.  Probably no one really cares about that though.  Unencrypted authentication is so 1989.

(In reply to comment #6)
> So, I don't think we want random things around the stack running random PAM
> modules to do a reauth with. gnome-screensaver just did that. It didn't run as
> root, either, so it looks like it would just break with random PAM modules that
> require it.
> 
> Ray is against a setuid helper to do the reauth, as that makes us more
> vulnerable to attacks. 

Note that ultimately, the way gnome-screensaver works[1] is that for the default case of pam_unix, there's a setuid /usr/sbin/unix_chkpwd program.  

Not saying that this is a sane long-term strategy; just listing the reason it works currently.

[1] Present tense as we still ship it in GNOME for fallback

(In reply to comment #7)

> 
> Sure, it is suboptimal, but you can not depend on the DM being there to catch
> you.

I didn't see a convincing argument that we can't. "Must be able to run with any DM" is not part of the GNOME mission statement. If it helps achieve a better user exerience, we can and should depend on gdm features that are necessary for that experience.

There may be pragmatic reasons for not graceful degradation of the user experience when running under dms, but "implement an alternate code path" for such situations doesn't sound attractive.

(In reply to comment #9)
> Note that ultimately, the way gnome-screensaver works[1] is that for the
> default case of pam_unix, there's a setuid /usr/sbin/unix_chkpwd program.  
> 
> Not saying that this is a sane long-term strategy; just listing the reason it
> works currently.
> 
> [1] Present tense as we still ship it in GNOME for fallback

But if there's a third party PAM module that relies on being root (Ray would have more details here, I've been told there are such things), the screensaver would fail.

(In reply to comment #7)
> If not, then at the very least you must disable those features so as not to
> break user systems. But the sane would be to implement an alternate code path
> that works without a supportive DM, just like it did before. For code
> perspective, it is likely that it would be the simplest to push the relevant
> code from GDM to libgdm and then call it from there in the Shell.

Again, this will not work, since the PAM worker process relies on being root. We will not use a setuid helper, because it does not guarantee that we start in a controlled environment.

If we want to have a common interface or piece of the stack that KDE/LightDM/GNOME/E/whatever can share that does secure and safe PAM authentication then I'm perfectly OK with that, and we certainly can define such an interface, even if only for LightDM/GNOME in the short term.

Having a broken workaround system for such an edge case is not acceptable to me.

(In reply to comment #11)
> But if there's a third party PAM module that relies on being root (Ray would
> have more details here, I've been told there are such things), the screensaver
> would fail.

If there is such a thing, then it would not have worked before, correct?

On the other hand there is a lot of people using KDM, LightDM and others, a bunch of people using remote terminal logins and a bunch of people doing some weird homegrown stuff and then calling startx.

What this does is breaks a lot of different existing usecases to make one vaguely unspecified usecase work. All because you introduce a braking of well established Linux/X abstraction layers without a proper fallback.

Making everyone else on the planet change their code to make your screensaver code a bit cleaner is not a solution.

With regards to ubuntu, I guess making the gnome-shell package conflict with the packages for all the other DMs than GDM and also strictly depend on GDM could solve the problem, without any change to gnome-shell, but surely you already thought about that and I'm missing something.

(In reply to comment #12)
> If there is such a thing, then it would not have worked before, correct?

setuid helper and code duplication was already given as answer
 
> On the other hand there is a lot of people using KDM, LightDM and others, a
> bunch of people using remote terminal logins and a bunch of people doing some
> weird homegrown stuff and then calling startx.

Can you provide clear data on how often those cases are used vs GDM? Seems better to argue with data than guessing.

All Ubuntu users have LightDM now by default, same with Xubuntu. KDE users on all distributions have KDM by default. All terminal client installations that I know of use their specific software for login (and that is hundreds of thousands of installations across just schools in Europe). And I also know a couple corporate deployments where they have custom login screens that later run startx.

I agree that the functionality itself is very cool and very clean, but forcing people to use GDM just to use Gnome is not cool. Imagine if KDE and XFCE did the same - a key part of the ?DM is to allow the user a choice between session types.

I am pretty sure that if this bug is not solved upstream, it will have to be solved at the distribution level anyway. It is better to have a single, central legacy workaround hack than 10 slightly different third-party ones.

While making gnome-shell conflict with all other DMs and depend on gdm would work technically, it would force people to replace a key part of their software stack and will cause unusable systems in the scenarios where another DM was chosen for a reason, such as above.

(In reply to comment #15)
> All Ubuntu users have LightDM now by default, same with Xubuntu. KDE users on
> all distributions have KDM by default. All terminal client installations that I
> know of use their specific software for login (and that is hundreds of
> thousands of installations across just schools in Europe). And I also know a
> couple corporate deployments where they have custom login screens that later
> run startx.

Could you expand a bit more?

LightDM need to be worked upon due to Ubuntu, that's clear.

Seems not too interesting that KDE users have KDM. Or XFCE/Xubuntu having LightDM.

Terminal clients: how many use GNOME 3? I thought most didn't support GNOME 3? Does that hardware allow for locking?

Corporate deployments: How many use GNOME 3? How far to they lag behind?

(In reply to comment #12)
> (In reply to comment #11)
> > But if there's a third party PAM module that relies on being root (Ray would
> > have more details here, I've been told there are such things), the screensaver
> > would fail.
> 
> If there is such a thing, then it would not have worked before, correct?

That's exactly what I'm saying. Either there are PAM modules that have setuid helpers (pam_unix), or they just flat out broke when plugged into gnome-screensaver. Again, Ray would have more information.

There's one additional solution, which probably makes the most sense; split out the PAM code used in gdm into an auth service, and have gdm, gnome-shell, gnome-screensaver, and polkit (and potentially other parts, like LightDM) all use that service. This would remove the dependency of the gdm daemon from gnome-shell.

Thoughts on this, Robert?

By the way, I was told it was the interest of the SSSD project to provide this service over DBus as a replacement for PAM, but it was not implemented for lack of resources.

Still, there is one problem: AFAIK, to refresh credentials correctly, at least for some modules (including kerberos) the reauthentication must run under the session (loginuid, kernel keyring) that it is unlocking.
This means that part of this auth agent must be either starting the PAM session entirely or somehow be started by whoever opens the session. In both cases it makes sense to be part of the display/login manager, and even if splitted it would not solve the startx case.

Hi Jasper,

I'm against implementing a public PAM interface into the LightDM daemon. This is a design I explored a while ago for GDM then LightDM but I'm uneasy about giving access to a root daemon without being sure what process is accessing it.

The design we have in LightDM is the greeter is solely responsible for authentication for login and screen locking (thus giving a consistent user experience). The safety of this is secured by using a private pipe between the daemon and the greeter process (the same way that GDM does it). The risk is the responsibility of the sysadmin who chooses which greeters are safe enough to get this access (i.e. by installing them). There have been some challenges [1] with doing it this way however.

Unfortunately we can't split the PAM code out of the display manager daemons (as much as I would like to) since we have to hold the process that PAM is authenticating and make it become the session [3].

Using PolicyKit was one idea I had thought about a while ago and it seems on the surface like a good fit. Unlocking is just an action that should have policy such as being able to be overridden by root. The problem is PK by design only has one UI for authentication and I don't know how we could override that in a lock screen (or if it would be safe to be able to).

So in summary, my thoughts are:

- I don't want to add a PAM interface to LightDM given that it has security implications and conflicts with the design for LightDM. Also due to the fact that other DMs will still have the same problem.

- Given we can't remove the PAM code from the display manager daemons (AFAICT) there isn't a good common place to put PAM authentication, but I do think PK is worth investigating if that would be possible / make sense.

- I would encourage you to consider moving the lock screen code into your greeter in the long term (given the problems in [1]) or consider using LightDM :) [4]

--Robert

[1] This relies on session switching to be reliable and and we haven't found to be the case with VT switching. We've been working on using a system compositor (i.e. Wayland) [2] that does the switching and that works well with some bugs requiring to be fixed.

[2] https://blueprints.launchpad.net/ubuntu/+spec/desktop-q-system-compositor

[3] Since PAM modules like to do things to the process like setenv, setgroups etc

[4] Please, no flames

(In reply to comment #20)
> Unfortunately we can't split the PAM code out of the display manager daemons
> (as much as I would like to) since we have to hold the process that PAM is
> authenticating and make it become the session [3].

The plan Ray and I had was to have a system service that the display managers would call, similar to how PAM auth works in gdm.

The gdm-session-worker runs a PAM conversation, and then forks and execs a session. For reauth, we simply ask the session worker to fork a new worker, start a PAM reauth conversation, and after it confirms, we kill it and we're done. This could be done by a DE-independent piece of code, like SSSD. Communication in GDM right now, is done by a private pipe where we talk peer-to-peer DBus.

All of this is obviously entirely out of reach for 3.6. For 3.6, I would suggest that we explore 2 things:

- Make gnome-shell cope with gdm not being available and just disable screen locking functionality in this case

- Make gnome-shell fall back to using gnome-screensaver for locking when the gdm authentication interface is not available

I would also suggest that the people who are interested in combining gnome with alternative dms should participate in this exploration.

Created attachment 223657 [details] [review]
ShellUserVerifier: catch DBus errors and report if GDM is not available

Instead of leaving the login or unlock dialogs in an inconsistent state,
report that GDM is not available to the user. For the unlock dialog,
we additionally unlock automatically.

Matthias: is this what you mean by option 1, more or less?

More or less, though this doesn't address the situation when gdm is not installed at all, right ? I mean, the import will blow up ?

(In reply to comment #25)
> More or less, though this doesn't address the situation when gdm is not
> installed at all, right ? I mean, the import will blow up ?

Yes, but AFAUI, the gdm package is already split in a daemon and a library part, and there is no problem for gnome-shell to depend on libgdm.

I agree - we actually started to have a runtime (in the sense of "gdm needs to be installed in order to run the shell", not "we depend on gdm running") dependency on libgdm a bit before the lock screen work.

*** Bug 683693 has been marked as a duplicate of this bug. ***

(In reply to comment #22)
> All of this is obviously entirely out of reach for 3.6. For 3.6, I would
> suggest that we explore 2 things:
> 
> - Make gnome-shell cope with gdm not being available and just disable screen
> locking functionality in this case
> 
> - Make gnome-shell fall back to using gnome-screensaver for locking when the
> gdm authentication interface is not available

I think we need one of these options anyway in case gdm becomes broken after the shell is started, or some shell extensions interfere with lockscreen code and the shell can no longer communicate with gdm.

For the latter case, at least make it an option to break through the lock screen. It'd be useful during shell extension development.

If gdm becomes broken (in the sense of crashing), the X server will go down, and take the session with it.
As far as shell extensions are concerned, since 3.5.91 they're disabled in the lock screen and unlock dialog, so they should not be a problem.

(In reply to comment #30)
> If gdm becomes broken (in the sense of crashing), the X server will go down,
> and take the session with it.

I can kill gdm process while gdm-simple-slave is still running (I don't know which one is talking to gnome-shell though). There could also be bugs in gdm that makes it stop responding to gnome-shell. The same applies to gnome-shell's bugs (that affects certain parts of the shell but not as a whole).

> As far as shell extensions are concerned, since 3.5.91 they're disabled in the
> lock screen and unlock dialog, so they should not be a problem.

In the context of extension development, the extension can leave some hooks in place when it is asked to be disabled. That can happen before lock screen is on and affect the lock screen.

(In reply to comment #26)
> (In reply to comment #25)
> > More or less, though this doesn't address the situation when gdm is not
> > installed at all, right ? I mean, the import will blow up ?
> 
> Yes, but AFAUI, the gdm package is already split in a daemon and a library
> part, and there is no problem for gnome-shell to depend on libgdm.

Ok, good enough for me.

Created attachment 223934 [details]
screenshot

This is what I get with this patch applied.

I think this is unacceptable for a few reasons.

1. The screen shield indicates that the screen is locked (even has a lock icon) when it is actually not locked. This is a big problem because it is a false sense of security.

2. Locks should never "fail open". If something goes wrong you should never pop open a lock.

3. The notification is unacceptable. It mentions implementation details (is almost entirely details), the "subject" line doesn't fit in the banner size, strange choice of icon, etc.

4. Doesn't seem to differentiate the case where the lock is supposed to work from the case where it fails unexpectedly.


I think there are at least two very different cases. Failure in a compatible OS and failure in an incompatible OS.

Basically, if the correct versions of all subsystems are present (ie. the OS is properly built) then we should consider a failure catastrophic. Basically fail-whale. You should never fail-open in this case. That would be a huge security hole. In GNOME 2 we always failed-closed in this case and we should continue to do that, I think. For the normal user this probably means the system "crashed" and a reboot is necessary - including doing whatever recovery/failsafe stuff the OS provides as needed. For the expert user this means killing and restarting components manually as is typical.

The other case is the one I have when I run jhbuild gnome-shell on top of, say, Fedora 17. Which is exactly the same scenario as running gnome-shell on top of say, Ubuntu. In this case we know that locking is never going to work. So we should act as if locking is disabled. We should not show a lock icon in the screen shield and not respond to control-alt-L or whatever the lock shortcut is. We should remove the Lock item from the user menu. etc.

Currently experiencing this as well; while I can't reasonably add to this detailed discussion, I'll shortly tell you my experience:

1. Can't start gdm due to DEBUG(+): Lost GDM name on bus

2. Removed gdm from runlevels, I run `startx` conditionally in .bashrc now.

3. Upon trying to type the password, I can _see_ the password and can't press the button to check the password.

4. Pressing ESC at this point just kills the locking screen itself as well, leaving me on a gray-ish shell screen... o_O

Bottom line of the story, I'm experiencing three bugs here disallowing me from locking unless I force `xscreensaver` to kick in, but that leaves my TTY1 wide open because I run `startx` from there.

What also bothers me is that locking no longer shows me my `xscreensaver`, which I'd rather like to see than a static background...

Sad to hear other DMs wouldn't be supported, some people prefer a lightweight Slim or some console based login (better than my .bashrc approach). Although I don't really mind it myself as I'd use gdm; going to try to file a bug for that now as well and then will file a bug for the "ESC kills lock screen" and "where's my sreen saver?" too...

An approach that doesn't require the daemon running sounds nice, as that gives the users freedom (the reason I'm running Gentoo for) and doesn't force you down certain parts of the OS; but feel free to pursue a solution that fits security better, because that on its own is of importance as well, seems the typical advantages / drawbacks weighting. Hope you can find the best of both worlds, thanks for discussing this, couldn't find this bug for a while...

Created attachment 224011 [details] [review]
Allow the shell to run without the screenshield

The screenshield requires gdm 3.5, which can be problematic in
jhbuild configurations, or distributions that don't use GDM as the display
manager. Allow transparent fallback to gnome-screensaver in that case.

This is an old patch from the first screen lock iteration, reworked to do
runtime detection of gdm. Requires a new DBus property, 'Version', not yet
merged, and therefore will cause fallback on all testers using GDM from the
distro (including those on Fedora 18).
Also, it uses synchronous DBus at startup to find if the screenshield is
supported. Unfortunately, I'm not sure it's possible to avoid that, without
another mass rework of the various interdependencies between shell components.
I've added a comment and hope that distros not planning to support anything
but gdm (including gnome-ostree) will patch isSupported() to return true without
further checks.

I admit that I don't understand the downsides of enabling basic lock support even if the user isn't running GDM. Why though are you recommending distros to remove that support?

(In reply to comment #36)
> I admit that I don't understand the downsides of enabling basic lock support
> even if the user isn't running GDM. Why though are you recommending distros to
> remove that support?

1) I'm not disabling basic lock support, I'm disabling integrated lock support in gnome-shell. If you (user, distro) want to have locking without GDM, you can use gnome-screensaver, like 3.4

2) I'm not recommending distros to patch isSupported() to return false, I'm recommending them to return true, so that we avoid useless sync IO in the common case and we give the user the full integrated 3.6 experience.

(In reply to comment #35)
> Created an attachment (id=224011) [details] [review]
> Allow the shell to run without the screenshield

This patch doesn't allow me to use the Gnome shell because it simply doesn't appear, I only get to see my background after executing `startx`. When I switch back to TTY1 I see as error that Main.screenShield.connect doesn't exist. I suppose your Fallback doesn't implement that function yet...

Created attachment 224226 [details] [review]
Allow the shell to run without the screenshield

The screenshield requires gdm 3.5, which can be problematic in
jhbuild configurations, or distributions that don't use GDM as the display
manager. Allow transparent fallback to gnome-screensaver in that case.

*** Bug 683757 has been marked as a duplicate of this bug. ***

we should get this reviewed and landed for .92 on Monday

Review of attachment 224226 [details] [review]:

::: js/ui/main.js
@@ +141,3 @@
+    if (UnlockDialog.isSupported()) {
+        screenShield = new ScreenShield.ScreenShield();
+        screenSaverDBus = new ShellDBus.ScreenSaverDBus();

To me, screenSaverDBus should be contained and constructed by the ScreenShield.

(and ExtensionSystemDBus should be contained constructed by the ExtensionSystem, but that's another patch)

::: js/ui/unlockDialog.js
@@ +50,3 @@
+
+        let version = result.deep_unpack()[0].deep_unpack();
+function isSupported() {

I don't understand this. Why are we comparing versions?

(In reply to comment #42)
> Review of attachment 224226 [details] [review]:
> 
> ::: js/ui/main.js
> @@ +141,3 @@
> +    if (UnlockDialog.isSupported()) {
> +        screenShield = new ScreenShield.ScreenShield();
> +        screenSaverDBus = new ShellDBus.ScreenSaverDBus();
> 
> To me, screenSaverDBus should be contained and constructed by the ScreenShield.
> 
> (and ExtensionSystemDBus should be contained constructed by the
> ExtensionSystem, but that's another patch)
> 
> ::: js/ui/unlockDialog.js
> @@ +50,3 @@
> +
> +        let version = result.deep_unpack()[0].deep_unpack();
> +function isSupported() {
> 
> I don't understand this. Why are we comparing versions?

Because 3.5.4 or less doesn't support OpenReauthenticationChannel, so jhbuild on currently stable distros would fail.
It's true that before 3.5.92 there is no Version property at all, so the actual comparison is moot, but it seemed cleaner to me to have a full Version property and check it, rather than some "ApiVersion" or "IsReauthSupported"

Created attachment 224412 [details] [review]
Allow the shell to run without the screenshield

The screenshield requires gdm 3.5, which can be problematic in
jhbuild configurations, or distributions that don't use GDM as the display
manager. Allow transparent fallback to gnome-screensaver in that case.

Review of attachment 224412 [details] [review]:

::: js/ui/unlockDialog.js
@@ +38,3 @@
+function isSupported() {
+    // DISTROS: please patch this away, it's a awful hack to allow
+    // running without GDM

I hate this ... can't we have a configure option instead of asking distros to patch the code?

(In reply to comment #45)
> Review of attachment 224412 [details] [review]:
> 
> ::: js/ui/unlockDialog.js
> @@ +38,3 @@
> +function isSupported() {
> +    // DISTROS: please patch this away, it's a awful hack to allow
> +    // running without GDM
> 
> I hate this ... can't we have a configure option instead of asking distros to
> patch the code?

How is a non-default (thus untested) config option different from a downstream patch? And at least, if it's a patch we don't appear to support it...

(In reply to comment #46)

> How is a non-default (thus untested) config option different from a downstream
> patch? And at least, if it's a patch we don't appear to support it...

Shipping the code in the first place seems to be a pretty clear statement about supporting it...

In fact, I was referring to the opposite (a patch or configuration option to remove that code), so yeah, it didn't make much sense in retrospect.
Still, I don't think a configuration option is right. I'll just remove the comment.

Created attachment 224653 [details] [review]
Allow the shell to run without the screenshield

The screenshield requires gdm 3.5, which can be problematic in
jhbuild configurations, or distributions that don't use GDM as the display
manager. Allow transparent fallback to gnome-screensaver in that case.

Created attachment 224656 [details] [review]
ShellUserVerifier: catch DBus errors and report them to the user

Instead of leaving the login or unlock dialogs in an inconsistent state,
catch DBus errors and show an Authentication Error message. The error
details are logged in the session logs.

This one is not strictly necessary if the other one lands, but it
provides a better experience for other errors, IMHO. In particular it
could help for transient GDM failures like those in bug 684172.
Breaks string freeze (new string "Authentication error").

Review of attachment 224653 [details] [review]:

OK we should go with that one for now. Code looks good.

Review of attachment 224656 [details] [review]:

Code looks good not sure about the strings though (need i18n approval and maybe a review by the designers).

Comment on attachment 224653 [details] [review]
Allow the shell to run without the screenshield

Attachment 224653 [details] pushed as de93677 - Allow the shell to run without the screenshield
I'll send a freeze break request for the other patch.

Created attachment 224661 [details]
New screenshot

This patch worked fine when I was running without gdm, but now that I fixed my gdm this patch broke it for me because it couldn't find a certain function right before the gdb login kicks in so I had to undo this patch. Dunno what the function was called (didn't write it down, can't find it in any logs) but I would advise you to test this patch on gdm as well.

This patch is tested in gdm, but you need 3.5.92 for the new Version DBus property.

Found it...

gnome-session[2103]: DEBUG(+): GsmDBusClient: obj_path=/org/gnome/SessionManager/Presence interface=org.freedesktop.DBus.Properties method=GetAll     JS ERROR: !!!   Exception was: TypeError: screenShield.showDialog is not a function     JS ERROR: !!!     message = '"screenShield.showDialog is not a function"'     JS ERROR: !!!     fileName = '"/usr/share/gnome-shell/js/ui/main.js"'     JS ERROR: !!!     lineNumber = '94'     JS ERROR: !!!     stack = '"createGDMSession()@/usr/share/gnome-shell/js/ui/main.js:94 ()@/usr/share/gnome-shell/js/ui/sessionMode.js:85 wrapper()@/usr/share/gjs-1.0/lang.js:204 start()@/usr/share/gnome-shell/js/ui/main.js:225 @<main>:1 "' Window manager warning: Log level 32: Execution of main.js threw exception: TypeError: screenShield.showDialog is not a function

This looks more like a JS error than a DBus error to me, so you might want to verify... If it is indeed fixed upstream, nvm my previous comment then.

It's kind of both... It's a DBus error that triggers ScreenShieldFallback, but that's not prepared to handle a GDM greeter (there is no showDialog() method), so it fails again with a JS error.
Please test again with gdm 3.5.92.

Giovanni, how does the latest patch compare to Jon's concerns in comment 33 ?
Also, I was a little confused that the screenshot seems to show just the regular authentication failure ?

Well, with the latest patch we fail in a controlled way, and this allows us to keep the screen locked, so we "fail close" if you want.
Also, we now differentiate cases where it's not supported (missing or wrong GDM on the bus) and cases where it failed for bugs, and only show this message in the latter case. I also remove all technical details from the message, leaving just "Authentication error".

I considered invoking the fail whale explicitly, but short of adding another method on the session bus, there is no way at the moment ("crashing" the shell will simply spawn another one), so this seemed the easiest way for 3.6.

(In reply to comment #61)
> Well, with the latest patch we fail in a controlled way, and this allows us to
> keep the screen locked, so we "fail close" if you want.

So if you aren't running GDM and the screen locks, it's impossible to open it back up, or? Should the screen saver pop back up? What's "Authentication error" for?

No, if you aren't running GDM we hit the fallback path. This is only for unexpected failures, and yes, in that case there is no way to open it except by going to a VT and killing the shell.

(In reply to comment #63)
> in that case there is no way to open it except by going to a VT and killing the shell.

That is not really what the string suggests though - it sounds a lot like "You mistyped your password, try again".
(No better suggestion from my side at this point unfortunately)

Well, the error is from initialization of the conversation, so slowness aside it's shown well before the password is inputted, and that should clarify that it's not a user error.

Ah, sounds good to me then.

Attachment 224656 [details] pushed as 10884ef - ShellUserVerifier: catch DBus errors and report them to the user

Got string freeze approval, pushing.

Created attachment 226782 [details] [review]
screenShield: explicitly load gnome-screensaver in fallback mode.

When running gnome-shell from lightDM, gnome-screensaver is no
longer auto-loaded. As a result the dbus calls for Lock user etc
will fail.

Created attachment 226783 [details] [review]
Move gnome-screensaver to the fallback session.

Created attachment 226784 [details] [review]
move gnome-screensaver desktop file out of autostart

When running gnome-shell with lightdm, gnome-screensaver never gets auto-started. As a result the fallback code in screenShield, fails trying to make
dbus calls. 

As was suggested by halfline on IRC, I have moved gnome-screensaver into the fallback session (removing autostart) and made the fallback code in screenshield explicitly launch gnome-screensaver.

Created attachment 226790 [details] [review]
screenShield: explicitly load gnome-screensaver in fallback mode.

When running gnome-shell from lightDM, gnome-screensaver is no
longer auto-loaded. As a result the dbus calls for Lock user etc
will fail.

This is an alternate patch that uses spawn instead, so it doesnt
depend on the gnome-session changes.

Created attachment 226795 [details] [review]
screenShield: explicitly load gnome-screensaver in fallback mode.

When running gnome-shell from lightDM, gnome-screensaver is no
longer auto-loaded. As a result the dbus calls for Lock user etc
will fail.

https://bugzilla.gnome.org/show_bug.cgi?id=683060
Bug-Ubuntu: https://launchpad.net/bugs/1064354

Created attachment 226796 [details] [review]
screenShield: explicitly load gnome-screensaver in fallback mode.

When running gnome-shell from lightDM, gnome-screensaver is no
longer auto-loaded. As a result the dbus calls for Lock user etc
will fail.

https://bugzilla.gnome.org/show_bug.cgi?id=683060
Bug-Ubuntu: https://launchpad.net/bugs/1064354

Review of attachment 226783 [details] [review]:

sure

Review of attachment 226784 [details] [review]:

looks right

Review of attachment 226796 [details] [review]:

::: js/ui/screenShield.js
@@ +847,2 @@
     _init: function() {
+        Util.spawn(['gnome-screensaver']);

Probably okay, though:

let app = Shell.AppSystem.get_default().lookup_app('gnome-screensaver.desktop');
app.activate();

might be slightly nicer. hard to say. I guess we don't need app window tracking or startup notification or anything, so maybe not.

Why not just remove the DO_NOT_AUTO_START?

it's not dbus activatable (intentionally)

(In reply to comment #77)
> Review of attachment 226796 [details] [review]:
> 
> ::: js/ui/screenShield.js
> @@ +847,2 @@
>      _init: function() {
> +        Util.spawn(['gnome-screensaver']);
> 
> Probably okay, though:
> 
> let app =
> Shell.AppSystem.get_default().lookup_app('gnome-screensaver.desktop');
> app.activate();
> 
> might be slightly nicer.

That's what attachment 226782 [details] [review] did. We could go with that for master and commit attachment 226796 [details] [review] on gnome-3-6 (so we don't require new releases for gnome-screensaver and gnome-session when doing a 3.6.2 release)

That'd be fine, or we could just use Util.spawn() for both, since it seems just as good here.

Comment on attachment 226784 [details] [review]
move gnome-screensaver desktop file out of autostart

Attachment 226784 [details] pushed as 1940dc6 - move gnome-screensaver desktop file out of autostart

Attachment 226796 [details] pushed as 418cf62 - screenShield: explicitly load gnome-screensaver in fallback mode.