dlech / KeeAgent

ssh agent plugin for KeePass 2.x
http://lechnology.com/software/keeagent
Other
522 stars 35 forks source link

0.13 Regression: key comment loading for OpenSSH private key #375

Closed topia closed 1 year ago

topia commented 1 year ago

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.

patch for KeeAgent ```diff 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 ```diff 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().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 @@ -24,9 +25,16 @@ public sealed class SshPrivateKey /// The decrypted parameters. public delegate AsymmetricKeyParameter DecryptFunc( GetPassphraseFunc getPassphrase, - IProgress progress + IProgress progress, + UpdateCommentFunc updateComment ); + /// + /// Called if DecryptFunc found key comment + /// + /// Found comment + public delegate void UpdateCommentFunc(string comment); + private readonly DecryptFunc decrypt; /// @@ -64,16 +72,18 @@ DecryptFunc decrypt /// Callback to get the passphrase. Can be null for unencrypted keys. /// /// Optional progress feedback. + /// Optional comment feedback. /// The decrypted private key parameters. /// /// This can be a long running/cpu intensive operation. /// public AsymmetricKeyParameter Decrypt( GetPassphraseFunc getPassphrase, - IProgress progress = null + IProgress progress = null, + UpdateCommentFunc updateComment = null ) { - return decrypt(getPassphrase, progress); + return decrypt(getPassphrase, progress, updateComment); } /// ```
dlech commented 1 year ago

This would be easier to review as a pull request.

topia commented 1 year ago

created pull requests: