search

kolban-google / sftp-gcs

An implementation of an SFTP to Google Cloud Storage bridge.
Apache License 2.0
86 stars 34 forks source link

Connection hanging instead of closing gracefully #23

Open jogoldberg opened 2 years ago

jogoldberg commented 2 years ago

When our sender attempts to upload files, they're experiencing the following:

"When I try to connect to their server interactively using the "sftp" client that comes with OpenSSH and I try to "quit" the connection the interactive client hangs. I have output from one of the attempts below with verbose options set to "-vv". Note that after the "input drain -> closed" portion we seem to be hanging when we try to disconnect from this customers server. (refer to attached log1.txt file)

The SFTP we are using to transfer files is from OpenSSH_7.4p1. Information about the SFTP protocol can be found here: https://datatracker.ietf.org/doc/html/rfc4254. When this client wants to disconnect it sends a packet of type 96 which indicates that we are finished sending data. After that the client expects the server to send a packet of type 97 indicating that we can close the the channel. However, the SFTP server the customer is using is not sending the type 97 packet. I don't see any options available in the version of the client that we have in production that would allow me to end the connection without doing the packet 96/packet 97 dance. Some verbose output is shown below.

sftp> quit
debug2: channel 0: read<=0 rfd 4 len 0
debug2: channel 0: read failed
debug2: channel 0: close_read
debug2: channel 0: input open -> drain
debug2: channel 0: ibuf empty
debug2: channel 0: send eof
debug3: send packet: type 96
debug2: channel 0: input drain -> closed

I have sent you the mentioned log1.txt via email.

kolban-google commented 2 years ago

We appear to have a candidate for a solution. Looking at the underlying SSH2 library that is used by this project, we find the following defect:

https://github.com/mscdex/ssh2/pull/1111

This is literally "in-flight" as of the time of this report. We looked at the suggested pull request for the SSH2 1111 bug and tested it locally (a patch) and it "seems" to have worked. We will continue to investigate but, at this point, I'm assuming/thinking that the root problem is a defect in SSH2 and when that is cured, the resolution will propagate up to this package too.

jogoldberg commented 2 years ago

We appear to have a candidate for a solution. Looking at the underlying SSH2 library that is used by this project, we find the following defect:

mscdex/ssh2#1111

This is literally "in-flight" as of the time of this report. We looked at the suggested pull request for the SSH2 1111 bug and tested it locally (a patch) and it "seems" to have worked. We will continue to investigate but, at this point, I'm assuming/thinking that the root problem is a defect in SSH2 and when that is cured, the resolution will propagate up to this package too.

I have manually patched in the change introduced by this PR and have confirmed it solves our issue. We're now just waiting for @mscdex to approve/release mscdex/ssh2#1111

jogoldberg commented 2 years ago

I can say based on testing that it will be Closed by mscdex/ssh2#1111