Bug 134494 - multiple gconfd's per session
multiple gconfd's per session
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: xinitrc (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: X/OpenGL Maintenance List
:
Depends On:
Blocks: FC3Target 131589 149475
  Show dependency treegraph
 
Reported: 2004-10-03 22:30 EDT by Bill Nottingham
Modified: 2014-03-16 22:48 EDT (History)
16 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2005-03-21 12:15:39 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
A patch (2.91 KB, patch)
2004-10-19 23:11 EDT, Søren Sandmann Pedersen
no flags Details | Diff
Soeren's fix, updated (3.00 KB, text/plain)
2004-10-20 16:48 EDT, Bill Nottingham
no flags Details
the updated updated patch (3.02 KB, patch)
2004-10-20 17:45 EDT, Bill Nottingham
no flags Details | Diff
ssh-agent prefix to launching session (2.35 KB, patch)
2004-12-09 20:53 EST, Paul Iadonisi
no flags Details | Diff
second attempt at fixing lingering ssh-agent problem (2.59 KB, patch)
2005-01-10 08:39 EST, Paul Iadonisi
no flags Details | Diff

  None (edit)
Description Bill Nottingham 2004-10-03 22:30:49 EDT
On every startup it reads a proxy setting from system/http_proxy and
tries to use it.

There is no way for the user to change this without gconf-editor;
the 'Network Proxy' prefs have no effect.
Comment 1 Bryan W Clark 2004-10-04 16:04:46 EDT
the 'Network Proxy' pref should affect the keys under /system/http_proxy/*

What is the output of: gconftool-2 --get /system/http_proxy/use_http_proxy

Ephy should be reading that key to determine using the proxy or not.
Comment 2 Bill Nottingham 2004-10-04 16:33:01 EDT
I think I've found the problem.

Hat->Preferences->Network Proxy

does *not* edit the gconf proxy stuff correctly.

Running gnome-network-preferences by hand does.
Comment 3 Bill Nottingham 2004-10-04 23:56:09 EDT
Tracking this down some more, it appears to be because.

a) Gnome, when started from GDM, starts a gconfd in /tmp
b) terminal runs with login shell, which sets TMPDIR=~/tmp
c) gconfd starts again
d) fun with two gconfds ensues.
Comment 4 Bryan W Clark 2004-10-05 12:20:28 EDT
Ok, moving this to gconf and retitling.
Comment 5 Mark McLoughlin 2004-10-07 11:11:34 EDT
... and re-assigning
Comment 6 Mark McLoughlin 2004-10-07 12:20:05 EDT
Ew. Ick. What the heck is setting $TMPDIR=~/tmp ?

This isn't going to screw GConf - you're probably seeing two copies of
bonobo-activation.

We could do the $GCONF_TMPDIR thing that Frederic had a patch for:

  http://mail.gnome.org/archives/gconf-list/2003-October/msg00026.html

But that'll only fix it for GConf, not bonobo-activation etc.
Comment 7 Bill Nottingham 2004-10-07 12:40:47 EDT
~/.bashrc is setting TMPDIR. The question is why that isn't in the
environment when running from GDM.
Comment 8 Christopher Blizzard 2004-10-13 11:51:46 EDT
How does gnome deal with .PAC files for proxy settings?
Comment 9 Christopher Blizzard 2004-10-18 17:43:32 EDT
Sorry, that was supposed to be in another bug.  Don't remember which one.
Comment 10 Havoc Pennington 2004-10-18 18:22:15 EDT
Discussing with Bill, we aren't fans of ignoring TMPDIR; we should do
the root cause thing and track this down to xinitrc/gdm and why the 
env variable isn't set from the beginning.
Comment 11 Ray Strode [halfline] 2004-10-19 21:37:04 EDT
So the root problem is TMPDIR is automatically stripped from the environment
before execing setgid binaries, and ssh-agent is setgid.  The easiest fix (I
think) is to modify /etc/X11/xdm/Xsession to use the alternative method of
invoking ssh-agent, which is doing `eval ssh-agent -s` before the exec calls.  
Comment 12 Ray Strode [halfline] 2004-10-19 21:38:40 EDT
Reassigning to X maintainers
Comment 13 Bill Nottingham 2004-10-19 21:55:35 EDT
Well, you still need the login shell fix.
Comment 14 Søren Sandmann Pedersen 2004-10-19 23:11:26 EDT
Created attachment 105490 [details]
A patch

A patch implemeting this fix.

I don't have access to the upstream of xinitrc, and I'd prefer if Mike would
glance over it to if it is looks basically sane.
Comment 15 Søren Sandmann Pedersen 2004-10-19 23:13:55 EDT
Ok, the 

    eval `ssh-agent -s`

should be

    eval `$SSH_AGENT -s`

Comment 16 Bill Nottingham 2004-10-19 23:26:59 EDT
To get the TMPDIR, you need to change Xclients from:

#!/bin/bash

to 

#!/bin/bash -l
Comment 17 Ray Strode [halfline] 2004-10-19 23:37:51 EDT
Hi Bill,
note that /etc/X11/xdm/Xsession calls Xclients like so:

    exec -l $SHELL -c "$SSH_AGENT $DBUS_LAUNCH $HOME/.Xclients"

using #!/bin/bash -l is bad because it will source the
.bash_profile/.bashrc in users' homedirs even if they have their login
shell set to something else.  

Not sure how I NOTABUG'd this, meant to reassign.
Comment 18 Bill Nottingham 2004-10-19 23:43:58 EDT
True. However, setting the -l on Xclients actually avoids the need for
the ssh-agent hack. Either or, I suppose.
Comment 19 Mike A. Harris 2004-10-20 01:44:53 EDT
This type of change seems overly risky at this late in the
development cycle.  Last minute xinitrc changes wont get
adequate end user testing to know if they both fix the problems
*and* do not break anything.  Every time in the past that we
have changed xinitrc at the last second it has caused great
pain once the OS is released.

Also, every previous OS has shipped with this like it is now,
so I don't see how this can be a blocker bug.  IMHO, this is
a "would be nice to fix, but the risks involved to make these
changes at this point are too great and we wont get proper
testing".

We need to include Kevin in on the decision for this and discuss
the specific risks involved and wether the benefits outweigh
the risks or not.  My personal opinion currently is that this
can wait for FC4 development to open up, or if we really do
have to fix it, we need to have adequate testing by a large
number of people so we don't have things blow up in our faces
when FC3 ships.

Feedback appreciated.

TIA
Comment 20 Bill Nottingham 2004-10-20 02:01:16 EDT
ssh-agent wasn't previously setgid in any release, so the previous
code would have worked fine. It's only broken for FC3 and later.
Comment 21 Ray Strode [halfline] 2004-10-20 02:20:20 EDT
Hi Bill,

setting -l in Xclients isn't really an ideal solution because it means
that 
      1) two login shells are started so sh startup scripts are
gettings sources twice
      2) all the other exec code paths in /etc/X11/xdm/Xsession won't
be fixed and thus will have TMPDIR stripped from their environment
Comment 22 Mike A. Harris 2004-10-20 04:15:55 EDT
Bill:  Ah, ok.  I'll investigate the ssh thing tomorrow.
Comment 23 Mike A. Harris 2004-10-20 16:38:00 EDT
Related bugs for reference:

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=7860
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=20781
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=50678
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=56822
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=59001
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=59003

"Note that tcsh explicitly disallows the use of -l and -c together,
 and only the "-" as argv[0][0] method can make it happen.  Clearly
 it was not intended to be used this way."


After reviewing all the bugs in bugzilla that are similar, while
it wasn't immediately obvious, it seems this type of issue has
been bouncing around back and forth every couple of OS releases,
where we change the behaviour for one bug, creating one or more
new bugs that can only be fixed by changing it back, having
previously forgotten about why we changed it a couple OS releases
before that.  ;o)

So, if we are going to make these types of changes again right
now, we will be just flipping the switch for which bugs we want
to have.  I think that the "solutions" that are always presented
are just looking at the one particular issue being reported and
ignoring previously reported issues.  We really need a solution
that solves all cases, not just the one being reported at a
particular time.

I can't see an obvious fix that solves this problem and allows
people to use tcsh, with no side effects, but if someone else
has a solution they believe solves all of the above bugs without
creating a new one, please attach a unified diff with explanation
for review, and the X Devel Team will discuss it and try to come
up with a solution.

Thanks in advance.

Comment 24 Bill Nottingham 2004-10-20 16:48:03 EDT
Created attachment 105548 [details]
Soeren's fix, updated

Here's Soeren's patch. I was wrong, the adding of -l is not needed.

This is correct because it maintains the use of ssh-agent, withuot having it
clear the environment for all subprocesses. The other change from his previous
patch is not removing the lines in xinitrc that called ssh-agent/dbus; I
believe you still want to call dbus-launch here.
Comment 25 Søren Sandmann Pedersen 2004-10-20 17:24:27 EDT
About these:

-	exec $SSH_AGENT $DBUS_LAUNCH $HOME/.Xclients || \
+	exec $DBUS_LAUNCH $HOME/.Xclients || \
 	exec $DBUS_LAUNCH $HOME/.Xclients

I think you may have overlooked the "|| \" at the end of the line.
Basically what it does currently is 

if (!(ssh_agent dbus Xclients))
{ 
    dbus XClients
}

ie., if we don't have ssh_agent, try with dbus. 

With the eval `ssh-agent -s`, we don't have to care at that point
whether we have ssh-agent, so we just run "dbus Xclients". So
basically we do still call dbus-launch.
Comment 26 Bill Nottingham 2004-10-20 17:33:05 EDT
The previous version had:

 if [ -f $HOME/.Xclients ]; then
-	exec $SSH_AGENT $DBUS_LAUNCH $HOME/.Xclients || \
 	exec $DBUS_LAUNCH $HOME/.Xclients
 elif [ -f /etc/X11/xinit/Xclients ]; then
-	exec $SSH_AGENT $DBUS_LAUNCH /etc/X11/xinit/Xclients || \
 	exec /etc/X11/xinit/Xclients
 else

The first part (removing the line) makes sense... the second part I'm
not so sure about.
Comment 27 Søren Sandmann Pedersen 2004-10-20 17:39:51 EDT
Ah, right. So what we want is actually

 if [ -f $HOME/.Xclients ]; then
-	exec $SSH_AGENT $DBUS_LAUNCH $HOME/.Xclients || \
-	exec $DBUS_LAUNCH $HOME/.Xclients
+	exec $DBUS_LAUNCH $HOME/.Xclients || \
+	exec $HOME/.Xclients
 elif [ -f /etc/X11/xinit/Xclients ]; then
-	exec $SSH_AGENT $DBUS_LAUNCH /etc/X11/xinit/Xclients || \
+	exec $DBUS_LAUNCH /etc/X11/xinit/Xclients || \
 	exec /etc/X11/xinit/Xclients

right?
Comment 28 Bill Nottingham 2004-10-20 17:45:34 EDT
Created attachment 105552 [details]
the updated updated patch

Yeah, that looks right.
Comment 29 Mike A. Harris 2004-10-20 18:12:54 EDT
Ok, this patch looks pretty good to me, and also reviewed by a
couple others who think it looks right too.  I'm committing
it to the next xinitrc build.
Comment 30 Mike A. Harris 2004-10-20 18:23:42 EDT
Ok, upon applying the patch, I noticed the code can be further
cleaned up and some redundancy removed.  The SSH_AGENT variable
is not used anywhere anymore except the one code block, and the
two sections can be combined and simplified.


SSH_AGENT=
[ -x /usr/bin/ssh-agent -a -z "$SSH_AGENT_PID" ] &&
SSH_AGENT="/usr/bin/ssh-agent"
if ! [ -z $SSH_AGENT ] ; then
    eval `$SSH_AGENT -s`
fi


Becomes simply:

# Start up ssh-agent if available and not already running.
[ -x /usr/bin/ssh-agent -a -z "$SSH_AGENT_PID" ] && eval
$(/usr/bin/ssh-agent -s)

I'm committing this fix.
Comment 31 Mike A. Harris 2004-10-20 18:52:21 EDT
xinitrc-4.0.14-1 contains the above fixes and is built internally.
Please test and provide feedback, and change status to
RAWHIDE or ASSIGNED.

TIA
Comment 32 Bill Nottingham 2004-10-20 18:55:39 EDT
Fixes the issue in my test environment - looks good!
Comment 33 Sitsofe Wheeler 2004-12-09 13:53:13 EST
I think the changes here have given rise to bug #138747 / bug #139372 ...
Comment 34 Paul Iadonisi 2004-12-09 20:53:51 EST
Created attachment 108291 [details]
ssh-agent prefix to launching session

I've commented on bug 139372 with a proposed fix for yet another bug (to add to
the list in comment 23 of this bug) that has been introduced by these
pull-this-way/push-that-way xinitrc changes (i.e.: fix one thing, break
another).  It I think will at least allow the fixes implemented in this bug to
be preserved and also fix the ssh-agent-hanging-around bug.

I will attach a patch to xinitrc, but I'll warn first that it re-introduces the
SSH_AGENT variable.

I looked at the list of bugs in Mike Harris' comment 23 of this bug and
couldn't see any way this would re-introduce any of them, but I am open to
correction.  It's not proactively fixing any of those bug either, but it allows
TMPDIR to still be propogated to gnome-session (or whatever) and dbus-launch
(if necessary?) without leaving stray ssh-agent processes around, which, though
maybe not major, is a bit of a security concern.  (Using ssh-agent on a
multi-user box is a security concern in itself [potentially], but that's a
whole other discussion.)
Comment 35 Paul Iadonisi 2004-12-10 01:10:41 EST
Comment on attachment 108291 [details]
ssh-agent prefix to launching session

skip that patch.  That'll teach me to post a patch before trying it out. 
Session seems to hang and I never get my desktop when I login.	This is
trickier than Hmm.  I thought.
Comment 36 Derek Poon 2004-12-10 12:21:52 EST
Now ssh-agent doesn't terminate when the user logs out.  When
previously doing
  /usr/bin/ssh-agent command
ssh-agent would automatically die when command terminated.

There ought to be a

  if [ -n "$SSH_AGENT_PID" ]; then
      kill "$SSH_AGENT_PID"
  fi

during the logout sequence, either in /etc/X11/xdm/Xsession or a
subshell thereof.  I think this means removing the exec's from
/etc/X11/xdm/Xsession.
Comment 37 Paul Iadonisi 2005-01-10 08:39:05 EST
Created attachment 109553 [details]
second attempt at fixing lingering ssh-agent problem

Okay, this one's tested.  Again, this doesn't proactively fix the bugs Mike
Harris listed, but I don't think it re-introduces any of them that have been
fixed.	Anyone else want to try it, please?  I'm using it without any obvious
problems.
Comment 38 Sitsofe Wheeler 2005-01-14 14:54:43 EST
This patch looks good to me. ssh-agent is started, if TMPDIR is set in ~/.bashrc
then it is set within a non-login gnome-terminal (if not then it doesn't exist),
SSH_AGENT_PID points to a correct ssh-agent and ssh-add appears to work. After
logging out ssh-agent is gone. Also if X crashes or is killed then ssh-agent
still quits.

Please roll this out.
Comment 39 Ian Mortimer 2005-02-16 17:42:59 EST
There's still something odd about /etc/X11/xdm/Xsession (and some
redundancy).  From the default Xsession for Fedora 3 this bit of code:

   42   # now, we see if xdm/gdm/kdm has asked for a specific environment
   43   case $# in
   44   1)
   45       if [ -x "$SWITCHDESKPATH/Xclients.$1" ]; then
   46          exec -l $SHELL -c "$SWITCHDESKPATH/Xclients.$1";
   47       fi;

short-circuits many of the cases below it because
/usr/share/switchdesk has an Xclients file for gnome, kde and twm (as
well as a lot of other window managers you might install).

In fact gnome is started with gnome-session and kde with startkde so
they don't match either that condition or the specific cases below it
but fall through to the default case:

   63       *)
   64          # GDM provies either a command line as the first
argument or
   65          # provides 'failsafe', 'default' or 'custom'.  KDM will
do the
   66          # same at some point
   67          if [ "$1" != "default" -a "$1" != "custom" ]; then
   68              exec -l $SHELL -c "$DBUS_LAUNCH $1"
   69          fi

I'm wondering if the first bit of code should also be patched to
ensure that all Window managers get started properly no matter how
they're selected:

   42   # now, we see if xdm/gdm/kdm has asked for a specific environment
   43   case $# in
   44   1)
   45       if [ -x "$SWITCHDESKPATH/Xclients.$1" ]; then
   46          exec -l $SHELL -c "$SSH_AGENT $DBUS_LAUNCH
$SWITCHDESKPATH/Xclients.$1";
   47       fi;

Comment 40 Tomas Mraz 2005-02-25 03:54:34 EST
Could bug 149475 be possibly related to the change introduced in this bug?
Comment 41 Mike A. Harris 2005-03-21 12:15:39 EST
Someone just brought this bug to my attention, indicating there are
patches here that haven't been applied and asking why.

There are patches here that haven't been applied, because the bug
is in a "CLOSED" state, so we'll never see them until someone reopens
the bug by setting the status to "ASSIGNED", or in this case, until
someone points the problem out in a different bug report.   ;o)

To avoid future confusion, please remember "MODIFIED" state means "We
believe this issue is fixed, but if it is not, set the bug back to
ASSIGNED and we will re-investigate, otherwise we will assume it is
indeed fixed and 'CLOSED'

The problem reporter(s) in the given bug, are to test the fix, and if
the problem is solved, to change the bug to "RAWHIDE" or whatever is
appropriate for the given issue.  If the bug still exists, indicating
the fix did not work properly, the people having the problem need to
change the bug state back to "ASSIGNED", effectively reopening the issue.

Now that someone has brought the patch from comment #37 to my attention,
I am applying it to the next xinitrc build.  Hopefully it solves the
problem and does not reintroduce any new ones.

Setting status to "RAWHIDE".
Comment 42 Gerald Teschl 2005-04-25 07:04:35 EDT
This also solves the ssh-agent, gconf issue here (and shows no side effects;-). I only see
some gam_server processes will keep running even after a user logs out from x11.
Please roll this out for rhel 4 as well.
Comment 48 Klaus Ethgen 2006-05-11 08:54:18 EDT
Hmm... The fix of this bug is just opening another with more problems. (See
also 148945, and this not only on fedora as also on EL4.)

There are two possible solutions I can imagine:
1. Just remove the sgid from ssh-agent (I see no reason why it have the bit)
or
2. Change the way dbus and ssh-agent is started (exec $DBUS_LAUNCH $SSH_AGENT
   ...)

The way ssh-agent is started in WS4 is not acceptable!

Note You need to log in before you can comment on or make changes to this bug.