Skip to content

Commit 14c652c

Browse files
scott-xuscott-xu
authored
Use System.Security.Cryptography for TripleDesCipher (#1546)
* Use System.Security.Cryptography in DesCipher and TripleDesCipher; Fall back to use BouncyCastle if BCL doesn't support * Drop DesCipher; Replace PKCS7Padding with BouncyCastle's implementation. * Restore `CbcCipherMode` * Restore AesCipherMode; Use BlockImpl instead of BouncyCastleImpl for 3DES-CFB on lower targets. * Restore the xml doc comment
1 parent 29997ae commit 14c652c

26 files changed

+751
-988
lines changed

‎README.md

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,6 @@ The main types provided by this library are:
121121
Private keys in OpenSSL traditional PEM format can be encrypted using one of the following cipher methods:
122122
* DES-EDE3-CBC
123123
* DES-EDE3-CFB
124-
* DES-CBC
125124
* AES-128-CBC
126125
* AES-192-CBC
127126
* AES-256-CBC

‎src/Renci.SshNet/ConnectionInfo.cs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,8 @@
1212
using Renci.SshNet.Security;
1313
using Renci.SshNet.Security.Cryptography;
1414
using Renci.SshNet.Security.Cryptography.Ciphers;
15-
using Renci.SshNet.Security.Cryptography.Ciphers.Modes;
15+
16+
using CipherMode = System.Security.Cryptography.CipherMode;
1617

1718
namespace Renci.SshNet
1819
{
@@ -372,7 +373,7 @@ public ConnectionInfo(string host, int port, string username, ProxyTypes proxyTy
372373
{ "aes128-cbc", new CipherInfo(128, (key, iv) => new AesCipher(key, iv, AesCipherMode.CBC, pkcs7Padding: false)) },
373374
{ "aes192-cbc", new CipherInfo(192, (key, iv) => new AesCipher(key, iv, AesCipherMode.CBC, pkcs7Padding: false)) },
374375
{ "aes256-cbc", new CipherInfo(256, (key, iv) => new AesCipher(key, iv, AesCipherMode.CBC, pkcs7Padding: false)) },
375-
{ "3des-cbc", new CipherInfo(192, (key, iv) => new TripleDesCipher(key, new CbcCipherMode(iv), padding: null)) },
376+
{ "3des-cbc", new CipherInfo(192, (key, iv) => new TripleDesCipher(key, iv, CipherMode.CBC, pkcs7Padding: false)) },
376377
};
377378

378379
HmacAlgorithms = new Dictionary<string, HashInfo>

‎src/Renci.SshNet/PrivateKeyFile.OpenSSH.cs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,8 @@
88
using Renci.SshNet.Security;
99
using Renci.SshNet.Security.Cryptography;
1010
using Renci.SshNet.Security.Cryptography.Ciphers;
11-
using Renci.SshNet.Security.Cryptography.Ciphers.Modes;
11+
12+
using CipherMode = System.Security.Cryptography.CipherMode;
1213

1314
namespace Renci.SshNet
1415
{
@@ -91,7 +92,7 @@ public Key Parse()
9192
{
9293
case "3des-cbc":
9394
ivLength = 8;
94-
cipherInfo = new CipherInfo(192, (key, iv) => new TripleDesCipher(key, new CbcCipherMode(iv), padding: null));
95+
cipherInfo = new CipherInfo(192, (key, iv) => new TripleDesCipher(key, iv, CipherMode.CBC, pkcs7Padding: false));
9596
break;
9697
case "aes128-cbc":
9798
cipherInfo = new CipherInfo(128, (key, iv) => new AesCipher(key, iv, AesCipherMode.CBC, pkcs7Padding: false));

‎src/Renci.SshNet/PrivateKeyFile.PKCS1.cs

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@
99
using Renci.SshNet.Common;
1010
using Renci.SshNet.Security;
1111
using Renci.SshNet.Security.Cryptography.Ciphers;
12-
using Renci.SshNet.Security.Cryptography.Ciphers.Modes;
13-
using Renci.SshNet.Security.Cryptography.Ciphers.Paddings;
12+
13+
using CipherMode = System.Security.Cryptography.CipherMode;
1414

1515
namespace Renci.SshNet
1616
{
@@ -51,13 +51,10 @@ public Key Parse()
5151
switch (_cipherName)
5252
{
5353
case "DES-EDE3-CBC":
54-
cipher = new CipherInfo(192, (key, iv) => new TripleDesCipher(key, new CbcCipherMode(iv), new PKCS7Padding()));
54+
cipher = new CipherInfo(192, (key, iv) => new TripleDesCipher(key, iv, CipherMode.CBC, pkcs7Padding: true));
5555
break;
5656
case "DES-EDE3-CFB":
57-
cipher = new CipherInfo(192, (key, iv) => new TripleDesCipher(key, new CfbCipherMode(iv), padding: null));
58-
break;
59-
case "DES-CBC":
60-
cipher = new CipherInfo(64, (key, iv) => new DesCipher(key, new CbcCipherMode(iv), new PKCS7Padding()));
57+
cipher = new CipherInfo(192, (key, iv) => new TripleDesCipher(key, iv, CipherMode.CFB, pkcs7Padding: false));
6158
break;
6259
case "AES-128-CBC":
6360
cipher = new CipherInfo(128, (key, iv) => new AesCipher(key, iv, AesCipherMode.CBC, pkcs7Padding: true));

‎src/Renci.SshNet/PrivateKeyFile.SSHCOM.cs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,8 @@
77
using Renci.SshNet.Common;
88
using Renci.SshNet.Security;
99
using Renci.SshNet.Security.Cryptography.Ciphers;
10-
using Renci.SshNet.Security.Cryptography.Ciphers.Modes;
10+
11+
using CipherMode = System.Security.Cryptography.CipherMode;
1112

1213
namespace Renci.SshNet
1314
{
@@ -51,7 +52,7 @@ public Key Parse()
5152
}
5253

5354
var key = GetCipherKey(_passPhrase, 192 / 8);
54-
var ssh2Сipher = new TripleDesCipher(key, new CbcCipherMode(new byte[8]), padding: null);
55+
var ssh2Сipher = new TripleDesCipher(key, new byte[8], CipherMode.CBC, pkcs7Padding: false);
5556
keyData = ssh2Сipher.Decrypt(reader.ReadBytes(blobSize));
5657
}
5758
else

‎src/Renci.SshNet/PrivateKeyFile.cs

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,9 +48,6 @@ namespace Renci.SshNet
4848
/// <description>DES-EDE3-CFB</description>
4949
/// </item>
5050
/// <item>
51-
/// <description>DES-CBC</description>
52-
/// </item>
53-
/// <item>
5451
/// <description>AES-128-CBC</description>
5552
/// </item>
5653
/// <item>

‎src/Renci.SshNet/Security/Cryptography/BlockCipher.cs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
using System;
22

3+
using Org.BouncyCastle.Crypto.Paddings;
4+
35
using Renci.SshNet.Common;
46
using Renci.SshNet.Security.Cryptography.Ciphers;
57
using Renci.SshNet.Security.Cryptography.Ciphers.Modes;
@@ -13,7 +15,7 @@ public abstract class BlockCipher : SymmetricCipher
1315
{
1416
private readonly CipherMode _mode;
1517

16-
private readonly CipherPadding _padding;
18+
private readonly IBlockCipherPadding _padding;
1719

1820
/// <summary>
1921
/// Gets the size of the block in bytes.
@@ -56,7 +58,7 @@ public byte BlockSize
5658
/// <param name="mode">Cipher mode.</param>
5759
/// <param name="padding">Cipher padding.</param>
5860
/// <exception cref="ArgumentNullException"><paramref name="key"/> is <see langword="null"/>.</exception>
59-
protected BlockCipher(byte[] key, byte blockSize, CipherMode mode, CipherPadding padding)
61+
protected BlockCipher(byte[] key, byte blockSize, CipherMode mode, IBlockCipherPadding padding)
6062
: base(key)
6163
{
6264
_blockSize = blockSize;
@@ -81,7 +83,9 @@ public override byte[] Encrypt(byte[] input, int offset, int length)
8183
if (_padding is not null)
8284
{
8385
paddingLength = _blockSize - (length % _blockSize);
84-
input = _padding.Pad(input, offset, length, paddingLength);
86+
input = input.Take(offset, length);
87+
Array.Resize(ref input, length + paddingLength);
88+
_ = _padding.AddPadding(input, length);
8589
length += paddingLength;
8690
offset = 0;
8791
}

‎src/Renci.SshNet/Security/Cryptography/Ciphers/AesCipher.BclImpl.cs

Lines changed: 12 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,8 @@ public override byte[] Encrypt(byte[] input, int offset, int length)
5252
{
5353
if (_aes.Mode is System.Security.Cryptography.CipherMode.CFB or System.Security.Cryptography.CipherMode.OFB)
5454
{
55+
// Manually pad the input for cfb and ofb cipher mode as BCL doesn't support partial block.
56+
// See https://github.com/dotnet/runtime/blob/e7d837da5b1aacd9325a8b8f2214cfaf4d3f0ff6/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/SymmetricPadding.cs#L20-L21
5557
paddingLength = BlockSize - (length % BlockSize);
5658
input = input.Take(offset, length);
5759
length += paddingLength;
@@ -69,6 +71,7 @@ public override byte[] Encrypt(byte[] input, int offset, int length)
6971

7072
if (paddingLength > 0)
7173
{
74+
// Manually unpad the output.
7275
Array.Resize(ref output, output.Length - paddingLength);
7376
}
7477

@@ -89,11 +92,12 @@ public override byte[] Decrypt(byte[] input, int offset, int length)
8992
{
9093
if (_aes.Mode is System.Security.Cryptography.CipherMode.CFB or System.Security.Cryptography.CipherMode.OFB)
9194
{
95+
// Manually pad the input for cfb and ofb cipher mode as BCL doesn't support partial block.
96+
// See https://github.com/dotnet/runtime/blob/e7d837da5b1aacd9325a8b8f2214cfaf4d3f0ff6/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/SymmetricPadding.cs#L20-L21
9297
paddingLength = BlockSize - (length % BlockSize);
93-
var newInput = new byte[input.Length + paddingLength];
94-
Buffer.BlockCopy(input, offset, newInput, 0, length);
95-
input = newInput;
96-
length = input.Length;
98+
input = input.Take(offset, length);
99+
length += paddingLength;
100+
Array.Resize(ref input, length);
97101
offset = 0;
98102
}
99103
}
@@ -107,6 +111,7 @@ public override byte[] Decrypt(byte[] input, int offset, int length)
107111

108112
if (paddingLength > 0)
109113
{
114+
// Manually unpad the output.
110115
Array.Resize(ref output, output.Length - paddingLength);
111116
}
112117

@@ -123,21 +128,11 @@ public override int DecryptBlock(byte[] inputBuffer, int inputOffset, int inputC
123128
throw new NotImplementedException($"Invalid usage of {nameof(DecryptBlock)}.");
124129
}
125130

126-
private void Dispose(bool disposing)
127-
{
128-
if (disposing)
129-
{
130-
_aes.Dispose();
131-
_encryptor.Dispose();
132-
_decryptor.Dispose();
133-
}
134-
}
135-
136131
public void Dispose()
137132
{
138-
// Do not change this code. Put cleanup code in 'Dispose(bool disposing)' method
139-
Dispose(disposing: true);
140-
GC.SuppressFinalize(this);
133+
_aes.Dispose();
134+
_encryptor.Dispose();
135+
_decryptor.Dispose();
141136
}
142137
}
143138
}

‎src/Renci.SshNet/Security/Cryptography/Ciphers/AesCipher.BlockImpl.cs

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
using System;
22
using System.Security.Cryptography;
33

4+
using Org.BouncyCastle.Crypto.Paddings;
5+
46
namespace Renci.SshNet.Security.Cryptography.Ciphers
57
{
68
public partial class AesCipher
@@ -11,7 +13,7 @@ private sealed class BlockImpl : BlockCipher, IDisposable
1113
private readonly ICryptoTransform _encryptor;
1214
private readonly ICryptoTransform _decryptor;
1315

14-
public BlockImpl(byte[] key, CipherMode mode, CipherPadding padding)
16+
public BlockImpl(byte[] key, CipherMode mode, IBlockCipherPadding padding)
1517
: base(key, 16, mode, padding)
1618
{
1719
var aes = Aes.Create();
@@ -33,21 +35,11 @@ public override int DecryptBlock(byte[] inputBuffer, int inputOffset, int inputC
3335
return _decryptor.TransformBlock(inputBuffer, inputOffset, inputCount, outputBuffer, outputOffset);
3436
}
3537

36-
private void Dispose(bool disposing)
37-
{
38-
if (disposing)
39-
{
40-
_aes.Dispose();
41-
_encryptor.Dispose();
42-
_decryptor.Dispose();
43-
}
44-
}
45-
4638
public void Dispose()
4739
{
48-
// Do not change this code. Put cleanup code in 'Dispose(bool disposing)' method
49-
Dispose(disposing: true);
50-
GC.SuppressFinalize(this);
40+
_aes.Dispose();
41+
_encryptor.Dispose();
42+
_decryptor.Dispose();
5143
}
5244
}
5345
}

‎src/Renci.SshNet/Security/Cryptography/Ciphers/AesCipher.CtrImpl.cs

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -105,20 +105,10 @@ private static void ArrayXOR(byte[] buffer, byte[] data, int offset, int length)
105105
}
106106
}
107107

108-
private void Dispose(bool disposing)
109-
{
110-
if (disposing)
111-
{
112-
_aes.Dispose();
113-
_encryptor.Dispose();
114-
}
115-
}
116-
117108
public void Dispose()
118109
{
119-
// Do not change this code. Put cleanup code in 'Dispose(bool disposing)' method
120-
Dispose(disposing: true);
121-
GC.SuppressFinalize(this);
110+
_aes.Dispose();
111+
_encryptor.Dispose();
122112
}
123113
}
124114
}

‎src/Renci.SshNet/Security/Cryptography/Ciphers/AesCipher.cs

Lines changed: 8 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
using System;
22
using System.Security.Cryptography;
33

4+
using Org.BouncyCastle.Crypto.Paddings;
5+
46
using Renci.SshNet.Security.Cryptography.Ciphers.Modes;
5-
using Renci.SshNet.Security.Cryptography.Ciphers.Paddings;
67

78
namespace Renci.SshNet.Security.Cryptography.Ciphers
89
{
@@ -17,8 +18,8 @@ public sealed partial class AesCipher : BlockCipher, IDisposable
1718
/// Initializes a new instance of the <see cref="AesCipher"/> class.
1819
/// </summary>
1920
/// <param name="key">The key.</param>
20-
/// <param name="mode">The mode.</param>
2121
/// <param name="iv">The IV.</param>
22+
/// <param name="mode">The mode.</param>
2223
/// <param name="pkcs7Padding">Enable PKCS7 padding.</param>
2324
/// <exception cref="ArgumentNullException"><paramref name="key"/> is <see langword="null"/>.</exception>
2425
/// <exception cref="ArgumentException">Keysize is not valid for this algorithm.</exception>
@@ -28,13 +29,13 @@ public AesCipher(byte[] key, byte[] iv, AesCipherMode mode, bool pkcs7Padding =
2829
if (mode == AesCipherMode.OFB)
2930
{
3031
// OFB is not supported on modern .NET
31-
_impl = new BlockImpl(key, new OfbCipherMode(iv), pkcs7Padding ? new PKCS7Padding() : null);
32+
_impl = new BlockImpl(key, new OfbCipherMode(iv), pkcs7Padding ? new Pkcs7Padding() : null);
3233
}
3334
#if !NET6_0_OR_GREATER
3435
else if (mode == AesCipherMode.CFB)
3536
{
3637
// CFB not supported on NetStandard 2.1
37-
_impl = new BlockImpl(key, new CfbCipherMode(iv), pkcs7Padding ? new PKCS7Padding() : null);
38+
_impl = new BlockImpl(key, new CfbCipherMode(iv), pkcs7Padding ? new Pkcs7Padding() : null);
3839
}
3940
#endif
4041
else if (mode == AesCipherMode.CTR)
@@ -76,24 +77,13 @@ public override byte[] Decrypt(byte[] input, int offset, int length)
7677
return _impl.Decrypt(input, offset, length);
7778
}
7879

79-
/// <summary>
80-
/// Dispose the instance.
81-
/// </summary>
82-
/// <param name="disposing">Set to True to dispose of resouces.</param>
83-
public void Dispose(bool disposing)
80+
/// <inheritdoc/>
81+
public void Dispose()
8482
{
85-
if (disposing && _impl is IDisposable disposableImpl)
83+
if (_impl is IDisposable disposableImpl)
8684
{
8785
disposableImpl.Dispose();
8886
}
8987
}
90-
91-
/// <inheritdoc/>
92-
public void Dispose()
93-
{
94-
// Do not change this code. Put cleanup code in 'Dispose(bool disposing)' method
95-
Dispose(disposing: true);
96-
GC.SuppressFinalize(this);
97-
}
9888
}
9989
}

0 commit comments

Comments
 (0)