diablodale / pinentry-wsl-ps1

GUI for GPG within Windows WSL for passwords, pinentry, etc.
Mozilla Public License 2.0
97 stars 9 forks source link

allow options to be passed to the script #4

Open Konfekt opened 4 years ago

Konfekt commented 4 years ago

This way, options can be set by, say NOTIFY=0 pinentry-wsl-ps1.sh; thus the script can be updated without losing once set options.

diablodale commented 4 years ago

Nice idea. Thanks for the contribution. For the implementation, perhaps using ${-} instead of ${:-}. The behavior changes when the incoming value is "" or null.

with ${-}

NOTIFY="" pinentry-wsl-ps1.sh
inside script NOTIFY=""

with ${:-}

NOTIFY="" pinentry-wsl-ps1.sh
inside script NOTIFY=1

And have you tested step 2 of setup using the VAR=VALUE pinentry-wsl-ps1.sh approach? A quick read of the gpg code makes me suspect this will fail. The pinentry-program expects a program path, not a script command. What did you see when testing?

An alternative might be to use the env var PINENTRY_USER_DATA described https://gnupg.org/documentation/manuals/gnupg.pdf and code https://github.com/gpg/gnupg/blob/fd79cadf7ba5ce45dfb5e266975f58bf5c7ce145/agent/call-pinentry.c#L425 The PINENTRY_USER_DATA isn't automatically parsed by gpg code, so pinentry-wsl-ps1.sh would need to manually parse it when it is received via the Assaun option OPTION pinentry-user-data for any settings contained within. For example https://kevinlocke.name/bits/2019/07/31/prefer-terminal-for-gpg-pinentry/

Konfekt commented 4 years ago

For the implementation, perhaps using ${-} instead of ${:-}

Well observed. The last commit adapts the code accordingly.

The pinentry-program expects a program path, not a script command. What did you see when testing?

I have no WSL system at hand, but this is rather a question of documenting the feature. I suppose then that env NOTIFY="" pinentry-wsl-ps1.sh instead of NOTIFIY="" pinentry-wsl-ps1.sh will go through fine?

diablodale commented 4 years ago

I do not think env will work either. Same problem as above, the gpg agent tries to run a program named env NOTIFY=""... with spaces in it which would naturally fail.

https://github.com/gpg/gnupg/blob/fd79cadf7ba5ce45dfb5e266975f58bf5c7ce145/agent/call-pinentry.c#L352

From what I can understand (unless you see otherwise in your testing), it is not possible to set environment variables on the "same line" as this pinentry program in the real-world. This is due to the gpg agent interface between that agent and a pinentry program. It might be possible to set NOTIFY in a global scope in which gpg-agent is launched and hope it is inherited by the pinentry program. Though a global env var named NOTIFY is somewhat concerning.

Testing is needed for your proposed code change. Trying your idea in a real-world WSL, Windows, GPG setup to see if it is successful.

I like your idea. But I predict problems that testing can surface or prove me wrong 😅

To move forward, you'll need to do the testing I've written about above to explore what works and not. Maybe its your envvar technique, maybe its PINENTRY_USER_DATA, or maybe something else. When you find a method that does work, then I'm happy to consider it 🙂

Konfekt commented 4 years ago

Okay, testing is called for this setup. In the meanwhile, alternative setups, such as wrapping pinentry-wsl-ps1.sh into a script that sets these variables, or exporting them before calling it (and unsetting them afterwards?!) could be still of use and be kept in mind.

Konfekt commented 3 years ago

Okay, indeed either

PERSISTENCE=${PERSISTENCE-""}
NOTIFY=${NOTIFY-"1"}
DEBUGLOG=${DEBUGLOG-""}

be set globally, or better pinentry-wsl-ps1 parse $PINENTRY_USER_DATA.

Because the same config can be used on different machines, it could also fall back to other pinentry programs adding a case "wls".

diablodale commented 3 years ago

I don't see any gpg2 standard PINENTRY_USER_DATA or PINENTRY_BINARY in gpg2 documentation that is focused on this narrow feature area. Where are you seeing these features/standards?

I only found https://www.gnupg.org/documentation/manuals/gnupg/GPG-Configuration.html#index-PINENTRY_005fUSER_005fDATA which passes data from agent->pinentry, and the related Assuan option https://www.gnupg.org/documentation/manuals/gnupg/GPGSM-OPTION.html. Both of them do not seem (to me) to be designed for a user to set options. And not designed for concatenating/appending options together...instead options are overwritten and therefore a user's options are easily overwritten by the agent->pinentry step.

Konfekt commented 3 years ago

There is no PINENTRY_BINARY in gpg2 documentation. From what I gathered, it is implemented in Opensuse.

PINENTRY_USER_DATA is documented on page 89 of the GPG documentation, Version 2.2.29 from June 2021. Not sure if this makes it a gpg2 standard. Citing

PINENTRY_USER_DATA ... is useful to convey extra information to a custom pinentry.

One use case could be a custom pinentry that opts for pinentry-wsl-ps1 on a WSL system.

diablodale commented 3 years ago

Thanks. Here are my thoughts....

  1. Now I understand that PINENTRY_USER_DATA is an env ​variable that is read by gpg-agent. Then gpg-agent sends the value to the pinentry program via something.
  2. It is unclear to me the via something. How it is sent? Env var? Assuan? Pinentry command? This is probably easy to determine by some trial/error.
  3. It is important that the value of PINENTRY_USER_DATA not be a source of script injection problems. It should have no expansion, no replacement, no interpretation, etc.

I don't have the bandwidth to code this. Do you want to submit a PR? I will gladly review a PR and do some testing on it.