search

dlech / KeeAgent

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

Opensshcert #310

Closed msabatier closed 2 years ago

msabatier commented 3 years ago

Hi David,

Here is a small patch proposal to add ability to load openssh certificates from -cert.pub file (or attachment) along with private key.

Let me know what you think, and if you would like me to adjust anything or may be add some tests.

Thanks

msabatier commented 3 years ago

Hi David,

Just did.

I have been doing some more tests and have found an issue with key removal not working consistently. Unfortunately this may reveal something a little more complex than I thought.

The issue comes from the fact that keys are identified / looked-up using different attributes in different places of the code:

The last method is an issue in my case as I need to insert 2 keys in the agent that share the same MD5 (because the private key is the same) and only differ by the Certificate attribute.

I think I know how to work around that for the UI part but I am really not sure about the keyFileMap part.

Would you have any suggestion for me on this ?

msabatier commented 3 years ago

I have fixed the inconsistency with key removal and cleaned up the implementation to avoid this loadCert flag that was less than ideal.

After looking at the keyFileMap topic again, I think the certificate does not need to be inserted along the private key. Just making sure that we don't remove the private key from the keyFileMap when we remove the cert from the agent should be enough.

Let me know what you think.

dlech commented 2 years ago

Thanks for your work getting started on this feature - it never would have happened without it. There have been a bunch of changes to the internals of how keys are loaded recently, and I've made some alternate changes to basically get this working. So I'm going to go ahead and close this PR. Feel free to open a new PR if you find any bugs.

dlech commented 2 years ago

Thanks for your work getting started on this feature - it never would have happened without it. There have been a bunch of changes to the internals of how keys are loaded recently, and I've made some alternate changes to basically get this working. So I'm going to go ahead and close this PR. Feel free to open a new PR if you find any bugs.

msabatier commented 2 years ago

Hi David, Thanks to you for looking into this and merging the change.

I did a first test with latest build and I get a different behavior than what I expected. When loading a key with a certificate from keepass, keeagent only loads the certificate and not the private key. Consequently authentication fails. I will open a separate issue on this with detailed logs.