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

CiscoIOSXE Authentication fails when login banner does not end with blank line #65

Closed
dpnetca opened this issue Feb 23, 2022 · 9 comments

Comments

@dpnetca
Copy link

dpnetca commented Feb 23, 2022

When connecting to a Cisco IOSXE device (have not tested with others) that contains a login banner, where the login banner does not end on a blank line, the SSH Authentication times out as the passwordPattern in func GetAuthPatterns() is not matched

Example with IOSv that ships with Cisco CML, out of the box, the default login banner configuration is this:

banner login ^C
**************************************************************************
* IOSv is strictly limited to use for evaluation, demonstration and IOS  *
* education. IOSv is provided as-is and is not supported by Cisco's      *
* Technical Advisory Center. Any use or disclosure, in whole or in part, *
* of the IOSv Software or Documentation to any third party for any       *
* purposes is expressly prohibited except as otherwise authorized by     *
* Cisco in writing.                                                      *
**************************************************************************^C

because the banner does not end on a blank line the password prompt appear in a format that does not match the regular expression for the passwordPattern defined in func GetAuthPatterns(): passwordPattern: regexp.MustCompile(`(?im)^(.*@.*)?password:\s?$`),

removing the banner, or Changing the banner as follows does solve this problem:

banner login ^C
**************************************************************************
* IOSv is strictly limited to use for evaluation, demonstration and IOS  *
* education. IOSv is provided as-is and is not supported by Cisco's      *
* Technical Advisory Center. Any use or disclosure, in whole or in part, *
* of the IOSv Software or Documentation to any third party for any       *
* purposes is expressly prohibited except as otherwise authorized by     *
* Cisco in writing.                                                      *
**************************************************************************
^C

However, could this also be solved by changing the password prompt regex? Changing the pattern to: passwordPattern: regexp.MustCompile(`(?im)^(.*)?password:\s?$`), does solve the problem, but not sure if that will cause other issues (if there was use cases that required the .*@.* to match instead of just .*)?

alternatively, is there the ability to pass a custom regex for this prompt in anywhere?

@dpnetca
Copy link
Author

dpnetca commented Feb 23, 2022

output w/ logging enabled:

2022/02/23 15:29:45 info::192.168.118.202::22::opening connection to '192.168.118.202' on port '22'
2022/02/23 15:29:45 debug::192.168.118.202::22::"attempting to open transport connection with the following command: [192.168.118.202 -p 22 -o ConnectTimeout=30 -o ServerAliveInterval=45 -l admin -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -F /home/denis/.ssh/config]
2022/02/23 15:29:45 debug::192.168.118.202::22::transport connection to host opened
2022/02/23 15:29:45 debug::192.168.118.202::22::attempting in channel ssh authentication
2022/02/23 15:29:45 debug::192.168.118.202::22::read: Warning: Permanently added '192.168.118.202' (RSA) to the list of known hosts.
2022/02/23 15:29:46 debug::192.168.118.202::22::read: C
**************************************************************************
* IOSv is strictly limited to use for evaluation, demonstration and IOS  *
* education. IOSv is provided as-is and is not supported by Cisco's      *
* Technical Advisory Center. Any use or disclosure, in whole or in part, *
* of the IOSv Software or Documentation to any third party for any       *
* purposes is expressly prohibited except as otherwise authorized by     *
* Cisco in writing.                                                      *
**************************************************************************
2022/02/23 15:29:46 debug::192.168.118.202::22::read: Password: 
2022/02/23 15:29:55 debug::192.168.118.202::22::attempting to acquire privilege level: privilege_exec
2022/02/23 15:29:55 write::192.168.118.202::22::write: 
2022/02/23 15:29:55 debug::192.168.118.202::22::read: 
2022/02/23 15:29:57 debug::192.168.118.202::22::read: Password: 
2022/02/23 15:30:05 failed to open driver; error: channel operation timed out
exit status 1

Also worth noting, but not really related(although this sent me troubleshooting wrong thing for a while) (not sure if this was intentional or not) in driver/base/base.go in func (d *Driver) Open() error { if there is an authentication error, in the code segment below it returns err which is nil, not authErr as a result calling function gets nill back for err, doesn't realize auth failed, and the code keeps running instead of detecting the authentication error and exiting out. you can see this in the above logging where while sitting at password prompt after authentication timed out it tried to do the privilege exec step

	err := d.Transport.Open()
	if err != nil {
		return err
	}

	var authErr error
	if d.TransportType == transport.SystemTransportName && !d.AuthBypass {
		_, authErr = d.Channel.AuthenticateSSH(d.AuthPassword, d.AuthPrivateKeyPassphrase)
	} else if d.TransportType == transport.TelnetTransportName && !d.AuthBypass {
		_, authErr = d.Channel.AuthenticateTelnet(d.AuthUsername, d.AuthPassword)
	}

	if authErr != nil {
		logging.LogError(
			d.FormatLogMessage("error", "authentication failed, connection not opened"),
		)

		return err
	}

@carlmontanari
Copy link
Contributor

Hey Denis!

Thanks for raising this and for including all the detail/logs!

Regarding the pattern stuff: probably the quickest fix is to use standard transport -- this will do the auth bits in its own process thing versus system transport doing it all "in band" -- thus eliminating the pattern from the auth process.

At this moment there isn't a way (short of updating yourself in source) to patch the patterns. In the python version we can update by just reaching into the channel like conn.channel._auth_password_pattern = blah but of course that isn't really an option in the go flavor right now. Generally speaking I don't want to change the pattern as this is the first time this issue has occurred and the multiline flag (the part that is breaking this basically) has tended to be a good thing to help prevent accidentally matching on stuff in banners and stuff.

All that said, I defo would like to find a way to at least have it be user settable as in the python flavor of scrapli -- I'll leave this open to noodle on that (or you are welcome to take a crack at a pr on it if you're keen).

if there is an authentication error, in the code segment below it returns err which is nil, not authErr

^^ regarding this -- can you elaborate a bit (maybe I need more caffeine -- its still reasonably early lol). I can defo see how returning nil from the channel authentication bits would be bad, but I'm not 100% on how that happened in your log output. Looks like it finds password prompt then just bailed out/moved on? I tested w/ localhost that if I have a bad password or something like that the channel auth returns a non-nil value, so that small test seems to work as expected. But clearly based on the log output there is something im missing!

Thanks again for raising this and all the detail!

Carl

@dpnetca
Copy link
Author

dpnetca commented Feb 24, 2022

"probably the quickest fix is to use standard transport" sounds good I will give that a test this weekend

"the multiline flag (the part that is breaking this basically) has tended to be a good thing to help prevent accidentally matching on stuff in banners and stuff." that makes sense

"(or you are welcome to take a crack at a pr on it if you're keen)." I'll noodle on this as well..I'm just starting to scratch the surface with go, much more comfortable in python (but digging my go experience so far), if I come up with any brilliant ideas I'll take a crack at it, but as of now I don't have anything good....

As for the second part (which should perhaps be a separate issue?) here is what is happening:

  1. In my code I create my driver as follows: driver, err := core.NewCoreDriver(... which is importing from github.com/scrapli/scrapligo/driver/core
  2. next, I run err = driver.Open()
  3. this runs calls the func (d *Driver) Open() error function in driver/network/network.go, first line there is err := d.Driver.Open()
  4. This then leads us to the func (d *Driver) Open() error { function in driver/base/base.go, which is where the code snipet above is from.
  5. Here is that code again with added comments labelled 'A' 'B' 'C' and 'D' to help me explain.

At Point B we run the authentication function, this is where it times out because the password prompt is never matched, so, at this point, authErr is not nil, it contains an error, so at Point C the if statement evaluates to true, and at point D we return err. however, err at this point will always be nil, because, at point A is the last time err was set, and if it was not nil it would of returned out at that point. So this return will always return nil.

So once it returns back to the calling function from step 3, err will not get the authentication error message, as such the the if err !=nil evaluates to false, and it continues on to run the d.OnOpen() call

func (d *Driver) Open() error {
	logging.LogDebug(
		d.FormatLogMessage(
			"info",
			fmt.Sprintf(
				"opening connection to '%s' on port '%d'",
				d.Host,
				d.Transport.BaseTransportArgs.Port,
			),
		),
	)
 // A
	err := d.Transport.Open()
	if err != nil {
		return err 
	}

	var authErr error
	if d.TransportType == transport.SystemTransportName && !d.AuthBypass {
// B
		_, authErr = d.Channel.AuthenticateSSH(d.AuthPassword, d.AuthPrivateKeyPassphrase)
	} else if d.TransportType == transport.TelnetTransportName && !d.AuthBypass {
		_, authErr = d.Channel.AuthenticateTelnet(d.AuthUsername, d.AuthPassword)
	}

// C
	if authErr != nil {
		logging.LogError(
			d.FormatLogMessage("error", "authentication failed, connection not opened"),
		)
// D
		return err
	}

	logging.LogDebug(d.FormatLogMessage("info", "connection to device opened successfully"))

	return nilattempting to acquire privilege level: privilege_exec
}

on the debug posted above these lines from the middle show the results, in the second last line we see it receive the password prompt. This is the point where we failed, the password was never entered in as the authentication timed out, however, instead of stopping and printing out the error at this point it continued on to the OnOpen step which I believe is where the "attempting to acquire privilege level: privilege_exec" the following debug (not posted below but in original message) shows a write and password re-prompt. I assume it sen "en" or something to that extent but have not validated that

* Cisco in writing.                                                      *
**************************************************************************
2022/02/23 15:29:46 debug::192.168.118.202::22::read: Password: 
2022/02/23 15:29:55 debug::192.168.118.202::22::attempting to acquire privilege level: privilege_exec

@hellt
Copy link
Collaborator

hellt commented Feb 24, 2022

Regarding the pattern stuff: probably the quickest fix is to use standard transport -- this will do the auth bits in its own process thing versus system transport doing it all "in band" -- thus eliminating the pattern from the auth process.

Hi @carlmontanari
can you help me to understand how system transport can solve this? Isn't system transport need to do the same and find the password prompt after the ssh established and login/motd banners are consumed?

Just wondering what magic does system transport have at this point

@carlmontanari
Copy link
Contributor

system transport is a fancy subprocess wrapper around /bin/ssh basically -- so "yeah" it is of course still doing auth, but its gotta literally hunt for the password prompt and such, whereas the "real" ssh clients are doing that stuff in some magic channel prior to just spewing bits to the clients terminal. As for solving it... the obvious fix (that I don't really want to do) would be to drop that multiline flag so the pattern finds the prompt even in this case w/ the banner ending and having the prompt run up into it.

will dig into the extra bits Denis wrote this weekend but wanted to reply to that since I had a sec!

@dpnetca
Copy link
Author

dpnetca commented Feb 26, 2022

confirmed changing to use standard transport type does work for login

@dpnetca
Copy link
Author

dpnetca commented Feb 26, 2022

doing some more debugging on the regex matching...

To make it easier to work with, shortened the banner to:

***********************
* login               *
* yadda some password *
* yadda yadda yadda   *
***********************

in channel\authenticate.go the func (c *Channel) authenticateSSH function passwordMatch := passwordPattern.Match(b) is the line that I believe should be matching when that executes, the value of b is:

warning: permanently added '192.168.118.202' (rsa) to the list of known hosts.

***********************
* login               *
* yadda some password *
* yadda yadda yadda   *
***********************password: 

found a few options for how to match this, nothing really creative... mostly changing the pattern from (?im)^(.*@.*)?password:\s?$ to (?im)^(.*)?password:\s?$ or (?im)(.*@.*)?password:\s?$ essentially dropping then ^ "at beginning of text or line" or the @ character in the first submatch group...I know there was concern about matching "password:" in the banner, could change the $ (end if line or text) to \z instead (end of text). that may help prevent against that (so (?im)^(.*)?password:\s?\z)

@carlmontanari
Copy link
Contributor

@dpnetca thanks for spelling that out (re the authErr thing) I was just blind and not seeing it! I've opened #66 to address both the authErr and providing the option to pass in patterns to override the default for password/passphrase and username (telnet). I'm not super wild about how scrapligo is structured (it was my first real attempt at a go project), and I don't super love this, but I think it addresses the issues so at least for now until I maybe eventually do a big overhaul this should do the trick!

I'll close this now so we can move the convo over to the PR -- if you could let us all know if this sorts both of those issues out or not there that would be much appreciated!

Thanks again for the issue and all the detail!

Carl

@carlmontanari
Copy link
Contributor

@dpnetca I'm going to end up removing that newline anchor I think.... too many folks won't get logs and do some investigating like you did and I think it'll end up being more of a problem than the thing the newline anchor tries to prevent! thanks again for the work on digging into this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants