py-junos-eznc icon indicating copy to clipboard operation
py-junos-eznc copied to clipboard

[WIP] Adds allow_agent option to Device() object

Open rsmekala opened this issue 5 years ago • 10 comments

Problem

User currently cannot control the value of allow_agent being passed to ncclient.connect().

Analysis

PyEZ internally sets the value of allow_agent before passing it to ncclient. It is set to True only if PyEZ cannot gather the passwd or ssh_private_keyfile.

In case the user has not specified passwd or ssh_private_keyfile, PyEZ will use the ssh_config to populate the variables. In this case, since PyEZ is able to gather the credentials allow_agent is set to False.

The above logic works fine for a simpler SSH configuration, where only ssh_private_keyfile, port and user are mentioned int SSH configuration, any additional options mentioned in the file are ignored.

Solution

To add a new optional argument bool allow_agent. There are 3 cases:

  1. allow_agent is True: PyEZ will not load the values of ssh_private_key_file or passwd and allow_agent will be passed as True to ncclient.connect().

  2. allow_agent is Fale: PyEZ will load the values of above variables if it is able to gather and allow_agent will be passed as False to ncclient.connect().

  3. allow_agent is not provided: PyEZ will fallback to default behavior and set the value of allow_agent at runtime, based on whether it is able to gather the credentials or not.

Note: PyEZ will prefer the user-provided value of allow_agent over the runtime value

Related issues: #648

rsmekala avatar Mar 28 '19 10:03 rsmekala

Can one of the admins verify this patch?

jnpr-community-netdev avatar Mar 28 '19 10:03 jnpr-community-netdev

Coverage Status

Coverage increased (+0.3%) to 96.77% when pulling 0e5ee3b17df0cee27de6367fa993b7aa9b6615ab on rsmekala:allow_agent into 96a647b77f9cac77c16ef59bd2cb4b28fa8a8005 on Juniper:master.

coveralls avatar Mar 28 '19 11:03 coveralls

This is a bit better than the current situation as we can workaround the inability to use an agent without modifying the code. However, this is still a bit weird to have to pass an additional option to make it work. This is also inconvenient for a lib: I'll have to beg any dependent project to expose this option.

From all the conversations around this we had, no one was able to explain why we try to disable the agent functionality in the first place. The regular OpenSSH client (ssh) doesn't have such an option: a user cannot ask to not use the agent. So, unless someone explains why we try to do things differently, a simpler solution would be to just set self._allow_agent to True. Could you tell me what happens for you when you set allow_agent to True?

As a compromise, in #648, I am disabling the agent only if a private key is specifically provided. When the private key is fetched from ~/.ssh/config, keep the agent in case the key is encrypted. This doesn't make sense for me, but at least, I don't need to change anything to make my setup work (since I am specifying the private key in my ~/.ssh/config).

vincentbernat avatar Apr 06 '19 10:04 vincentbernat

@stacywsmith Can you please provide your input on the issue ??

rsmekala avatar Apr 08 '19 10:04 rsmekala

@rsmekala I'm really not sure why allow_agent is being set as it currently is. Maybe you can review #281 where it was introduced. If possible, I would recommend against adding yet another parameter to Device(). I'd prefer @vincentbernat suggestion to just leave allow_agent enabled all the time, but I'm not 100% sure that doesn't break anything. Again, the info in #281 might provide some clue about the reasons for the current behavior.

stacywsmith avatar Apr 08 '19 15:04 stacywsmith

In #281, there is the mention of a popup when enabling the agent. I suppose this is a reference to OSX. Can someone on OSX test if there is a popup when enabling the agent?

vincentbernat avatar Apr 08 '19 15:04 vincentbernat

I agree with @vincentbernat here. To be honest, I also don't understand why py-junos-eznc has to have all of this complex logic around SSH options, including this, but also hardcoding a path to .ssh/config (under os.getenv('HOME')), as well as options like identityfile etc.

In the eznc -> ncclient -> paramiko chain, you'd think that the SSH client behavior would be something left to the SSH implementation (= paramiko), which would in turn parse an OpenSSH-compatible .ssh/config (or not), and not something the higher level libraries would have to each bother with.

The current code is very simplistic and broken with so many different ways -- anything but a very simple configuration will not work. allow_agent is only part of the problem. ProxyCommand (#153) is another one. Pretty sure ProxyJump wouldn't work either. etc.

paravoid avatar Apr 27 '19 12:04 paravoid

Can one of the admins verify this patch?

jnpr-community-netdev avatar Jun 11 '19 10:06 jnpr-community-netdev

Pretty sure ProxyJump wouldn't work either. etc.

Can confirm ProxyJump does not work, See #1073. I'm with @paravoid, I can't understand why this module can't leave SSH to a downstream library like paramiko.

jameskirsop avatar Aug 31 '20 05:08 jameskirsop

https://github.com/Juniper/py-junos-eznc/pull/1284 is an updated version of this patch.

Curiously ProxyJump doesn't work with this PR combined with https://github.com/Juniper/ansible-junos-stdlib/pull/634 but ProxyCommand does. I've never used ProxyJump before though so it might not work for me anyway.

Host juniperdevice
    User bob
# Does work
    ProxyCommand ssh -l gaima8 jumphost -W %h:%p
# Doesn't have any effect
#   ProxyJump gaima8@jumphost

"bob" is my user on the juniperdevice but not my user locally.

gaima8 avatar Nov 24 '23 23:11 gaima8