Skip to content

Cisco connection issue fix #841

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

Closed
wants to merge 9 commits into from
Closed

Conversation

likeMyCoffee
Copy link

Certain Cisco devices do not adhere to RFC4342 and do not reply if the client identifies first.
Since identifcation can be in random order it will give random connection issues because the SSH_MSG_KEXINIT will not be sent if the client is faster.

Since SSH.Net is not at fault and compatibility with Cisco (and possibly other) devices is something that can easily be supported I've written this modification.

Added LazyIdentification to the ConnectionInfo object to allow late identification in ProtocolVersionExchange.
Overloaded 'Start' function to keep the original functionality and tests intact.

Highly likely fixes issues #752, #778, #469 and might help with others like #798, #767, #807

A user of the SSH.Net library can specify the use of LazyIdentification by setting the flag in the ConnectionInfo class instance that is fed to SshClient.

I hope you are willing to add this Cisco/legacy support into your product in some way, shape or form.

Certain Cisco devices do not adhere to RFC4342 and do not reply if the client identifies first.
Since identifcation can be in random order it will give random connection issues because the SSH_MSG_KEXINIT will not be sent if the client is faster.

Since SSH.Net is not at fault and compatibility with Cisco (and possibly other) devices is something that can easily be supported I've written this modification.

Added LazyIdentification to the ConnectionInfo object to allow late identification in ProtocolVersionExchange.
Overloaded 'Start' function to keep the original functionality and tests intact.

Highly likely fixes issues sshnet#752, sshnet#778, sshnet#469 and might help with sshnet#798, sshnet#767, sshnet#807
@jgarciadelanoceda
Copy link

jgarciadelanoceda commented Oct 4, 2021

I think that it could solve my problem (#798)
Because I forgot to tell that the connection device that the program was connecting to was a Cisco device.

This PR is what happens to me in some devices. If the connection is done with version 2016.1.0 then the first device that sends the Protocol Identification is the server (A cisco device) whereas if the connection is done with version 2020.0.1 the first device that sends the Protocol Identification is the client.

Some Cisco Devices do not send the message Key Exchange Init if the first device to identify is the client (Others do it), when the server does not send the Key Exchange Init message the connection hangs forever

@NeoMorfeo
Copy link

@likeMyCoffee It works so fine for CISCO IOSvl2 15.2 on a GNS3 virtualization, and really nice!!!!

So many thanks, please @darinkes could you merge this, because it will help us a lot :)

@RobinBeismann
Copy link

Hey @darinkes,

sorry to poke again, but could we have this merged? This would help a lot people over at Posh-SSH to connect to Cisco IOS Devices.

@darinkes
Copy link
Collaborator

I'm not the one you have to poke :)
ping @drieseng

@yala40
Copy link

yala40 commented Jul 28, 2022

Hello @drieseng,
could you please have a look on this issue, it could help a lot of Cisco admins using Posh-SSH with Cisco devices that do not adhere to RFC4342 (like me).

many thanks in advance

best regards

@Jeriffe
Copy link

Jeriffe commented Jul 28, 2022 via email

@asmith3006
Copy link

Is there any way to get this resolved? What needs to be done to get this merged, please?
Thanks.

@likeMyCoffee
Copy link
Author

I'm amazed that my 10 lines of easy coding have not been added to the project after so long...

I even added comments and kept to the style guide to make it as painless as possible.

Hopefully a developer will soon look at the trivial lines of code and add them, opening up a whole line of Cisco device support, that your customer-base so desperately needs... It will most likely resolve 10 or so issues.

@fredebk
Copy link

fredebk commented Mar 22, 2023

Looks like there are some conflicts that need to be resolved.

I would also be very interested in having this feature/bug fix merged into the main branch.

@likeMyCoffee
Copy link
Author

Even though the check errors probably have nothing to do with my code, I think I'm gong to have to figure out how to get the checks to pass before the devs in here are going to even look into it :-/

@likeMyCoffee
Copy link
Author

likeMyCoffee commented Mar 27, 2023

Since this pull request is going nowhere I'm trying to resolve things.

*edited

Conflict has been merged, so that's out of the way.

One test is failing due to a Mock issue.
Other tests are failing because ports are exhausted (Only one usage of each socket address (protocol/network address/port) is normally permitted.).

Drilling down into the test suite I think I have found the cause of the failing tests.
Due to the overloaded constructor defined in my modified IProtocolVersionExchange the Mock fails to create an instance because there are now two definitions (unambiguous). The test then fails and presumably leaves a socket open which causes the block of related tests to all fail.

Solutions to fixing the tests/testresults

  1. bring IProtocolVersionExchange back to one definition somehow
  2. the tests need to be expanded to also check the added LazyAuthentication feature

Option 1 should result in cleanest solution if I can somehow pull it off.... :-)

@likeMyCoffee likeMyCoffee requested a review from drieseng as a code owner March 27, 2023 20:23
Default parameter instead of overloading
Can't delete commits so reverted back manually.
Optional parameters not possible
Optional parameters not possible. Reverted back using commit.
@likeMyCoffee
Copy link
Author

likeMyCoffee commented Mar 27, 2023

Tried to solve the test issues in the Github webgui because I didn't have the time to set up my machine.

Tried changing the (I)ProtocolVersionExchange to a single constructor with an optional parameter. This is not supported, so that didn't work.

Found out that the webgui cannot delete commits so I left a trail of commits to end up at the same point as commit 8a4fcf3

I'm leaving it up to the SSH.Net team to find a way to implement it into their code, whilst keeping their tests working.

@DanielHabenicht
Copy link

Is there a way to use this as a workaround in the current version? (so to say without building your own nuget package)

@likeMyCoffee
Copy link
Author

Is there a way to use this as a workaround in the current version? (so to say without building your own nuget package)

The answer is in your name (in German): Habe nicht (you don't get it without the change)

@scott-xu
Copy link
Collaborator

How about naming the property as ServerFirstIdentification or WaitServerIdentification or maybe some other elegant name.
LazyIdentification is a little bit lazy. People may get confused when first read it.

@Rob-Hague
Copy link
Collaborator

FYI I have seen the packet traces in e.g. darkoperator/Posh-SSH#522 and darkoperator/Posh-SSH#442 and believe that these would be fixed by sending the client KEXINIT without waiting for the server KEXINIT. As is also the conclusion in #767 (comment) with #839.

The client KEXINIT behaviour is something that we should fix anyway, but obviously there could be further issues that this PR addresses, relating to identification. It would be great to have a demonstration of those if you are able to come up with one.

@likeMyCoffee
Copy link
Author

I no longer have access to Cisco devices so I can't do any additional work ;-(

@darkoperator
Copy link

darkoperator commented Nov 19, 2023 via email

@geoffstewart
Copy link

Here's a zip of 2 pcaps to a cisco device. One is from openssh and the other from sshnet.
cisco-pcaps.zip

@Rob-Hague
Copy link
Collaborator

@geoffstewart Thanks for that 👍 I think that shows that #972 would fix that case. Will be interesting to see if there are any further issues or not.

@WojciechNagorski
Copy link
Collaborator

It's done in #1274

@WojciechNagorski WojciechNagorski added this to the 2023.0.1 milestone Dec 21, 2023
@WojciechNagorski
Copy link
Collaborator

The 2023.0.1 version has been released to Nuget: https://www.nuget.org/packages/SSH.NET/2023.0.1

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

Successfully merging this pull request may close these issues.