Horton

Re: Allow file inclusion in pg_hba and pg_ident files

Michael Paquier <michael@paquier.xyz>

View thread View on postgr.es
From: Michael Paquier <michael@paquier.xyz>
To: Julien Rouhaud <rjuju123@gmail.com>
Cc: Aleksander Alekseev <aleksander@timescale.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Nathan Bossart <nathandbossart@gmail.com>
Date: 2022-03-28T07:20:07Z
Lists: pgsql-hackers 
On Sun, Mar 27, 2022 at 05:52:22PM +0800, Julien Rouhaud wrote:
> I didn't like the various suggestions, as it would mean to scatter the tests
> all over the place.  The whole point of those views is indeed to check the
> current content of a file without applying the configuration change (not on
> Windows or EXEC_BACKEND, but there's nothing we can do there), so let's use
> this way.  I added a naive src/test/authentication/003_hba_ident_views.pl test
> that validates that specific new valid and invalid lines in both files are
> correctly reported.  Note that I didn't update those tests for the file
> inclusion.

I see.  The tests ought to be more extensive though, as hba.c checks
for multiple or missing fields for a varying number of expecting
parameters.  Here are the patterns that would cover most of the
failures of the backend.  For hba.conf:
+host
+host incorrect_db
+host incorrect_db incorrect_user
+host incorrect_db incorrect_user incorrect_host

For pg_ident.conf:
+# Error with incomplete lines.
+incorrect_map
+incorrect_map os_user
+# Errors with lines that have multiple values, for each field.
+incorrect_map_1,incorrect_map_2
+incorrect_map os_user_1,os_user_2
+incorrect_map os_user pg_role_1,pg_role_2

Then I was thinking about doing full scan of each view and could the
expected errors with some GROUP BY magic.  See the attached, for
reference, but it would fail with EXEC_BACKEND on WIN32.

> Note that those tests fail on Windows (and I'm assuming on EXEC_BACKEND
> builds), as they're testing invalid files which by definition prevent any
> further connection attempt.  I'm not sure what would be best to do here, apart
> from bypassing the invalid config tests on such platforms.  I don't think that
> validating that trying to connect on such platforms when an invalid
> pg_hba/pg_ident file brings anything.

Hmm, indeed.  I have been looking at that and that's annoying.  As you
mentioned off-list, in order to know if a build has an emulation of
fork() that would avoid failures with new connection attempts after
including some dummy entries in pg_hba.conf or pg_ident.conf, we'd
need to look at EXEC_BACKEND (well, mostly), as of:
- CFLAGS in pg_config, or a query on pg_config() (does not work with
MSVC as CFLAGS is not set there).
- A potential check on pg_config{_manual}.h.
- Perhaps an extra dependency on $windows_os, discarding incorrectly
cygwin that should be able to work.

We could use a failure path for each psql command rather than a SKIP
block, as you told me, if the psql fails and check that we get some
error strings related to the loading of auth files.  However, I am
scared of this design in the long-term as it could cause the tests to
pass with a failure triggered on platforms and/or configurations where
we should have a success.  So, I am tempted to drop the ball for now
with the TAP part.

The patch still has value for the end-user.  I have checked the
backend part, and I did not notice any obvious issue.  There is one
thing that I am wondering though: should we change the two queries in 
sysviews.sql so as we check that there are zero errors in the two
views when the files are parsed?  This simple change would avoid
mistakes for users running installcheck on a production installation.
--
Michael

Commits

  1. Add TAP tests for include directives in HBA end ident files

  2. Introduce variables for initial and max nesting depth on configuration files

  3. Add support for file inclusions in HBA and ident configuration files

  4. Add missing initialization in tokenize_expand_file() for output list

  5. Rework memory contexts in charge of HBA/ident tokenization

  6. Add error context callback when tokenizing authentication files

  7. Invent open_auth_file() in hba.c to refactor authentication file opening

  8. Use AbsoluteConfigLocation() when building an included path in hba.c

  9. Move code related to configuration files in directories to new file

  10. doc: Fix some descriptions related to pg_ident_file_mappings

  11. Add rule_number to pg_hba_file_rules and map_number to pg_ident_file_mappings

  12. Refactor code handling the names of files loaded in hba.c

  13. Make consistent a couple of log messages when parsing HBA files

  14. Use hba_file/ident_file GUCs rather than pg_hba.conf/pg_ident.conf in logs

  15. Fix path reference when parsing pg_ident.conf for pg_ident_file_mappings

  16. Add system view pg_ident_file_mappings

  17. Modify query on pg_hba_file_rules to check for errors in regression tests

  18. Refactor code related to pg_hba_file_rules() into new file