Here is the fix:

I do think that switching from our own copied-around-code to a wider shared component for single-application setups does make very much sense rather than try to battle-harden our own code against scenarios of various likeliness.

I would suggest to move the is_elevated check before checking for running instances and then always terminate the process. For those footgunners we can add a Registry key AllowRunningAsAdmin=footgun as HKCU which prints it only as a warning.

See also T5248

I think we have a regression with this change. This is the old behaviour (gnupg 2.2 in this case, though)

Minimum fix is:

Well, I don't think we'll add platform-specific X11 code to pinentry-qt just to check for an invalid DISPLAY. We are using Qt so that we don't have to deal with platform-specific stuff. I have no intention to look into this and, given Wayland, investing any more time in X11 feels wasted. We might accept a patch that can be used by all GUI pinentries to check for a usable DISPLAY.

"ikloecker (Ingo Klöcker)" wrote:

ikloecker added a comment.

How is "invalid DISPLAY" defined? `DISPLAY=invalid`? Anything that's not `DISPLAY=:<some number>`? Why do screen and tmux have to use an extra-wurst?

[...]

For my own understanding I repeat your explanation with some changes
for clarity:

@werner I can confirm that we've tested the patch and it seems to fix the issue in our setup.

The attack works like this: An unprivileged user starts an application which creates a window like the one Kleopatra looks for. Then the normal user (or an admin) starts Kleopatra. Kleopatra finds the existing window (it looks for any window with the right name) and grants the unprivileged process full access to the Kleopatra process. Now the unprivileged process can do anything the Kleopatra process can do.

This is not a security bug. Consider: The user starts kleopatra as administrator (via runas or an administrator terminal) and then starts a second kleopatra to have a "privilege escalation" - So what is the point - if you can do runas you already have all the privileges you could get with this privilege escalation.

Maybe. EncryptionResult has a list of invalid recipients and I've changed the code to show the Retry dialog only if there's at least one invalid recipient.

Your suggestion sounds ok to me, maybe with a slight change for the message: "Failed to encrypt the notepad because at least on certificate could not be validated."

I tried to add the list of invalid recipients to the message box, but it seems that gpgsm stops the validation of the certificates at the first invalid recipient. I got only the first Bob certificate reported as invalid recipient when I tried to encrypt to both Bob certificates so that it doesn't make sense to list the (incomplete) list of invalid recipients. It also means that Kleopatra cannot update the invalid recipient certificates because it knows only of one invalid certificate.

Ideally the certificate would change, but Kleopatra has no idea that this certificate turned out to be not valid. In fact, Kleopatra doesn't even know that the encryption failed because of some certificate. It could have failed for any other reason (e.g. full disk). Kleopatra only knows that an error occurred and offers to retry with lower security. (I looked at GpgOL and it does the same.)

GpgOL/Web is likely also affected.

yes, basically it's what we want.