Skip to content

Commit 24838e6

Browse files
Rob-HagueWojciechNagorski
Rob-Hague
and
WojciechNagorski
authored
Make keys immutable (#1264)
This makes it easier to reason about Key instances in e.g. DigitalSignature implementations, because we know that the Key is initialised with its data and will not change. Co-authored-by: Wojciech Nagórski <[email protected]>
1 parent e998d87 commit 24838e6

File tree

15 files changed

+295
-516
lines changed

15 files changed

+295
-516
lines changed

‎src/Renci.SshNet/Common/SshData.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,7 @@ protected ulong ReadUInt64()
231231
/// <returns>
232232
/// The <see cref="string"/> that was read.
233233
/// </returns>
234-
protected string ReadString(Encoding encoding)
234+
protected string ReadString(Encoding encoding = null)
235235
{
236236
return _stream.ReadString(encoding);
237237
}

‎src/Renci.SshNet/Common/SshDataStream.cs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -249,12 +249,14 @@ public ulong ReadUInt64()
249249
/// <summary>
250250
/// Reads the next <see cref="string"/> data type from the SSH data stream.
251251
/// </summary>
252-
/// <param name="encoding">The character encoding to use.</param>
252+
/// <param name="encoding">The character encoding to use. Defaults to <see cref="Encoding.UTF8"/>.</param>
253253
/// <returns>
254254
/// The <see cref="string"/> read from the SSH data stream.
255255
/// </returns>
256-
public string ReadString(Encoding encoding)
256+
public string ReadString(Encoding encoding = null)
257257
{
258+
encoding ??= Encoding.UTF8;
259+
258260
var length = ReadUInt32();
259261

260262
if (length > int.MaxValue)

‎src/Renci.SshNet/ConnectionInfo.cs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -396,16 +396,16 @@ public ConnectionInfo(string host, int port, string username, ProxyTypes proxyTy
396396

397397
HostKeyAlgorithms = new Dictionary<string, Func<byte[], KeyHostAlgorithm>>
398398
{
399-
{ "ssh-ed25519", data => new KeyHostAlgorithm("ssh-ed25519", new ED25519Key(), data) },
400-
{ "ecdsa-sha2-nistp256", data => new KeyHostAlgorithm("ecdsa-sha2-nistp256", new EcdsaKey(), data) },
401-
{ "ecdsa-sha2-nistp384", data => new KeyHostAlgorithm("ecdsa-sha2-nistp384", new EcdsaKey(), data) },
402-
{ "ecdsa-sha2-nistp521", data => new KeyHostAlgorithm("ecdsa-sha2-nistp521", new EcdsaKey(), data) },
399+
{ "ssh-ed25519", data => new KeyHostAlgorithm("ssh-ed25519", new ED25519Key(new SshKeyData(data))) },
400+
{ "ecdsa-sha2-nistp256", data => new KeyHostAlgorithm("ecdsa-sha2-nistp256", new EcdsaKey(new SshKeyData(data))) },
401+
{ "ecdsa-sha2-nistp384", data => new KeyHostAlgorithm("ecdsa-sha2-nistp384", new EcdsaKey(new SshKeyData(data))) },
402+
{ "ecdsa-sha2-nistp521", data => new KeyHostAlgorithm("ecdsa-sha2-nistp521", new EcdsaKey(new SshKeyData(data))) },
403403
#pragma warning disable SA1107 // Code should not contain multiple statements on one line
404-
{ "rsa-sha2-512", data => { var key = new RsaKey(); return new KeyHostAlgorithm("rsa-sha2-512", key, data, new RsaDigitalSignature(key, HashAlgorithmName.SHA512)); } },
405-
{ "rsa-sha2-256", data => { var key = new RsaKey(); return new KeyHostAlgorithm("rsa-sha2-256", key, data, new RsaDigitalSignature(key, HashAlgorithmName.SHA256)); } },
404+
{ "rsa-sha2-512", data => { var key = new RsaKey(new SshKeyData(data)); return new KeyHostAlgorithm("rsa-sha2-512", key, new RsaDigitalSignature(key, HashAlgorithmName.SHA512)); } },
405+
{ "rsa-sha2-256", data => { var key = new RsaKey(new SshKeyData(data)); return new KeyHostAlgorithm("rsa-sha2-256", key, new RsaDigitalSignature(key, HashAlgorithmName.SHA256)); } },
406406
#pragma warning restore SA1107 // Code should not contain multiple statements on one line
407-
{ "ssh-rsa", data => new KeyHostAlgorithm("ssh-rsa", new RsaKey(), data) },
408-
{ "ssh-dss", data => new KeyHostAlgorithm("ssh-dss", new DsaKey(), data) },
407+
{ "ssh-rsa", data => new KeyHostAlgorithm("ssh-rsa", new RsaKey(new SshKeyData(data))) },
408+
{ "ssh-dss", data => new KeyHostAlgorithm("ssh-dss", new DsaKey(new SshKeyData(data))) },
409409
};
410410

411411
CompressionAlgorithms = new Dictionary<string, Type>

‎src/Renci.SshNet/PrivateKeyFile.cs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -573,12 +573,14 @@ private static Key ParseOpenSshV1Key(byte[] keyFileData, string passPhrase)
573573
switch (keyType)
574574
{
575575
case "ssh-ed25519":
576-
// public key
577-
publicKey = privateKeyReader.ReadBignum2();
576+
// https://datatracker.ietf.org/doc/html/draft-miller-ssh-agent-11#section-3.2.3
578577

579-
// private key
578+
// ENC(A)
579+
_ = privateKeyReader.ReadBignum2();
580+
581+
// k || ENC(A)
580582
unencryptedPrivateKey = privateKeyReader.ReadBignum2();
581-
parsedKey = new ED25519Key(publicKey.Reverse(), unencryptedPrivateKey);
583+
parsedKey = new ED25519Key(unencryptedPrivateKey);
582584
break;
583585
case "ecdsa-sha2-nistp256":
584586
case "ecdsa-sha2-nistp384":

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

Lines changed: 53 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
using System;
2+
23
using Renci.SshNet.Common;
34
using Renci.SshNet.Security.Cryptography;
45

@@ -15,57 +16,27 @@ public class DsaKey : Key, IDisposable
1516
/// <summary>
1617
/// Gets the P.
1718
/// </summary>
18-
public BigInteger P
19-
{
20-
get
21-
{
22-
return _privateKey[0];
23-
}
24-
}
19+
public BigInteger P { get; }
2520

2621
/// <summary>
2722
/// Gets the Q.
2823
/// </summary>
29-
public BigInteger Q
30-
{
31-
get
32-
{
33-
return _privateKey[1];
34-
}
35-
}
24+
public BigInteger Q { get; }
3625

3726
/// <summary>
3827
/// Gets the G.
3928
/// </summary>
40-
public BigInteger G
41-
{
42-
get
43-
{
44-
return _privateKey[2];
45-
}
46-
}
29+
public BigInteger G { get; }
4730

4831
/// <summary>
4932
/// Gets public key Y.
5033
/// </summary>
51-
public BigInteger Y
52-
{
53-
get
54-
{
55-
return _privateKey[3];
56-
}
57-
}
34+
public BigInteger Y { get; }
5835

5936
/// <summary>
6037
/// Gets private key X.
6138
/// </summary>
62-
public BigInteger X
63-
{
64-
get
65-
{
66-
return _privateKey[4];
67-
}
68-
}
39+
public BigInteger X { get; }
6940

7041
/// <summary>
7142
/// Gets the length of the key.
@@ -94,46 +65,70 @@ protected internal override DigitalSignature DigitalSignature
9465
}
9566

9667
/// <summary>
97-
/// Gets or sets the public.
68+
/// Gets the DSA public key.
9869
/// </summary>
9970
/// <value>
100-
/// The public.
71+
/// An array whose values are:
72+
/// <list>
73+
/// <item><term>0</term><description><see cref="P"/></description></item>
74+
/// <item><term>1</term><description><see cref="Q"/></description></item>
75+
/// <item><term>2</term><description><see cref="G"/></description></item>
76+
/// <item><term>3</term><description><see cref="Y"/></description></item>
77+
/// </list>
10178
/// </value>
10279
public override BigInteger[] Public
10380
{
10481
get
10582
{
10683
return new[] { P, Q, G, Y };
10784
}
108-
set
109-
{
110-
if (value.Length != 4)
111-
{
112-
throw new InvalidOperationException("Invalid public key.");
113-
}
114-
115-
_privateKey = value;
116-
}
11785
}
11886

11987
/// <summary>
12088
/// Initializes a new instance of the <see cref="DsaKey"/> class.
12189
/// </summary>
122-
public DsaKey()
90+
/// <param name="publicKeyData">The encoded public key data.</param>
91+
public DsaKey(SshKeyData publicKeyData)
12392
{
124-
_privateKey = new BigInteger[5];
93+
if (publicKeyData is null)
94+
{
95+
throw new ArgumentNullException(nameof(publicKeyData));
96+
}
97+
98+
if (publicKeyData.Name != "ssh-dss" || publicKeyData.Keys.Length != 4)
99+
{
100+
throw new ArgumentException($"Invalid DSA public key data. ({publicKeyData.Name}, {publicKeyData.Keys.Length}).", nameof(publicKeyData));
101+
}
102+
103+
P = publicKeyData.Keys[0];
104+
Q = publicKeyData.Keys[1];
105+
G = publicKeyData.Keys[2];
106+
Y = publicKeyData.Keys[3];
125107
}
126108

127109
/// <summary>
128110
/// Initializes a new instance of the <see cref="DsaKey"/> class.
129111
/// </summary>
130-
/// <param name="data">DER encoded private key data.</param>
131-
public DsaKey(byte[] data)
132-
: base(data)
112+
/// <param name="privateKeyData">DER encoded private key data.</param>
113+
public DsaKey(byte[] privateKeyData)
133114
{
134-
if (_privateKey.Length != 5)
115+
if (privateKeyData is null)
116+
{
117+
throw new ArgumentNullException(nameof(privateKeyData));
118+
}
119+
120+
var der = new DerData(privateKeyData);
121+
_ = der.ReadBigInteger(); // skip version
122+
123+
P = der.ReadBigInteger();
124+
Q = der.ReadBigInteger();
125+
G = der.ReadBigInteger();
126+
Y = der.ReadBigInteger();
127+
X = der.ReadBigInteger();
128+
129+
if (!der.IsEndOfData)
135130
{
136-
throw new InvalidOperationException("Invalid private key.");
131+
throw new InvalidOperationException("Invalid private key (expected EOF).");
137132
}
138133
}
139134

@@ -147,7 +142,11 @@ public DsaKey(byte[] data)
147142
/// <param name="x">The x.</param>
148143
public DsaKey(BigInteger p, BigInteger q, BigInteger g, BigInteger y, BigInteger x)
149144
{
150-
_privateKey = new BigInteger[5] { p, q, g, y, x };
145+
P = p;
146+
Q = q;
147+
G = g;
148+
Y = y;
149+
X = x;
151150
}
152151

153152
/// <summary>

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

Lines changed: 26 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,7 @@ namespace Renci.SshNet.Security
1111
/// </summary>
1212
public class ED25519Key : Key, IDisposable
1313
{
14-
#pragma warning disable IDE1006 // Naming Styles
15-
#pragma warning disable SX1309 // Field names should begin with underscore
16-
private readonly byte[] privateKey = new byte[Ed25519.ExpandedPrivateKeySizeInBytes];
17-
#pragma warning restore SX1309 // Field names should begin with underscore
18-
#pragma warning restore IDE1006 // Naming Styles
1914
private ED25519DigitalSignature _digitalSignature;
20-
private byte[] _publicKey = new byte[Ed25519.PublicKeySizeInBytes];
2115
private bool _isDisposed;
2216

2317
/// <summary>
@@ -32,20 +26,16 @@ public override string ToString()
3226
}
3327

3428
/// <summary>
35-
/// Gets or sets the public.
29+
/// Gets the Ed25519 public key.
3630
/// </summary>
3731
/// <value>
38-
/// The public.
32+
/// An array with <see cref="PublicKey"/> encoded at index 0.
3933
/// </value>
4034
public override BigInteger[] Public
4135
{
4236
get
4337
{
44-
return new BigInteger[] { _publicKey.ToBigInteger2() };
45-
}
46-
set
47-
{
48-
_publicKey = value[0].ToByteArray().Reverse().TrimLeadingZeros().Pad(Ed25519.PublicKeySizeInBytes);
38+
return new BigInteger[] { PublicKey.ToBigInteger2() };
4939
}
5040
}
5141

@@ -78,52 +68,46 @@ protected internal override DigitalSignature DigitalSignature
7868
/// <summary>
7969
/// Gets the PublicKey Bytes.
8070
/// </summary>
81-
public byte[] PublicKey
82-
{
83-
get
84-
{
85-
return _publicKey;
86-
}
87-
}
71+
public byte[] PublicKey { get; }
8872

8973
/// <summary>
9074
/// Gets the PrivateKey Bytes.
9175
/// </summary>
92-
public byte[] PrivateKey
93-
{
94-
get
95-
{
96-
return privateKey;
97-
}
98-
}
76+
public byte[] PrivateKey { get; }
9977

10078
/// <summary>
10179
/// Initializes a new instance of the <see cref="ED25519Key"/> class.
10280
/// </summary>
103-
public ED25519Key()
81+
/// <param name="publicKeyData">The encoded public key data.</param>
82+
public ED25519Key(SshKeyData publicKeyData)
10483
{
105-
}
84+
if (publicKeyData is null)
85+
{
86+
throw new ArgumentNullException(nameof(publicKeyData));
87+
}
10688

107-
/// <summary>
108-
/// Initializes a new instance of the <see cref="ED25519Key"/> class.
109-
/// </summary>
110-
/// <param name="pk">pk data.</param>
111-
public ED25519Key(byte[] pk)
112-
{
113-
_publicKey = pk.TrimLeadingZeros().Pad(Ed25519.PublicKeySizeInBytes);
89+
if (publicKeyData.Name != "ssh-ed25519" || publicKeyData.Keys.Length != 1)
90+
{
91+
throw new ArgumentException($"Invalid Ed25519 public key data ({publicKeyData.Name}, {publicKeyData.Keys.Length}).", nameof(publicKeyData));
92+
}
93+
94+
PublicKey = publicKeyData.Keys[0].ToByteArray().Reverse().TrimLeadingZeros().Pad(Ed25519.PublicKeySizeInBytes);
95+
PrivateKey = new byte[Ed25519.ExpandedPrivateKeySizeInBytes];
11496
}
11597

11698
/// <summary>
11799
/// Initializes a new instance of the <see cref="ED25519Key"/> class.
118100
/// </summary>
119-
/// <param name="pk">pk data.</param>
120-
/// <param name="sk">sk data.</param>
121-
public ED25519Key(byte[] pk, byte[] sk)
101+
/// <param name="privateKeyData">
102+
/// The private key data <c>k || ENC(A)</c> as described in RFC 8032.
103+
/// </param>
104+
public ED25519Key(byte[] privateKeyData)
122105
{
123-
_publicKey = pk.TrimLeadingZeros().Pad(Ed25519.PublicKeySizeInBytes);
124106
var seed = new byte[Ed25519.PrivateKeySeedSizeInBytes];
125-
Buffer.BlockCopy(sk, 0, seed, 0, seed.Length);
126-
Ed25519.KeyPairFromSeed(out _publicKey, out privateKey, seed);
107+
Buffer.BlockCopy(privateKeyData, 0, seed, 0, seed.Length);
108+
Ed25519.KeyPairFromSeed(out var publicKey, out var privateKey, seed);
109+
PublicKey = publicKey;
110+
PrivateKey = privateKey;
127111
}
128112

129113
/// <summary>

0 commit comments

Comments
 (0)