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

Add support for Zlib compression (.NET 6.0 onward only) #1326

Merged
merged 27 commits into from
Apr 17, 2024

Conversation

scott-xu
Copy link
Collaborator

@scott-xu scott-xu commented Feb 17, 2024

This PR adds support for [email protected] described in https://www.openssh.com/txt/draft-miller-secsh-compression-delayed-00.txt

Resolves #1130
Resolves #559
Resolves #336
Resolves #119

Notes:

  1. This PR only adds compression support for .NET 6.0+.

  2. "none" is the first and preferred compression method which means no compression by default, just like ssh_config:

    Compression
    Specifies whether to use compression. The argument must
    be yes or no (the default).

    The description of ssh -C option explains the reason:

    Compression is desirable on
    modem lines and other slow connections, but will only
    slow down things on fast networks.

  3. zlib (pre-auth) is not supported in OpenSSH server (sshd) but is listed in OpenSSH client (ssh).
    Since it is not easy to do integration-test for zlib (pre-auth), this PR does not add zlib (pre-auth).

README.md Show resolved Hide resolved
src/Renci.SshNet/Compression/ZlibStream.cs Outdated Show resolved Hide resolved
@scott-xu scott-xu marked this pull request as draft March 17, 2024 02:46
@scott-xu
Copy link
Collaborator Author

scott-xu commented Mar 21, 2024

With this PR, SSH.NET targeted to frameworks below .NET 6.0 can easily use 3rd party library to do the compression. For example: https://github.com/scott-xu/SSH.NET.Ionic.Zlib/blob/main/src/SshNet.IonicZlib/ZLib.cs

@scott-xu scott-xu marked this pull request as ready for review March 21, 2024 13:56
@scott-xu scott-xu requested a review from Rob-Hague March 23, 2024 09:01
@scott-xu
Copy link
Collaborator Author

scott-xu commented Mar 23, 2024

The decompression works when receive SSH_MSG_GLOBAL_REQUEST.
The decompression stops working when receive SSH_MSG_CHANNEL_OPEN_CONFIRMATION (compared with non-compression mode). It tries to decompress byte array: [20 13 74 63 12 112 208 192 0 16]. No idea what's going on here. Thoughts? @Rob-Hague

@scott-xu scott-xu marked this pull request as ready for review March 24, 2024 03:27
@scott-xu
Copy link
Collaborator Author

Ready to review @Rob-Hague @WojciechNagorski

Copy link
Collaborator

@Rob-Hague Rob-Hague left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice. Is there a reason for not testing "zlib" as well?

@scott-xu
Copy link
Collaborator Author

scott-xu commented Mar 24, 2024

Nice. Is there a reason for not testing "zlib" as well?

Because sshd doesn't support "zlib"(pre-auth) anymore.

@scott-xu scott-xu self-assigned this Apr 4, 2024
@scott-xu scott-xu changed the title Integrate ZLibStream from .NET 6.0+ with SSH.NET. Add support for Zlib compression (.NET 6.0 onward) Apr 4, 2024
Copy link
Collaborator

@Rob-Hague Rob-Hague left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, in that case I suggest we do not support the "zlib" algorithm since we can't test it easily. What do you think?

This is good to go otherwise.

src/Renci.SshNet/ConnectionInfo.cs Outdated Show resolved Hide resolved
@scott-xu scott-xu changed the title Add support for Zlib compression (.NET 6.0 onward) Add support for Zlib compression (.NET 6.0 onward only) Apr 5, 2024
@Rob-Hague
Copy link
Collaborator

Thanks, this is good to merge.

@scott-xu
Copy link
Collaborator Author

scott-xu commented Apr 9, 2024

@WojciechNagorski Is there any other thing I can do to get this merged? Thanks!

@WojciechNagorski
Copy link
Collaborator

Yes. You can ask @Rob-Hague for review. I do not have time now.

@Rob-Hague
Copy link
Collaborator

I approved it but it needs CODEOWNERS approval:

image

No rush

@WojciechNagorski
Copy link
Collaborator

I've added @Rob-Hague to code owners.

@Rob-Hague Rob-Hague merged commit b553f81 into sshnet:develop Apr 17, 2024
1 check passed
@scott-xu
Copy link
Collaborator Author

Perfect!

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