dlech / KeeAgent

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

Comments of keys not shown #348

Closed Hackerpcs closed 2 years ago

Hackerpcs commented 2 years ago

As described here https://github.com/dlech/KeeAgent/issues/341#issuecomment-1133722216

until this build https://github.com/dlech/KeeAgent/actions/runs/2290793867 I can see the comments of the keys

previous previous2

on the latest v0.13.1 I can't

latest latest2

dlech commented 2 years ago

Which private key file format are you using?

Hackerpcs commented 2 years ago
-----BEGIN OPENSSH PRIVATE KEY-----
foobarbaz
-----END OPENSSH PRIVATE KEY-----
dlech commented 2 years ago

KeeAgent currently uses only the public key comment. For this file format, the comment is encrypted, so it is only available after the private key has been decrypted. As a workaround, you might be able to add a .pub file containing the public key and the comment as an attachment so that KeeAgent will pick up the comment.

We could also consider adding a Comment property to SshPrivateKey (to be populated here) for private key comments and use this as a fallback when a public key comment is not provided.

Hackerpcs commented 2 years ago

My keys aren't encrypted, they are generated with this command without entering a passphrase

ssh-keygen -t rsa -b 4096 -C "comment" -f "key_rsa4096"

It worked till that version (which I currently use) though, couldn't it be done in the stable also like it was there?

dlech commented 2 years ago

Most people seem to use encrypted keys and decrypting the key even when it is not being used caused performance problems for newer keys formats with CPU intensive key derivation functions. Fixing that problem has lead to this problem.

We could add a hack that to the effect of "if the key is not encrypted, use the comment", but that would only work for people with unencrypted keys. Attaching a separate public key file that includes the comment is a more universal workaround.

egfx-notifications commented 2 years ago

I just updated from KeeAgent 0.12.1 to 0.13.1 and can confirm @Hackerpcs findings

I have both private and public keys in KeePass and configured for KeeAgent usage. Starting with 0.13.1 I no longer see the comment

If I go back to KeeAgent 0.12.1 it works again.

Hackerpcs commented 2 years ago

Personally I use this build https://github.com/dlech/KeeAgent/actions/runs/2290793867 that both works (regarding issue https://github.com/dlech/KeeAgent/issues/341, Windows 10 21h2) and has comments.

egfx-notifications commented 2 years ago

Didn't manage to do a git bisect yet, but looking at the commits since 0.12.1 I consider this the most likely subject 122735404915168cd30de57eb6326294f5a83d25

egfx-notifications commented 2 years ago

Personally I use this build https://github.com/dlech/KeeAgent/actions/runs/2290793867 that both works (regarding issue #341, Windows 10 21h2) and has comments.

That would be two commits before the commit I suspect, so that sounds legit

egfx-notifications commented 2 years ago

KeeAgent currently uses only the public key comment. For this file format, the comment is encrypted, so it is only available after the private key has been decrypted. As a workaround, you might be able to add a .pub file containing the public key and the comment as an attachment so that KeeAgent will pick up the comment.

We could also consider adding a Comment property to SshPrivateKey (to be populated here) for private key comments and use this as a fallback when a public key comment is not provided.

@dlech Looking at the diff between 0.12.1 and 0.13.1 I think I understand what you meant with the encrypted comment. Unfortunately the public key attachment I also have in place is not helping because the code only ever checks for a public key attachment if the private key doesn't have a public key included. So my readable public key attachment is ignored, because a public key is already present in the private key, but I can't access the comment because it is encrypted.

mplattner commented 2 years ago

I have the same problem: since the update to v0.13.1.0 my keys' comments are not displayed anymore, beside for one old and used rsa1024 key. All keys (including the old one) are encrypted.

attisan commented 2 years ago

I have the same problem: since the update to v0.13.1.0 my keys' comments are not displayed anymore, beside for one old and used rsa1024 key. All keys (including the old one) are encrypted.

same here. in my case public key comments for ssh-ed25519 aren't showing up anymore

KuttKatrea commented 2 years ago

What about a setting to use the entry title as the comment? Is that feasible @dlech?

Wouldn't require a public key nor decrypting the private key

dlech commented 2 years ago

That sounds like a good idea for a fallback.

KuttKatrea commented 2 years ago

That sounds like a good idea for a fallback.

I created a PR: https://github.com/dlech/KeeAgent/pull/355

attisan commented 2 years ago

I wonder if this (perhaps optionally) shouldn't be the default as it is more accessible / editable.

mplattner commented 2 years ago

The comment of my key (ed25519) is still not shown in v0.13.2. The comment field is just empty. It does work correctly with v.0.12.1.

dlech commented 2 years ago

This issue was closed since the workaround of including a .pub file is fixed. The breaking change of encrypted comments not working as before for some keys is an unfortunate side effect of performance improvements, so can't technically be fixed without causing performance issues again.

We should probably start a new issue for the suggestion in https://github.com/dlech/KeeAgent/issues/348#issuecomment-1225254088 (with relevant discussion in #355) as an alternate solution if using .pub files is not sufficient.

mplattner commented 2 years ago

As @egfx-notifications mentioned above, using v0.13.2, adding a .pub file does not fix this for me - the comment is still empty.

The pub file contains ssh-ed25519 AAAA...V mycomment. @egfx-notifications said its due to the public key being included in the private key file. I found no way to remove the public key from the private key file.

egfx-notifications commented 2 years ago

The fix provided in 90a0ce2effa79871d97679e7f08f981afebdedeb is incomplete. The Read() method in SshAgentLib only uses the provided public key if a public key is not included in the private key, so WithPublicKey() method needs to be used instead.

I can work on a PR later today.

jaxFF commented 1 year ago

Hi.

I'm still facing the same issue on the most recent KeeAgent build (0.13.2.0) with my ed25519 keypair. My pub key looks as follows, I believe there may be an issue with parsing spaces in the comment or something.

ssh-ed25519 AA....zq user@desktop commentpt2 anotherpartof comment 20221015

I'm not familiar with the codebase and couldn't easily push a PR. If someone was willing to create a PR that would allow the plugin to fall back to using the title AND OR comment for the password entry, that would be wonderful, as I use the same comment for my entry as the one stored in my pub key.

I think it would be more accessible to users if you could specify to use the password entry's name OR comment, for the key info comment.

This is how the key with the comment is appearing in KeeAgent and my ssh-agent. (screenshots redacted)

The most recent PR regarding this issue does not fix it. I'll probably revert to 0.12.1 for now as previously suggested.

dlech commented 1 year ago

Since this issue has been closed, can you please start a new issue and attach an example key that reproduces the issue?

jaxFF commented 1 year ago

Since this issue has been closed, can you please start a new issue and attach an example key that reproduces the issue?

Sure thing, please view issue #369. Thanks