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

SSH from .NET Core support #69130

Open
juzuluag opened this issue May 10, 2022 · 24 comments
Open

SSH from .NET Core support #69130

juzuluag opened this issue May 10, 2022 · 24 comments
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Net.Security
Milestone

Comments

@juzuluag
Copy link

During the work with a customer, discovered that .NET core doesn't contain SSH which was crucial for the development of the engagement. Using a 3rd party NuGet package (known as SSH.NET) was the workaround, which unfortunately is not well maintained and supported, which brings the following issues:

  • Security:
    Lack of more/new secure encryption algorithms given the fact the github repository lacks continuous support.
  • Asynchronous support:
    Current package is implemented using AMP (Asynchronous Programming Model), which is the legacy asynchronous implementation, compared to what most recent versions of dotnet .NET that supports – TAP (Task-based Asynchronous Pattern), which is what was used by customer.
  • Lack of Documentation:
    It required to spend good amount of time reading SSH.NET repository, on how the package was implemented, look at some tests how those were written to have a good idea on how to consume the SSH client provided by the package.

it is very common to find SSH as a standard mechanism to communicate between services across different platform, which brings the issue here whether this should be considered as workable item.

@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@ghost ghost added the untriaged New issue has not been triaged by the area owner label May 10, 2022
@ghost
Copy link

ghost commented May 10, 2022

Tagging subscribers to this area: @dotnet/ncl, @vcsjones
See info in area-owners.md if you want to be subscribed.

Issue Details

During the work with a customer, discovered that .NET core doesn't contain SSH which was crucial for the development of the engagement. Using a 3rd party NuGet package (known as SSH.NET) was the workaround, which unfortunately is not well maintained and supported, which brings the following issues:

  • Security:
    Lack of more/new secure encryption algorithms given the fact the github repository lacks continuous support.
  • Asynchronous support:
    Current package is implemented using AMP (Asynchronous Programming Model), which is the legacy asynchronous implementation, compared to what most recent versions of dotnet .NET that supports – TAP (Task-based Asynchronous Pattern), which is what was used by customer.
  • Lack of Documentation:
    It required to spend good amount of time reading SSH.NET repository, on how the package was implemented, look at some tests how those were written to have a good idea on how to consume the SSH client provided by the package.

it is very common to find SSH as a standard mechanism to communicate between services across different platform, which brings the issue here whether this should be considered as workable item.

Author: juzuluag
Assignees: -
Labels:

area-System.Net.Security, untriaged

Milestone: -

@wfurt
Copy link
Member

wfurt commented May 10, 2022

related to #40424 and #31437. We may choose to contribute to https://github.com/sshnet/SSH.NET and perhaps bring it under https://dotnetfoundation.org/ umbrella.

@antonfirsov
Copy link
Member

antonfirsov commented May 12, 2022

Triage: we want to see how much demand is from community, and consider it in the future. Please upvote top post if interested.

@antonfirsov antonfirsov removed the untriaged New issue has not been triaged by the area owner label May 12, 2022
@antonfirsov antonfirsov added this to the Future milestone May 12, 2022
@karelz karelz added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label May 12, 2022
@karelz
Copy link
Member

karelz commented May 12, 2022

Previously we had interest in:

@vcsjones
Copy link
Member

vcsjones commented May 12, 2022

I think @tmds made or started an SSH client library and noted that there are some possible missing cryptographic pieces, particularly around DiffieHellman, if I recall correctly. SSH needs the "raw" DiffieHellman secret where we don't provide it. SSH.NET gets around this by implementing a lot of the cryptography by hand or borrowing from BouncyCastle.

Is that right @tmds?

@tmds
Copy link
Member

tmds commented May 20, 2022

That's right. There is no API that returns the shared secret. And if I remember correctly, Windows itself doesn't provide one.

After trying the managed implementation on top of .NET crypto APIs, I did another prototype that wraps libssh. The code is at https://github.com/tmds/Tmds.Ssh. (The managed implementation is under the release/v0.1 branch.)

@iSazonov
Copy link
Contributor

/cc @SteveL-MSFT Could you please share thoughts on the possibility of implementing this request?

@vcsjones
Copy link
Member

Windows itself doesn't provide one.

I think this is possible with BCryptDeriveKey and specifying BCRYPT_KDF_RAW_SECRET. It's only on Windows 10+, and would be somewhat tricky to make work as it's a BCrypt API, whereas the ECDiffieHellmanCng implementation is built on top of NCrypt (to support CngKey and the like). For whatever reason the NCryptDeriveKey does not support raw values.

But I digress.

@vcsjones
Copy link
Member

To follow up on raw DH, .NET 8 has DeriveRawSecretAgreement so the D-H blocker has been removed.

@tmds
Copy link
Member

tmds commented May 3, 2024

I started the Tmds.Ssh repo with a managed implementation. After I got stuck on the DH secret, I switched it to wrap libssh instead. I'm going to circle back to the managed implementation and see if I can now get it to fully work using only .NET BCL crypto.

I'm posting here to raise some awareness. It may take some weeks until I actually get this done.

@tmds
Copy link
Member

tmds commented May 17, 2024

see if I can now get it to fully work using only .NET BCL crypto.

I have this working. Here are some "getting started" steps: https://github.com/tmds/Tmds.Ssh?tab=readme-ov-file#getting-started.

If you have feedback, you can create an issue or starting a discussion in the repo.

@tmds
Copy link
Member

tmds commented May 18, 2024

I'm looking at the algorithms my OpenSSH client would prefer to use (see below), and these are some preferable algorithms that .NET doesn't provide APIs for:

This is the list of algorithms currently provided through BCL crypto in Tmds.Ssh: https://github.com/tmds/Tmds.Ssh?tab=readme-ov-file#tmdsssh.

KEX algorithms: curve25519-sha256,curve25519-sha256@libssh.org,ecdh-sha2-nistp256,ecdh-sha2-nistp384,ecdh-sha2-nistp521,diffie-hellman-group-exchange-sha256,diffie-hellman-group14-sha256,diffie-hellman-group16-sha512,diffie-hellman-group18-sha512,ext-info-c,kex-strict-c-v00@openssh.com
host key algorithms: ssh-ed25519-cert-v01@openssh.com,ecdsa-sha2-nistp256-cert-v01@openssh.com,ecdsa-sha2-nistp384-cert-v01@openssh.com,ecdsa-sha2-nistp521-cert-v01@openssh.com,sk-ssh-ed25519-cert-v01@openssh.com,sk-ecdsa-sha2-nistp256-cert-v01@openssh.com,rsa-sha2-512-cert-v01@openssh.com,rsa-sha2-256-cert-v01@openssh.com,ssh-ed25519,ecdsa-sha2-nistp256,ecdsa-sha2-nistp384,ecdsa-sha2-nistp521,sk-ssh-ed25519@openssh.com,sk-ecdsa-sha2-nistp256@openssh.com,**rsa-sha2-512,rsa-sha2-256**
ciphers ctos: aes256-gcm@openssh.com,chacha20-poly1305@openssh.com,aes256-ctr,aes128-gcm@openssh.com,aes128-ctr
ciphers stoc: aes256-gcm@openssh.com,chacha20-poly1305@openssh.com,aes256-ctr,aes128-gcm@openssh.com,aes128-ctr
MACs ctos: hmac-sha2-256-etm@openssh.com,hmac-sha1-etm@openssh.com,umac-128-etm@openssh.com,hmac-sha2-512-etm@openssh.com,hmac-sha2-256,hmac-sha1,umac-128@openssh.com,hmac-sha2-512
MACs stoc: hmac-sha2-256-etm@openssh.com,hmac-sha1-etm@openssh.com,umac-128-etm@openssh.com,hmac-sha2-512-etm@openssh.com,hmac-sha2-256,hmac-sha1,umac-128@openssh.com,hmac-sha2-512
compression ctos: none,zlib@openssh.com,zlib
compression stoc: none,zlib@openssh.com,zlib

@vcsjones
Copy link
Member

vcsjones commented May 19, 2024

@tmds

ChaCha20 (needed separate from ChaCha20-Poly1305 for implementing chacha20-poly1305@openssh.com)

You can implement ChaCha20 with ChaCha20Poly1305 (basically remove the Poly1305).

Basically, you just use Encrypt with ChaCha20Poly1305 and throw away the tag (which is produced by Poly1305). So, that takes care of encrypt.

For Decrypt, since ChaCha20 is a stream cipher that derives a key stream and XORs it with the data, that means Encrypt is the exact same thing as decrypt since XOR is associative and commutative. So to decrypt it without a tag, you just encrypt it again (and throw away the tag).

Here is an implementation of ChaCha20:

public sealed class ChaCha20 : IDisposable {
    private readonly ChaCha20Poly1305 _chaCha20Poly1305;

    public ChaCha20(byte[] key) => _chaCha20Poly1305 = new(key);
    public ChaCha20(ReadOnlySpan<byte> key) => _chaCha20Poly1305 = new(key);

    public void Encrypt(byte[] nonce, byte[] plaintext, byte[] ciphertext) {
        ArgumentNullException.ThrowIfNull(nonce);
        ArgumentNullException.ThrowIfNull(plaintext);
        ArgumentNullException.ThrowIfNull(ciphertext);

        Span<byte> tagThrowAway = stackalloc byte[16];
        _chaCha20Poly1305.Encrypt(nonce, plaintext, ciphertext, tagThrowAway);
    }

    public void Encrypt(ReadOnlySpan<byte> nonce, ReadOnlySpan<byte> plaintext, Span<byte> ciphertext) {
        Span<byte> tagThrowAway = stackalloc byte[16];
        _chaCha20Poly1305.Encrypt(nonce, plaintext, ciphertext, tagThrowAway);
    }

    // ChaCha20 is a stream cipher that builds a keystream and XORs the keystream with the data.
    // So encrypt and decrypt are actually the same thing. We use encrypt here because it allows us to throw away
    // the tag, essentially decryption without providing a tag.

    public void Decrypt(byte[] nonce, byte[] ciphertext, byte[] plaintext) =>
        Encrypt(nonce, ciphertext, plaintext);

    public void Decrypt(ReadOnlySpan<byte> nonce, ReadOnlySpan<byte> ciphertext, Span<byte> plaintext) =>
        Encrypt(nonce, ciphertext, plaintext);

    public void Dispose() => _chaCha20Poly1305.Dispose();
}

And to test it:

ReadOnlySpan<byte> key = [
    00, 01, 02, 03, 04, 05, 06, 07, 08, 09, 10, 11, 12, 13, 14, 15,
    16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31];

ChaCha20 chaCha20 = new(key);
ReadOnlySpan<byte> plaintext = "Ladies and Gentlemen of the class of '99: If I could offer you only one tip for the future, sunscreen would be it."u8;
ReadOnlySpan<byte> nonce = [0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x4a, 0x00, 0x00, 0x00, 0x00];
Span<byte> ciphertextBuffer = stackalloc byte[plaintext.Length];
Span<byte> plaintextBuffer = stackalloc byte[plaintext.Length];

chaCha20.Encrypt(nonce, plaintext, ciphertextBuffer);
Console.WriteLine(Convert.ToHexString(ciphertextBuffer));

chaCha20.Decrypt(nonce, ciphertextBuffer, plaintextBuffer);
Console.WriteLine(Encoding.UTF8.GetString(plaintextBuffer));

We can see this roundtrips the encryption and decryption appropriately, and produces the same cipher text (Ciphertext Sunscreen) as the RFC.

This does mean the implementation is doing Poly1305 for no reason, but Poly1305 overhead is fairly small.

AES+CTR (default ssh-keygen cypher)

I don't know if .NET will ever have a CTR mode in the box. But, this one is reasonable to implement on top of AES-ECB.

Ed25519 and Curve25519

There is no good work around for these currently, unfortunately, and nor are they trivial to implement. As you have found we have (several) tracking issues for various 25519 curve uses, but impeded by some operating systems.

@tmds
Copy link
Member

tmds commented May 21, 2024

You can implement ChaCha20 with ChaCha20Poly1305 (basically remove the Poly1305).

@vcsjones thanks for providing code for this!

this one is reasonable to implement on top of AES-ECB.

If you could provide an implementation, it would be very welcome too.

In the next months I want to release a 0.2 version that includes both a managed and libssh based implementation.
After that, I will drop the libssh based implementation.

@tmds
Copy link
Member

tmds commented May 21, 2024

use Encrypt with ChaCha20Poly1305 and throw away the tag

@vcsjones I'm doing some reading on ChaCha20Poly1305. As I understand it, the algorithm has a block counter and counter zero is used to derive a key. The counter then increments for encrypting the plaintext. To implement chacha20-poly1305@openssh.com (spec) we need to encrypt with the zero block counter. I think this means we can't use ChaCha20Poly1305.Encrypt (because the counter used on the plaintext > 0). Is that right?

@scott-xu
Copy link
Contributor

scott-xu commented May 25, 2024

use Encrypt with ChaCha20Poly1305 and throw away the tag

@vcsjones I'm doing some reading on ChaCha20Poly1305. As I understand it, the algorithm has a block counter and counter zero is used to derive a key. The counter then increments for encrypting the plaintext. To implement chacha20-poly1305@openssh.com (spec) we need to encrypt with the zero block counter. I think this means we can't use ChaCha20Poly1305.Encrypt (because the counter used on the plaintext > 0). Is that right?

I got the exact problem when try to bring ChaCha20Poly1305 into SSH.NET. There's nowhere to set the counter.

@tmds
Copy link
Member

tmds commented May 25, 2024

I think we need a class to provide ChaCha20 separately which could be something like:

namespace System.Security.Cryptography;

class ChaCha20 : IDisposable
{
    static bool IsSupported;

    ChaCha20(ReadOnlySpan<byte> key, int initialCounter);

    void Transform(ReadOnlySpan<byte> nonce, ReadOnlySpan<byte> input, Span<byte> output);
}

This would be usable for the packet length field encryption of chacha20-poly1305@openssh.com.

For the entire packet encryption, I think the algorithm described in chacha20-poly1305@openssh.com matches with what ChaCha20Poly1305 provides.

@vcsjones
Copy link
Member

int initialCounter

Unfortunately a number of the platform crypto primitives do not expose unauthenticated ChaCha20 which accepts an initial counter.

  • Windows does not
  • macOS / iOS does not
  • Linux (OpenSSL) does with EVP_chacha20
  • Android kinda does with ChaCha20ParameterSpec. I say "kinda" because it was just added in API Level 35 (Android 15) which is unreleased.

I also do not know if there is much appetite for adding unauthenticated ChaCha20, that's a @GrabYourPitchforks and @bartonjs question.

@scott-xu
Copy link
Contributor

scott-xu commented May 26, 2024

I'm trying to bring chacha20-poly1305@openssh.com into SSH.NET.

  1. Use hand-written managed ChaCha20 class to encrypt the packet length field.
  2. Use BCL ChaCha20Poly1305 to encrypt packet payload.

However, the openssh server doesn't like the encrypted packet tag and closes the connection immediately.
The server log says: ssh_dispatch_run_fatal: Connection from 127.0.0.1 port 33182: message authentication code incorrect [preauth]

Here's the PR: sshnet/SSH.NET#1416

@tmds
Copy link
Member

tmds commented May 27, 2024

do not expose unauthenticated ChaCha20 which accepts an initial counter.

@vcsjones do you know if they start counting at zero?

Here's the PR: sshnet/SSH.NET#1416

@scott-xu It looks like you got it working (CI is passing). You aren't using the BCL ChaCha20Poly1305. Is it not usable for encrypting the packet payload per chacha20-poly1305@openssh.com?

@scott-xu
Copy link
Contributor

scott-xu commented May 27, 2024

@tmds I'm now using both home-made managed ChaCha20 and managed Poly1305 to encrypt the packet payload and compute tag because BCL's ChaCha20Poly1305 follows RFC8439 (section 2.8) to pad the AAD and the CipherText and append 2 lengths at the end before compute the tag.

Finally, the Poly1305 function is called with the Poly1305 key
calculated above, and a message constructed as a concatenation of
the following:

 *  The AAD

 *  padding1 -- the padding is up to 15 zero bytes, and it brings
    the total length so far to an integral multiple of 16.  If the
    length of the AAD was already an integral multiple of 16 bytes,
    this field is zero-length.

 *  The ciphertext

 *  padding2 -- the padding is up to 15 zero bytes, and it brings
    the total length so far to an integral multiple of 16.  If the
    length of the ciphertext was already an integral multiple of 16
    bytes, this field is zero-length.

 *  The length of the additional data in octets (as a 64-bit
    little-endian integer).

 *  The length of the ciphertext in octets (as a 64-bit little-
    endian integer).

while the OpenSSH's ChaCha20Poly1305 does not pad nor append lengths. See the draft (section 4)

the MAC tag calculated using Poly1305 with this
key over the ciphertext of the packet length and the payload
together.

@scott-xu
Copy link
Contributor

int initialCounter

Unfortunately a number of the platform crypto primitives do not expose unauthenticated ChaCha20 which accepts an initial counter.

  • Windows does not
  • macOS / iOS does not
  • Linux (OpenSSL) does with EVP_chacha20
  • Android kinda does with ChaCha20ParameterSpec. I say "kinda" because it was just added in API Level 35 (Android 15) which is unreleased.

I also do not know if there is much appetite for adding unauthenticated ChaCha20, that's a @GrabYourPitchforks and @bartonjs question.

@vcsjones Do you mean ChaCha20 or ChaCha20 with initial counter? I ask because eventually I found that there's nothing special for the initial counter about ChaCha20 in SSH protocol.
If there's individual ChaCha20 and Poly1305 class in BCL, it would save a lot of work to implement chacha20-poly1305@openssh.com. And it would be much more performance efficient and secure.

I'm trying to check the Windows CNG myself. However, there's even no "chacha20poly1305" found. Am I looking at the wrong doc?

@tmds
Copy link
Member

tmds commented Sep 20, 2024

On the topic of the original issue: I don't think it is likely .NET will include an in-box SSH client because SSH is extensible for algorithms, and the SSH use-case alone is not sufficient for the .NET team to provide these algorithms.

Tmds.Ssh should now have a feature set that meets the needs of most .NET SSH use-cases. I'm interested in any feedback you have about the library (bugs, missing features, API, ...). To give feedback, you can open an issue in the repo or start a discussion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Net.Security
Projects
None yet
Development

No branches or pull requests

9 participants