Fix SSH Agent broken decrypt button

[AlexpFr]

• Fixes The Decrypt button of the SSH agent never activates. #10637

The reason is that the activation condition is: if (!key.comment().isEmpty() || !key.encrypted()).
But the comment is never empty because when opening the key we have:

if (key.comment().isEmpty() && !key.encrypted()) {
    key.setComment(username);
}

if (key.comment().isEmpty() && !key.encrypted()) {
    key.setComment(fileName);
}

I also took the opportunity to modify the text to be copied to the clipboard when the key is not decrypted.
Indeed, the undecrypted private key exposes the public key in clear text, but not its comment.

• setPlainText(tr("(encrypted)"));
  to
• setPlainText(key.publicKey() + tr("(comment is encrypted)"));

Testing strategy

Tested manually with ed25519 crypted key.

Type of change

✅ Bug fix (non-breaking change that fixes an issue)

[droidmonkey]

The way the key is handled makes this rather confusing. Comment, encrypted, publickey, etc. We shouldn't have to check so many conditions to display simple data. How can we improve this overall? @hifi

[AlexpFr]

The presentation of data in Keepassxc is identical to that of openSSH tools and is closely related to the SSH private key file format. Naturally, if the private key is password-free, all information is directly visible.

Currently, the only way to simplify the display of SSH key information in Keepassxc would be to decrypt the keys when opening the entry.

However, this presents a major drawback:

• The decryption process, due to the Key Derivation Function (KDF), takes about 0.2 seconds with the default number of rounds (16 rounds). However, many users follow online tutorials that recommend adjusting the number of rounds to 100, 256, or even more than 1000 rounds.

This means that an entry containing an SSH key encoded with a high number of KDF rounds would take several seconds to open. Simplifying the display of SSH keys in Keepassxc would therefore negatively impact the overall responsiveness of the application.

@hifi had already noted this issue here:

• SSH Agent: Make adding keys on database unlock asynchronous #4688

Changing the behavior and display of information is not just a bug fix.

This PR is simply aimed at fixing the decrypt button. :)

[droidmonkey]

That is not what I was saying. The underlying data structure that contains the keys has a confusing API.

[AlexpFr]

Do you want me to look into refactoring this part with a "case switch" , for example ?

[droidmonkey]

No. For example, all these checks for comment existing and decrypted and such... those should be done within the key.comment() function which would return the appropriate response given the conditions of the key. Doing all those checks at the UI layer is confusing and non-repeatable in case we want to implement the same behavior elsewhere. You should simply have to do the setText(key.comment()) call and move on.

Could also be done in a new function that makes it obvious we are preparing data for display: key.resolveComment() or key.displayComment()

[AlexpFr]

Ok, I'll see if I can do that within a reasonable amount of time.

[AlexpFr]

I removed the key checks from the UI layer without creating new functions.

Your idea of ​​putting this in the OpenSSHKey class is appealing, but it would require deeper code modifications.
'OpenSSHKey' would need 'username' and 'filename' which are not accessible to it.

If we do that, we might as well go all the way and move this part of toOpenSSHKey() function into the OpenSSHKey class :

    if (!key.parsePKCS1PEM(privateKeyData)) {
        m_error = key.errorString();
        return false;
    }

    if (key.encrypted() && (decrypt || key.publicKey().isEmpty())) {
        if (!key.openKey(password)) {
            m_error = key.errorString();
            return false;
        }
    }

    if (key.comment().isEmpty()) {
        key.setComment(username);
    }

    if (key.comment().isEmpty()) {
        key.setComment(fileName);
    }

But that's no longer a fix..."

[droidmonkey]

Yah I like these changes now, no need to go further at this point. Thank you!

[AlexpFr]

Thank you for your help.
Before merging the PR, I've asked a question on Transifex regarding a gender issue for the translation of the word encoded in some languages.
I hope we can find an elegant solution!

[droidmonkey]

Doesn't need to hold up this PR, translations are independent of pull requests. However you do need to make format

[AlexpFr]

Format done !

[AlexpFr]

Hello, is this PR ready for a merge? Do I need to review certain points?
Thanks.