leamas / ddupdate

Update DNS Data for Dynamic IP Addresses
MIT License
40 stars 28 forks source link

RFE: finer grained auth with providers #52

Closed g2p closed 2 years ago

g2p commented 2 years ago

Currently the netrc-based system picks passwords based on just the hostname of the API server. We need a way to discriminate multiple accounts at the same provider when selecting login and password. In particular, with Google Domains, multiple records appear as separate accounts, so this is needed to support updating multiple records.

Might be solved with #23 or possibly separately.

leamas commented 2 years ago

hm... .netrc basically maps a hostname to a password. This is a documented, well established behaviour and it's probably a Bad Idea to add something to the hostname -- this will break the file when used in other contexts. Ergo: no reasonable solution as long as using .netrc.

Likewise, the idea to invent some other file for this purpose is, well... let's not do that.

Which basically leaves the option to have this actual while handling #23. There is now code supporting multiple configuration file sections (#25), this is a step forward also when thinking about this.

leamas commented 2 years ago

More thinking. The hostname of the API server might not be unique when there are several accounts on the same API server,

However, the hostname we configure, something like myhost.someservice.net, has to be unique, right? That is, if we use this as key rather than the API hostname we should be fine.

Since netrc supports an account token it should be possible to implement this on current netrc solution, decoupling it from #23.

@g2p: Thoughts?

leamas commented 2 years ago

... which actually boils down to that the current system fundamentally should work out of the box if writing a plugin for, say, Google domains. The only thing required by the plugin is to use

user, password = get_netrc_auth(hostname)

instead of

user, password = get_netrc_auth('domains.google.com')

which should work just fine. However, ddupdate-config would not make the right things here so it would require manual editing of ~/.netrc.

@g2p : Let me know what you think. If you plan to make a plugin for google domains, I might look into ddupdate-config (which is some code I'm not that proud of :( ). If you don't have any such plans, let's just close this issue as wontfix for now.

EDIT: If you need also the account we could modify get_netrc_auth() to support also this, like

user, password, account = get_netrc_auth('domains.google.com', include_account = True)

For ddupdate-config we would need to mark the plugin as using configured hostname rather than API hostname, probably just an attribute like netrc_hostname which could be set to 'config' or so, defaulting to 'api' if not existing.

g2p commented 2 years ago

I've looked at how curl handles netrc and have an idea that should work with the configuration system. I'm pleased that it now handles multiple sections by the way.

Have get_netrc_auth take a username parameter, have configuration sections look for an optional login and pass it to get_netrc_auth. If get_netrc_auth is given a login, it needs to find exactly one matching entry for the (machine, login) pair. If it isn't given one, it needs to find exactly one matching entry for the machine. For plugins that don't actually use the login (password is an API key), the login can still be used as a discriminant.

leamas commented 2 years ago

hm... I don't really follow you here.

Have get_netrc_auth take a username parameter,

OK, something like user, password = get_netrc_auth('domains.google.com', 'user_foo') ?

have configuration sections look for an optional login and pass it to get_netrc_auth

Here, I'm lost...

g2p commented 2 years ago
[domain1]
hostname = blah.dev
service-plugin = domains.google.com
login = foo

becomes

user, password = get_netrc_auth('domains.google.com', 'foo')

Whereas just

[domain1]
hostname = blah.dev
service-plugin = domains.google.com

becomes

user, password = get_netrc_auth('domains.google.com', None)
g2p commented 2 years ago

With a netrc file like this:

machine domains.google.com login foo password p4ss
machine domains.google.com login bar password pa55

get_netrc_auth('domains.google.com', 'foo') returns 'foo', 'p4ss' whereas get_netrc_auth('domains.google.com', None) is invalid due to being ambiguous. If there is just one netrc line for the domain, omitting the login keeps working as before.

leamas commented 2 years ago

ok, the 'configuration sections' wording fooled me

The drawback here is that netrc will contain multiple lines with the same machine. This is not really how things are intended to work, and the standard python bindings just not support it [1]

For that reason, the approach to use the configured hostname as key has an advantage -- the configured host is a DNS name which by definition is unique. Can you see any drawbacks with this?

g2p commented 2 years ago

Ah, I looked at the stdlib module, there's no issue with the parsing, just with the way it stuffs everything into a hosts[machine] dictionary. It wouldn't be too hard to add an accounts[(machine, login)] dictionary as well, using the stdlib code as a starting point.

Your idea works as well, but as far as drawbacks, the configured host might have its own netrc entry for unrelated reasons, maybe for curl or ftp.

leamas commented 2 years ago

It wouldn't be too hard to add an accounts[(machine, login)] dictionary as well, using the stdlib code as a starting point.

Sure, but forking standard libraries is seldom a good idea.

the configured host might have its own netrc entry for unrelated reasons, maybe for curl or ftp.

True. So this is about forking a system library vs collisions when using netrc for a configured host for other purposes.

As I see it, using the configured host is overall a more simple solution. It also maintains the netrc semantics as described in the manpage -- if we have two lines with the same machine the use of .netrc becomes more or less undefined when running for example ftp.

The "collision" drawback exists. Perhaps we could handle it by adding a fake TLD to the configured host entries. That is, a host like myhost.someservice.com is stored in netrc as myhost.someservice.com.ddupdate. This would never match a netrc entry used for other purposes, and would still be easy to handle for ddupdate.

Thoughts?

leamas commented 2 years ago

ping?

g2p commented 2 years ago

My preferred way is still the way curl does it, which uses the login. My next preferred way might be to alias hosts under a .ddupdate tld, but it requires care if you still want to handle netrc entries that predate the change.

leamas commented 2 years ago

Seems that we still disagree. OTOH, in the end I am the maintainer here, and we need to make a decision.

So, in other words: can you live with the "aliased hosts" solution?

g2p commented 2 years ago

Sure, I can. Thank you.

leamas commented 2 years ago

I'll add a parameter to get_netrc_auth()as described above, in some meantime while I'm bisecting the kernel.

With that change you should be fine to develop and test a google-domains plugin. You would need to edit the .netrc manually.

OK?

g2p commented 2 years ago

I'm willing to send a PR that does both (update get_netrc_auth and the domains plugin) if we go with the tld as described here: https://github.com/leamas/ddupdate/issues/52#issuecomment-1023124796