Control: tags 927105 - moreinfo
Hi Daniel,
On 4/15/19 9:17 PM, Daniel Kahn Gillmor wrote:
On Sun 2019-04-14 23:25:14 -0700, Zephaniah E. Loss-Cutler-Hull wrote:
Looking at the code, it sure looks like it tries to handle this, by
checking to see if there is a DBUS_SESSION_BUS_ADDRESS (which there is,
inherited from the gpg-agent), if a gcr system prompt is available, and
trying to see if the screen is locked, however in my testing none of
these actually seem to work to detect that, indeed, the screen is locked
and the user isn't at the desktop any more.
To me the obvious solution is to also check and see if there is a
display set, using the same logic as pinentry-gtk-2, I have some fear
that this will break a pure wayland environment (one with no xwayland
involved), however I don't actually have one of those handy to test
with. If someone with a wayland environment could test this that would
be appreciated.
As far as i'm aware, the current state is a deliberate choice by
upstream (and it's also one that i think is the right answer) -- GNOME3
is not X11-only, and pinentry-gnome3 doesn't bother communicating with
the X11 session via $DISPLAY at all. As you point out, your proposed
patch seems likely to fail in environments like Wayland.
I need to try and find the time to spin up a Wayland native VM and do
some testing to see if DISPLAY is getting set in all reasonable cases.
I say reasonable because no X11 programs could be run in that
environment, and that doesn't seem like a horribly likely configuration.
If you don't want to talk to a GNOME3 session, you can change your
pinentry explicitly to be something other than pinentry-gnome3 :)
But that's _way_ uglier! :)
That said, are you certain that GNOME desktop is is locked when you
conduct your test? What screenlocking mechanism are you using? If the
screen is *unlocked* then i think there's a decent argument for wanting
the prompt to happen on the screen anyway. But if the screen is locked,
and it can fall back to curses maybe i can agree with you that it
should?
So for the sake of immediate convenience, I'm doing some of the testing
while I write this email on a Ubuntu 18.04 box, but the behaviour
appears to be the same.
System was locked with the lock screen hot key, password is required to
unlock, and monitors were powered down by the system. So definitely locked.
ssh to the system, attempt to use 'pass' to access a password fails with:
gpg: decryption failed: No secret key
More interestingly calling pinentry-gnome3 directly fails with an
'Operation Cancelled' error:
(Doing this without the echo so there is a real TTY doesn't have any
impact on the results.)
~$ echo 'GETPIN' | pinentry-gnome3
OK Pleased to meet you
ERR 83886179 Operation cancelled <Pinentry>
Doing the same thing on an unlocked desktop brings up the pin prompt,
and returns the results:
~$ echo GETPIN | pinentry-gnome3
OK Pleased to meet you
D 1234
OK
Now, with all of that said...
If you have any patch that you want to propose that falls back to curses
if both:
* gcr fails because the screen is locked, and
* a tty is available
Then i would be happy to help you convince upstream (e.g. via
https://dev.gnupg.org/) that this should be adopted. I've suggested
this approach before in #842015, but i haven't had the time (or the
experience with using dbus to monitor a GNOME session lock, e.g. [0]) to
implement it myself.
There may also be a race condition here -- between testing for the
screen lock and using gcr, or vice versa. Perhaps gcr itself is what
needs to be able to fail cleanly (thereby triggering the fallback) if
the screen is locked? that seems less likely to have a race. I'm open
to different implementation ideas, anyway, but testing for $DISPLAY
seems like the wrong choice.
(This may be a little disjointed, I'm writing as I debug.)
I think that there is a bug in how it is detecting the screen lock,
because pe_gnome_screen_locked has a comment that the check it is doing
is intended to be equivalent of:
dbus-send --print-reply=literal --session --dest=org.gnome.ScreenSaver
/org/gnome/ScreenSaver org.gnome.ScreenSaver.GetActive
Except that this command when run from the command line is correctly
detecting that the screen is locked.
Which means that either pe_gnome_screen_locked isn't working correctly,
or it somehow isn't getting called.
So, a few minutes of debugging later, in looks like
g_dbus_connection_call_sync is returning NULL, instead of a correct value.
Which, as long as there is no error, fails silently. But there is an
error, a timeout. Timeouts are explicitly getting ignored, but why is
there a timeout?
So, referencing:
https://developer.gnome.org/gio//2.50/GDBusConnection.html#g-dbus-connection-call-sync
The third to the last argument is timeout_msec:
the timeout in milliseconds, -1 to use the default timeout or G_MAXINT
for no timeout
Except that the code is passing in a 0, not -1, not some other value,
and not G_MAXINT. Which appears to be causing an immediate timeout, and
so the 'is the screen locked' check simply fails, every single time.
Which means that the correct fix is to simply change the timeout from 0
to something sane.
Do you see any reason why accepting the default timeout with -1 wouldn't
be the correct thing to do here?
A tested patch is attached.
Now, I'm still not entirely convinced on the logic that a remote session
should trigger a desktop PIN entry, but that seems more like an upstream
design question of intent instead of something to try and fix in Debian.
I'm happy to try and do some work to implement a check that works on
Wayland as well if upstream has any interest in it, but first we would
need to confirm the intent.
Regards,
Zephaniah E. Loss-Cutler-Hull.
--dkg
[0]
https://elementaryos.stackexchange.com/questions/14083/dbus-signal-for-session-lock-unlock
Description: Use the default dbus timeout.
At the moment, every single check to see if the screen is locked fails
immediately with a timeout, which is then silently ignored.
.
This is happening because the definition of the timeout argument is a
millisecond count, with -1 as the (unspecified) default, and G_MAXINT for no
timeout.
.
This means 0 times out immediately, and that leads to a complete inability for
pinentry-gnome3 to fallback to curses input when the screen is locked.
.
(Which then prevents a remote session from ever asking for a PIN, even when
the local session is locked.)
Author: Zephaniah E. Loss-Cutler-Hull <zephan...@gmail.com>
---
The information above should follow the Patch Tagging Guidelines, please
checkout http://dep.debian.net/deps/dep3/ to learn about the format. Here
are templates for supplementary fields that you might want to add:
Bug-Debian: https://bugs.debian.org/927105
Last-Update: 2019-04-17
--- pinentry-1.1.0.orig/gnome3/pinentry-gnome3.c
+++ pinentry-1.1.0/gnome3/pinentry-gnome3.c
@@ -446,7 +446,7 @@ pe_gnome_screen_locked (void)
NULL,
((const GVariantType *) "(b)"),
G_DBUS_CALL_FLAGS_NO_AUTO_START,
- 0,
+ -1,
NULL,
&error);
g_object_unref(dbus);
