search

sshnet / SSH.NET

SSH.NET is a Secure Shell (SSH) library for .NET, optimized for parallelism.
http://sshnet.github.io/SSH.NET/
MIT License
3.96k stars 931 forks source link

Support AesGcm cipher #1364

Closed scott-xu closed 5 months ago

scott-xu commented 6 months ago

This PR adds support for aes128-gcm@openssh.com, aes256-gcm@openssh.com ciphers. This PR does not add chacha20-poly1305@openssh.com cipher. Fix https://github.com/sshnet/SSH.NET/issues/792 Fix https://github.com/sshnet/SSH.NET/issues/1356 Fix https://github.com/sshnet/SSH.NET/issues/774 Fix https://github.com/sshnet/SSH.NET/issues/773 Fix https://github.com/sshnet/SSH.NET/issues/555 Fix https://github.com/sshnet/SSH.NET/issues/477 Fix https://github.com/sshnet/SSH.NET/issues/994

WojciechNagorski commented 6 months ago

I will review in next few days.

scott-xu commented 5 months ago

The OpenSSH version of AES-GCM (https://github.com/openssh/openssh-portable/blob/43e7c1c07cf6aae7f4394ca8ae91a3efc46514e2/PROTOCOL#L82) states:

Additionally, if AES-GCM is selected as the cipher [...] there doesn't have to be a matching MAC.

could you please implement that?

I think the "matching MAC" means the matching MAC algorithm. See code here: https://github.com/sshnet/SSH.NET/pull/1364/files#diff-8cb58eb5577b3c26c820c320ea463952036f2aeadf28470a6265575168756164R1520-R1528

Rob-Hague commented 5 months ago

That's good but I think if we offer:

Ciphers: aes128-gcm@openssh.com
HMACs: hmac-sha2-256

and they offer:

Ciphers: aes128-gcm@openssh.com
HMACs: hmac-sha1

then we are going to error out here despite not using the HMAC

https://github.com/sshnet/SSH.NET/blob/db3d7e8d03d0a852cd7d16e1dee95a72939de317/src/Renci.SshNet/Security/KeyExchange.cs#L99-L107

WojciechNagorski commented 5 months ago

@Rob-Hague Can I ask you for a review? My time is limited now.

scott-xu commented 5 months ago

I renamed the source branch and this PR is closed automatically. I just created another PR https://github.com/sshnet/SSH.NET/pull/1369

zybexXL commented 5 months ago

You mean #1369. This PR could be reopened to keep the existing comment history.

scott-xu commented 5 months ago

@zybexXL thanks! I corrected the comments just now. Unfortunately, this PR cannot be reopened. The button is grayed out.

2024-04-04 174558

zybexXL commented 5 months ago

Admins/Moderators should be able to reopen it.

Rob-Hague commented 5 months ago

Presumably, you will at least need to restore the branch ref with the same name as before

scott-xu commented 5 months ago

The PR is closed but this thread is not deleted. The history is not lost. Is there anything can't be done without this PR to reopen?