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

Support ChaCha20Poly1305 encrypted private keys #210

Closed
jborean93 opened this issue Aug 5, 2024 · 3 comments · Fixed by #225
Closed

Support ChaCha20Poly1305 encrypted private keys #210

jborean93 opened this issue Aug 5, 2024 · 3 comments · Fixed by #225

Comments

@jborean93
Copy link
Contributor

The PR #207 implements support for private keys encrypted with the various AES ciphers. This issue is for tracking support for ChaCha20Poly1305 encrypted keys.

I've held off from implementing support for this cipher in that PR as I'm not familiar with the details but here are some details I used when I got it working in that PR.

  • KeyLength is 64 but only the first 32 bytes are used in the decrypter key below
  • IVLength is 0

Code to decrypt

static class ChaCha20Decrypter
{
    public static byte[] Decrypt(ReadOnlySpan<byte> key, ReadOnlySpan<byte> data)
    {
        // https://github.com/openssh/openssh-portable/blob/master/PROTOCOL.chacha20poly1305
        Span<byte> nonce = stackalloc byte[12];
        Span<byte> tag = stackalloc byte[16];
        tag[0] = 1;  // K_2 sets the counter to 1
        byte[] decData = new byte[data.Length];

        using var chacha = new ChaCha20Poly1305(key);
        chacha.Encrypt(nonce, data, decData, tag);

        return decData;
    }
}

I'm unsure how to use the remaining 32 bytes of the generated key but it seems like it might be part of the authentcation/tag data.

@scott-xu
Copy link

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 whereas OpenSSH's ChaCha20Poly1305 does not pad nor append lengths. See the draft (section 4)

I understand this library tries to avoid 3rd party dependency as possible as it can. However, based on the fact that BCL doesn't support all required ciphers/algorithms, we can try our best to use BCL's implementation and falls back the 3rd party library. e.g. BouncyCastle.

Some references:
sshnet/SSH.NET#1450
sshnet/SSH.NET#1453
sshnet/SSH.NET#1447
sshnet/SSH.NET#1416

@jborean93
Copy link
Contributor Author

This is specifically for OpenSSH private keys and not the ongoing encryption of packets, The example code I shared above is all from the BCL and worked in the tests I had. Is there something I was missing or are you just talking about ChaCha20Poly1305 in general?

@jborean93
Copy link
Contributor Author

Now that the PR has been merged here is a diff that "works". I'm just unsure if it's fine to use and whether things like the associated data is always going to be a fixed size for this scenario.

diff --git a/src/Tmds.Ssh/AlgorithmNames.cs b/src/Tmds.Ssh/AlgorithmNames.cs
index 9256b01..30536b9 100644
--- a/src/Tmds.Ssh/AlgorithmNames.cs
+++ b/src/Tmds.Ssh/AlgorithmNames.cs
@@ -51,6 +51,8 @@ static class AlgorithmNames // TODO: rename to KnownNames
     public static Name Aes128Gcm => new Name(Aes128GcmBytes);
     private static readonly byte[] Aes256GcmBytes = "aes256-gcm@openssh.com"u8.ToArray();
     public static Name Aes256Gcm => new Name(Aes256GcmBytes);
+    private static readonly byte[] ChaCha20Poly1305Bytes = "chacha20-poly1305@openssh.com"u8.ToArray();
+    public static Name ChaCha20Poly1305 => new Name(ChaCha20Poly1305Bytes);
 
     // KDF algorithms:
     private static readonly byte[] BCryptBytes = "bcrypt"u8.ToArray();
diff --git a/src/Tmds.Ssh/OpenSshKeyCipher.cs b/src/Tmds.Ssh/OpenSshKeyCipher.cs
index 17ea1b7..6844175 100644
--- a/src/Tmds.Ssh/OpenSshKeyCipher.cs
+++ b/src/Tmds.Ssh/OpenSshKeyCipher.cs
@@ -61,6 +61,14 @@ public static bool TryGetCipher(Name name, [NotNullWhen(true)] out OpenSshKeyCip
             { AlgorithmNames.Aes256Ctr, CreateAesCtrCipher(32) },
             { AlgorithmNames.Aes128Gcm, CreateAesGcmCipher(16) },
             { AlgorithmNames.Aes256Gcm, CreateAesGcmCipher(32) },
+            {
+                AlgorithmNames.ChaCha20Poly1305,
+                new OpenSshKeyCipher(
+                    keyLength: 64,
+                    ivLength: 0,
+                    (ReadOnlySpan<byte> key, Span<byte> _1, ReadOnlySpan<byte> ciphertext, ReadOnlySpan<byte> tag)
+                        => DecryptChaCha20Poly1305(key[..32], ciphertext, tag),
+                    tagLength: 16) },
         };
 
     private static OpenSshKeyCipher CreateAesCbcCipher(int keyLength)
@@ -99,4 +107,18 @@ private static byte[] DecryptAesGcm(ReadOnlySpan<byte> key, Span<byte> iv, ReadO
         aesGcm.Decrypt(iv, data, tag, plaintext, null);
         return plaintext;
     }
+
+    private static byte[] DecryptChaCha20Poly1305(ReadOnlySpan<byte> key, ReadOnlySpan<byte> ciphertext, ReadOnlySpan<byte> associatedData)
+    {
+        // https://github.com/openssh/openssh-portable/blob/master/PROTOCOL.chacha20poly1305
+        Span<byte> nonce = stackalloc byte[12];
+        Span<byte> tag = stackalloc byte[16];
+        tag[0] = 1;  // K_2 sets the counter to 1
+        byte[] decData = new byte[ciphertext.Length];
+
+        using var chacha = new ChaCha20Poly1305(key);
+        chacha.Encrypt(nonce, ciphertext, decData, tag, associatedData);
+
+        return decData;
+    }
 }
diff --git a/test/Tmds.Ssh.Tests/PrivateKeyCredentialTests.cs b/test/Tmds.Ssh.Tests/PrivateKeyCredentialTests.cs
index da418e5..ea0d935 100644
--- a/test/Tmds.Ssh.Tests/PrivateKeyCredentialTests.cs
+++ b/test/Tmds.Ssh.Tests/PrivateKeyCredentialTests.cs
@@ -55,6 +55,7 @@ await RunWithKeyConversion(_sshServer.TestUserIdentityFile, async (string localK
     [InlineData("aes256-ctr")]
     [InlineData("aes128-gcm@openssh.com")]
     [InlineData("aes256-gcm@openssh.com")]
+    [InlineData("chacha20-poly1305@openssh.com")]
     public async Task OpenSshRsaKey(string? cipher)
     {
         await RunWithKeyConversion(_sshServer.TestUserIdentityFile, async (string localKey) =>

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

Successfully merging a pull request may close this issue.

2 participants