Make password retrieval per-storage and explicit

[untitaker]

At the moment password retrieval is a bit flaky: Vdirsyncer "just tries" a series of methods and sends the next-best thing to the server. This is very problematic because in the case of a failing custom command, vdirsyncer might send a wrong password (from a password store not intended by the user) to the server. One could call this leaking secrets.

I've worked around this for now by aborting this process if the custom command fails, but we have to seriously rethink how password retrieval is supposed to work.

[untitaker]

Also I think it's somewhat unfortunate that vdirsyncer only resolves passwords by hostname. This becomes a problem if one wants to sync multiple accounts from the same server. This problem vanishes if password config becomes per-storage. I wonder if we should copy offlineimap's "pythonfile" for this or stick to shell commands.

[untitaker]

I thought a lot about this and the only way I can see working is the following. Instead of:

[storage foo]
param = secretValue

you write one of this:

fetch:param = ["command", "mycommand"]
fetch:param = ["keychain", "passwordname"]

The fetching would happen without the storage's knowledge, which is why with netrc one would have to explicitly set the hostname:

fetch:param = ["netrc", "example.com"]

I wonder if it's not more straightforward to just cat a file instead of that abomination.

[untitaker]

@geier @hobarrera You were rather critical of removing netrc support, would the kind of support as shown above still be worth it?

[WhyNotHugo]

I personally run a custom command, and think that it's the most flexible choice (eg: you can even grep netrc if you want to remove support for that).

I don't really follow what you're proposing with fetch:param, can you rephrase a bit?

[untitaker]

Not sure how to phrase it differently. Basically instead of this:

[general]
password_command = "cat secret.txt"

[storage foo]
type = carddav
username = foobar

you write this:

[storage foo]
type = carddav
username = foobar
fetch:password = ["command", "cat", "secret.txt"]

The nice thing is that you can apply this to arbitrary params:

[storage foo]
type = carddav
fetch:username = ["command", ...]
fetch:password = ["command", ...]

[WhyNotHugo]

Oh, I get it now. This looks like a great idea, really really flexible. Not sure what the issue with storing usernames (or other settings except password) in the config would be though.

[untitaker]

I only saw that offlineimap supported this, and I guess it's a nice to have.

My main motivation is, however, that this removes all password-fetching code from vdirsyncer.storage.

[WhyNotHugo]

My main motivation is, however, that this removes all password-fetching code from vdirsyncer.storage.

That makes a bunch of sense, IMHO. If properly done, this can be easily reusable too.

[ghost]

Sorry to reopen this thread, but with this new method is it always possible to get the password from the command-line (like sudo) without using any script or extra file ?
If so, how to do it ?

[untitaker]

I've removed that feature since you can implement it as custom command. Do people actually use this?

[ghost]

I sync my calendars manually and not so often. I do not want to write explicitly my passwords in the config file or use a thirdparty to enter a password.
What could be the simplest command to achieve such behavior?
If it's not possible or too cumbersome, I will look at the keyring method.

[untitaker]

I thought about something like ["command", "bash", "-c", "read -p 'Password: ' -s $x; echo $x"], but I realize that probably wouldn't work because "command" captures all output. Also, now that I've typed it out, it looks horrible.

I've re-added a way to prompt for the password.

[ghost]

The prompt introduced in 21b1baf is working nicely, thanks for getting back this functionality.
Would it just be possible to add a new line between the different operations (syncing and prompting), because when syncing multiple calendars, the following appears (where CalendarN are different):

Syncing Calendar1Password for Calendar2:
Detected change in config file, discovering collections for Calendar3Password for Calendar4:

[untitaker]

This must've been an issue with the old behavior too, can you confirm that?