Allow the user to configure a pinentry #202
Conversation
(and optionally a passphrase entry). It uses `pinentry` by default, which should be available if gpg is installed. It looks roughly similar to the existing pinentry.
libagent/device/trezor.py
Outdated
| encoding='utf-8', | ||
| stdin=subprocess.PIPE, stdout=subprocess.PIPE, | ||
| ) | ||
| except OSError as e: |
There was a problem hiding this comment.
This is Py2 specific. How would you handle compatibility in this case? Catch both? Wrap subprocess?
libagent/gpg/__init__.py
Outdated
| @@ -255,6 +266,8 @@ def main(device_type): | |||
| p.add_argument('-t', '--time', type=int, default=int(time.time())) | |||
| p.add_argument('-v', '--verbose', default=0, action='count') | |||
| p.add_argument('-s', '--subkey', default=False, action='store_true') | |||
| p.add_argument('-p', '--pinentry') | |||
| p.add_argument('-pa', '--passentry') | |||
There was a problem hiding this comment.
Maybe these would be better as long options only.
rendaw
force-pushed
the
configure-pinentry
branch
from
c9ffe6e
to
4a06ccb
Compare
|
I noticed I missed SSH config. Any suggestions on how to access the config there? Should that just read gpg-agent.conf? Will fix remaining lint issues tomorrow. Edit: I have SSH load the gpg-agent.conf to get the pinentry programs and it also uses the log level configured there now. |
rendaw
force-pushed
the
configure-pinentry
branch
from
4a06ccb
to
bb9ec4d
Compare
|
@rendaw Please let me know when I can review this PR :) |
|
Ah, please review it when you are available! :) I've tested the GPG portion and added the SSH config, and the linting passes. I think it's complete at this point. I haven't switched over to using trezor-agent keys yet though for day to day work, so I'd like to do that. |
|
Found this regarding default pinentry program logic: https://github.com/gpg/gnupg/blob/660eafa3a9f68e116e9b0597edc317d8ff90f9b2/common/homedir.c#L932 - so there's more that could be done to come up with fallback names for Windows. At the moment though it will still use the builtin pinentry so it shouldn't cause problems at least. |
|
I just figured out that SSH operation is completely separate from the GPG setup and ring, so the configuration files might not be there. Do you have any suggestions how to handle the pinentry/passphrase entry configuration for SSH in that case? Edit: On second thought, since this is by default a GPG pinentry program and protocol maybe it's reasonable to ask them to complete the GPG setup first. |
rendaw
force-pushed
the
configure-pinentry
branch
from
16f62b2
to
da59ba4
Compare
|
@romanz I made the pinentry SSH config a normal command line argument which seems to be working alright, although I think there's an issue with configargparse (bw2/ConfigArgParse#114) which makes the config file unusable. I don't think there's anything left to do prior to review, and I've tested it for both SSH and GPG usage locally (seems to work well). In the meantime if I'll continue to use it myself and fix issues if I find any. |
There was a problem hiding this comment.
First of all - many thanks for effort adding the needed functionality, and sorry for taking so long to review it...
I've understood that my original design of the PIN/passphrase entry is not extensible enough - so I've opened #211 - which should make the integration with trezor-gpg-pinentry-tk much easier.
Could you please take a look at #211?
|
Closing in favor of #211 |
(and optionally a passphrase entry).
It uses
pinentryby default, which should be available if gpg is installed, then falls back to the existing builtin pinentry. I also had to parse the options passed to the agent to pass them to the pinentry - this communicates things like the X11 display and user's current TTY so the pinentry can communicate properly.