Rewrite the ssh-agent module; rename it to ssh #425

Closed
sorin-ionescu opened this Issue May 1, 2013 · 76 comments

5 participants

@sorin-ionescu

I am rewriting the ssh-agent module. I need volunteers to test it on Linux since I can not test it on Mac OS X due to custom Apple SSH implementation.

@cordarei

I can test it on Ubuntu and Arch Linux (no X server). I made some changes myself to work better with Ubuntu and other systems that start X with ssh-agent; you can see my version here if you want: https://github.com/cordarei/prezto/blob/master/modules/ssh-agent/init.zsh .

@sorin-ionescu

Thanks, @cordarei.

I invite @nasenatmer and @ColinHebert into this issue.

I have pushed a WIP to branch module/ssh.

The module is disabled on Mac OS X because Apple provide special SSH support via launchd.

Is loading custom identities needed? ssh-add with no arguments adds default ones.

The check for an existing ssh-agent is not a adequate. gpg-agent with SSH support can replace ssh-agent; it could also be argued that it's a better agent overall. On Mac OS X, ssh-agent is not running until a program communicates with $SSH_AUTH_SOCK, which is set to /tmp/launch-<random>/Listeners.

It may be better to check for a valid $SSH_AUTH_SOCK instead of checking for agent PIDs.

Lastly, does the persistent SSH authentication socket for screen/tmux work?

@cordarei

I don't use the custom identities functionality. Is there anyone who does?

I also don't use the ssh forwarding option; I'm not actually sure what the persistent socket is supposed to do, in fact.

@nasenatmer

Preface: I'm not using ssh-agent personally and would probably prefer gpg-agent instead, but I don't think it hurts to have it in prezto.

Looking at the branch, I think multiple identities don't hurt so much as to be left out, although @cordarei is probably right in assuming that there's not many use cases for them… but certainly, for some people it might be nice to have multiple keys for different purposes…

@sorin-ionescu : Yes, checking for $SSH_AUTH_SOCK makes more sense than to check for the agent PID.

@nasenatmer

I merged the ssh branch and activated ssh in my zpreztorc, but when starting a new zsh, I get the following:

jakob ❯ zsh                                                                                                                                   ⏎ ~/.ssh
Identity added: /home/jakob/.ssh/id_rsa (/home/jakob/.ssh/id_rsa)
/home/jakob/.zprezto/modules/ssh/init.zsh:export:35: not valid in this context: /tmp/ssh-eiU9qJ9tb9xi/agent.10641

print $SSH_AUTH_SOCK && print $_ssh_agent_persistent_auth_sock yields

/tmp/ssh-eiU9qJ9tb9xi/agent.10641
/tmp/ssh-agent-jakob-screen

and ls -l /tmp/ssh-agent-jakob-screen: lrwxrwxrwx 1 jakob users 33 May 3 16:58 /tmp/ssh-agent-jakob-screen -> /tmp/ssh-eiU9qJ9tb9xi/agent.10641

@sorin-ionescu

I fixed line 35: 5632b4f.

@sorin-ionescu

Is the identity added line output to stdin or stderr? Perhaps it's possible to silent it if it's on stderr.

@sorin-ionescu

I can't silence stdin because it may prompt the user for a password.

@nasenatmer

@sorin-ionescu seems to be stderr:

jakob ssh-agent ❯ ssh-add 2>/dev/null                                                                                                   ⏎ ✱ ~/.z/m/ssh
Enter passphrase for /home/jakob/.ssh/id_rsa:
jakob ssh-agent ❯

However, ssh-add prompts the user for the password each time they spawn a new tty. Interestingly, once I've supplied it with the password, I can just press Enter to get rid of it, the identity is still there in ssh-add -l

@sorin-ionescu

Try 8dec5ad.

@sorin-ionescu

If that doesn't work, try 40e219b.

@nasenatmer

How can I cherry-pick upstream commits?

@sorin-ionescu

git cherry-pick SHA-1

@cordarei

It doesn't work in the case where ssh-agent is started with the X session; there should be a check before sourcing the environment file whether the SSH_AGENT_PID (or SSH_AUTH_SOCK) exists, and in that case it needs to call ssh-add (it should probably check whether the keys have been added or not as well).

@nasenatmer

@sorin-ionescu sorry, it was late yesterday, could have guessed that with fetching first, I could just cherry-pick as usual…

Back to topic: I've applied 8dec5ad, killed ssh-agent and first thing coming up on starting a new shell is:

Could not open a connection to your authentication agent.

However, pgrep ssh-agent returns the same PID as $SSH_AGENT_PID. The agent has no identities loaded, however. (yes, my default identities exist and are for instance used by git to push my commits to github).

Generally, if I execute ssh-add once, add my identities and then open new instances of zsh in tmux, I don't get any questions anymore about passwords for my identities.


With 40e219b, I get the following after starting a new terminal:

sed: -e expression #1, char 21: Invalid character class name
Could not open a connection to your authentication agent.

pgrep ssh-agent yields a PID, but none of the SSH_* variables is set.

@sorin-ionescu

@cordarei Yes, I have said as much. One should check if SSH_AUTH_SOCK is valid. Do you know how? I don't mean to just check for the existence of the socket, but whether one is cable to communicate with said socket.

@nasenatmer So, I guess, GNU sed doesn't support that expression.

@sorin-ionescu

@cordarei You're right, it assumes that ssh-add was called by whomever started the agent. It's possible to get a list of loaded identities with ssh-add -l. Why doesn't the X add identities?

@sorin-ionescu

@nasenatmer 7432d61 should fix the sed problem.

@nasenatmer

@sorin-ionescu yep, works nicely now! Only one password request, no sed errors.

@cordarei

@sorin-ionescu I use Xubuntu, which doesn't have as complete a default configuration as Gnome, and I haven't had a reason to set up a graphical passphrase entry because I only use ssh from the terminal.

From some searching, it seems there is no good way to tell if a socket is valid or not: http://stackoverflow.com/questions/7405932/how-to-know-whether-any-process-is-bound-to-a-unix-domain-socket
but keychain seems to just check if the file exists: https://github.com/funtoo/keychain/blob/master/keychain.sh#L379

I think this mostly makes sense because the only way the SSH_AUTH_SOCK variable is going to be outdated is if we save the value in $_ssh_agent_env and then source it after a reboot without checking. I think if the SSH_AUTH_SOCK file exists we can assume that it is the correct socket for the current ssh-agent process (I think this should work on OSX >Leopard as well?), and either avoid sourcing $_ssh_agent_env or even rm it ; otherwise we can use $_ssh_agent_env as usual.

@sorin-ionescu

I see. What's SSH2?

I'm thinking that it might be useful to replace the ssh module and the gpg module with keychain.

@sorin-ionescu

8439402 now looks at SSH_AUTH_SOCK.

This doesn't solve the non-loaded identities problem.

I could loop over the output of ssh-add -l and check for each identity or (($(ssh-add -l | wc -l) > 0)) to check for any identity.

@sorin-ionescu

I have made a ssh identies check in 0b4522b. Unfortunately, I cannot
count since, at least on OS X, ssh-add doesn't use stderr properly;
so, there is never wc -l count of 0.

@nasenatmer

@sorin-ionescu I see where your reasoning comes from (suggesting replacing it all with keychain) but shouldn't that be the decision of the users? Open Source Software is also about choice and no matter people prefer to have gpg-agent with or without ssh support, only ssh-agent (cause they don't need gpg functionalities) or keychain, I think they should decide themselves…

What do you mean with "This doesn't solve the non-loaded identities problem."? Here, as I said, it worked out already (starting ssh-add only during initialisation of ssh-agent).

Oh, but check for identities (0b4522b) is also quite nice.

@sorin-ionescu

@nasenatmer keychain takes care of everything better than I ever I will though these modules that I don't use.

See @cordarei's comments about the non-loaded identities problem.

@nasenatmer

Alright, I've introduced a check for enable-ssh-support in module gpg and copied the check for loaded identities from here, see d075cc9dc9c1e9a4b20dd33ce8aaf6b93c760778. As soon as module/ssh merges into master, I'll open a pull request.

@sorin-ionescu
@nasenatmer

Swish! I think it is necessary in that case, however, to export the SSH relevant variables to $_ssh_agent_env before pmodload ssh so that the ssh module can properly deal with them. I did that in 615c4c7978f36d86d39f10ee6a5a8defe6b76450 . Hm, diff not available says github. So then, check Lines 21-25.

@nasenatmer

The variables have to be source before pmodload ssh as in e8ec2a573081e6d387d9c2ffdd36747f0b23b53b.

@nasenatmer

@sorin-ionescu I like 3fd807f! Bien hecho!

@sorin-ionescu

It works?

@nasenatmer
  • only one process of gpg-agent running.
  • variables have same values as actually running processes.
  • variables are properly exported.
  • haven't checked ssh module with the new setup. Tomorrow.
@ColinHebert

Okay I'm a bit late to the party but from what I understand there is no reason to use both the gpg-agent with ssh and the ssh-agent.

The check if [[ ! -S "$SSH_AUTH_SOCK" ]]; then is a bit useless then. If SSH_AUTH_SOCK is already set there is nothing to do really (it's already set, what else could be done?)

On the other hand if it isn't set (this means that gpg-agent isn't started or doesn't support ssh) ssh-agent is started, but what happen if I have window A with ssh-agent started and I open window B, SSH_AUTH_SOCK isn't set yet and window B will now try to start ssh-agent.

Or did I misunderstand something?

@sorin-ionescu

@ColinHebert There are two modules called GPG and SSH.

Presently, they launch their respective agents. In addition to launching its agent, the SSH module also loads identities into an agent, any agent that implements the OpenSSH Agent protocol.

When gpg-agent with OpenSSH Agent protocol emulation is loaded, it has no identities to serve, and they must be added. Instead of duplicating code from the SSH module, the GPG module loads the SSH module. Since $SSH_AUTH_SOCK is detected, into won't start ssh-agent; it will only load identities via ssh-add.

Since agents are reused, starting new TTYs will not result in duplicate agents.

@nasenatmer
  • ssh module alone also works as expected.
@nasenatmer

Hm, I just saw that $_ssh_agent_env proper in the current version doesn't hold the correct variables since it is linked to _gpg_agent_info. But the variables are exported, so I guess that's enough then? $_ssh_agent_env isn't a default location anyways…

@nasenatmer

I was airing my surprise at discovering that the output of print $SSH_AUTH_SOCK and cat ~/.ssh/environment-$HOST did not match. But I guess as long as the variables are exported properly and ssh is only pmodloaded and not standing on its own feet it's not a real problem?

@sorin-ionescu

Why should it match? It's no longer relevant when gpg-agent is the acting agent. Unless you can think of interoperability problems with other shells or programs, I don't think this is a major issue.

@nasenatmer

Yeah, it's just some aesthetic feeling that urges me. Probably not rational.

@sorin-ionescu

Instead of setting $_ssh_agent_env in the GPG module, we could symlink the .gpg-agent-info file.

@nasenatmer

Oh no. Imagine somebody switching back to ssh-agent who would then overwrite .gpg-agent-info. I guess in that case we've reached an optimum consensus, my autistic urge for the perfectly clean solution be better not always fulfilled.

@sorin-ionescu

No, in the GPG module, we could do the following:

rm -f "$_ssh_agent_env"
ln -s "$_gpg_agent_info" "$_ssh_agent_env"
pmodload 'ssh'
@nasenatmer

Haha, what did I start there? I don't think that's really necessary as ~/.ssh/environment-$HOST isn't a default file anyway. And I don't see the benefit of symlinking over a simple grep SSH $_gpg_agent_info >! $_ssh_agent_env for that matter.

@sorin-ionescu

What is the default location of that file?

ln -sf "$_gpg_agent_info" "$_ssh_agent_env" is simpler.

@nasenatmer

I think there is no default environment file (unlike as for gpg-agent), or at least I can't find it in the manual. Another reason to leave it as it is now.

@sorin-ionescu

Man pages are not identical throughout the UNIX ecosystem. Some are barebones; others are detailed. Keep looking.

@nasenatmer

Nope, not much variation in regards to ssh-agent mans and I couldn't find any man that prescribes a default location. Also, ssh-agent doesn't have an option to write down an env file.

@sorin-ionescu

I was speaking of something like fb31669.

@nasenatmer

In Line 32 of gpg/init.zsh replace $_gpg_agent_conf with $_gpg_agent_info. Otherwise, I'm still not convinced that this solution is in any way more elegant.

@sorin-ionescu
@nasenatmer

I think I made it clear several times that I didn't insist on that "feature" and I do it once more and for the last time now.

@nasenatmer

Hm, last reboot gave me the error the ln couln't symlink since file ~/.ssh/environment-$HOST existed already, this time I get the following: ln: cannot remove ‘/home/jakob/.ssh/environment-x220’: No such file or directory Although it exists…

@nasenatmer

This happens since I've got a session started in tmux which spawns quite a few ttys in a short time, so that the ln in one has just deleted ~/.ssh/environment-$HOST when the other ln stats it. So either copying or grepping might then be the solution which will certainly produce less error output.

@sorin-ionescu

Hmm, it seems that there are race conditions with using a symlink. We'll go back to using the variable.

That said, I have found another problem. If ssh-agent is not started by Prezto, there is no environment file to source.

@nasenatmer

So that means? grep 'SSH' "$_gpg_agent_info" >! "$_ssh_agent_env"?

@sorin-ionescu

No. Whomever started it may not have written an environment file at whatever location. Sometimes these agents are started as gpg-agent ssh-agent $SHELL. The variables are inherited but there is no file.

@nasenatmer

Alright, so that leaves us at a point where we go back to the behaviour from 3fd807f where we just set _ssh_agent_env="$_gpg_agent_info"? Currently, the module is broken and there's not much missing, just a decision basically, to make it work again.

@nasenatmer

Nice!

@gmaghera gmaghera added a commit to gmaghera/prezto that referenced this issue May 19, 2013
@gmaghera gmaghera Merge remote-tracking branch 'upstream/master'
* upstream/master:
  Make gpg-agent and ssh-agent work with each other
  [Fix #425] Rewrite module ssh-agent; rename it to ssh
  [Fix #103] Add documentation for editor
  Remove the git-info SIGINT message
  [Fix #307] Do not auto-off git-info
  Remove ununsed variable
  Clarify Git listing aliases descriptions
  Swap aliases gsd and gsL
  Rename alias gRc to gRp
  [Fix #221] Add a simple git-info
  [#221] Do not format undefined zstyles
  Initialize ahead and behind local variables
  Add rar command to archive module
  Refactor Emacs module
  Load completion for Carton
ac72c9a
@admk admk added a commit to admk/prezto that referenced this issue May 20, 2013
@admk admk Merge branch 'master' of https://github.com/sorin-ionescu/prezto into…
… HEAD

* 'master' of https://github.com/sorin-ionescu/prezto: (35 commits)
  Make gpg-agent and ssh-agent work with each other
  [Fix #425] Rewrite module ssh-agent; rename it to ssh
  [Fix #103] Add documentation for editor
  Remove the git-info SIGINT message
  [Fix #307] Do not auto-off git-info
  Remove ununsed variable
  Clarify Git listing aliases descriptions
  Swap aliases gsd and gsL
  Rename alias gRc to gRp
  [Fix #221] Add a simple git-info
  [#221] Do not format undefined zstyles
  Initialize ahead and behind local variables
  Add rar command to archive module
  Refactor Emacs module
  Load completion for Carton
  Correct syntax error in variable assignment
  Ensure that the tmux server is started
  [Fix #426] Correct syntax error in variable assignment
  [Fix #419] Rewrite module gpg-agent; rename it to gpg
  [Fix #52] Add zstyles to configure history-substring-search
  ...
a19cdee
@gudleik gudleik added a commit that referenced this issue May 23, 2013
@gudleik gudleik Merge remote-tracking branch 'upstream/master'
* upstream/master:
  Make gpg-agent and ssh-agent work with each other
  [Fix #425] Rewrite module ssh-agent; rename it to ssh
  [Fix #103] Add documentation for editor
  Remove the git-info SIGINT message
  [Fix #307] Do not auto-off git-info
  Remove ununsed variable
  Clarify Git listing aliases descriptions
  Swap aliases gsd and gsL
  Rename alias gRc to gRp
  [Fix #221] Add a simple git-info
  [#221] Do not format undefined zstyles
  Initialize ahead and behind local variables
  Add rar command to archive module
  Refactor Emacs module
  Load completion for Carton

Conflicts:
	runcoms/zpreztorc
b5a1d0a
@stefanfrede stefanfrede pushed a commit that referenced this issue May 25, 2013
Stefan Frede Merge remote-tracking branch 'upstream/master'
* upstream/master:
  [Fix #436] Remove Bombich rsync references
  Add the RubyGems bin directory to PATH on other Unix systems
  Do not substitute /tmp since $TMPDIR is always set
  [Fix #437] Always set $TMPDIR
  Make gpg-agent and ssh-agent work with each other
  [Fix #425] Rewrite module ssh-agent; rename it to ssh
  [Fix #103] Add documentation for editor
  Remove the git-info SIGINT message
  [Fix #307] Do not auto-off git-info
  Remove ununsed variable
  Clarify Git listing aliases descriptions
  Swap aliases gsd and gsL
  Rename alias gRc to gRp
  [Fix #221] Add a simple git-info
  [#221] Do not format undefined zstyles
  Initialize ahead and behind local variables
  Add rar command to archive module
  Refactor Emacs module
  Load completion for Carton

Conflicts:
	runcoms/zpreztorc
5258a51
@trongrg trongrg added a commit to trongrg/prezto that referenced this issue May 27, 2013
@trongrg trongrg Merge remote-tracking branch 'upstream/master'
* upstream/master: (39 commits)
  [Fix #436] Remove Bombich rsync references
  Add the RubyGems bin directory to PATH on other Unix systems
  Do not substitute /tmp since $TMPDIR is always set
  [Fix #437] Always set $TMPDIR
  Make gpg-agent and ssh-agent work with each other
  [Fix #425] Rewrite module ssh-agent; rename it to ssh
  [Fix #103] Add documentation for editor
  Remove the git-info SIGINT message
  [Fix #307] Do not auto-off git-info
  Remove ununsed variable
  Clarify Git listing aliases descriptions
  Swap aliases gsd and gsL
  Rename alias gRc to gRp
  [Fix #221] Add a simple git-info
  [#221] Do not format undefined zstyles
  Initialize ahead and behind local variables
  Add rar command to archive module
  Refactor Emacs module
  Load completion for Carton
  Correct syntax error in variable assignment
  ...
8e4a2f3
@adamrights adamrights added a commit to adamrights/prezto that referenced this issue Jun 7, 2013
@adamrights adamrights Merge branch 'master' of https://github.com/sorin-ionescu/prezto
* 'master' of https://github.com/sorin-ionescu/prezto: (42 commits)
  Rename archive module functions
  [Fix #436] Update link to Bombich rsync
  Revert "[Fix #436] Remove Bombich rsync references"
  [Fix #436] Remove Bombich rsync references
  Add the RubyGems bin directory to PATH on other Unix systems
  Do not substitute /tmp since $TMPDIR is always set
  [Fix #437] Always set $TMPDIR
  Make gpg-agent and ssh-agent work with each other
  [Fix #425] Rewrite module ssh-agent; rename it to ssh
  [Fix #103] Add documentation for editor
  Remove the git-info SIGINT message
  [Fix #307] Do not auto-off git-info
  Remove ununsed variable
  Clarify Git listing aliases descriptions
  Swap aliases gsd and gsL
  Rename alias gRc to gRp
  [Fix #221] Add a simple git-info
  [#221] Do not format undefined zstyles
  Initialize ahead and behind local variables
  Add rar command to archive module
  ...

Conflicts:
	runcoms/zpreztorc
c5bd540
@douglasdrumond douglasdrumond added a commit to douglasdrumond/prezto that referenced this issue Jun 24, 2013
@douglasdrumond douglasdrumond Merge branch 'master' of github.com:eee19/prezto
* 'master' of github.com:eee19/prezto: (28 commits)
  Rename archive module functions
  [Fix #436] Update link to Bombich rsync
  Revert "[Fix #436] Remove Bombich rsync references"
  [Fix #436] Remove Bombich rsync references
  Add the RubyGems bin directory to PATH on other Unix systems
  Do not substitute /tmp since $TMPDIR is always set
  [Fix #437] Always set $TMPDIR
  Make gpg-agent and ssh-agent work with each other
  [Fix #425] Rewrite module ssh-agent; rename it to ssh
  [Fix #103] Add documentation for editor
  Remove the git-info SIGINT message
  [Fix #307] Do not auto-off git-info
  Remove ununsed variable
  Clarify Git listing aliases descriptions
  Swap aliases gsd and gsL
  Rename alias gRc to gRp
  [Fix #221] Add a simple git-info
  [#221] Do not format undefined zstyles
  Initialize ahead and behind local variables
  Add rar command to archive module
  ...
7fe0563
@agrimaldi agrimaldi added a commit that referenced this issue Jul 11, 2013
@agrimaldi agrimaldi Merge remote-tracking branch 'upstream/master'
* upstream/master: (52 commits)
  Rename archive module functions
  [Fix #436] Update link to Bombich rsync
  Revert "[Fix #436] Remove Bombich rsync references"
  [Fix #436] Remove Bombich rsync references
  Add the RubyGems bin directory to PATH on other Unix systems
  Do not substitute /tmp since $TMPDIR is always set
  [Fix #437] Always set $TMPDIR
  Make gpg-agent and ssh-agent work with each other
  [Fix #425] Rewrite module ssh-agent; rename it to ssh
  [Fix #103] Add documentation for editor
  Remove the git-info SIGINT message
  [Fix #307] Do not auto-off git-info
  Remove ununsed variable
  Clarify Git listing aliases descriptions
  Swap aliases gsd and gsL
  Rename alias gRc to gRp
  [Fix #221] Add a simple git-info
  [#221] Do not format undefined zstyles
  Initialize ahead and behind local variables
  Add rar command to archive module
  ...

Conflicts:
	runcoms/zpreztorc
c0f4127
@mfussenegger

Does this actually work for anybody correctly? I had to adjust it a bit, see mfussenegger/prezto@f5b868a

@nasenatmer

It works correctly here, yes. However, I'm using gpg module which pmodloads ssh.

@sorin-ionescu

Does 9be7481 fix it?

@nasenatmer

Works as well, here, but here it's probably up to @mfussenegger to check his context.

@nasenatmer

However, today I got the following warning in a tmux session:
ln: cannot remove ‘/tmp/jakob/ssh-agent.sock’: No such file or directory

To be more precise, I don't automatically start tmux whenever a terminal window opens, but I invoke tmux with tmux attach and have a set of panes being created, of which one shows above errors.

@sorin-ionescu

@nasenatmer That's what it is supposed to prevent. An if statement is probably failing. SSH behaves differently on Mac OS X. So, I have to guess how it should work on Linux.

@mfussenegger

Same issue for me with 9be7481 opening another terminal spawns a new ssh-agent and I've to enter the passphrase again.

@sorin-ionescu

I don't understand why it's doing that. There is something wrong with L23-L34. I can't fix it on a Mac since SSH on a Mac behaves differently. I don't get multiple ssh-agent instances.

@nasenatmer

I'm a bit speechless. When I killed gpg-agent and re-ran tmux attach, I suddenly hat several /tmp/gpg-XYZ dirs and multiple instances of gpg-agent. After logging out and in again, however, this time it works fine.

@sorin-ionescu

Race condition?

@jeffknupp jeffknupp pushed a commit to jeffknupp/prezto that referenced this issue Oct 15, 2013
@sorin-ionescu [Fix #425] Rewrite module ssh-agent; rename it to ssh 9767a0c
@linuslundahl linuslundahl added a commit to linuslundahl/prezto that referenced this issue Oct 17, 2013
@sorin-ionescu [Fix #425] Rewrite module ssh-agent; rename it to ssh b17df1c
@zeroasterisk zeroasterisk added a commit to zeroasterisk/prezto that referenced this issue Oct 22, 2013
@sorin-ionescu [Fix #425] Rewrite module ssh-agent; rename it to ssh 03c1986
@nasenatmer

This error still exists. So although it only happens if ssh is pmodloaded via gpg, it is a bug here in ssh since it started ocurring due to the changes in ssh module and was not there in previous versions. What would you need to debug this?

Note: This only happens at the start (though not every time) of a tmux session when the window is split and new panes are created quite fastly after each other and these lines here are probably executed in parallel (I don't know he exact terminology but sounds like a race condition?):

# Create a persistent SSH authentication socket.
if [[ -S "$SSH_AUTH_SOCK" && "$SSH_AUTH_SOCK" != "$_ssh_agent_sock" ]]; then
  ln -sf "$SSH_AUTH_SOCK" "$_ssh_agent_sock"
  export SSH_AUTH_SOCK="$_ssh_agent_sock"
fi

What I mean is such that while the one pane is executing ln -sf … and has just deleted the old symlink, the next one can't remove it. May it be better to do the if loop the other way round? I.e.

if [[ -S "$SSH_AUTH_SOCK" && "$SSH_AUTH_SOCK" = "$_ssh_agent_sock" ]]; then
else
  ln -sf "$SSH_AUTH_SOCK" "$_ssh_agent_sock"
  export SSH_AUTH_SOCK="$_ssh_agent_sock"
fi

(I don't know whether it's possible to put else dircetly afer then) or would that produce the same error?

@sorin-ionescu

If there is a race condition, there is not much I can do about it other than telling you to set up the environment before you start tmux. This code should only run once via zprofile not interactively via zshrc, but I have no clean solution as of yet.

@nasenatmer

I don't understand. gpg (and thereby implicitly ssh) is loaded due to its presence in .zpreztorc in the zstyle :prezto:load' pmodule part. What does that have to do with .zprofile?

And actually, the environment should be set up before I start the tmux session as I don't start it with xfce4-terminal automatically, but first start my terminal and then manually enter tmux attach. So there, it shouldn't reiterate through the whole ssh module any more any ways, no?

But it does: if I add the following line after line 34

echo "ln is being executed at this stage"

I see this message in every pane that is created by tmux, so there must be something wrong with the if condition.

@nasenatmer

Debugging this further, if I add the following into the above mentioned code block:

# Create a persistent SSH authentication socket.
if [[ -S "$SSH_AUTH_SOCK" && "$SSH_AUTH_SOCK" != "$_ssh_agent_sock" ]]; then
    print $SSH_AUTH_SOCK
    print $_ssh_agent_sock
  ln -sf "$SSH_AUTH_SOCK" "$_ssh_agent_sock"
  export SSH_AUTH_SOCK="$_ssh_agent_sock"
fi

I get the following output when starting a session or terminal:

/tmp/gpg-lCFUr1/S.gpg-agent.ssh
/tmp/jakob/ssh-agent.sock

First line is empty, thus $SSH_AUTH_SOCK is not properly exported. Now where should that have happened again? Update: I mispelt the variable in the above code example, and when I correct it, I get the correct variable.

But again, as you said: The ssh module shouldn't be run with every new shell shouldn't it?

@nasenatmer

How about adding a check in L30 of gpg module whether a link exists between $SSH_AUTH_SOCK and $_ssh_agent_sock or so and if not, then pmodload ssh?

@sorin-ionescu

The link can be stale; so, no.

@nasenatmer

Well, then duplicate L34 of ssh module before L30 of gpg and thus save the whole pmodload ssh procedure in case it's not needed.

if [[ -S "$SSH_AUTH_SOCK" && "$SSH_AUTH_SOCK" != "$_ssh_agent_sock" ]]; then

Granted, code redundancy is not nice, but having shell going through the whole ssh module and even having a race condition in there is not really more elegant.

@lildude lildude pushed a commit to lildude/prezto that referenced this issue Jan 12, 2014
@sorin-ionescu [Fix #425] Rewrite module ssh-agent; rename it to ssh 879325b
@lackac lackac added a commit to lackac/prezto that referenced this issue Jan 19, 2014
@sorin-ionescu [Fix #425] Rewrite module ssh-agent; rename it to ssh 84a683b
@matthoffman matthoffman added a commit to matthoffman/oh-my-zsh that referenced this issue Sep 18, 2014
@sorin-ionescu [Fix #425] Rewrite module ssh-agent; rename it to ssh c19284b
@fanf fanf added a commit to fanf/prezto that referenced this issue Nov 12, 2015
@sorin-ionescu [Fix #425] Rewrite module ssh-agent; rename it to ssh c68fee9
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment