Third party libraries - Githubissues

sshnet / SSH.NET

SSH.NET is a Secure Shell (SSH) library for .NET, optimized for parallelism.

http://sshnet.github.io/SSH.NET/

MIT License

4.02k stars 934 forks source link

Third party libraries #1271

Open WojciechNagorski opened 11 months ago

WojciechNagorski commented 11 months ago

Does anyone know why SSH.NET uses copies of third-party libraries:

BouncyCastle: Based on Version 1.8.3 from http://www.bouncycastle.org/csharp/ (currently there is 2.2.1)
Chaos.NaCl: https://github.com/CodesInChaos/Chaos.NaCl/commit/2c861348dc45369508e718aa08611c53b53553db

source of information: https://github.com/sshnet/SSH.NET/pull/496#issuecomment-573557849

I wonder if it would be better to use Nuget. We would receive security updates, bug fixes, and optimizations.

I looked through the code coverage and for the most part, the copied code is not covered by tests.

@scott-xu @Rob-Hague @drieseng @jacobslusser

Rob-Hague commented 11 months ago

I don't know why it was copied, but one possible explanation and downside of using the nuget package: the bouncycastle binary is nearly 7 megabytes.

The size wouldn't be a blocker for me (I would prefer the nuget). I imagine we wouldn't need Chaos.NaCl: I think bouncycastle could be used for Ed25519

I looked through the code coverage and for the most part, the copied code is not covered by tests.

Most of the internal code is unused: #1140

Rob-Hague commented 11 months ago

cc @darinkes

jacobslusser commented 11 months ago

@WojciechNagorski, I think you raise an excellent question. If we are counting votes, I would be in favor of using the third-party nuget packages instead of copying the code.

darinkes commented 11 months ago

At that time there were no usable NuGets (Chaos.NaCl still hasnt) and BouncyCastle is a huge bloat we just needed a very small part of. Thats why we went the route to import only needed stuff.

drieseng commented 11 months ago

I'm ok with switching to NuGet.

I suppose the reasons for including the source code were:

Reduce the number of - direct or indirect - dependencies to the absolute minimum hereby avoid dll/assembly hell.
Reduce the on-disk foodprint of SSH.NET.
Support for legacy target frameworks.

Perhaps the last one was - at that time - the most important reason.

WojciechNagorski commented 4 months ago

The BouncyCastle code has been removed. I think we can get rid of Chaos.NaCI now.