Skip to content

Commit

Permalink
use out parameter for private key comment
Browse files Browse the repository at this point in the history
  • Loading branch information
dlech committed Feb 4, 2023
1 parent a8c2963 commit 6809f3e
Show file tree
Hide file tree
Showing 8 changed files with 96 additions and 52 deletions.
11 changes: 9 additions & 2 deletions SshAgentLib/IAgent.cs
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,15 @@ public static ISshKey AddKeyFromFile(
throw new FileNotFoundException("this key requires a .pub file");
}

var comment = publicKey.Comment;
var decryptedKey = privateKey.Decrypt(getPassPhraseCallback, progress, newComment => comment = newComment);
var decryptedKey = privateKey.Decrypt(getPassPhraseCallback, progress, out var comment);

// prefer public key comment if there is one, otherwise fall back
// to private key comment
if (!string.IsNullOrWhiteSpace(publicKey.Comment))
{
comment = publicKey.Comment;
}

var key = new SshKey(
publicKey.Parameter,
decryptedKey,
Expand Down
12 changes: 7 additions & 5 deletions SshAgentLib/Keys/OpensshPrivateKey.cs
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
// SPDX-License-Identifier: MIT
// Copyright (c) 2015,2022 David Lechner <david@lechnology.com>
// Copyright (c) 2015,2022-2023 David Lechner <david@lechnology.com>

using System;
using System.IO;
using System.Security.Cryptography;
using System.Text;
using dlech.SshAgentLib;
using Org.BouncyCastle.Crypto;
using Org.BouncyCastle.Crypto.Parameters;
using Org.BouncyCastle.Security;
using Org.BouncyCastle.Utilities.IO.Pem;
Expand Down Expand Up @@ -126,7 +125,11 @@ public static SshPrivateKey Read(Stream stream)
var publicKeyBlob = parser.ReadBlob();
var privateKeyBlob = parser.ReadBlob();

SshPrivateKey.DecryptFunc decrypt = (getPassphrase, progress, updateComment) =>
SshPrivateKey.DecryptFunc decrypt = (
SshPrivateKey.GetPassphraseFunc getPassphrase,
IProgress<double> progress,
out string comment
) =>
{
var keyAndIV = new byte[32 + 16];
Expand Down Expand Up @@ -210,8 +213,7 @@ public static SshPrivateKey Read(Stream stream)
out var application
);
var privateKey = privateKeyParser.ReadSsh2KeyData(publicKey);
var comment = privateKeyParser.ReadString();
updateComment?.Invoke(comment);
comment = privateKeyParser.ReadString();
// TODO: should we throw if nonce/cert is not null?
// TODO: do we expect application?
Expand Down
11 changes: 8 additions & 3 deletions SshAgentLib/Keys/PemPrivateKey.cs
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
// SPDX-License-Identifier: MIT
// Copyright (c) 2022 David Lechner <david@lechnology.com>
// Copyright (c) 2022-2023 David Lechner <david@lechnology.com>

using System;
using System.IO;
using System.Linq;
using System.Text;
using System.Text.RegularExpressions;
using dlech.SshAgentLib;
using Org.BouncyCastle.Crypto;
using Org.BouncyCastle.OpenSsl;
using Org.BouncyCastle.Utilities.IO.Pem;
Expand All @@ -33,8 +32,14 @@ public static SshPrivateKey Read(Stream stream)

var isEncrypted = pem.Headers.Cast<PemHeader>().Any(h => h.Name == "DEK-Info");

SshPrivateKey.DecryptFunc decrypt = (getPassphrase, progress, updateComment) =>
SshPrivateKey.DecryptFunc decrypt = (
SshPrivateKey.GetPassphraseFunc getPassphrase,
IProgress<double> progress,
out string comment
) =>
{
comment = "";
var keyPair = ReadKeyPair(new StringReader(contents), getPassphrase);
// REVISIT: should we validate match with public key?
Expand Down
10 changes: 8 additions & 2 deletions SshAgentLib/Keys/PuttyPrivateKey.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: MIT
// Copyright (c) 2012-2013,2015,2017,2022 David Lechner <david@lechnology.com>
// Copyright (c) 2012-2013,2015,2017,2022-2023 David Lechner <david@lechnology.com>

using System;
using System.Collections.Generic;
Expand Down Expand Up @@ -155,8 +155,14 @@ public static SshPrivateKey Read(Stream stream)
reader.ReadHeader(version == "1" ? HeaderKey.PrivateHash : HeaderKey.PrivateMAC)
);

SshPrivateKey.DecryptFunc decrypt = (getPassphrase, progress, updateComment) =>
SshPrivateKey.DecryptFunc decrypt = (
SshPrivateKey.GetPassphraseFunc getPassphrase,
IProgress<double> progress,
out string comment2
) =>
{
comment2 = comment;
byte[] decryptedPrivateKeyBlob;
byte[] macKey;
Expand Down
24 changes: 15 additions & 9 deletions SshAgentLib/Keys/SshPrivateKey.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@

// SPDX-License-Identifier: MIT
// Copyright (c) 2022 David Lechner <david@lechnology.com>
// Copyright (c) 2022-2023 David Lechner <david@lechnology.com>

using System;
using System.IO;
Expand All @@ -20,13 +19,16 @@ public sealed class SshPrivateKey
/// A callback to get the passphrase. Can be <c>null</c> if the private key is not encrypted.
/// </param>
/// <param name="progress">
/// Optional progress callback. Returns normalized progres 0 to 1.
/// Optional progress callback. Returns normalized progress 0 to 1.
/// </param>
/// <param name="comment">
/// The private key comment.
/// </param>
/// <returns>The decrypted parameters.</returns>
public delegate AsymmetricKeyParameter DecryptFunc(
GetPassphraseFunc getPassphrase,
IProgress<double> progress,
UpdateCommentFunc updateComment
out string comment
);

/// <summary>
Expand Down Expand Up @@ -71,19 +73,23 @@ DecryptFunc decrypt
/// <param name="getPassphrase">
/// Callback to get the passphrase. Can be <c>null</c> for unencrypted keys.
/// </param>
/// <param name="progress">Optional progress feedback.</param>
/// <param name="updateComment">Optional comment feedback.</param>
/// <param name="progress">
/// Optional progress feedback.
/// </param>
/// <param name="comment">
/// The private key comment.
/// </param>
/// <returns>The decrypted private key parameters.</returns>
/// <remarks>
/// This can be a long running/cpu intensive operation.
/// </remarks>
public AsymmetricKeyParameter Decrypt(
GetPassphraseFunc getPassphrase,
IProgress<double> progress = null,
UpdateCommentFunc updateComment = null
IProgress<double> progress,
out string comment
)
{
return decrypt(getPassphrase, progress, updateComment);
return decrypt(getPassphrase, progress, out comment);
}

/// <summary>
Expand Down
34 changes: 22 additions & 12 deletions SshAgentLibTests/Keys/OpensshPrivateKeyTests.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: MIT
// Copyright (c) 2022 David Lechner <david@lechnology.com>
// Copyright (c) 2022-2023 David Lechner <david@lechnology.com>

using System;
using System.Text;
Expand Down Expand Up @@ -34,10 +34,11 @@ public void TestThatReadingRsaPrivateKeyWorks()
var pubKey = (RsaKeyParameters)key.PublicKey.Parameter;
Assert.That(pubKey.Modulus, Is.EqualTo(new BigInteger(n, 16)));

var privParam = key.Decrypt(null);
var privParam = key.Decrypt(null, null, out var comment);

Assert.That(privParam.IsPrivate);
Assert.That(privParam, Is.TypeOf<RsaPrivateCrtKeyParameters>());
Assert.That(comment, Is.Empty);

var privKey = (RsaPrivateCrtKeyParameters)privParam;

Expand All @@ -46,7 +47,7 @@ public void TestThatReadingRsaPrivateKeyWorks()
Assert.That(privKey.Q, Is.EqualTo(new BigInteger(q, 16)));

// ensure that decrypting a second time works
privParam = key.Decrypt(null);
privParam = key.Decrypt(null, null, out comment);
Assert.That(privParam.IsPrivate);
}

Expand All @@ -69,10 +70,11 @@ public void TestThatReadingRsaPrivateKeyWithPasswordWorks()
var pubKey = (RsaKeyParameters)key.PublicKey.Parameter;
Assert.That(pubKey.Modulus, Is.EqualTo(new BigInteger(n, 16)));

var privParam = key.Decrypt(() => Encoding.UTF8.GetBytes(pw));
var privParam = key.Decrypt(() => Encoding.UTF8.GetBytes(pw), null, out var comment);

Assert.That(privParam.IsPrivate);
Assert.That(privParam, Is.TypeOf<RsaPrivateCrtKeyParameters>());
Assert.That(comment, Is.Empty);

var privKey = (RsaPrivateCrtKeyParameters)privParam;

Expand All @@ -81,7 +83,7 @@ public void TestThatReadingRsaPrivateKeyWithPasswordWorks()
Assert.That(privKey.Q, Is.EqualTo(new BigInteger(q, 16)));

// ensure that decrypting a second time works
privParam = key.Decrypt(() => Encoding.UTF8.GetBytes(pw));
privParam = key.Decrypt(() => Encoding.UTF8.GetBytes(pw), null, out comment);
Assert.That(privParam.IsPrivate);
}

Expand All @@ -102,10 +104,11 @@ public void TestThatReadingDsaPrivateKeyWorks()
var pubKey = (DsaPublicKeyParameters)key.PublicKey.Parameter;
Assert.That(pubKey.Y, Is.EqualTo(new BigInteger(pub, 16)));

var privParam = key.Decrypt(null);
var privParam = key.Decrypt(null, null, out var comment);

Assert.That(privParam.IsPrivate);
Assert.That(privParam, Is.TypeOf<DsaPrivateKeyParameters>());
Assert.That(comment, Is.Empty);

var privKey = (DsaPrivateKeyParameters)privParam;

Expand All @@ -131,10 +134,11 @@ public void TestThatReadingDsaPrivateKeyWithPasswordWorks()
var pubKey = (DsaPublicKeyParameters)key.PublicKey.Parameter;
Assert.That(pubKey.Y, Is.EqualTo(new BigInteger(pub, 16)));

var privParam = key.Decrypt(() => Encoding.UTF8.GetBytes(pw));
var privParam = key.Decrypt(() => Encoding.UTF8.GetBytes(pw), null, out var comment);

Assert.That(privParam.IsPrivate);
Assert.That(privParam, Is.TypeOf<DsaPrivateKeyParameters>());
Assert.That(comment, Is.Empty);

var privKey = (DsaPrivateKeyParameters)privParam;

Expand Down Expand Up @@ -163,10 +167,11 @@ public void TestThatReadingEcdsaPrivateKeyWorks()
);
Assert.That(pubKey.Q, Is.EqualTo(pubKey.Parameters.Curve.DecodePoint(Hex.Decode(pub))));

var privParam = key.Decrypt(null);
var privParam = key.Decrypt(null, null, out var comment);

Assert.That(privParam.IsPrivate);
Assert.That(privParam, Is.TypeOf<ECPrivateKeyParameters>());
Assert.That(comment, Is.Empty);

var privKey = (ECPrivateKeyParameters)privParam;

Expand Down Expand Up @@ -199,10 +204,11 @@ public void TestThatReadingEcdsaPrivateKeyWithPasswordWorks()
);
Assert.That(pubKey.Q, Is.EqualTo(pubKey.Parameters.Curve.DecodePoint(Hex.Decode(pub))));

var privParam = key.Decrypt(() => Encoding.UTF8.GetBytes(pw));
var privParam = key.Decrypt(() => Encoding.UTF8.GetBytes(pw), null, out var comment);

Assert.That(privParam.IsPrivate);
Assert.That(privParam, Is.TypeOf<ECPrivateKeyParameters>());
Assert.That(comment, Is.Empty);

var privKey = (ECPrivateKeyParameters)privParam;

Expand All @@ -219,7 +225,8 @@ public void TestThatReadingEd25519PrivateKeyWorks()

Assert.That(key.PublicKey.Parameter, Is.TypeOf<Ed25519PublicKeyParameters>());

var privParam = key.Decrypt(null);
var privParam = key.Decrypt(null, null, out var comment);
Assert.That(comment, Is.EqualTo("ED25519 test key #1"));

Assert.That(privParam.IsPrivate);
Assert.That(privParam, Is.TypeOf<Ed25519PrivateKeyParameters>());
Expand All @@ -237,10 +244,11 @@ public void TestThatReadingEd25519PrivateKeyWithPasswordWorks()

Assert.That(key.PublicKey.Parameter, Is.TypeOf<Ed25519PublicKeyParameters>());

var privParam = key.Decrypt(() => Encoding.UTF8.GetBytes(pw));
var privParam = key.Decrypt(() => Encoding.UTF8.GetBytes(pw), null, out var comment);

Assert.That(privParam.IsPrivate);
Assert.That(privParam, Is.TypeOf<Ed25519PrivateKeyParameters>());
Assert.That(comment, Is.EqualTo("ED25519 test key #1"));
}

[Test]
Expand All @@ -253,10 +261,12 @@ public void TestThatReadingEd25519PrivateKey2Works()

Assert.That(key.PublicKey.Parameter, Is.TypeOf<Ed25519PublicKeyParameters>());

var privParam = key.Decrypt(null);
var privParam = key.Decrypt(null, null, out var comment);

Assert.That(privParam.IsPrivate);
Assert.That(privParam, Is.TypeOf<Ed25519PrivateKeyParameters>());
// upstream file has typo
Assert.That(comment, Is.EqualTo("ED25519 test key #1"));
}
}
}
17 changes: 10 additions & 7 deletions SshAgentLibTests/Keys/PemPrivateKeyTests.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: MIT
// Copyright (c) 2022 David Lechner <david@lechnology.com>
// Copyright (c) 2022-2023 David Lechner <david@lechnology.com>

using System.IO;
using System.Text;
Expand All @@ -24,7 +24,7 @@ public void TestThatReadingRsaKeyWorks(string baseName, bool isEncrypted)
var q = ReadStringResourceFile("OpenSshTestData", $"{baseName}.param.q");
var pw = ReadStringResourceFile("OpenSshTestData", "pw");

var suffix = isEncrypted ? "_pw" : "";
var suffix = isEncrypted ? "_pw" : string.Empty;
var file = OpenResourceFile("OpenSshTestData", $"{baseName}{suffix}");
var key = SshPrivateKey.Read(file);

Expand All @@ -39,8 +39,9 @@ public void TestThatReadingRsaKeyWorks(string baseName, bool isEncrypted)
// decrypt multiple times to check for state corruption
for (int i = 0; i < 2; i++)
{
var privParam = key.Decrypt(getPassphrase);
var privParam = key.Decrypt(getPassphrase, null, out var comment);
Assert.That(privParam, Is.TypeOf<RsaPrivateCrtKeyParameters>());
Assert.That(comment, Is.Empty);

var rsa = (RsaPrivateCrtKeyParameters)privParam;
Assert.That(rsa.Modulus, Is.EqualTo(new BigInteger(n, 16)));
Expand All @@ -56,7 +57,7 @@ public void TestThatReadingDsaKeyWorks(string baseName, bool isEncrypted)
var priv = ReadStringResourceFile("OpenSshTestData", $"{baseName}.param.priv");
var pw = ReadStringResourceFile("OpenSshTestData", "pw");

var suffix = isEncrypted ? "_pw" : "";
var suffix = isEncrypted ? "_pw" : string.Empty;
var file = OpenResourceFile("OpenSshTestData", $"{baseName}{suffix}");
var key = SshPrivateKey.Read(file);

Expand All @@ -71,8 +72,9 @@ public void TestThatReadingDsaKeyWorks(string baseName, bool isEncrypted)
// decrypt multiple times to check for state corruption
for (int i = 0; i < 2; i++)
{
var privParam = key.Decrypt(getPassphrase);
var privParam = key.Decrypt(getPassphrase, null, out var comment);
Assert.That(privParam, Is.TypeOf<DsaPrivateKeyParameters>());
Assert.That(comment, Is.Empty);

var dsa = (DsaPrivateKeyParameters)privParam;
Assert.That(dsa.X, Is.EqualTo(new BigInteger(priv, 16)));
Expand All @@ -87,7 +89,7 @@ public void TestThatReadingEcdsaKeyWorks(string baseName, bool isEncrypted)
var priv = ReadStringResourceFile("OpenSshTestData", $"{baseName}.param.priv");
var pw = ReadStringResourceFile("OpenSshTestData", "pw");

var suffix = isEncrypted ? "_pw" : "";
var suffix = isEncrypted ? "_pw" : string.Empty;
var file = OpenResourceFile("OpenSshTestData", $"{baseName}{suffix}");
var key = SshPrivateKey.Read(file);

Expand All @@ -102,8 +104,9 @@ public void TestThatReadingEcdsaKeyWorks(string baseName, bool isEncrypted)
// decrypt multiple times to check for state corruption
for (int i = 0; i < 2; i++)
{
var privParam = key.Decrypt(getPassphrase);
var privParam = key.Decrypt(getPassphrase, null, out var comment);
Assert.That(privParam, Is.TypeOf<ECPrivateKeyParameters>());
Assert.That(comment, Is.Empty);

var ecdsa = (ECPrivateKeyParameters)privParam;
Assert.That(ecdsa.D, Is.EqualTo(new BigInteger(priv, 16)));
Expand Down
Loading

0 comments on commit 6809f3e

Please sign in to comment.