Closed jborean93 closed 2 months ago
tmds
commented
3 months ago For encrypted keys, I would like to start with an in-box implementation of AES+CTR only, to avoid a 3rd party dependency.
Per https://github.com/dotnet/runtime/issues/69130#issuecomment-2119035245 it should be implementable on top of AES-ECB.
If you want to try to implement that let me know, otherwise I'll make some time for it.
jborean93
commented
3 months ago For encrypted keys, I would like to start with an in-box implementation of AES+CTR only, to avoid a 3rd party dependency.
Unfortunately the bcrypt kdf blocker mentioned means we can’t implemented any cipher algorithm with the OpenSSH format. The bcrypt kdf derives the AES key from the password so this is unfortunately a must have.
SSH.NET implements their own kdf implementation in managed code. The Python script I wrote utilises some rust library to do the work. Unfortunately there’s really no palatable option here except to:
tmds
commented
3 months ago I prefer to have a managed kdf implementation, either written from scratch or permissively copied.
tmds
commented
3 months ago @jborean93 are you interested to make an in-box implementation for the aes-ctr encryption. Otherwise, I can look into it.
Some additional context: I consider this library in early preview state which means there is room to learn and explore different options. Some time next year I want there to be a "stable" release.
jborean93
commented
3 months ago I'm happy to do so, when researching earlier it didn't seem that hard to implement aes-ctr support. Unfortunately it's not something that can be used until the KDF side is solved. I'll add in SSH.NET's KDF implementation on my test branch to try it out and we can have a look at where we want to go from there.
jborean93
commented
2 months ago The PR #207 implemented encryption support for OpenSSH keys but didn't do so for the old RSA PKCS#1 format as it hasn't been the default since OpenSSH 7.8 (2018). Just in case there is demand for this here is a diff that can re-add it back in
diff --git a/src/Tmds.Ssh/AlgorithmNames.cs b/src/Tmds.Ssh/AlgorithmNames.cs
index 9256b01..a318b46 100644
--- a/src/Tmds.Ssh/AlgorithmNames.cs
+++ b/src/Tmds.Ssh/AlgorithmNames.cs
@@ -52,6 +52,14 @@ static class AlgorithmNames // TODO: rename to KnownNames
private static readonly byte[] Aes256GcmBytes = "aes256-gcm@openssh.com"u8.ToArray();
public static Name Aes256Gcm => new Name(Aes256GcmBytes);
+ // PKCS#1 encryption algorithms.
+ private static readonly byte[] Pkcs1Aes128CbcBytes = "AES-128-CBC"u8.ToArray();
+ public static Name Pkcs1Aes128Cbc => new Name(Pkcs1Aes128CbcBytes);
+ private static readonly byte[] Pkcs1Aes192CbcBytes = "AES-192-CBC"u8.ToArray();
+ public static Name Pkcs1Aes192Cbc => new Name(Pkcs1Aes192CbcBytes);
+ private static readonly byte[] Pkcs1Aes256CbcBytes = "AES-256-CBC"u8.ToArray();
+ public static Name Pkcs1Aes256Cbc => new Name(Pkcs1Aes256CbcBytes);
+
// KDF algorithms:
private static readonly byte[] BCryptBytes = "bcrypt"u8.ToArray();
public static Name BCrypt => new Name(BCryptBytes);
diff --git a/src/Tmds.Ssh/PrivateKeyParser.RsaPkcs1.cs b/src/Tmds.Ssh/PrivateKeyParser.RsaPkcs1.cs
index 5b52da0..d3c4ded 100644
--- a/src/Tmds.Ssh/PrivateKeyParser.RsaPkcs1.cs
+++ b/src/Tmds.Ssh/PrivateKeyParser.RsaPkcs1.cs
@@ -5,6 +5,7 @@
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;
using System.Security.Cryptography;
+using System.Text;
namespace Tmds.Ssh;
@@ -17,6 +18,7 @@ partial class PrivateKeyParser
internal static bool TryParseRsaPkcs1PemKey(
ReadOnlySpan<byte> keyData,
Dictionary<string, string> metadata,
+ Func<string?> passwordPrompt,
[NotNullWhen(true)] out PrivateKey? privateKey,
[NotNullWhen(false)] out Exception? error)
{
@@ -26,8 +28,54 @@ internal static bool TryParseRsaPkcs1PemKey(
{
if (metadata.TryGetValue("DEK-Info", out var dekInfo))
{
- error = new NotImplementedException($"PKCS#1 key decryption is not implemented.");
- return false;
+ string? password = passwordPrompt();
+ if (password is null)
+ {
+ error = new FormatException($"The key is encrypted but no password was provided.");
+ return false;
+ }
+
+ int dekIdx = dekInfo.IndexOf(',');
+ if (dekIdx == -1)
+ {
+ error = new FormatException($"Failed to decrypt PKCS#1 RSA key, unknown DEK-Info '{dekInfo}'.");
+ return false;
+ }
+
+ Name algoName = new Name(dekInfo.Substring(0, dekIdx));
+ byte[] iv = Convert.FromHexString(dekInfo.AsSpan(dekIdx + 1));
+
+ int keySize;
+ if (algoName == AlgorithmNames.Pkcs1Aes128Cbc)
+ {
+ keySize = 16;
+ }
+ else if (algoName == AlgorithmNames.Pkcs1Aes192Cbc)
+ {
+ keySize = 24;
+ }
+ else if (algoName == AlgorithmNames.Pkcs1Aes256Cbc)
+ {
+ keySize = 32;
+ }
+ else
+ {
+ error = new NotSupportedException($"PKCS#1 RSA encryption algo {algoName} not supported.");
+ return false;
+ }
+
+ // Yes this is an MD5 hash and 1 round, PKCS#1 is old and uses
+ // some weak cryptography components.
+ byte[] key = Pbkdf1(
+ HashAlgorithmName.MD5,
+ Encoding.UTF8.GetBytes(password),
+ iv.AsSpan(0, 8),
+ 1,
+ keySize);
+
+ using Aes aes = Aes.Create();
+ aes.Key = key;
+ keyData = aes.DecryptCbc(keyData, iv, PaddingMode.PKCS7);
}
rsa.ImportRSAPrivateKey(keyData, out int bytesRead);
@@ -48,4 +96,59 @@ internal static bool TryParseRsaPkcs1PemKey(
return false;
}
}
+
+ /// <summary>
+ /// Modified version of PBKDF1 to derive a key for a PKCS#1 encrypted cipher.
+ /// The modifications allow for deriving a key larger than the used hash length.
+ /// </summary>
+ private static byte[] Pbkdf1(
+ HashAlgorithmName hashName,
+ ReadOnlySpan<byte> data,
+ ReadOnlySpan<byte> salt,
+ int rounds,
+ int keySize)
+ {
+ using var hash = IncrementalHash.CreateHash(hashName);
+
+ // Our initial output needs to be a multiple of the hash length.
+ // The desired size is trimmed on return.
+ int totalSize = (keySize + hash.HashLengthInBytes - 1) & ~(hash.HashLengthInBytes - 1);
+ byte[] output = new byte[totalSize];
+
+ // We may need to derive a key that is larger than the hash length.
+ // This is a deviation from the PBKDF1 spec but is needed for PKCS#1
+ // cipher keys. We repeat the process for the same amount of rounds
+ // but start with the existing output data.
+ int outWritten = 0;
+ while (outWritten < output.Length)
+ {
+ ReadOnlySpan<byte> hashData = data;
+ ReadOnlySpan<byte> saltData = salt;
+
+ // First round should include the existing output data if any.
+ if (outWritten > 0)
+ {
+ hash.AppendData(output.AsSpan(0, outWritten));
+ }
+
+ for (int i = 0; i < rounds; i++)
+ {
+ hash.AppendData(hashData);
+ if (saltData.Length > 0)
+ {
+ hash.AppendData(saltData);
+ }
+
+ hash.GetHashAndReset(output.AsSpan(outWritten));
+
+ // Next rounds should use the hash as the data and no salt.
+ hashData = output.AsSpan(outWritten, hash.HashLengthInBytes);
+ saltData = Span<byte>.Empty;
+ }
+
+ outWritten += hash.HashLengthInBytes;
+ }
+
+ return output.AsSpan(0, keySize).ToArray();
+ }
}
diff --git a/src/Tmds.Ssh/PrivateKeyParser.cs b/src/Tmds.Ssh/PrivateKeyParser.cs
index 4d25b5d..9f78c41 100644
--- a/src/Tmds.Ssh/PrivateKeyParser.cs
+++ b/src/Tmds.Ssh/PrivateKeyParser.cs
@@ -103,7 +103,7 @@ internal static bool TryParsePrivateKeyFile(string filename, Func<string?> passw
switch (keyFormat)
{
case "-----BEGIN RSA PRIVATE KEY-----":
- return TryParseRsaPkcs1PemKey(keyData, metadata, out privateKey, out error);
+ return TryParseRsaPkcs1PemKey(keyData, metadata, passwordPrompt, out privateKey, out error);
case "-----BEGIN OPENSSH PRIVATE KEY-----":
return TryParseOpenSshKey(keyData, passwordPrompt, out privateKey, out error);
default:
diff --git a/test/Tmds.Ssh.Tests/PrivateKeyCredentialTests.cs b/test/Tmds.Ssh.Tests/PrivateKeyCredentialTests.cs
index da418e5..551da7a 100644
--- a/test/Tmds.Ssh.Tests/PrivateKeyCredentialTests.cs
+++ b/test/Tmds.Ssh.Tests/PrivateKeyCredentialTests.cs
@@ -19,13 +19,25 @@ public PrivateKeyCredentialTests(SshServer sshServer)
_sshServer = sshServer;
}
- [Fact]
- public async Task Pkcs1RsaKey()
+ [Theory]
+ [InlineData(null)]
+ [InlineData("aes128")]
+ [InlineData("aes192")]
+ [InlineData("aes256")]
+ public async Task Pkcs1RsaKey(string? algo)
{
await RunWithKeyConversion(_sshServer.TestUserIdentityFile, async (string localKey) =>
{
await EncryptSshKey(localKey, "PEM", null, null);
- return new PrivateKeyCredential(localKey);
+
+ if (string.IsNullOrWhiteSpace(algo))
+ {
+ return new PrivateKeyCredential(localKey);
+ }
+
+ await RunBinary("openssl", "pkey", "-in", localKey, "-inform", "PEM", "-out", $"{localKey}.rsa", "-traditional", $"-{algo}", "-passout", $"pass:{TestPassword}");
+ File.Move($"{localKey}.rsa", localKey, overwrite: true);
+ return new PrivateKeyCredential(localKey, TestPassword);
}, async (c) => await c.ConnectAsync());
}This is part of 0.5.0 which was just published to nuget.org.
This issue tries to track what needs to be done to support using encrypted private keys. I tried starting to implement support for this in a branch https://github.com/jborean93/Tmds.Ssh/tree/key-encryption but hit a few blockers. So far there are two types of keys that are supported (PKCS#1 and OpenSSH) but PKCS#8 may be a possible future addition.
RSA PKCS#1: -----BEGIN RSA PRIVATE KEY-----
RSA PKCS#1 is more of a legacy format that was generated by `ssh-keygen -m PEM ...` or older OpenSSL versions. OpenSSL can still generate these keys but on v3 it requires you to add the `-traditional` argument to commands like `genrsa` and `pkey`. You can generate some test keys with: ```bash openssl genrsa -out pkcs1_rsa_plain -traditional openssl pkey -inform PEM -in pkcs1_rsa_plain -out pkcs1_rsa_des3 -des3 -traditional -passout pass:password openssl pkey -inform PEM -in pkcs1_rsa_plain -out pkcs1_rsa_aes128 -aes128 -traditional -passout pass:password openssl pkey -inform PEM -in pkcs1_rsa_plain -out pkcs1_rsa_aes256 -aes256 -traditional -passout pass:password ``` The first problem is that these keys add custom metadata to the key payload that required changes to the parser in the code. This is not insurmountable just something to keep in mind. For example one encrypted with AES 256 looks like ``` -----BEGIN RSA PRIVATE KEY----- Proc-Type: 4,ENCRYPTED DEK-Info: AES-256-CBC,080255EB001B0585AC8F5559BCFA4270 ...base64 data... -----END RSA PRIVATE KEY----- ``` There does not seem to be a builtin way to easily decrypt this format without a 3rd party lib or a lot of code. In my `key-encryption` branch I used `BouncyCastle.Cryptography` with something like the below. ```csharp using var reader = new StringReader(fileContent); using var pemReader = new PemReader(reader, new PlaintextPassword(passphrase.ToArray())); var pemObject = pemReader.ReadObject(); if (pemObject is AsymmetricCipherKeyPair keyPair && keyPair.Private is RsaPrivateCrtKeyParameters rsaParameters) { RSAParameters parameters = DotNetUtilities.ToRSAParameters(rsaParameters); RSA rsa = RSA.Create(); rsa.ImportParameters(parameters); } ``` It looks like `BouncyCastle` also only work with strings that have only ASCII chars. I had a brief look at their code and it seems like they are just casting the chars to a byte which will only work if the char is < 128. https://github.com/bcgit/bc-csharp/blob/63b151ba120be022bde4dfa013371d0259595099/crypto/src/util/Strings.cs#L60-L68 Trying to implement this manually may not be worth the effort as well considering the format is old and it can contain multiple encryption types so I see that to support this we add a dep on `BouncyCastle.Cryptography` and try and submit a PR there to support passwords encoded as UTF-8. I'm unsure what your stance is on 3rd party deps with this library though.TLDR: Requires a dep on
BouncyCastle.Cryptographyand only supports ASCII chars (right now)OpenSSH (currently RSA only): -----BEGIN OPENSSH PRIVATE KEY-----
This is a custom format used by OpenSSH and the code already has a parser for this format it just needs to implement support for decrypting these keys. I wrote [parse_openssh_key.py](https://gist.github.com/jborean93/7c8c21142f75a2e412436c39472c4356) as a tool to help me understand this format and how the encryption works. It's not usable by this library but is a useful tool for testing. There are two cryptographic operations that are involved with these keys: + KDF to generate the cipher key + Cipher decryption OpenSSH only uses the `bcrypt` KDF right now to derive the cipher key from the password and unfortunately there are no libraries I can find that can do this for us. `BouncyCastle` has a BCrypt hashing function but it seems like the BCrypt KDF function used within these keys is different. `BCrypt.NET` also suffers from the same problem where it has the hashing function but not the required KDF function. `SSH.NET` uses their own custom implementation which is quite complex https://github.com/sshnet/SSH.NET/blob/develop/src/Renci.SshNet/Security/Cryptography/Bcrypt.cs. I'm unsure if there are some native libraries we can use but the Python library I used in my test uses the Rust package [bcrypt-pbkdf](https://github.com/RustCrypto/password-hashes/tree/master/bcrypt-pbkdf). Once the KDF problem has been solved the next part is to decrypt the bytes, currently the default cipher algorithm is `aes256-ctr` but you can list most of these ciphers with `ssh -Q ciphers` giving you: + `3des-cbc` + `aes[128|192|256]-[cbc|ctr]` + `aes[128|256]-gcm@openssh.com` + `chacha20-poly1305@openssh.com` We should be able to use the AES ciphers using the builtin AES classes in .NET. The CTR mode isn't natively exposed but it doesn't seem to onerous to use the ECB mode with some extra code to get working. 3DES is also exposed in .NET but I'm unsure if it's worth the effort. The ChaCha20 might be blocked as mentioned in https://github.com/dotnet/runtime/issues/69130.TLDR: No BCrypt KDF implementation available in .NET so either requires our own impl or dep on some native lib.
Possible Future for PKCS#8: -----BEGIN ENCRYPTED PRIVATE KEY-----
It may be worthwhile looking into supporting PKCS#8 encoded keys which is a newer format emitted by `openssl`. [RSA.ImportFromEncryptedPem](https://learn.microsoft.com/en-us/dotnet/api/system.security.cryptography.rsa.importfromencryptedpem?view=net-8.0) and [ECDsa.ImportFromEncryptedPem](https://learn.microsoft.com/en-us/dotnet/api/system.security.cryptography.ecdsa.importfromencryptedpem?view=net-6.0) support this format so shouldn't be hard to add support for. The downside is figuring out what key type to use as it might require some parsing of the ASN.1 data or just to try both. I don't believe an ed25519 based key supports this format, `ssh-keygen -t ed25519 -m PEM` just outputs the key in the OpenSSH format.TLDR: Should be easy to implement but unsure if it's a common enough scenario to warrant the time and effort