Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Linux SSH / OpenGear read_until_prompt fails to detect enable mode when prompt is only single character #2866

Closed
mirceaulinic opened this issue Jul 21, 2022 · 5 comments
Labels

Comments

@mirceaulinic
Copy link
Contributor

mirceaulinic commented Jul 21, 2022

Steps to reproduce:

  1. Initialise a connection with an OpenGear device, using the Linux SSH driver:
>>> from netmiko import ConnectHandler
>>> device = ConnectHandler(device_type="linux", host="some-opengear", username="root", password="pass", secret="pass", fast_cli=False)
  1. Attempt to enter in enable mode:
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/local/lib/python3.9/site-packages/netmiko/linux/linux_ssh.py", line 150, in enable
    raise ValueError(msg)
ValueError: 

Netmiko failed to elevate privileges.

Please ensure you pass the sudo password into ConnectHandler
using the 'secret' argument and that the user has sudo
permissions.

The previous command does send the sudo -s command as it should, the device returns a line containing the # as expected, but it fails on check_enable_mode.

After digging deeper, it seems like the issue is caused by the read_until_prompt method that is misbehaving. To reproduce, use the device defined previously:

>>> device.write_channel(device.RETURN)
>>> output = device.read_channel()
>>> results = re.split("(.*)", output, maxsplit=1, flags=re.I)
>>> output, match_str, buffer = results
>>> output
''
>>> match_str
''
>>> output + match_str
''
>>> buffer
'\n# \n# \n# '
>>> 

Can notice that it sends the return, it reads the prompt correctly, but the regular expression split renders output + match_str to be an empty string. As a result, check_enable_mode will never find the # prompt in the empty string, and therefore check_enable_mode will always return False in https://github.com/ktbyers/netmiko/blob/v4.1.1/netmiko/base_connection.py#L1861.

The above is a simplified version of https://github.com/ktbyers/netmiko/blob/v4.1.1/netmiko/base_connection.py#L614-L640 to reproduce it step-by-step. The pattern .* comes from https://github.com/ktbyers/netmiko/blob/v4.1.1/netmiko/base_connection.py#L731-L733 as self.base_prompt is empty string. So everything seems to be caused by the incorrectly detected prompt.

TL;DR: self.base_prompt should be # but it's ''.

If I change the line https://github.com/ktbyers/netmiko/blob/v4.1.1/netmiko/base_connection.py#L1289 to

self.base_prompt = prompt

Everything seems to work fine, but might break other things? If so, perhaps something like the following would be more appropriate?

if len(prompt.strip()) == 1:
    self.base_prompt = prompt
else:
    self.base_prompt = prompt[:-1]

In fact, I can't think of a case where we need to strip the last character of a prompt, but perhaps there's more background to this than I'm aware of.

@ktbyers
Copy link
Owner

ktbyers commented Jul 21, 2022

self.base_prompt is the prompt without the terminating prompt character.

In other words, if the prompt is foo# then self.base_prompt should be foo.

The reason for this is that the terminating character frequently changes as you can see with the sudo -s where it might go from foo> to foo# and Netmiko needs something for the terminating read pattern (i.e. Netmiko wants to only have the part that doesn't change i.e. foo so that the read completes successfully upon state changes on the device).

But obviously that is problematic in cases where the prompt is only the the terminating character and nothing else.

So one solution is to just raise an error or exception if self.base_prompt is ever a null-string as that should be invalid (and instruct the end-user that they need to set their device's prompt to a longer value).

Another option would be to modify the default set_base_prompt behavior such that if len(prompt) == 1, then just set the self.base_prompt to that and live with the consequences of that. I could definitely see how that would be problematic for Netmiko reading properly, however (if the device state changes).

Do you have a way of testing this failing napalm-opengear (if we make this latter fix)?

I am a bit concerned that allowing a prompt of only the terminating character, then we would just be deferring failures to a later place in the Netmiko code (which would be harder to debug as far as why the failure happened).

@mirceaulinic
Copy link
Contributor Author

Thanks for looking into this and for your notes @ktbyers!

Might also be worth pointing out that this only happens on Netmiko 4.x, no issues with 3.x.

So one solution is to just raise an error or exception if self.base_prompt is ever a null-string as that should be invalid (and instruct the end-user that they need to set their device's prompt to a longer value).

I'm not aware of a way to do this. For example, the hostname is configured, but the prompt is always #. I've never seen an OpenGear with a different prompt, but I'll ask around and will let you know.

Do you have a way of testing this failing napalm-opengear (if we make this latter fix)?

Yes, no problem with that...

Would it be a good idea to have a global argument such as global_pattern that read_until_pattern would use when set (as opposed to being passed for every call)? e.g.

device = ConnectHandler(device_type="linux", host="some-opengear", username="root", password="pass", secret="pass", fast_cli=False, global_pattern="#")

Or perhaps expected_prompt that we fallback to if self.base_prompt is empty? e.g.

device = ConnectHandler(device_type="linux", host="some-opengear", username="root", password="pass", secret="pass", fast_cli=False, expected_prompt="#")

And then after https://github.com/ktbyers/netmiko/blob/v4.1.1/netmiko/base_connection.py#L1289 we add

if not self.base_prompt:
    self.base_prompt = expected_prompt

Just trying to think of other alternatives. Regardless, I'm curious what's the reasoning for having self.base_prompt = prompt[:-1] (instead of just self.base_prompt = prompt) if you recall?

@ktbyers
Copy link
Owner

ktbyers commented Jul 21, 2022

Might also be worth pointing out that this only happens on Netmiko 4.x, no issues with 3.x.

Yes, that one is hard as a lot of things changed from Netmiko 3.x to Netmiko 4.x. The main thing I can think of is that Netmiko 3.x had a pretty fundamental bug with respect to reading beyond pattern (i.e. it would/could read beyond the pattern). This was fixed in Netmiko 4.x.

In general for terminal servers I recommend the use of the terminal_server device_type as terminal servers are generally strange beasts. Of course, Netmiko is usually used for just going through them to get to end-devices and not actually for configuring and managing them directly.

It is likely (or at least possible) that we should create a Netmiko specific device_type for opengear (if we actually want it to work). In other words, I would expect to be able to set the prompt on all linux platforms so not being able to set the prompt is pretty problematic.

I previously started moving (in a small way) towards a class attribute to indicate a terminating pattern, but it is definitely not ubiquitous and would (likely) be a very large overhaul to apply this generally.

See an example here:

https://github.com/ktbyers/netmiko/blob/develop/netmiko/linux/linux_ssh.py#L18

Let me research a bit about what is a practical solution to fix this in the near term.

@ktbyers
Copy link
Owner

ktbyers commented Jul 21, 2022

 Regardless, I'm curious what's the reasoning for having self.base_prompt = prompt[:-1] 
(instead of just self.base_prompt = prompt) if you recall?

For self.base_prompt, I want something that is enduring (not likely to change).

For example, in the below:

cisco3>enable
Password: 
cisco3#conf t
Enter configuration commands, one per line.  End with CNTL/Z.
cisco3(config)#

I want to be able to go through the state changes and still have the netmiko read succeed. So I want the part of the prompt that is common to all of these cases i.e. cisco3. So that is what is meant by the base in self.base_prompt.

Obviously, that doesn't always work, but that is why the last character is dropped (to try to make Netmiko output reading more reliable).

Linux behavior can already be problematic here (relative to platforms that are very Cisco IOS like).

For example:

(yang_test) [ktbyers@pydev2 ~]$ sudo -s
(yang_test) [root@pydev2 ktbyers]# 

i.e. the prompt here is changing on more things than just the last character.

mirceaulinic added a commit to napalm-automation-community/napalm-opengear that referenced this issue Jul 26, 2022
Until there will be a more permanent solution in Netmiko, in order to
have this driver usable with version 4.x, we need to "help" Netmiko when
it fails to detect the prompt. Strictly speaking, as discussed under
ktbyers/netmiko#2866 Netmiko works as
designed, the ``base_prompt`` attribute is what we'd expect to be, but
when OpenGear doesn't set anything on the prompt other than ``$`` or ``#``
depending on the user elevation, then Netmiko fails to properly read
(more specifically, won't know when to stop).

I also took this chance to change the behaviour in ``open()`` to only
invoke ``enable()`` when the user is not root -- this saves one
unnecessary operation when we use the root user.
@ktbyers ktbyers added the bug label Aug 2, 2022
@ktbyers ktbyers changed the title Linux SSH / OpenGear read_until_prompt fails to detect enable mode Linux SSH / OpenGear read_until_prompt fails to detect enable mode when prompt is only single charater Aug 8, 2022
@ktbyers ktbyers changed the title Linux SSH / OpenGear read_until_prompt fails to detect enable mode when prompt is only single charater Linux SSH / OpenGear read_until_prompt fails to detect enable mode when prompt is only single character Aug 8, 2022
@ktbyers
Copy link
Owner

ktbyers commented Aug 8, 2022

Should be fixed here:

#2891

@ktbyers ktbyers closed this as completed Aug 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants