plattfot / pinentry-rofi

Rofi frontend to pinentry
43 stars 5 forks source link

High CPU Load when secret key is not available (e.g. YubiKey not inserted) #4

Closed leona-ya closed 4 years ago

leona-ya commented 4 years ago

I use pinentry-rofi with my GPG-Key saved on my YubiKey (as smartcard). When I try to use GPG without having the YubiKey inserted, pinentry-rofi does not show an error message an creates a high CPU load and temperature. After inserting the YubiKey the load falls and the message for entering the password shows up

plattfot commented 4 years ago

Not sure if this the fault of pinentry-rofi, as it only sets up the interface for typing in the password. It should just wait for input and not waste cpu cycles spinning in some loop. Or abort if it gets a command it doesn't understand. I could be wrong though.

I only have a yubikey standard which I bought in 2013 so cannot really reproduce this. What is the behavior of other pinentry programs? Do they error out?

n0emis commented 4 years ago

I can reproduce this. It seems like pinentry-rofi crashes and get's respawned by gpg-agent, because it changed its PID in a matter of seconds. I had to kill my gpg-agent to stop this.

plattfot commented 4 years ago

Ok, cool. Then that explains the high CPU load.

I changed the behavior when it encounter an unknown command. Instead of bailing out, it now print an error message with the gpg error code for not implemented.

I also fixed an issue with it not exiting when the command BYE is given.

And lastly I added an option --log <logfile>, where <logfile> is a path to a file. It will then write out unknown commands it encounters to it, which hopefully can help debug stuff like this. It will append the PID to the filename. If you have added pinentry-rofi-guile to /usr/bin/pinentry, usage would be

#!/bin/sh

exec /usr/bin/pinentry-rofi-guile --log /tmp/pinentry-rofi "$@"

All this are added to the high_cpu_load_4 branch. Please test it out and see if it fixes the issue. And if not please attach the log of one run. That would help me tremendously to track down the issue. Just make sure no sensitive information is included in the log.

Thanks

n0emis commented 4 years ago

The following message seemed to appear a lot in the logs:

Unknown command: "CONFIRM"
plattfot commented 4 years ago

Ok, it's probably trying to pop up a window of some sort. My guess is notify the user that it cannot find the key and either continue or cancel. Need to check the source code for some other pinentry implementation and see if I can figure it out. Should be doable to pop up a rofi window with the message.

plattfot commented 4 years ago

Added a pop up when the CONFIRM command is used, please test it out as I have no way of reproducing this.

Thanks

n0emis commented 4 years ago

Thanks, that already works well. (With some minor displaying issues, see the attached screenshot) Screenshot from 2020-05-02 13-57-52 ~@n0emis @em0lar

n0emis commented 4 years ago

Is this also the CONFIRM pop-up? Because it has the same issue with linebreaks... Screenshot from 2020-05-02 14-08-04

plattfot commented 4 years ago

Glad to hear it works. Yeah both are the CONFIRM pop up. I'll look into why the linebreaks isn't working. And also fix the prompt, it shouldn't say Passphrase::

plattfot commented 4 years ago

Try it now, it should properly handle newlines in the description.

n0emis commented 4 years ago

Hmm, unfortunately not for me. I can still see \n but no newlines in the output.

plattfot commented 4 years ago

Ok, strange. Can you run the script manually and type in after the prompt OK Please go ahead

SETDESC Foo\nBar
CONFIRM

Do you see a newline in the pop up? Or is it Foo\nBar?

You can exit the script with either ctrl+c or type BYE.

leona-ya commented 4 years ago

It's really strange. When I do it manually it works (it has a new line and no \n in it)

n0emis commented 4 years ago

I have a solution: instead of replacing "\\\\n" you have to search for "%0A". Then it works.

The pinentry docs (for example at http://info2html.sourceforge.net/cgi-bin/info2html-demo/info2html?(pinentry)Protocol) states the following:

Note that all strings are expected to be encoded as UTF-8; PINENTRY takes care of converting it to the locally used codeset. To include linefeeds or other special characters, you may percent-escape them (e.g., a line feed is encoded as %0A, the percent sign itself is encoded as %25, etc.).

plattfot commented 4 years ago

Thanks @n0emis, changed the \\\\n to %0A hopefully that works.

And thanks for finding the manual protocol, found some other things pinentry-rofi isn't supporting. For example handling underline, better default handling of the ok and cancel button and handling CONFIRM --one-button/ MESSAGE. But they're out of the scope of this issue so I haven't pushed them to the high_cpu_load_4 branch.

n0emis commented 4 years ago

Thanks for looking into this. That has fixed our problems...

plattfot commented 4 years ago

Perfect, then I'll close this issue and merge the branch to master. Please reopen if that's not the case.