Add support for AEAD ChaCha20Poly1305 Cipher #1416

scott-xu · 2024-05-26T02:35:11Z

This PR adds support for chacha20-poly1305@openssh.com described in https://datatracker.ietf.org/doc/html/draft-josefsson-ssh-chacha20-poly1305-openssh-00

Resolves #1356

scott-xu · 2024-05-26T04:01:42Z

Note: It does not work currently. (That's why it is still in draft)
The integration test fails and the server says "message authentication code incorrect"

output from server:

debug1: sshd version OpenSSH_8.9, OpenSSL 3.0.2 15 Mar 2022
debug1: private host key #0: ssh-rsa SHA256:i0kFdfQjSliPLw28b6R4VgUQJG9OasHV24TbSSeoysQ
debug1: private host key #1: ecdsa-sha2-nistp256 SHA256:BABbQ4XyDj1SwjEpqk1w4nmxxzHXcinwU7tp0GIlcmg
debug1: private host key #2: ssh-ed25519 SHA256:AKLEi6kJlwZgvCn/ngyN3JXiZLVJ+4GvwU0Lb4u5QsA
debug1: rexec_argv[0]='/usr/sbin/sshd'
debug1: rexec_argv[1]='-d'
debug1: rexec_argv[2]='-D'
debug1: Set /proc/self/oom_score_adj from 0 to -1000
debug1: Bind to port 22 on 0.0.0.0.
Server listening on 0.0.0.0 port 22.
debug1: Bind to port 22 on ::.
Server listening on :: port 22.
debug1: Server will not fork when running in debugging mode.
debug1: rexec start in 5 out 5 newsock 5 pipe -1 sock 8
debug1: sshd version OpenSSH_8.9, OpenSSL 3.0.2 15 Mar 2022
debug1: private host key #0: ssh-rsa SHA256:i0kFdfQjSliPLw28b6R4VgUQJG9OasHV24TbSSeoysQ
debug1: private host key #1: ecdsa-sha2-nistp256 SHA256:BABbQ4XyDj1SwjEpqk1w4nmxxzHXcinwU7tp0GIlcmg
debug1: private host key #2: ssh-ed25519 SHA256:AKLEi6kJlwZgvCn/ngyN3JXiZLVJ+4GvwU0Lb4u5QsA
debug1: inetd sockets after dupping: 3, 3
Connection from 127.0.0.1 port 33182 on 127.0.0.1 port 22 rdomain ""
debug1: Local version string SSH-2.0-OpenSSH_8.9p1 Ubuntu-3ubuntu0.6
debug1: Remote protocol version 2.0, remote software version Renci.SshNet.SshClient.0.0.1
debug1: compat_banner: no match: Renci.SshNet.SshClient.0.0.1
debug1: permanently_set_uid: 108/65534 [preauth]
debug1: list_hostkey_types: rsa-sha2-512,rsa-sha2-256,ecdsa-sha2-nistp256,ssh-ed25519 [preauth]
debug1: SSH2_MSG_KEXINIT sent [preauth]
debug1: SSH2_MSG_KEXINIT received [preauth]
debug1: kex: algorithm: curve25519-sha256 [preauth]
debug1: kex: host key algorithm: ssh-ed25519 [preauth]
debug1: kex: client->server cipher: chacha20-poly1305@openssh.com MAC: <implicit> compression: none [preauth]
debug1: kex: server->client cipher: chacha20-poly1305@openssh.com MAC: <implicit> compression: none [preauth]
debug1: expecting SSH2_MSG_KEX_ECDH_INIT [preauth]
debug1: SSH2_MSG_KEX_ECDH_INIT received [preauth]
debug1: ssh_packet_send2_wrapped: resetting send seqnr 3 [preauth]
debug1: rekey out after 134217728 blocks [preauth]
debug1: SSH2_MSG_NEWKEYS sent [preauth]
debug1: expecting SSH2_MSG_NEWKEYS [preauth]
debug1: ssh_packet_read_poll2: resetting read seqnr 3 [preauth]
debug1: SSH2_MSG_NEWKEYS received [preauth]
debug1: rekey in after 134217728 blocks [preauth]
debug1: KEX done [preauth]
ssh_dispatch_run_fatal: Connection from 127.0.0.1 port 33182: message authentication code incorrect [preauth]
debug1: do_cleanup [preauth]
debug1: monitor_read_log: child log fd closed
debug1: do_cleanup
debug1: Killing privsep child 17109
debug1: audit_event: unhandled event 12

'padding_length', 'payload', and 'random padding' MUST be a multiple of the cipher block size or 8, whichever is larger. See https://www.rfc-editor.org/rfc/rfc4253#section-6

Rob-Hague · 2024-05-26T16:10:32Z

Another option would be to try it against the BC branch #1370

scott-xu · 2024-05-27T11:31:47Z

Another option would be to try it against the BC branch #1370

Yes, we can leverage BC if the final decision is to ref BC as the dependent nuget package.
At current stage, since we've already had Poly1305 in Chao.Nacl, it is straightforward to just use it.

Rob-Hague · 2024-07-12T06:41:12Z

I've looked at this, it's cool that you got it working, but to be honest I am not comfortable in reviewing and signing off on an encryption implementation

WojciechNagorski · 2024-07-16T09:29:54Z

I've reviewed this PR and overall it looks great. For me, we can merge it as is or wait for #1370 (comment)
@Rob-Hague it is up to you.

WojciechNagorski · 2024-07-16T20:59:12Z

#1370 is merged.

… into chacha20-poly1305

…acha20-poly1305

…er is deleted

test/Renci.SshNet.IntegrationTests/CipherTests.cs

Rob-Hague · 2024-07-23T12:39:14Z

src/Renci.SshNet/Session.cs

+                // First block is not encrypted in AES GCM mode.
+                if (_serverCipher is not null
+#if NET6_0_OR_GREATER
+        and not Security.Cryptography.Ciphers.AesGcmCipher
+#endif
+                    )
                {
-                    firstBlock = _serverCipher.Decrypt(firstBlock);
+                    _serverCipher.SetSequenceNumber(_inboundPacketSequence);
+
+                    // First block is not encrypted in ETM mode.
+                    if (_serverMac == null || !_serverEtm)
+                    {
+                        plainFirstBlock = _serverCipher.Decrypt(firstBlock);
+                    }


This whole method makes me shed a tear. Maybe one day we can design something easier to comprehend. I have a bit of an idea

It's becoming "nobody knows why but it just works" 🤣
Looking forward your idea.

Co-authored-by: Rob Hague <rob.hague00@gmail.com>

Rob-Hague · 2024-07-23T13:39:29Z

appveyor.yml


  test_script:
    - sh: echo "Run unit tests"
    - sh: dotnet test -f net8.0 -c Debug --no-restore --no-build --results-directory artifacts --logger Appveyor --logger "console;verbosity=normal" --logger "liquid.md;LogFileName=linux_unit_test_net_8_report.md" -p:CollectCoverage=true -p:CoverletOutputFormat=cobertura -p:CoverletOutput=../../artifacts/linux_unit_test_net_8_coverage.xml test/Renci.SshNet.Tests/Renci.SshNet.Tests.csproj
    - sh: echo "Run integration tests"
    - sh: dotnet test -f net8.0 -c Debug --no-restore --no-build --results-directory artifacts --logger Appveyor --logger "console;verbosity=normal" --logger "liquid.md;LogFileName=linux_integration_test_net_8_report.md" -p:CollectCoverage=true -p:CoverletOutputFormat=cobertura -p:CoverletOutput=../../artifacts/linux_integration_test_net_8_coverage.xml test/Renci.SshNet.IntegrationTests/Renci.SshNet.IntegrationTests.csproj
+    - sh: dotnet test -f net48 -c Debug --no-restore --no-build --results-directory artifacts --logger Appveyor --logger "console;verbosity=normal" --logger "liquid.md;LogFileName=linux_integration_test_net_48_report.md" -p:CollectCoverage=true -p:CoverletOutputFormat=cobertura -p:CoverletOutput=../../artifacts/linux_integration_test_net_48_coverage.xml --filter Name=ChaCha20Poly1305 test/Renci.SshNet.IntegrationTests/Renci.SshNet.IntegrationTests.csproj


If the .NET Framework integration tests do actually work on linux/mono, I think we should just run the whole suite, but we can test it in another PR

Rob-Hague

LGTM if CI passes

scott-xu added 2 commits May 25, 2024 11:24

Implements ChaCha20 cipher algorithm.

0880a6a

Implements chacha20-poly1305@openssh.com

5471a32

scott-xu mentioned this pull request May 26, 2024

SSH from .NET Core support dotnet/runtime#69130

Open

scott-xu added 3 commits May 26, 2024 14:01

Update Cipher.cs

da9bc08

Update ChaCha20Poly1305Cipher.cs

aaf49a1

Note that the length of the concatenation of 'packet_length',

7780ff8

'padding_length', 'payload', and 'random padding' MUST be a multiple of the cipher block size or 8, whichever is larger. See https://www.rfc-editor.org/rfc/rfc4253#section-6

scott-xu added 3 commits May 27, 2024 01:25

Use Chaos.Nacl Poly1305Donna

d29d8de

Fix build. Fix typo. Update README

9983a31

Update README.md

920d681

scott-xu changed the title ~~Add support for AEAD ChaCha20Poly1305 Cipher (.NET 6.0 onward only)~~ Add support for AEAD ChaCha20Poly1305 Cipher May 26, 2024

scott-xu added 3 commits May 27, 2024 07:35

Fix build

bea3b48

Remove trailing whitespace

ec84664

Fix build

eecffe6

scott-xu marked this pull request as ready for review May 28, 2024 13:26

scott-xu requested review from drieseng, WojciechNagorski and Rob-Hague as code owners May 28, 2024 13:26

WojciechNagorski mentioned this pull request Jul 16, 2024

Replace internal BouncyCastle with NuGet package #1370

Merged

scott-xu added 4 commits July 17, 2024 07:27

Merge branch 'chacha20-poly1305' of https://github.com/scott-xu/SSH.NET…

b67024c

… into chacha20-poly1305

Merge branch 'chacha20-poly1305' of https://github.com/scott-xu/SSH.NET…

1bbca33

… into chacha20-poly1305

Change to BouncyCastle

898cf08

Merge branch 'develop' of https://github.com/scott-xu/SSH.NET into ch…

7a057de

…acha20-poly1305

scott-xu marked this pull request as draft July 17, 2024 10:36

Inherit from SymmetricCipher instead of StreamCipher since StreamCiph…

b600dd2

…er is deleted

scott-xu added 2 commits July 17, 2024 21:02

Resolve conflicts

5dbb8d7

Move field to local variable

15be6bc

scott-xu marked this pull request as ready for review July 17, 2024 13:18

Compute poly key stream once

fa69068

Rob-Hague reviewed Jul 23, 2024

View reviewed changes

scott-xu and others added 3 commits July 23, 2024 20:44

Update test/Renci.SshNet.IntegrationTests/CipherTests.cs

90826d3

Co-authored-by: Rob Hague <rob.hague00@gmail.com>

Fix build; Add net48 integration test in CI

4ff14c6

Merge branch 'develop' into chacha20-poly1305

379ef1f

Rob-Hague reviewed Jul 23, 2024

View reviewed changes

Rob-Hague approved these changes Jul 23, 2024

View reviewed changes

WojciechNagorski approved these changes Jul 23, 2024

View reviewed changes

scott-xu and others added 4 commits July 24, 2024 06:29

Merge branch 'develop' into chacha20-poly1305

9046899

Merge branch 'develop' into chacha20-poly1305

9f3eafd

Merge branch 'develop' into chacha20-poly1305

566fa98

Merge branch 'develop' into chacha20-poly1305

6106068

Rob-Hague merged commit 486b69d into sshnet:develop Jul 27, 2024
1 check passed

scott-xu deleted the chacha20-poly1305 branch July 27, 2024 08:13

scott-xu mentioned this pull request Aug 14, 2024

Support ChaCha20Poly1305 encrypted private keys tmds/Tmds.Ssh#210

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for AEAD ChaCha20Poly1305 Cipher #1416

Add support for AEAD ChaCha20Poly1305 Cipher #1416

scott-xu commented May 26, 2024

scott-xu commented May 26, 2024

Rob-Hague commented May 26, 2024

scott-xu commented May 27, 2024

Rob-Hague commented Jul 12, 2024

WojciechNagorski commented Jul 16, 2024

WojciechNagorski commented Jul 16, 2024

Rob-Hague Jul 23, 2024

scott-xu Jul 23, 2024 •

edited

Loading

Rob-Hague Jul 23, 2024

Rob-Hague left a comment

Add support for AEAD ChaCha20Poly1305 Cipher #1416

Add support for AEAD ChaCha20Poly1305 Cipher #1416

Conversation

scott-xu commented May 26, 2024

scott-xu commented May 26, 2024

Rob-Hague commented May 26, 2024

scott-xu commented May 27, 2024

Rob-Hague commented Jul 12, 2024

WojciechNagorski commented Jul 16, 2024

WojciechNagorski commented Jul 16, 2024

Rob-Hague Jul 23, 2024

Choose a reason for hiding this comment

scott-xu Jul 23, 2024 • edited Loading

Choose a reason for hiding this comment

Rob-Hague Jul 23, 2024

Choose a reason for hiding this comment

Rob-Hague left a comment

Choose a reason for hiding this comment

scott-xu Jul 23, 2024 •

edited

Loading