Subject: Re: Bug#642012: x11-common: ssh-agent Xsession script does not check if gpg-agent will enable SSH support

From: Luca Capello <[email-masked]>
Date: Mon, 26 Sep 2011 16:36:52 +0200

block 642012 by 642021
thanks

Hi there!

On Mon, 19 Sep 2011 14:57:14 +0200, Julien Cristau wrote:
> On Sun, Sep 18, 2011 at 21:51:21 +0200, Luca Capello wrote:
>> This leaves the bug opened: I would be glad to explore other solutions,
>> but AFAIK without checking gpg.conf and gpg-agent.conf there is no way
>> to know *beforehand* 1) if gpg-agent will run and 2) if the latter will
>> provide SSH support.

This is the real problem.

>> Please note that until now ssh-agent is *never* started if gpg-agent has
>> been started at least once with SSH support, for the following reasons
>> (and this is another bug, no matter what):
>> 
>> 1) 90gpg-agent is sourced before 90x11-common_ssh-agent
>> 2) gpg-agent does not remove its "PID" file when exiting, see #642021
>
> Sounds like that should be fixed.

Patch sent upstream and block added.

  <http://news.gmane.org/find-root.php?message_id=1316457193-26043-1-git-send-email-luca%40pca.it>
  <http://bugs.debian.org/cgi-bin/bugreport.cgi?msg=10;bug=642021>

>> 3) 90gpg-agent sources the "PID" file above, which means that
>>    SSH_AUTH_SOCK is defined *before* any gpg-agent is started at all
>
> Shouldn't the "if ! $GPGAGENT 2>/dev/null; then" line in 90gpg-agent be
> followed by unsetting the variables (and maybe removing the file) it
> just read since it found out they don't work?

Good catch, I will follow-up on the other bug report.  However,
unsetting (at least) the SSH_AUTH_SOCK variable is not correct, because
it could be defined in ~/.Xsessionrc.

>> 4) 90x11-common_ssh-agent starts ssh-agent only if SSH_AUTH_SOCK is
>>    empty, which is not the case as per point 3
[...]
>> IMHO the real bug is to try to start ssh-agent in a system-wide fashion
>> via /etc/X11/Xsession.options, while this is (clearly) a user option.
>> This is also why I fear the new Xsession "use-gpg-agent" option at
>> <http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=412993#20>.  The fact
>> that ssh_config does not have any way to define that we want the agent
>> is probably the original cause of this bug.
>> 
> Can we switch the order so that 1) doesn't apply?  And turn ssh-agent
> into a no-op when it's started by gpg-agent with ssh support (assuming
> it's not already)?

I still fail to see your solution: when both Xsession scripts do their
checks there is no agent running at all, so reverting the order should
not change anything.  Again, how do you know that gpg-agent will be
started with SSH support?

Please also note that unfortunately ssh-agent starts a new instance even
if another one is already running:
=====
$ gpg-agent
gpg-agent: gpg-agent running and available

$ export | grep -e GPG -e SSH
declare -x GPG_AGENT_INFO="/tmp/gpg-g4sLIi/S.gpg-agent:1943:1"
declare -x SSH_AGENT_PID="1943"
declare -x SSH_AUTH_SOCK="/tmp/gpg-Gy6MdS/S.gpg-agent.ssh"

$ ssh-agent
SSH_AUTH_SOCK=/tmp/ssh-CdRxIRsi7792/agent.7792; export SSH_AUTH_SOCK;
SSH_AGENT_PID=7793; export SSH_AGENT_PID;
echo Agent pid 7793;

$ ssh-agent
SSH_AUTH_SOCK=/tmp/ssh-PsTeGnGE7845/agent.7845; export SSH_AUTH_SOCK;
SSH_AGENT_PID=7846; export SSH_AGENT_PID;
echo Agent pid 7846;

$ export | grep -e GPG -e SSH
declare -x GPG_AGENT_INFO="/tmp/gpg-g4sLIi/S.gpg-agent:1943:1"
declare -x SSH_AGENT_PID="1943"
declare -x SSH_AUTH_SOCK="/tmp/gpg-Gy6MdS/S.gpg-agent.ssh"

$ ls -l /tmp/ | grep -e gpg -e ssh
drwx------ 2 luca luca  60 Sep 25 17:29 gpg-g4sLIi
drwx------ 2 luca luca  60 Sep 25 17:29 gpg-Gy6MdS
drwx------ 2 luca luca  60 Sep 26 15:19 ssh-CdRxIRsi7792
drwx------ 2 luca luca  60 Sep 26 15:20 ssh-PsTeGnGE7845
=====

>> Finally, may I ask why this file is not provided by openssh-client?  I
>> could not find any reference in the x11-common changelog.Debian nor
>> x11-common Recommends:/Suggests:/Enhances: openssh-client.
>> 
> The changelog suggests this was already in xfree86-common with the
> initial xfree86 4.0 upload 11 years ago.  I could go look for earlier
> changelogs, but I guess "hysterical raisins" pretty much covers it?

I came to the same conclusion.  However, I still think openssh-client
would be a better place, because until now ssh-agent is started
unconditionally without asking the user (and FWIW not event the
sysadmin).  The fact that there is no way to have ssh-agent "configured"
through a user variable changes the whole situations, so I will not
bother any more with this.

Attached a Git patch to add the Enhances: above, including dbus-x11 for
the very same reason.

Thx, bye,
Gismo / Luca

From 95a9f699207107621cf59443efd7b2a1f2effa0e Mon Sep 17 00:00:00 2001
From: Luca Capello <[email-masked]>
Date: Mon, 26 Sep 2011 16:31:54 +0200
Subject: [PATCH] debian/control: x11-common Enhances: dbus-x11 and
 openssh-client

Please see #642012 for the full rationale.
---
 debian/changelog |    9 +++++++++
 debian/control   |    1 +
 2 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/debian/changelog b/debian/changelog
index f2198f9..361e81a 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,12 @@
+xorg (1:7.6+10) UNRELEASED; urgency=low
+
+  [ Luca Capello ]
+  * debian/control:
+    + add dbus-x11 and openssh-client to x11-common's Enhances: (both
+      because of their Xsession.d scripts).
+
+ --
+
 xorg (1:7.6+9) unstable; urgency=low
 
   [ Julien Cristau ]
diff --git a/debian/control b/debian/control
index b825221..b96fe2c 100644
--- a/debian/control
+++ b/debian/control
@@ -16,6 +16,7 @@ Package: x11-common
 Architecture: all
 Multi-Arch: foreign
 Depends: ${misc:Depends}, lsb-base (>= 1.3-9ubuntu2)
+Enhances: dbus-x11 openssh-client
 Description: X Window System (X.Org) infrastructure
  x11-common contains the filesystem infrastructure required for further
  installation of the X Window System in any configuration; it does not
-- 
1.7.6.3