Added ability to specify agent socket

[alongoldboim]

Adding the option to specify a socket that will be used, this is useful in case we have multiple agents running and multiple users using the net-ssh at the same time (that's why changing the ENV variable is not really practical).

[mfazekas]

Thanks much for the PR, can you please describe your usecase?
Rather than passing a connection have you considered passing a factory that makes the connection?

[alongoldboim]

@mfazekas didn't quite get your comment, could you elaborate please?

[mfazekas]

Sorry i was not reading your change carefully enough. I assumed agent_socket is an actual socket, but since you're passing it to agent_socket_factory.open it's likely meant to be an address rather than socket.

So your version is this:

Net::SSH::start(user,host,agent_socket_addres:'/foo/bar')

what i was asking is this:

Net::SSH::start(user,host,agent_socket_factory: ->{ UNIXSocket.open('/foo/bar') })

But again, please explain your usecase.

[alongoldboim]

Adding the option to specify a socket that will be used, this is useful in case we have multiple agents running and multiple users using the net-ssh at the same time (that's why changing the ENV variable is not really practical).

I've placed the desc later on :).
@mfazekas thanks for the review, added needed fixes.

[mfazekas]

multiple users with multiple agents sounds an esoteric usecase.

I find custom_socket_factory name confusing. We only use it for agent connections, so the name would be better to reflect that.

[alongoldboim]

Its a multi tenant app that uses net-ssh for some actions, the user inserts his private key, hence all the users passwords will be cached on the same ssh-agent which is a security issue.
The name you suggested 'agent_socket_factory' is the name of the existing function, so i can't use it, what other name do you suggest?

[simon3z]

multiple users with multiple agents sounds an esoteric usecase.

Hi @mfazekas think of a multi-tenant management application.
You want different users to be able to manage hosts belonging to their tenant... but you wouldn't want to collect all the keys in the same ssh-agent.

And remember that "multiple users with multiple agents" is the original design of ssh-agent. In a linux machine you run an ssh-agent per user, because ssh-agent is not multi-tenant itself and you don't want to cache all the keys in a single place for all the users.

[mfazekas]

I'd suggest renamig that method it can be just socket_class it's a private method anyway. And have the option named as agent_socket_factory or agent_connection_factory.

[mfazekas]

@simon3z, @alongoldboim thanks for clarifying the usecase, it makes sense that way.

[alongoldboim]

@mfazekas Needed changes were made and tests fixed, please review again :)

[mfazekas]

can't we just write session vs self.session

[alongoldboim]

fixed.

[mfazekas]

Looks good to me, a test would be good to have.

[alongoldboim]

@mfazekas Added test to make sure that if we pass a lambda it will use its value instead of the regular ENV.

[mfazekas]

Thanks much!

[simon3z]

@mfazekas thanks!

@mfazekas is there any chance to have a new release soon?

[mfazekas]

i'll try to release 4.0.0.alpha4 this weekend

[alongoldboim]

Hey @mfazekas, I'm trying the new fix and i got a small problem you might help me with, I'm trying to do a nested ssh and for some reason the forwarding doesn't work (works if agent_socket_factory isn't passed), debugging the code it seems it uses the correct agent with the correct socket, what did i miss ? :)

require 'net/ssh'
agent_socket = "/tmp/ssh/ssh_a"
FileUtils.mkdir_p '/tmp/ssh_manageiq'
system "ssh-agent -a #{agent_socket}"
Net::SSH.start('10.35.4.213', 'root', :paranoid => false, :forward_agent => true, :agent_socket_factory => ->{ UNIXSocket.open(agent_socket) },
               :key_data => ['key']) do |ssh|
  res = ssh.exec!("ssh -o 'StrictHostKeyChecking no' root@10.35.4.163" + " echo $?")
end

the problem is that the agent.identities arn't being set.

[alongoldboim]

@mfazekas i though net-ssh will add the key we connect with to the running agent automatically, apparently it doesn't, ill just do it manually, maybe we should consider adding the current key to the agent by default?

[mfazekas]

I assume there might be usecase where auto adding the programatically defined key_data to agent automatically is not desirable. So maybe an option to add that to session.

[alongoldboim]

@mfazekas your right, when ill get some time ill do a patch for that as well.
Regarding the release any ETA on that?