Remove configuration options

[dominikschulz]

gopass supports lots of configuration options.
This offers a lot of flexbility but adds lot of cognitive overhead to users and developers and makes testing all combinations quite hard.

We should try to reduce the number of configuration options to a bare minimum.

I propose the following changes:

Option	Action	Default	Comment
alwaystrust	remove	true	When encrypting the trust of a recipient shouldn't matter
askformore	keep	true	Nice feature, but we should not ask for more info by default
autoimport	keep	true	Missing recipient keys are super annoying and they virtually don't hurt
autopull	remove	true	If autopush is enabled we should always do autopull as well
autopush	keep	true	There must be a way to disable automatic git pushes
checkrecipients	remove	?	Should we drop invalid recipients by default?
cliptimeout	keep	60
debug	remove	false	Use GOPASS_DEBUG instead
gitrecurse	remove	true	Provide a command line flag to disable this per invocation
loadkeys	remove	true	Missing recipient keys are super annoying
nocolor	remove	false	Use GOPASS_NOCOLOR instead
noconfirm	keep	false	Confirming recipients per encryption is helpful, but sometimes annoying
nopager	remove	?	Should we drop this?
path	keep
persistkeys	remove	true	We should always persist keys on adding recipients as it doesn't hurt in any way
safecontent	keep	false

[zwopir]

nopager: can be dropped, if we use a isTTY or whatever check.

autopull and autopush are not independent config parameters. Maybe we can put them in one config option autorepoaccess = (on|off|pull). If we do, we need a better name than autorepoaccess.

checkrecipients should take into account if the secret can be encrypted for a users own gpg identity. It should at least encrypt with that identity, even if other recipients fail.

[metalmatze]

I agree that we can get rid of most of them. Providing them via env seems way easier.
Combining autopull & autopush is 💯 Maybe call it gitsync gitautosync?!

[frntn]

@dominikschulz does the old configuration file will be compatible when updating gopass ?
If not how can we migrate without loosing expected we had set up ?

Thanks

[dominikschulz]

The old configuration will be compatible, but some behavior may change depending on the values you used.

If you have specific concerns we will try to address them before the next release.

[frntn]

As of written, my concern was about autopull/autopush not appearing in the source code anymore...
So I was wondering how backward compatibility will be achieved here 🤔

This morning I have faced an actual issue : a new secret insertion have been rejected.
In fact, I have lost the ability to alwaystrust as it has been removed from config 😞

bash$ gopass edit team-devel/foo/bar
Unuseable public keys detected (IGNORING FOR ENCRYPTION):
  - 0xAD01D1EDC256A009 - John Doe <john.doe@anon.com>
gopass: Encrypting /foo/bar for these recipients:
 - 0xD385289FA809234F - Matthieu Fronton <m@tthieu.fr>

Do you want to continue? [Y/n]: n

Error: user aborted

[dominikschulz]

alwaystrust should be always true, if it isn't in master that would be a release critical bug.

[frntn]

Yep I had noticed this.
But it's not the behavior I see.
Not even in master branch 😞

bash$  go get -u -v github.com/justwatchcom/gopass
github.com/justwatchcom/gopass (download)

bash$ gopass version
gopass 0.0.0+HEAD go1.8.3 linux amd64
  GPG: 2.1.11
  Git: 2.7.4

bash$ git describe
v1.2.0-44-g916ab24

bash$ gopass edit team-devel/foo/bar
Unuseable public keys detected (IGNORING FOR ENCRYPTION):
  - 0xAD01D1EDC256A009 - John Doe <john.doe@anon.com>
gopass: Encrypting /foo/bar for these recipients:
 - 0xD385289FA809234F - Matthieu Fronton <m@tthieu.fr>

Do you want to continue? [Y/n]: n

Error: user aborted

I'd be happy to help for testing purpose, so i'll clone this whole setup for later
(while reverting in parallel for my actual day2day work)

Thanks.

[frntn]

@dominikschulz ?

[dominikschulz]

Will try to look into this soon.

[dominikschulz]

Ok, after testing and reading your bug report twice I finally noticed the important detail: Unuseable public keys detected (IGNORING FOR ENCRYPTION):.

Fact is that AlwaysTrust is always true for some time, but unfortunately CheckRecipients is always true as well, that way unseable keys won't even be passed to GPG. So AlwaysTrust doesn't matter anymore.

For now we should disable CheckRecipients until we figure out how to handle this.

[tcheneau]

Hi,

I know most of what has been discussed here as been merged already, but I can't help but notice a possible security issue here. I can open a separate issue if needed.

The alwaystrust config option has been removed and now the alwaystrust argument is always passed to gpg. If the autoimport feature is enabled (default true), I see a huge security issue.

If an attacker manages to commit to my repository a GPG key I do not trust, and I'm not careful when pulling (can easily be overlooked if there is a lot of changes in the history), it means that I will soon start reencoding my password with the attacker's public key. My understanding is that if alwaystrust was not set, the pubkey of the attacker would be added to my keyring, but gopass would not reencrypt my password file with its pubkey (would probably spit out an error message).

I actually was confronted to this behavior this very afternoon when trying to set up the tool with a colleague of mine and it certainly does not look like a sane default behavior. Am I missing something obvious here?

Regards,
Tony

[tcheneau]

Never mind my previous comment: I opened a separate issue for this problem (#305) in order to give it a better visibility.

[timkelty]

Has persistkeys been removed?
I noticed it is still referenced in the README, but gopass config persistkeys true seems to have no affect on the config.

[dominikschulz]

Yes, persistkeys is indeed gone and now always on by default.
Thanks for pointing out the issue with the README, has just been fixed.