py-junos-eznc
py-junos-eznc copied to clipboard
Juniper
Do not provide the key if it comes from the SSH configuration
This is a followup of #628 where this change was initially pushed but
reverted because I didn't remember why I did it. Before #628, when an
identity is provided in the SSH configuration, it was not copied in
_conf_ssh_private_key_file due to a bug. After fixing the bug in #628,
the key is now copied.
However, the SSH configuration is provided to the connect() method
which will use it if needed. Therefore, this is not needed. Moreover, if
the key is provided by an agent and/or encrypted, this won't work as,
later in the code, allow_agent will be set to False due to the
presence of a private key.
Coverage decreased (-0.0006%) to 98.975% when pulling 51a68d1567eab00dc0ff5b7b6deee20bb76c5318 on vincentbernat:fix/private-ssh-key into 999c88374d7668da503ef2feb31ff29674382794 on Juniper:master.
An alternative solution would be to enable the user to say if they want to use the agent or not. allow_agent=None in the function signature. if allow_agent is None, use the current heuristic to decide its value, otherwise leave it as is.
IMO, we should always use the agent: ssh doesn't offer the option to not query the agent but I suppose people put that for some reason.
Is it something to do for this PR to be merged? People using an agent to store their SSH keys cannot use junos-eznc without this patch as agent is bypassed when providing a key directly to junos-eznc or through ~/.ssh/config.
Sorry for the delay @vincentbernat I need to test this change before I can merge. I will do it next week.
Hi @paravoid, Can you explain the scenario of your test. In my test environment, I dont see any issue.
allow_agent is set to False when there is any explicit value passed by user for passwd/ssh private key. If both are None allow_agent is set to True and hence its upto ncclient/paramiko to make use of ssh config file/identity to make the connection to the device.
Sure! Selecting a SSH key file is orthogonal to using an agent (an agent may have multiple keys installed, and a user may want to pick different ones for different hosts).
In my case, I have an .ssh/config that looks a little bit like this:
Host router.example.org
User paravoid
IdentityFile ~/.ssh/id_rsa.network
…and this works as intended with OpenSSH. I actually have a much more complicated config with statements such as UserKnownHostsFile, IdentitiesOnly, ProxyJump etc., but this is just a simplified PoC of the bug in question.
The code right now does this:
allow_agent = bool((self._auth_password is None) and
(self._ssh_private_key_file is None))
…with self._conf_ssh_private_key_file = found.get('identityfile'), hence why this is broken.
It really feels odd to me that py-junos-eznc has logic around SSH config options and their combinations. It doesn't feel like like an SSH configuration logic belongs in a Juniper-specific module at all. More broadly, it looks like in the NAPALM->py-junos-eznc->ncclient->paramiko chain… there is some logic around SSH private key files, passwords and/or agent use in each of those modules :( This makes things very complicated to handle corner cases, debug or add new features for (e.g. ProxyJump)…
This bug is still biting me 2 years later. Again, the root cause of all this pain is the ability to disable querying local agent. This is not an option the regular ssh client allows. An easy fix would be to always set to True and see who complains.
Sorry @vincentbernat for late response, we will update you about this by EOD today with the code changes we plan. You can go through the plan. If all good, will take it forward.
@vincentbernat We will be going with allow_agent logic. A new option allow_agent would be added to the Device function.
A more detailed view of the changes.
Option bool allow_agent
Default False
-
When set to
True, Pyez will forward thessh_configfile to the paramiko agent instead of parsing it. -
When set to
False, Pyez will fallback to default behavior i.e Pyez will parse thessh_configand the file will not be forwarded to paramiko agent.
Any comments/inputs on the logic ??
@vincentbernat @paravoid The functionality will be merged via #920. Can you please review the changes, and let us know if you have any additional requirements ??
https://github.com/Juniper/py-junos-eznc/pull/1284 is another go at resolving this. In my case I have an IdentityFile defined at the top level of my ~/.ssh/config but for the Juniper devices I must use a yubikey so have no private key on disk to use for them.
With allow_agent defined it simply won't look for an IdentityFile and pass allow_agent=True to ncclient.
I wanted to get consensus here before opening yet another pull request, but after years, still no consensus. Nobody was able to explain why we try to disable the agent, something that is not possible with "ssh" from OpenSSH. I think the best way forward is to mimic this and always leave the agent enabled. Nobody ever demonstrated this would break their workflow. Adding more options and workarounds will just makes the code more complex and brittle. I have opened #1285 to remove the logic around disabling the agent and it should be favored over of the current PR.