Skip to content

Commit 29997ae

Browse files
scott-xuRob-Hague
scott-xu
and
Rob-Hague
authored
Add padding when encrypt and remove padding when decrypt (#1545)
* Tighten private key checking to reveal padding issue * `Encrypt` should take into account padding for length of `inputBuffer` passed to `EncryptBlock` if padding is specified, no matter input is divisible or not. * `Decrypt` should take into account unpadding for the final output if padding is specified. * `Decrypt` should take into account *manual* padding for length of `inputBuffer` passed to `DecryptBlock` and unpadding for the final output if padding is not specified and mode is CFB or OFB. * `Encrypt` should take into account *manual* padding for length of `inputBuffer` passed to `EncryptBlock` and unpadding for the final output if padding is not specified and mode is CFB or OFB. * Rectify DES cipher tests. There's no padding in the data. * Borrow `PadCount` method from BouncyCastle * Manually pad input in CTR mode as well. Update AesCipherTest. Co-Authored-By: Rob Hague <[email protected]> * Manually pad/unpad for Aes CFB/OFB mode * Update test/Renci.SshNet.Tests/Classes/Security/Cryptography/Ciphers/AesCipherTest.Gen.cs.txt Co-authored-by: Rob Hague <[email protected]> * Re-generate AES cipher tests --------- Co-authored-by: Rob Hague <[email protected]> Co-authored-by: Rob Hague <[email protected]>
1 parent 42d75bc commit 29997ae

File tree

13 files changed

+1196
-550
lines changed

13 files changed

+1196
-550
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ public Key Parse()
5454
cipher = new CipherInfo(192, (key, iv) => new TripleDesCipher(key, new CbcCipherMode(iv), new PKCS7Padding()));
5555
break;
5656
case "DES-EDE3-CFB":
57-
cipher = new CipherInfo(192, (key, iv) => new TripleDesCipher(key, new CfbCipherMode(iv), new PKCS7Padding()));
57+
cipher = new CipherInfo(192, (key, iv) => new TripleDesCipher(key, new CfbCipherMode(iv), padding: null));
5858
break;
5959
case "DES-CBC":
6060
cipher = new CipherInfo(64, (key, iv) => new DesCipher(key, new CbcCipherMode(iv), new PKCS7Padding()));

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
using Renci.SshNet.Security;
99
using Renci.SshNet.Security.Cryptography.Ciphers;
1010
using Renci.SshNet.Security.Cryptography.Ciphers.Modes;
11-
using Renci.SshNet.Security.Cryptography.Ciphers.Paddings;
1211

1312
namespace Renci.SshNet
1413
{
@@ -52,7 +51,7 @@ public Key Parse()
5251
}
5352

5453
var key = GetCipherKey(_passPhrase, 192 / 8);
55-
var ssh2Сipher = new TripleDesCipher(key, new CbcCipherMode(new byte[8]), new PKCS7Padding());
54+
var ssh2Сipher = new TripleDesCipher(key, new CbcCipherMode(new byte[8]), padding: null);
5655
keyData = ssh2Сipher.Decrypt(reader.ReadBytes(blobSize));
5756
}
5857
else

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

Lines changed: 45 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
using System;
22

3+
using Renci.SshNet.Common;
34
using Renci.SshNet.Security.Cryptography.Ciphers;
5+
using Renci.SshNet.Security.Cryptography.Ciphers.Modes;
46

57
namespace Renci.SshNet.Security.Cryptography
68
{
@@ -75,18 +77,29 @@ protected BlockCipher(byte[] key, byte blockSize, CipherMode mode, CipherPadding
7577
/// </returns>
7678
public override byte[] Encrypt(byte[] input, int offset, int length)
7779
{
78-
if (length % _blockSize > 0)
80+
var paddingLength = 0;
81+
if (_padding is not null)
7982
{
80-
if (_padding is null)
81-
{
82-
throw new ArgumentException(string.Format("The data block size is incorrect for {0}.", GetType().Name), "data");
83-
}
84-
85-
var paddingLength = _blockSize - (length % _blockSize);
83+
paddingLength = _blockSize - (length % _blockSize);
8684
input = _padding.Pad(input, offset, length, paddingLength);
8785
length += paddingLength;
8886
offset = 0;
8987
}
88+
else if (length % _blockSize > 0)
89+
{
90+
if (_mode is CfbCipherMode or OfbCipherMode or CtrCipherMode)
91+
{
92+
paddingLength = _blockSize - (length % _blockSize);
93+
input = input.Take(offset, length);
94+
length += paddingLength;
95+
Array.Resize(ref input, length);
96+
offset = 0;
97+
}
98+
else
99+
{
100+
throw new ArgumentException(string.Format("The data block size is incorrect for {0}.", GetType().Name), "data");
101+
}
102+
}
90103

91104
var output = new byte[length];
92105
var writtenBytes = 0;
@@ -108,6 +121,11 @@ public override byte[] Encrypt(byte[] input, int offset, int length)
108121
throw new InvalidOperationException("Encryption error.");
109122
}
110123

124+
if (_padding is null && paddingLength > 0)
125+
{
126+
Array.Resize(ref output, output.Length - paddingLength);
127+
}
128+
111129
return output;
112130
}
113131

@@ -122,16 +140,21 @@ public override byte[] Encrypt(byte[] input, int offset, int length)
122140
/// </returns>
123141
public override byte[] Decrypt(byte[] input, int offset, int length)
124142
{
143+
var paddingLength = 0;
125144
if (length % _blockSize > 0)
126145
{
127-
if (_padding is null)
146+
if (_padding is null && _mode is CfbCipherMode or OfbCipherMode or CtrCipherMode)
147+
{
148+
paddingLength = _blockSize - (length % _blockSize);
149+
input = input.Take(offset, length);
150+
length += paddingLength;
151+
Array.Resize(ref input, length);
152+
offset = 0;
153+
}
154+
else
128155
{
129156
throw new ArgumentException(string.Format("The data block size is incorrect for {0}.", GetType().Name), "data");
130157
}
131-
132-
input = _padding.Pad(_blockSize, input, offset, length);
133-
offset = 0;
134-
length = input.Length;
135158
}
136159

137160
var output = new byte[length];
@@ -154,6 +177,16 @@ public override byte[] Decrypt(byte[] input, int offset, int length)
154177
throw new InvalidOperationException("Encryption error.");
155178
}
156179

180+
if (_padding is not null)
181+
{
182+
paddingLength = _padding.PadCount(output);
183+
}
184+
185+
if (paddingLength > 0)
186+
{
187+
Array.Resize(ref output, output.Length - paddingLength);
188+
}
189+
157190
return output;
158191
}
159192

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

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,12 +47,31 @@ public override byte[] Encrypt(byte[] input, int offset, int length)
4747
return _encryptor.TransformFinalBlock(input, offset, length);
4848
}
4949

50+
var paddingLength = 0;
51+
if (length % BlockSize > 0)
52+
{
53+
if (_aes.Mode is System.Security.Cryptography.CipherMode.CFB or System.Security.Cryptography.CipherMode.OFB)
54+
{
55+
paddingLength = BlockSize - (length % BlockSize);
56+
input = input.Take(offset, length);
57+
length += paddingLength;
58+
Array.Resize(ref input, length);
59+
offset = 0;
60+
}
61+
}
62+
5063
// Otherwise, (the most important case) assume this instance is
5164
// used for one direction of an SSH connection, whereby the
5265
// encrypted data in all packets are considered a single data
5366
// stream i.e. we do not want to reset the state between calls to Encrypt.
5467
var output = new byte[length];
5568
_ = _encryptor.TransformBlock(input, offset, length, output, 0);
69+
70+
if (paddingLength > 0)
71+
{
72+
Array.Resize(ref output, output.Length - paddingLength);
73+
}
74+
5675
return output;
5776
}
5877

@@ -65,12 +84,32 @@ public override byte[] Decrypt(byte[] input, int offset, int length)
6584
return _decryptor.TransformFinalBlock(input, offset, length);
6685
}
6786

87+
var paddingLength = 0;
88+
if (length % BlockSize > 0)
89+
{
90+
if (_aes.Mode is System.Security.Cryptography.CipherMode.CFB or System.Security.Cryptography.CipherMode.OFB)
91+
{
92+
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;
97+
offset = 0;
98+
}
99+
}
100+
68101
// Otherwise, (the most important case) assume this instance is
69102
// used for one direction of an SSH connection, whereby the
70103
// encrypted data in all packets are considered a single data
71104
// stream i.e. we do not want to reset the state between calls to Decrypt.
72105
var output = new byte[length];
73106
_ = _decryptor.TransformBlock(input, offset, length, output, 0);
107+
108+
if (paddingLength > 0)
109+
{
110+
Array.Resize(ref output, output.Length - paddingLength);
111+
}
112+
74113
return output;
75114
}
76115

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,5 +54,12 @@ public byte[] Pad(byte[] input, int paddinglength)
5454
/// The padded data array.
5555
/// </returns>
5656
public abstract byte[] Pad(byte[] input, int offset, int length, int paddinglength);
57+
58+
/// <summary>
59+
/// Gets the padd count from the specified input.
60+
/// </summary>
61+
/// <param name="input">The input.</param>
62+
/// <returns>The padd count.</returns>
63+
public abstract int PadCount(byte[] input);
5764
}
5865
}

‎src/Renci.SshNet/Security/Cryptography/Ciphers/Paddings/PKCS7Padding.cs

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

3+
using Renci.SshNet.Common;
4+
35
namespace Renci.SshNet.Security.Cryptography.Ciphers.Paddings
46
{
57
/// <summary>
@@ -45,5 +47,26 @@ public override byte[] Pad(byte[] input, int offset, int length, int paddingleng
4547

4648
return output;
4749
}
50+
51+
/// <inheritdoc/>
52+
public override int PadCount(byte[] input)
53+
{
54+
var padValue = input[input.Length - 1];
55+
int count = padValue;
56+
var position = input.Length - count;
57+
58+
var failed = (position | (count - 1)) >> 31;
59+
for (var i = 0; i < input.Length; ++i)
60+
{
61+
failed |= (input[i] ^ padValue) & ~((i - position) >> 31);
62+
}
63+
64+
if (failed != 0)
65+
{
66+
throw new SshException("pad block corrupted");
67+
}
68+
69+
return count;
70+
}
4871
}
4972
}

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

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -113,16 +113,19 @@ public DsaKey(byte[] privateKeyData)
113113
{
114114
ThrowHelper.ThrowIfNull(privateKeyData);
115115

116-
var der = new AsnReader(privateKeyData, AsnEncodingRules.DER).ReadSequence();
117-
_ = der.ReadInteger(); // skip version
116+
var keyReader = new AsnReader(privateKeyData, AsnEncodingRules.DER);
117+
var sequenceReader = keyReader.ReadSequence();
118+
keyReader.ThrowIfNotEmpty();
118119

119-
P = der.ReadInteger();
120-
Q = der.ReadInteger();
121-
G = der.ReadInteger();
122-
Y = der.ReadInteger();
123-
X = der.ReadInteger();
120+
_ = sequenceReader.ReadInteger(); // skip version
124121

125-
der.ThrowIfNotEmpty();
122+
P = sequenceReader.ReadInteger();
123+
Q = sequenceReader.ReadInteger();
124+
G = sequenceReader.ReadInteger();
125+
Y = sequenceReader.ReadInteger();
126+
X = sequenceReader.ReadInteger();
127+
128+
sequenceReader.ThrowIfNotEmpty();
126129

127130
DSA = LoadDSA();
128131
}

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

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -218,18 +218,21 @@ public EcdsaKey(string curve, byte[] publickey, byte[] privatekey)
218218
/// <param name="data">DER encoded private key data.</param>
219219
public EcdsaKey(byte[] data)
220220
{
221-
var der = new AsnReader(data, AsnEncodingRules.DER).ReadSequence();
222-
_ = der.ReadInteger(); // skip version
221+
var keyReader = new AsnReader(data, AsnEncodingRules.DER);
222+
var sequenceReader = keyReader.ReadSequence();
223+
keyReader.ThrowIfNotEmpty();
223224

224-
var privatekey = der.ReadOctetString().TrimLeadingZeros();
225+
_ = sequenceReader.ReadInteger(); // skip version
225226

226-
var s0 = der.ReadSequence(new Asn1Tag(TagClass.ContextSpecific, 0, isConstructed: true));
227+
var privatekey = sequenceReader.ReadOctetString().TrimLeadingZeros();
228+
229+
var s0 = sequenceReader.ReadSequence(new Asn1Tag(TagClass.ContextSpecific, 0, isConstructed: true));
227230
var curve = s0.ReadObjectIdentifier();
228231

229-
var s1 = der.ReadSequence(new Asn1Tag(TagClass.ContextSpecific, 1, isConstructed: true));
232+
var s1 = sequenceReader.ReadSequence(new Asn1Tag(TagClass.ContextSpecific, 1, isConstructed: true));
230233
var pubkey = s1.ReadBitString(out _);
231234

232-
der.ThrowIfNotEmpty();
235+
sequenceReader.ThrowIfNotEmpty();
233236

234237
_impl = Import(curve, pubkey, privatekey);
235238
}

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

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -161,19 +161,22 @@ public RsaKey(byte[] privateKeyData)
161161
{
162162
ThrowHelper.ThrowIfNull(privateKeyData);
163163

164-
var der = new AsnReader(privateKeyData, AsnEncodingRules.DER).ReadSequence();
165-
_ = der.ReadInteger(); // skip version
166-
167-
Modulus = der.ReadInteger();
168-
Exponent = der.ReadInteger();
169-
D = der.ReadInteger();
170-
P = der.ReadInteger();
171-
Q = der.ReadInteger();
172-
DP = der.ReadInteger();
173-
DQ = der.ReadInteger();
174-
InverseQ = der.ReadInteger();
175-
176-
der.ThrowIfNotEmpty();
164+
var keyReader = new AsnReader(privateKeyData, AsnEncodingRules.DER);
165+
var sequenceReader = keyReader.ReadSequence();
166+
keyReader.ThrowIfNotEmpty();
167+
168+
_ = sequenceReader.ReadInteger(); // skip version
169+
170+
Modulus = sequenceReader.ReadInteger();
171+
Exponent = sequenceReader.ReadInteger();
172+
D = sequenceReader.ReadInteger();
173+
P = sequenceReader.ReadInteger();
174+
Q = sequenceReader.ReadInteger();
175+
DP = sequenceReader.ReadInteger();
176+
DQ = sequenceReader.ReadInteger();
177+
InverseQ = sequenceReader.ReadInteger();
178+
179+
sequenceReader.ThrowIfNotEmpty();
177180

178181
RSA = RSA.Create();
179182
RSA.ImportParameters(GetRSAParameters());

0 commit comments

Comments
 (0)