Re: host name support in pg_hba.conf

Peter Eisentraut <peter_e(at)gmx(dot)net> writes:
> Hopefully final patch, which addresses the above issues, adds some
> documentation enhancements, and the possibility to quote host names (in
> case someone wants to have a host named "samehost").

A few minor gripes:

> + If a host name is specified (anything that is not an IP address
> + or a special key word is processed as a potential host name),
> + that name is compared with the result of a reverse name
> + resolution of the client's IP address (e.g., reverse DNS
> + lookup, if DNS is used). If there is a match, then a forward
> + name resolution (e.g., forward DNS lookup) is performed on the
> + host name to check if it resolves to an IP address that is
> + equal to the client's IP address. If both directions match,
> + then the entry is considered to match.

I think the reason you're getting repeated questions about why the
reverse DNS lookup is needed is that the documentation fails to explain
that. It'd be helpful if this part of the docs pointed out why the
apparently-extra lookup is necessary. If I understand correctly, the
point is that we do the reverse lookup only once per connection (when
first finding a name-based pg_hba.conf entry) and that saves us having
to do forward lookup on all the name-based entries that don't match.

Which means BTW that the technique loses if the first name-based entry
matches the connection, and only wins when the match comes at the third
or later name-based entry. Are we really sure this is a good tradeoff?
Are we sure there shouldn't be a way to turn it off? I'm a bit
concerned about the fact that the argument seems to be "better
performance" versus "makes it completely useless for some use cases",
as here:
http://archives.postgresql.org/pgsql-hackers/2010-08/msg00726.php

Another smaller point is that it might be helpful if the wording didn't
make it sound like we expect the forward DNS lookup to produce just
one IP address. Perhaps "If there is a match, then a forward name
resolution (e.g., forward DNS lookup) is performed on the host name to
check if any of the addresses it resolves to are equal to the client's
IP address."

In check_hostname():

> + port->remote_hostname = strdup(remote_hostname);

Seems like this had better be pstrdup(), or at least have an error
check. Dumping core at the next line is not a real acceptable
substitute for an "out of memory" error.

> + /* Lookup IP from host name and check against original IP */
> + ret = getaddrinfo(port->remote_hostname, NULL, NULL, &gai_result);
> + if (ret != 0)
> + ereport(ERROR,
> + (errcode(ERRCODE_CONFIG_FILE_ERROR),
> + errmsg("getaddrinfo failed on \"%s\": %s",
> + port->remote_hostname, gai_strerror(ret))));

Is ERRCODE_CONFIG_FILE_ERROR really the most appropriate code here?

regards, tom lane