[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

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



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

On Mon, 26 Sep 2011 16:36:52 +0200, Luca Capello wrote:
> 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>

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

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

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

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.

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

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>

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

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 <luca@pca.it>
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

Attachment: pgpOnLJrHY32z.pgp
Description: PGP signature


Reply to: