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

From: Luca Capello <[email-masked]>
Date: Sun, 18 Sep 2011 16:53:13 +0200

Package: x11-common
Version: 1:7.6+8
Severity: important
File: /etc/X11/Xsession.d/90x11-common_ssh-agent
Affects: openssh-client gnupg-agent
Tags: patch

Hi there!

As the title suggests, if you install both openssh-client and
gnupg-agent and you enable the SSH support in the latter, ssh-agent is
anyway started by the Xsession script:
=====
$ ps aux | grep agent
luca     16399  0.0  0.0  12236   304 ?        Ss   16:09   0:00 \
 /usr/bin/ssh-agent \
 /usr/bin/gpg-agent --daemon --sh --write-env-file=/home/luca/.gnupg/gpg-agent-info-gismo \
 /usr/bin/ck-launch-session \
 /usr/bin/dbus-launch --exit-with-session /home/luca/.xsession

luca     16400  0.0  0.0  18968   424 ?        Ss   16:09   0:00 \
 /usr/bin/gpg-agent --daemon --sh --write-env-file=/home/luca/.gnupg/gpg-agent-info-gismo \
 /usr/bin/ck-launch-session \
 /usr/bin/dbus-launch --exit-with-session /home/luca/.xsession

$ ls -l /tmp | grep -e gpg -e ssh
drwx------ 2 luca luca  60 Sep 18 16:09 gpg-0drpxf
drwx------ 2 luca luca  60 Sep 18 16:09 gpg-xSZr4Z
drwx------ 2 luca luca  60 Sep 18 16:09 ssh-GmzGjVQ16373

$ grep -s '^[[:space:]]*use-agent' ~/.gnupg/gpg.conf
use-agent

$ grep -s '^[[:space:]]*enable-ssh-support' ~/.gnupg/gpg-agent.conf
enable-ssh-support

$ export | grep -e GPG -e SSH
declare -x GPG_AGENT_INFO="/tmp/gpg-0drpxf/S.gpg-agent:16400:1"
declare -x SSH_AGENT_PID="16400"
declare -x SSH_AUTH_SOCK="/tmp/gpg-xSZr4Z/S.gpg-agent.ssh"
=====

Indeed the check for $SSH_AUTH_SOCK is not enough: this variable is not
set given that gpg-agent has not been started yet.  Here is the patch:

--8<---------------cut here---------------start------------->8---
--- 90x11-common_ssh-agent.ORG
+++ 90x11-common_ssh-agent
@@ -9,10 +9,15 @@
 if has_option use-ssh-agent; then
   if [ -x "$SSHAGENT" ] && [ -z "$SSH_AUTH_SOCK" ] \
      && [ -z "$SSH2_AUTH_SOCK" ]; then
-    STARTSSH=yes
-    if [ -f /usr/bin/ssh-add1 ] && cmp -s $SSHAGENT /usr/bin/ssh-agent2; then
-      # use ssh-agent2's ssh-agent1 compatibility mode
-      SSHAGENTARGS=-1
+    # check if gpg-agent will be started with SSH support
+    : ${GNUPGHOME=$HOME/.gnupg}
+    if ! ( grep -qs '^[[:space:]]*use-agent' "$GNUPGHOME/gpg.conf" && \
+           grep -qs '^[[:space:]]*enable-ssh-support' "$GNUPGHOME/gpg-agent.conf" ); then
+      STARTSSH=yes
+      if [ -f /usr/bin/ssh-add1 ] && cmp -s $SSHAGENT /usr/bin/ssh-agent2; then
+        # use ssh-agent2's ssh-agent1 compatibility mode
+        SSHAGENTARGS=-1
+      fi
     fi
   fi
 fi
--8<---------------cut here---------------end--------------->8---

Thx, bye,
Gismo / Luca

-- System Information:
Debian Release: wheezy/sid
  APT prefers unstable
  APT policy: (990, 'unstable'), (1, 'experimental')
Architecture: amd64 (x86_64)

Kernel: Linux 3.1.0-rc4-amd64 (SMP w/2 CPU cores)
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash

Versions of packages x11-common depends on:
ii  debconf [debconf-2.0]  1.5.41
ii  lsb-base               3.2-28

x11-common recommends no packages.

x11-common suggests no packages.

-- debconf information:
  x11-common/xwrapper/allowed_users: Console Users Only
  x11-common/xwrapper/actual_allowed_users: console

---

From: Julien Cristau <[email-masked]>
Date: Sun, 18 Sep 2011 17:05:37 +0200

> +++ 90x11-common_ssh-agent
NAK, as far as I'm concerned this script has no business looking around
in gpg.conf.

Cheers,
Julien

---

From: Luca Capello <[email-masked]>
Date: Sun, 18 Sep 2011 17:05:40 +0200

affects 642012 + openssh-client gnupg-agent
thanks

Hi there!

> Affects: openssh-client gnupg-agent

It seems that the BTS does not understand 'affects' as a pseudo-header
or I do not know how to use it correctly, fixed.

Thx, bye,
Gismo / Luca

---

From: Luca Capello <[email-masked]>
Date: Sun, 18 Sep 2011 21:51:21 +0200

Hi there!

>> --8<---------------cut here---------------start------------->8---
[patch]
> NAK, as far as I'm concerned this script has no business looking around

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.

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
3) 90gpg-agent sources the "PID" file above, which means that
   SSH_AUTH_SOCK is defined *before* any gpg-agent is started at all
4) 90x11-common_ssh-agent starts ssh-agent only if SSH_AUTH_SOCK is
   empty, which is not the case as per point 3

Here is the patch to test the behavior above:

--8<---------------cut here---------------start------------->8---
--- 90x11-common_ssh-agent.ORG
+++ 90x11-common_ssh-agent
@@ -14,6 +14,11 @@
       # use ssh-agent2's ssh-agent1 compatibility mode
       SSHAGENTARGS=-1
     fi
+  else
+    cat <<EOF >>"$HOME"/.xsession-errors
+/etc/X11/Xsession.d/90x11-common_ssh-agent: SSH_AUTH_SOCK='$SSH_AUTH_SOCK'
+/etc/X11/Xsession.d/90x11-common_ssh-agent: not starting ssh-agent
+EOF
   fi
 fi

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.

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.

Thx, bye,
Gismo / Luca

---

From: Julien Cristau <[email-masked]>
Date: Mon, 19 Sep 2011 14:57:14 +0200

> 2) gpg-agent does not remove its "PID" file when exiting, see #642021

Sounds like that should be fixed.

>    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?

> +++ 90x11-common_ssh-agent
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)?

> 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?

Cheers,
Julien

---

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

block 642012 by 642021
thanks

Hi there!

>> to know *beforehand* 1) if gpg-agent will run and 2) if the latter will

This is the real problem.

> 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>

> 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.

>>    empty, which is not the case as per point 3
[...]
> into a no-op when it's started by gpg-agent with ssh support (assuming

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
=====

> 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

---

From: Luca Capello <[email-masked]>
Date: Tue, 21 Feb 2012 13:36:11 +0100

usertags 642012 + pca.it-authentication
thanks

Hi there!

Hellekin, the patch you have sent is referring to another bug, see:

  <http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=444103#15>

BTW, please always check if you need to Cc: other people interested in
the bug (you did not Cc: me, so I was not aware of your replies).

>   <http://bugs.debian.org/cgi-bin/bugreport.cgi?msg=10;bug=642021>

Upstream's opinion is that the "PID" file must not be removed:

  <http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=642021#33>

> it could be defined in ~/.Xsessionrc.

According to its manpage, it is ssh-agent that sets this variable, so
any value in ~/.Xsessionrc (sourced by 40x11-common_xsessionrc, so
before any 90*agent) should be simply ignored:

  A UNIX-domain socket is created and the name of this socket is
  stored in the SSH_AUTH_SOCK environment variable.  The socket is
  made accessible only to the current user.  This method is easily
  abused by root or another instance of the same user.

However, given upstream's opinion on the "PID" file not to be removed, I
would simply unset the variables.  This at least to be sure that
variables from dead gpg-agent processes will not influence the current
login.

> started with SSH support?

The key is SSH_AUTH_SOCK, which should be *anyway* set is gpg-agent
should be started with the SSH support, which is covered by:

  <http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=444103#15>

Git patch attached.  Test packages fixing #444103, #642012 and #642021
are available at:

  <http://people.debian.org/~gismo/tmp/gnupg2_2.0.18-3~gismo444103.642012.642021.1.dsc>

> Attached a Git patch to add the Enhances: above, including dbus-x11 for

The last sentence is independent of the gpg-agent stuff, can it be
included anyway?

Thx, bye,
Gismo / Luca

From 2125295b8c8348839ddf91db9637b1d97a3d3de2 Mon Sep 17 00:00:00 2001
From: Luca Capello <[email-masked]>
Date: Tue, 21 Feb 2012 12:55:28 +0100
Subject: [PATCH 2/3] debian/gnupg-agent.xsession: (#642012) fix
 90x11-common_ssh-agent

---
 debian/changelog            |    2 ++
 debian/gnupg-agent.xsession |   14 ++++++++++++++
 2 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/debian/changelog b/debian/changelog
index 37379b0..0321edf 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -4,6 +4,8 @@ gnupg2 (2.0.18-3) UNRELEASED; urgency=low
   * debian/gnupg-agent.xsession:
     + enable the SSH support according to $GNUPGHOME/gpg-agent.conf
       (Closes: #444103).
+    + fix behavior with /etc/X11/Xsession.d/90x11-common_ssh-agent
+      (Closes: #642012).
 
  --
 
diff --git a/debian/gnupg-agent.xsession b/debian/gnupg-agent.xsession
index d129884..fe2b275 100644
--- a/debian/gnupg-agent.xsession
+++ b/debian/gnupg-agent.xsession
@@ -14,10 +14,24 @@ if grep -qs '^[[:space:]]*use-agent' "$GNUPGHOME/gpg.conf" "$GNUPGHOME/options"
    # Invoking gpg-agent with no arguments exits successfully if the agent
    # is already running as pointed by $GPG_AGENT_INFO
    if ! $GPGAGENT 2>/dev/null; then
+       ## <http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=642012#22>
+       # if there is no gpg-agent running, then "$PID_FILE" comes from
+       # an old gpg-agent
+       if [ -w "$PID_FILE" ]; then
+           rm -rf "$PID_FILE"
+       fi
+       unset GPG_AGENT_INFO
+       unset SSH_AGENT_PID
+       unset SSH_AUTH_SOCK
+
        ## <http://bugs.debian.org/444103>
        # check if the SSH support should be enabled
        if grep -qs '^[[:space:]]*enable-ssh-support' "$GNUPGHOME/gpg-agent.conf"; then
           ENABLESSH='--enable-ssh-support'
+          ## <http://bugs.debian.org/642012>
+          # prevent /etc/X11/Xsession.d/90x11-common_ssh-agent to start
+          # ssh-agent, gpg-agent will take care of its functionality
+          SSH_AUTH_SOCK=true
        fi
 
        STARTUP="$GPGAGENT --daemon $ENABLESSH --sh --write-env-file=$PID_FILE $STARTUP"
-- 
1.7.8.3