0.13 Regression: key comment loading for OpenSSH private key

[topia]

I think this is the last item for my migration blocker.
I understand we don't have any clean solutions for that because OpenSSH private key stores a comment in an encrypted blob.

By the way, I created a hacky patch to resolve this problem, to start discussing.

I confirmed the comment loading for the following areas, which don't work with 0.13.

• Entry Dialog (comment in KeeAgent Tab)
• Sending the key comment to SSH-Agent (client mode)

patch for KeeAgent

diff --git a/KeeAgent/ExtensionMethods.cs b/KeeAgent/ExtensionMethods.cs
index 8e3185a..1f3d735 100644
--- a/KeeAgent/ExtensionMethods.cs
+++ b/KeeAgent/ExtensionMethods.cs
@@ -365,26 +365,37 @@ namespace KeeAgent
     public static ISshKey GetSshKey(this PwEntry entry)
     {
       var settings = entry.GetKeeAgentSettings();
+      return settings.GetSshKey(entry.Binaries, entry.GetPassphrase);
+    }
+
+    public static ISshKey GetSshKey(this EntrySettings settings, ProtectedBinaryDictionary binaries, PwEntry entry)
+    {
+      return settings.GetSshKey(binaries, entry.GetPassphrase);
+    }
+
+    static ISshKey GetSshKey(this EntrySettings settings, ProtectedBinaryDictionary binaries, SshPrivateKey.GetPassphraseFunc getPassphrase)
+    {
 
       if (!settings.AllowUseOfSshKey) {
         return null;
       }
 
-      var publicKey = settings.TryGetSshPublicKey(entry.Binaries);
+      var publicKey = settings.TryGetSshPublicKey(binaries);
 
       if (publicKey == null) {
         throw new PublicKeyRequiredException();
       }
 
-      var privateKey = settings.GetSshPrivateKey(entry.Binaries);
+      var privateKey = settings.GetSshPrivateKey(binaries);
 
       AsymmetricKeyParameter parameter;
+      var comment = publicKey.Comment;
 
       if (privateKey.HasKdf) {
         // if there is a key derivation function, decrypting could be slow,
         // so show a progress dialog
         var dialog = new DecryptProgressDialog();
-        dialog.Start((p) => privateKey.Decrypt(() => entry.GetPassphrase(), p));
+        dialog.Start((p) => privateKey.Decrypt(getPassphrase, p, newComment => comment = newComment));
         dialog.ShowDialog();
 
         if (dialog.DialogResult == DialogResult.Abort) {
@@ -394,13 +405,13 @@ namespace KeeAgent
         parameter = (AsymmetricKeyParameter)dialog.Result;
       }
       else {
-        parameter = privateKey.Decrypt(() => entry.GetPassphrase());
+        parameter = privateKey.Decrypt(getPassphrase, null, newComment => comment = newComment);
       }
 
       var key = new SshKey(
         publicKey.Parameter,
         parameter,
-        publicKey.Comment,
+        comment,
         publicKey.Nonce,
         publicKey.Certificate);
 
diff --git a/KeeAgent/UI/EntryPanel.cs b/KeeAgent/UI/EntryPanel.cs
index 8ffa85d..4ab5d21 100644
--- a/KeeAgent/UI/EntryPanel.cs
+++ b/KeeAgent/UI/EntryPanel.cs
@@ -135,8 +135,13 @@ namespace KeeAgent.UI
 
       try {
         var key = CurrentSettings.TryGetSshPublicKey(pwEntryForm.EntryBinaries);
+        var comment = key.Comment;
+        if (string.IsNullOrEmpty(comment)) {
+          var privateKey = CurrentSettings.GetSshKey(pwEntryForm.EntryBinaries, pwEntryForm.EntryRef);
+          comment = privateKey.Comment;
+        }
 
-        commentTextBox.Text = key.Comment;
+        commentTextBox.Text = comment;
         fingerprintTextBox.Text = key.Sha256Fingerprint;
         publicKeyTextBox.Text = key.AuthorizedKeysString;
         copyPublicKeyButton.Enabled = true;

patch for SshAgentLib

 SshAgentLib/IAgent.cs                 |  6 ++++--
 SshAgentLib/Keys/OpensshPrivateKey.cs |  6 +++---
 SshAgentLib/Keys/PemPrivateKey.cs     |  2 +-
 SshAgentLib/Keys/PuttyPrivateKey.cs   |  2 +-
 SshAgentLib/Keys/SshPrivateKey.cs     | 16 +++++++++++++---
 5 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/SshAgentLib/IAgent.cs b/SshAgentLib/IAgent.cs
index 44af24b..9ae369c 100644
--- a/SshAgentLib/IAgent.cs
+++ b/SshAgentLib/IAgent.cs
@@ -108,10 +108,12 @@ public static class IAgentExt
                 throw new FileNotFoundException("this key requires a .pub file");
             }
 
+            var comment = publicKey.Comment;
+            var decryptedKey = privateKey.Decrypt(getPassPhraseCallback, progress, newComment => comment = newComment);
             var key = new SshKey(
                 publicKey.Parameter,
-                privateKey.Decrypt(getPassPhraseCallback, progress),
-                publicKey.Comment,
+                decryptedKey,
+                comment,
                 publicKey.Nonce,
                 publicKey.Certificate,
                 publicKey.Application
diff --git a/SshAgentLib/Keys/OpensshPrivateKey.cs b/SshAgentLib/Keys/OpensshPrivateKey.cs
index 81a94db..7f7670e 100644
--- a/SshAgentLib/Keys/OpensshPrivateKey.cs
+++ b/SshAgentLib/Keys/OpensshPrivateKey.cs
@@ -126,7 +126,7 @@ public static SshPrivateKey Read(Stream stream)
             var publicKeyBlob = parser.ReadBlob();
             var privateKeyBlob = parser.ReadBlob();
 
-            SshPrivateKey.DecryptFunc decrypt = (getPassphrase, progress) =>
+            SshPrivateKey.DecryptFunc decrypt = (getPassphrase, progress, updateComment) =>
             {
                 var keyAndIV = new byte[32 + 16];
 
@@ -210,8 +210,8 @@ public static SshPrivateKey Read(Stream stream)
                     out var application
                 );
                 var privateKey = privateKeyParser.ReadSsh2KeyData(publicKey);
-                // var comment = privateKeyParser.ReadString();
-                // TODO: what to do with comment?
+                var comment = privateKeyParser.ReadString();
+                updateComment?.Invoke(comment);
                 // TODO: should we throw if nonce/cert is not null?
                 // TODO: do we expect application?
 
diff --git a/SshAgentLib/Keys/PemPrivateKey.cs b/SshAgentLib/Keys/PemPrivateKey.cs
index c3c5abe..5ef2c52 100644
--- a/SshAgentLib/Keys/PemPrivateKey.cs
+++ b/SshAgentLib/Keys/PemPrivateKey.cs
@@ -33,7 +33,7 @@ public static SshPrivateKey Read(Stream stream)
 
             var isEncrypted = pem.Headers.Cast<PemHeader>().Any(h => h.Name == "DEK-Info");
 
-            SshPrivateKey.DecryptFunc decrypt = (getPassphrase, progress) =>
+            SshPrivateKey.DecryptFunc decrypt = (getPassphrase, progress, updateComment) =>
             {
                 var keyPair = ReadKeyPair(new StringReader(contents), getPassphrase);
 
diff --git a/SshAgentLib/Keys/PuttyPrivateKey.cs b/SshAgentLib/Keys/PuttyPrivateKey.cs
index c16c154..898dbf3 100644
--- a/SshAgentLib/Keys/PuttyPrivateKey.cs
+++ b/SshAgentLib/Keys/PuttyPrivateKey.cs
@@ -155,7 +155,7 @@ public static SshPrivateKey Read(Stream stream)
                 reader.ReadHeader(version == "1" ? HeaderKey.PrivateHash : HeaderKey.PrivateMAC)
             );
 
-            SshPrivateKey.DecryptFunc decrypt = (getPassphrase, progress) =>
+            SshPrivateKey.DecryptFunc decrypt = (getPassphrase, progress, updateComment) =>
             {
                 byte[] decryptedPrivateKeyBlob;
                 byte[] macKey;
diff --git a/SshAgentLib/Keys/SshPrivateKey.cs b/SshAgentLib/Keys/SshPrivateKey.cs
index 79fbe52..88fe338 100644
--- a/SshAgentLib/Keys/SshPrivateKey.cs
+++ b/SshAgentLib/Keys/SshPrivateKey.cs
@@ -1,3 +1,4 @@
+
 // SPDX-License-Identifier: MIT
 // Copyright (c) 2022 David Lechner <david@lechnology.com>
 
@@ -24,9 +25,16 @@ public sealed class SshPrivateKey
         /// <returns>The decrypted parameters.</returns>
         public delegate AsymmetricKeyParameter DecryptFunc(
             GetPassphraseFunc getPassphrase,
-            IProgress<double> progress
+            IProgress<double> progress,
+            UpdateCommentFunc updateComment
         );
 
+        /// <summary>
+        /// Called if DecryptFunc found key comment
+        /// </summary>
+        /// <param name="comment">Found comment</param>
+        public delegate void UpdateCommentFunc(string comment);
+
         private readonly DecryptFunc decrypt;
 
         /// <summary>
@@ -64,16 +72,18 @@ DecryptFunc decrypt
         /// 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>
         /// <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
+            IProgress<double> progress = null,
+            UpdateCommentFunc updateComment = null
         )
         {
-            return decrypt(getPassphrase, progress);
+            return decrypt(getPassphrase, progress, updateComment);
         }
 
         /// <summary>

[dlech]

This would be easier to review as a pull request.

[topia]

created pull requests:

• KeeAgent side: support reading key comment from private key (only openssh do it) #376
• SshAgentLib side: implement reading comment from openssh private key SshAgentLib#23