Hi, On Tue, Jul 19, 2022 at 03:13:12PM +0900, Michael Paquier wrote: > On Mon, Jul 18, 2022 at 03:11:51PM +0800, Julien Rouhaud wrote: > > > I'm not really sure what should be done here. The best compromise I can > > think > > of is to split the tests in 3 parts: > > > > 1) view reporting with various inclusions using safe_psql() > > You mean in the case where the HBA and indent files can be loaded, > so as it is possible to peek at the system views without the > EXEC_BACKEND problem, right?
Yes, testing the general behavior when there are no errors in the auth files. > > 2) log error reporting > > This one should be reliable and stable enough by parsing the logs of > the backend, thanks to connect_ok() and connect_fails(). I meant testing the postgres logs reporting, something like generating entirely bogus files, restarting, checking that the start failed and verify that the logs are expected. With a variation that if only the pg_ident.conf is bogus, the server will (and should) still start, so both needs to be tested separately. > > 3) view reporting with various inclusions errors, using safe_psql() > > > > And when testing 3), detect first if we can still connect after introducing > > errors. If not, assume this is Windows / EXEC_BACKEND and give up here > > without > > reporting an error. Otherwise continue, and fail the test if we later can't > > connect anymore. As discussed previously, detecting that the build is using > > the fork emulation code path doesn't seem worthwhile so guessing it from the > > psql error may be a better approach. > > Yeah, we could do that. Now we may also fail on other patterns, so we > would need to make sure that a set of expected error outputs are the > ones generated? I'd be fine to give up testing the error output > generated in the system views at the end. Without a persistent > connection state with the same kind of APIs as any of the drivers able > to do so, that's going to be a PITA to maintain. Yes, or just checking that the error is expected on the psql side as the server will report it. I've been working on all of that and came up with the attached v8. - 0001: I modified things as discussed previously to report the real auth file names rather than the hardcoded "pg_ident.conf" and "pg_hba.conf" in the various log messages - 0002 and 0003 are minor fixes to error logs more consistent - 0004: the rule_number / mapping_number addition in the views in a separate commit - 0005: the main file inclusion patch. Only a few minor bugfix since previous version discovered thanks to the tests (a bit more about it after), and documentation tweaks based on previous discussions - 0006: the pg_hba_matches() POC, no changes About the regression tests: I added a new 003_file_inclusion.pl in src/test/authentication TAP tests to do things as previously described, ie test both extisting behavior and new features added, for working and non working scenarios. The view error reporting are bypassed for Windows / EXEC_BACKEND build by detecting a connection failure with the expected error reported by the server. If the error doesn't match, the rest of the tests are still skipped but overall test will fail due to the mismatch in the reported error. It's a bit troublesome to write such tests, as you need to keep in sync the various rule and line number, format things differently depending on whether you want to check the logs or the view with each new tests added and so on. To make the tests easier to write (and to maintain) I added some wrapper functions to add lines in the wanted files that allow to automatically generates the wanted regex (for the logs) or output (for the view). There are still things to be careful about and a bit of duplication, but this way I can easily modify any test, or add new ones, without the need to modify everything around. As written, it also uses the same included files for the log error reporting and the view error reporting, so there's no need to define everything twice. The tests work as expected locally on a normal build and an EXEC_BACKEND build, and also on the CI.
>From 7232b708099eec0fd27a43ed722388f4988655da Mon Sep 17 00:00:00 2001 From: Julien Rouhaud <julien.rouh...@free.fr> Date: Fri, 3 Jun 2022 15:56:23 +0800 Subject: [PATCH v8 1/6] Use real file names rather than pg_hba.conf/pg_ident.conf in messages. --- src/backend/postmaster/postmaster.c | 7 ++++--- src/backend/utils/init/postinit.c | 3 ++- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c index e541b16bdb..8bcfedef64 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -1419,7 +1419,8 @@ PostmasterMain(int argc, char *argv[]) * since there is no way to connect to the database in this case. */ ereport(FATAL, - (errmsg("could not load pg_hba.conf"))); + /* translator: %s is a configuration file */ + (errmsg("could not load %s", HbaFileName))); } if (!load_ident()) { @@ -2769,11 +2770,11 @@ SIGHUP_handler(SIGNAL_ARGS) if (!load_hba()) ereport(LOG, /* translator: %s is a configuration file */ - (errmsg("%s was not reloaded", "pg_hba.conf"))); + (errmsg("%s was not reloaded", HbaFileName))); if (!load_ident()) ereport(LOG, - (errmsg("%s was not reloaded", "pg_ident.conf"))); + (errmsg("%s was not reloaded", IdentFileName))); #ifdef USE_SSL /* Reload SSL configuration as well */ diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c index 29f70accb2..8801a5f5f5 100644 --- a/src/backend/utils/init/postinit.c +++ b/src/backend/utils/init/postinit.c @@ -217,7 +217,8 @@ PerformAuthentication(Port *port) * since there is no way to connect to the database in this case. */ ereport(FATAL, - (errmsg("could not load pg_hba.conf"))); + /* translator: %s is a configuration file */ + (errmsg("could not load %s", HbaFileName))); } if (!load_ident()) -- 2.37.0
>From 45c22e6186ded15605d07fc4c8abc38a1f9f2852 Mon Sep 17 00:00:00 2001 From: Julien Rouhaud <julien.rouh...@free.fr> Date: Fri, 29 Jul 2022 14:47:09 +0800 Subject: [PATCH v8 2/6] Add file name / file line context for incorrect regex in ident files. For consistency with all other error messages report those information, which are indeed useful to debug configuration errors. Author: Julien Rouhaud Reviewed-by: FIXME Discussion: https://postgr.es/m/20220223045959.35ipdsvbxcstrhya%40jrouhaud --- src/backend/libpq/hba.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c index 327a4b42af..1ad09f7dc6 100644 --- a/src/backend/libpq/hba.c +++ b/src/backend/libpq/hba.c @@ -2372,7 +2372,9 @@ parse_ident_line(TokenizedAuthLine *tok_line, int elevel) ereport(elevel, (errcode(ERRCODE_INVALID_REGULAR_EXPRESSION), errmsg("invalid regular expression \"%s\": %s", - parsedline->ident_user + 1, errstr))); + parsedline->ident_user + 1, errstr), + errcontext("line %d of configuration file \"%s\"", + line_num, IdentFileName))); *err_msg = psprintf("invalid regular expression \"%s\": %s", parsedline->ident_user + 1, errstr); -- 2.37.0
>From 775098bfffaef9f3f380b64c94c94479e9c2513c Mon Sep 17 00:00:00 2001 From: Julien Rouhaud <julien.rouh...@free.fr> Date: Fri, 29 Jul 2022 17:21:49 +0800 Subject: [PATCH v8 3/6] Standardize IDENT_FIELD_ABSENT error messages The rest of the code emit the file name and line in a distinct errcontext so make IDENT_FIELD_ABSENT consistent. This will help an upcoming commit will add regression tests for variour error scenario. Author: Julien Rouhaud Reviewed-by: FIXME Discussion: https://postgr.es/m/20220223045959.35ipdsvbxcstrhya%40jrouhaud --- src/backend/libpq/hba.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c index 1ad09f7dc6..deee05c197 100644 --- a/src/backend/libpq/hba.c +++ b/src/backend/libpq/hba.c @@ -900,8 +900,9 @@ do { \ if (!field) { \ ereport(elevel, \ (errcode(ERRCODE_CONFIG_FILE_ERROR), \ - errmsg("missing entry in file \"%s\" at end of line %d", \ - IdentFileName, line_num))); \ + errmsg("missing entry at end of line"), \ + errcontext("line %d of configuration file \"%s\"", \ + line_num, IdentFileName))); \ *err_msg = psprintf("missing entry at end of line"); \ return NULL; \ } \ -- 2.37.0
>From dba686a27746a3235d97b2c11439f1c13166deda Mon Sep 17 00:00:00 2001 From: Julien Rouhaud <julien.rouh...@free.fr> Date: Mon, 30 May 2022 10:59:51 +0800 Subject: [PATCH v8 4/6] Add rule_number / mapping_number to the pg_hba/pg_ident views. Author: Julien Rouhaud Discussion: https://postgr.es/m/20220223045959.35ipdsvbxcstrhya%40jrouhaud --- doc/src/sgml/system-views.sgml | 22 +++++++++++++ src/backend/utils/adt/hbafuncs.c | 50 ++++++++++++++++++++++------- src/include/catalog/pg_proc.dat | 11 ++++--- src/test/regress/expected/rules.out | 10 +++--- 4 files changed, 72 insertions(+), 21 deletions(-) diff --git a/doc/src/sgml/system-views.sgml b/doc/src/sgml/system-views.sgml index 26ce83eb9b..b4ee40c042 100644 --- a/doc/src/sgml/system-views.sgml +++ b/doc/src/sgml/system-views.sgml @@ -991,6 +991,18 @@ </thead> <tbody> + <row> + <entry role="catalog_table_entry"><para role="column_definition"> + <structfield>rule_number</structfield> <type>int4</type> + </para> + <para> + Rule number of this rule among all rules if the rule is valid, otherwise + null. This indicates the order in which each rule will be considered + until the first matching one, if any, is used to perform authentication + with the client. + </para></entry> + </row> + <row> <entry role="catalog_table_entry"><para role="column_definition"> <structfield>line_number</structfield> <type>int4</type> @@ -1131,6 +1143,16 @@ </thead> <tbody> + <row> + <entry role="catalog_table_entry"><para role="column_definition"> + <structfield>mapping_number</structfield> <type>int4</type> + </para> + <para> + Mapping number, in priority order, of this mapping if the mapping is + valid, otherwise null + </para></entry> + </row> + <row> <entry role="catalog_table_entry"><para role="column_definition"> <structfield>line_number</structfield> <type>int4</type> diff --git a/src/backend/utils/adt/hbafuncs.c b/src/backend/utils/adt/hbafuncs.c index 9e5794071c..c9be4bff1f 100644 --- a/src/backend/utils/adt/hbafuncs.c +++ b/src/backend/utils/adt/hbafuncs.c @@ -26,10 +26,12 @@ static ArrayType *get_hba_options(HbaLine *hba); static void fill_hba_line(Tuplestorestate *tuple_store, TupleDesc tupdesc, - int lineno, HbaLine *hba, const char *err_msg); + int rule_number, int lineno, HbaLine *hba, + const char *err_msg); static void fill_hba_view(Tuplestorestate *tuple_store, TupleDesc tupdesc); static void fill_ident_line(Tuplestorestate *tuple_store, TupleDesc tupdesc, - int lineno, IdentLine *ident, const char *err_msg); + int mapping_number, int lineno, IdentLine *ident, + const char *err_msg); static void fill_ident_view(Tuplestorestate *tuple_store, TupleDesc tupdesc); @@ -157,7 +159,7 @@ get_hba_options(HbaLine *hba) } /* Number of columns in pg_hba_file_rules view */ -#define NUM_PG_HBA_FILE_RULES_ATTS 9 +#define NUM_PG_HBA_FILE_RULES_ATTS 10 /* * fill_hba_line @@ -165,6 +167,7 @@ get_hba_options(HbaLine *hba) * * tuple_store: where to store data * tupdesc: tuple descriptor for the view + * rule_number: unique rule identifier among all valid rules * lineno: pg_hba.conf line number (must always be valid) * hba: parsed line data (can be NULL, in which case err_msg should be set) * err_msg: error message (NULL if none) @@ -174,7 +177,8 @@ get_hba_options(HbaLine *hba) */ static void fill_hba_line(Tuplestorestate *tuple_store, TupleDesc tupdesc, - int lineno, HbaLine *hba, const char *err_msg) + int rule_number, int lineno, HbaLine *hba, + const char *err_msg) { Datum values[NUM_PG_HBA_FILE_RULES_ATTS]; bool nulls[NUM_PG_HBA_FILE_RULES_ATTS]; @@ -193,6 +197,11 @@ fill_hba_line(Tuplestorestate *tuple_store, TupleDesc tupdesc, memset(nulls, 0, sizeof(nulls)); index = 0; + /* rule_number */ + if (err_msg) + nulls[index++] = true; + else + values[index++] = Int32GetDatum(rule_number); /* line_number */ values[index++] = Int32GetDatum(lineno); @@ -336,7 +345,7 @@ fill_hba_line(Tuplestorestate *tuple_store, TupleDesc tupdesc, else { /* no parsing result, so set relevant fields to nulls */ - memset(&nulls[1], true, (NUM_PG_HBA_FILE_RULES_ATTS - 2) * sizeof(bool)); + memset(&nulls[2], true, (NUM_PG_HBA_FILE_RULES_ATTS - 3) * sizeof(bool)); } /* error */ @@ -359,6 +368,7 @@ fill_hba_view(Tuplestorestate *tuple_store, TupleDesc tupdesc) FILE *file; List *hba_lines = NIL; ListCell *line; + int rule_number = 0; MemoryContext linecxt; MemoryContext hbacxt; MemoryContext oldcxt; @@ -393,7 +403,11 @@ fill_hba_view(Tuplestorestate *tuple_store, TupleDesc tupdesc) if (tok_line->err_msg == NULL) hbaline = parse_hba_line(tok_line, DEBUG3); - fill_hba_line(tuple_store, tupdesc, tok_line->line_num, + /* No error, set a new rule number */ + if (tok_line->err_msg == NULL) + rule_number++; + + fill_hba_line(tuple_store, tupdesc, rule_number, tok_line->line_num, hbaline, tok_line->err_msg); } @@ -430,8 +444,8 @@ pg_hba_file_rules(PG_FUNCTION_ARGS) PG_RETURN_NULL(); } -/* Number of columns in pg_ident_file_mappings view */ -#define NUM_PG_IDENT_FILE_MAPPINGS_ATTS 5 +/* Number of columns in pg_hba_file_mappings view */ +#define NUM_PG_IDENT_FILE_MAPPINGS_ATTS 6 /* * fill_ident_line: build one row of pg_ident_file_mappings view, add it to @@ -439,6 +453,7 @@ pg_hba_file_rules(PG_FUNCTION_ARGS) * * tuple_store: where to store data * tupdesc: tuple descriptor for the view + * mapping_number: unique rule identifier among all valid rules * lineno: pg_ident.conf line number (must always be valid) * ident: parsed line data (can be NULL, in which case err_msg should be set) * err_msg: error message (NULL if none) @@ -448,7 +463,8 @@ pg_hba_file_rules(PG_FUNCTION_ARGS) */ static void fill_ident_line(Tuplestorestate *tuple_store, TupleDesc tupdesc, - int lineno, IdentLine *ident, const char *err_msg) + int mapping_number, int lineno, IdentLine *ident, + const char *err_msg) { Datum values[NUM_PG_IDENT_FILE_MAPPINGS_ATTS]; bool nulls[NUM_PG_IDENT_FILE_MAPPINGS_ATTS]; @@ -461,6 +477,11 @@ fill_ident_line(Tuplestorestate *tuple_store, TupleDesc tupdesc, memset(nulls, 0, sizeof(nulls)); index = 0; + /* mapping_number */ + if (err_msg) + nulls[index++] = true; + else + values[index++] = Int32GetDatum(mapping_number); /* line_number */ values[index++] = Int32GetDatum(lineno); @@ -473,7 +494,7 @@ fill_ident_line(Tuplestorestate *tuple_store, TupleDesc tupdesc, else { /* no parsing result, so set relevant fields to nulls */ - memset(&nulls[1], true, (NUM_PG_IDENT_FILE_MAPPINGS_ATTS - 2) * sizeof(bool)); + memset(&nulls[2], true, (NUM_PG_IDENT_FILE_MAPPINGS_ATTS - 3) * sizeof(bool)); } /* error */ @@ -495,6 +516,7 @@ fill_ident_view(Tuplestorestate *tuple_store, TupleDesc tupdesc) FILE *file; List *ident_lines = NIL; ListCell *line; + int mapping_number = 0; MemoryContext linecxt; MemoryContext identcxt; MemoryContext oldcxt; @@ -529,8 +551,12 @@ fill_ident_view(Tuplestorestate *tuple_store, TupleDesc tupdesc) if (tok_line->err_msg == NULL) identline = parse_ident_line(tok_line, DEBUG3); - fill_ident_line(tuple_store, tupdesc, tok_line->line_num, identline, - tok_line->err_msg); + /* No error, set a new mapping number */ + if (tok_line->err_msg == NULL) + mapping_number++; + + fill_ident_line(tuple_store, tupdesc, mapping_number, + tok_line->line_num, identline, tok_line->err_msg); } /* Free tokenizer memory */ diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat index 2e41f4d9e8..e544f9f758 100644 --- a/src/include/catalog/pg_proc.dat +++ b/src/include/catalog/pg_proc.dat @@ -6128,15 +6128,16 @@ { oid => '3401', descr => 'show pg_hba.conf rules', proname => 'pg_hba_file_rules', prorows => '1000', proretset => 't', provolatile => 'v', prorettype => 'record', proargtypes => '', - proallargtypes => '{int4,text,_text,_text,text,text,text,_text,text}', - proargmodes => '{o,o,o,o,o,o,o,o,o}', - proargnames => '{line_number,type,database,user_name,address,netmask,auth_method,options,error}', + proallargtypes => '{int4,int4,text,_text,_text,text,text,text,_text,text}', + proargmodes => '{o,o,o,o,o,o,o,o,o,o}', + proargnames => '{rule_number,line_number,type,database,user_name,address,netmask,auth_method,options,error}', prosrc => 'pg_hba_file_rules' }, { oid => '6250', descr => 'show pg_ident.conf mappings', proname => 'pg_ident_file_mappings', prorows => '1000', proretset => 't', provolatile => 'v', prorettype => 'record', proargtypes => '', - proallargtypes => '{int4,text,text,text,text}', proargmodes => '{o,o,o,o,o}', - proargnames => '{line_number,map_name,sys_name,pg_username,error}', + proallargtypes => '{int4,int4,text,text,text,text}', + proargmodes => '{o,o,o,o,o,o}', + proargnames => '{mapping_number,line_number,map_name,sys_name,pg_username,error}', prosrc => 'pg_ident_file_mappings' }, { oid => '1371', descr => 'view system lock information', proname => 'pg_lock_status', prorows => '1000', proretset => 't', diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out index 7ec3d2688f..79408710e0 100644 --- a/src/test/regress/expected/rules.out +++ b/src/test/regress/expected/rules.out @@ -1337,7 +1337,8 @@ pg_group| SELECT pg_authid.rolname AS groname, WHERE (pg_auth_members.roleid = pg_authid.oid)) AS grolist FROM pg_authid WHERE (NOT pg_authid.rolcanlogin); -pg_hba_file_rules| SELECT a.line_number, +pg_hba_file_rules| SELECT a.rule_number, + a.line_number, a.type, a.database, a.user_name, @@ -1346,13 +1347,14 @@ pg_hba_file_rules| SELECT a.line_number, a.auth_method, a.options, a.error - FROM pg_hba_file_rules() a(line_number, type, database, user_name, address, netmask, auth_method, options, error); -pg_ident_file_mappings| SELECT a.line_number, + FROM pg_hba_file_rules() a(rule_number, line_number, type, database, user_name, address, netmask, auth_method, options, error); +pg_ident_file_mappings| SELECT a.mapping_number, + a.line_number, a.map_name, a.sys_name, a.pg_username, a.error - FROM pg_ident_file_mappings() a(line_number, map_name, sys_name, pg_username, error); + FROM pg_ident_file_mappings() a(mapping_number, line_number, map_name, sys_name, pg_username, error); pg_indexes| SELECT n.nspname AS schemaname, c.relname AS tablename, i.relname AS indexname, -- 2.37.0
>From b47d5fd1ecbf32baba12a07d061c062c6585fe14 Mon Sep 17 00:00:00 2001 From: Julien Rouhaud <julien.rouh...@free.fr> Date: Mon, 30 May 2022 11:15:06 +0800 Subject: [PATCH v8 5/6] Allow file inclusion in pg_hba and pg_ident files. pg_hba.conf file now has support for "include", "include_dir" and "include_if_exists" directives, which work similarly to the same directives in the postgresql.conf file. This fixes a possible crash if a secondary file tries to include itself as there's now a nesting depth check in the inclusion code path, same as the postgresql.conf. Many regression tests added to cover both the new directives, but also error detection for the whole pg_hba / pg_ident files. Catversion is bumped. Author: Julien Rouhaud Reviewed-by: FIXME Discussion: https://postgr.es/m/20220223045959.35ipdsvbxcstrhya%40jrouhaud --- doc/src/sgml/client-auth.sgml | 86 ++- doc/src/sgml/system-views.sgml | 22 +- src/backend/libpq/hba.c | 485 ++++++++++--- src/backend/libpq/pg_hba.conf.sample | 25 +- src/backend/libpq/pg_ident.conf.sample | 15 +- src/backend/utils/adt/hbafuncs.c | 43 +- src/backend/utils/misc/guc-file.l | 250 +++---- src/include/catalog/pg_proc.dat | 12 +- src/include/libpq/hba.h | 5 +- src/include/utils/guc.h | 2 + .../authentication/t/003_file_inclusion.pl | 657 ++++++++++++++++++ src/test/regress/expected/rules.out | 6 +- 12 files changed, 1325 insertions(+), 283 deletions(-) create mode 100644 src/test/authentication/t/003_file_inclusion.pl diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml index 433759928b..e4eacab4a5 100644 --- a/doc/src/sgml/client-auth.sgml +++ b/doc/src/sgml/client-auth.sgml @@ -89,8 +89,23 @@ </para> <para> - Each record specifies a connection type, a client IP address range - (if relevant for the connection type), a database name, a user name, + Each record can either be an inclusion directive or an authentication + record. Inclusion directives specify files that can be included, which + contains additional records. The records will be inserted in lieu of the + inclusion records. Those records only contains two fields: the + <literal>include</literal>, <literal>include_if_exists</literal> or + <literal>include_dir</literal> directive and the file or directory to be + included. The file or directory can be a relative of absolute path, and can + be double quoted if needed. For the <literal>include_dir</literal> form, + all files not starting with a <literal>.</literal> and ending with + <literal>.conf</literal> will be included. Multiple files within an include + directory are processed in file name order (according to C locale rules, + i.e., numbers before letters, and uppercase letters before lowercase ones). + </para> + + <para> + Each authentication record specifies a connection type, a client IP address + range (if relevant for the connection type), a database name, a user name, and the authentication method to be used for connections matching these parameters. The first record with a matching connection type, client address, requested database, and user name is used to perform @@ -103,21 +118,57 @@ <para> A record can have several formats: <synopsis> -local <replaceable>database</replaceable> <replaceable>user</replaceable> <replaceable>auth-method</replaceable> <optional><replaceable>auth-options</replaceable></optional> -host <replaceable>database</replaceable> <replaceable>user</replaceable> <replaceable>address</replaceable> <replaceable>auth-method</replaceable> <optional><replaceable>auth-options</replaceable></optional> -hostssl <replaceable>database</replaceable> <replaceable>user</replaceable> <replaceable>address</replaceable> <replaceable>auth-method</replaceable> <optional><replaceable>auth-options</replaceable></optional> -hostnossl <replaceable>database</replaceable> <replaceable>user</replaceable> <replaceable>address</replaceable> <replaceable>auth-method</replaceable> <optional><replaceable>auth-options</replaceable></optional> -hostgssenc <replaceable>database</replaceable> <replaceable>user</replaceable> <replaceable>address</replaceable> <replaceable>auth-method</replaceable> <optional><replaceable>auth-options</replaceable></optional> -hostnogssenc <replaceable>database</replaceable> <replaceable>user</replaceable> <replaceable>address</replaceable> <replaceable>auth-method</replaceable> <optional><replaceable>auth-options</replaceable></optional> -host <replaceable>database</replaceable> <replaceable>user</replaceable> <replaceable>IP-address</replaceable> <replaceable>IP-mask</replaceable> <replaceable>auth-method</replaceable> <optional><replaceable>auth-options</replaceable></optional> -hostssl <replaceable>database</replaceable> <replaceable>user</replaceable> <replaceable>IP-address</replaceable> <replaceable>IP-mask</replaceable> <replaceable>auth-method</replaceable> <optional><replaceable>auth-options</replaceable></optional> -hostnossl <replaceable>database</replaceable> <replaceable>user</replaceable> <replaceable>IP-address</replaceable> <replaceable>IP-mask</replaceable> <replaceable>auth-method</replaceable> <optional><replaceable>auth-options</replaceable></optional> -hostgssenc <replaceable>database</replaceable> <replaceable>user</replaceable> <replaceable>IP-address</replaceable> <replaceable>IP-mask</replaceable> <replaceable>auth-method</replaceable> <optional><replaceable>auth-options</replaceable></optional> -hostnogssenc <replaceable>database</replaceable> <replaceable>user</replaceable> <replaceable>IP-address</replaceable> <replaceable>IP-mask</replaceable> <replaceable>auth-method</replaceable> <optional><replaceable>auth-options</replaceable></optional> +include <replaceable>file</replaceable> +include_if_exists <replaceable>file</replaceable> +include_dir <replaceable>directory</replaceable> +local <replaceable>database</replaceable> <replaceable>user</replaceable> <replaceable>auth-method</replaceable> <optional><replaceable>auth-options</replaceable></optional> +host <replaceable>database</replaceable> <replaceable>user</replaceable> <replaceable>address</replaceable> <replaceable>auth-method</replaceable> <optional><replaceable>auth-options</replaceable></optional> +hostssl <replaceable>database</replaceable> <replaceable>user</replaceable> <replaceable>address</replaceable> <replaceable>auth-method</replaceable> <optional><replaceable>auth-options</replaceable></optional> +hostnossl <replaceable>database</replaceable> <replaceable>user</replaceable> <replaceable>address</replaceable> <replaceable>auth-method</replaceable> <optional><replaceable>auth-options</replaceable></optional> +hostgssenc <replaceable>database</replaceable> <replaceable>user</replaceable> <replaceable>address</replaceable> <replaceable>auth-method</replaceable> <optional><replaceable>auth-options</replaceable></optional> +hostnogssenc <replaceable>database</replaceable> <replaceable>user</replaceable> <replaceable>address</replaceable> <replaceable>auth-method</replaceable> <optional><replaceable>auth-options</replaceable></optional> +host <replaceable>database</replaceable> <replaceable>user</replaceable> <replaceable>IP-address</replaceable> <replaceable>IP-mask</replaceable> <replaceable>auth-method</replaceable> <optional><replaceable>auth-options</replaceable></optional> +hostssl <replaceable>database</replaceable> <replaceable>user</replaceable> <replaceable>IP-address</replaceable> <replaceable>IP-mask</replaceable> <replaceable>auth-method</replaceable> <optional><replaceable>auth-options</replaceable></optional> +hostnossl <replaceable>database</replaceable> <replaceable>user</replaceable> <replaceable>IP-address</replaceable> <replaceable>IP-mask</replaceable> <replaceable>auth-method</replaceable> <optional><replaceable>auth-options</replaceable></optional> +hostgssenc <replaceable>database</replaceable> <replaceable>user</replaceable> <replaceable>IP-address</replaceable> <replaceable>IP-mask</replaceable> <replaceable>auth-method</replaceable> <optional><replaceable>auth-options</replaceable></optional> +hostnogssenc <replaceable>database</replaceable> <replaceable>user</replaceable> <replaceable>IP-address</replaceable> <replaceable>IP-mask</replaceable> <replaceable>auth-method</replaceable> <optional><replaceable>auth-options</replaceable></optional> </synopsis> The meaning of the fields is as follows: <variablelist> + <varlistentry> + <term><literal>include</literal></term> + <listitem> + <para> + This line will be replaced with the content of the given file. + </para> + </listitem> + </varlistentry> + + <varlistentry> + <term><literal>include_if_exists</literal></term> + <listitem> + <para> + This line will be replaced with the content of the given file if the + file exists and can be read. Otherwise, a message will be logged to + indicate that the file is skipped. + </para> + </listitem> + </varlistentry> + + <varlistentry> + <term><literal>include_dir</literal></term> + <listitem> + <para> + This line will be replaced with the content of all the files found in + the directory, if they don't start with a <literal>.</literal> and end + with <literal>.conf</literal>, processed in file name order (according + to C locale rules, i.e., numbers before letters, and uppercase letters + before lowercase ones). + </para> + </listitem> + </varlistentry> + <varlistentry> <term><literal>local</literal></term> <listitem> @@ -837,8 +888,10 @@ local db1,db2,@demodbs all md5 cluster's data directory. (It is possible to place the map file elsewhere, however; see the <xref linkend="guc-ident-file"/> configuration parameter.) - The ident map file contains lines of the general form: + The ident map file contains lines of two general form: <synopsis> +<replaceable>include</replaceable> <replaceable>file</replaceable> +<replaceable>include_dir</replaceable> <replaceable>directory</replaceable> <replaceable>map-name</replaceable> <replaceable>system-username</replaceable> <replaceable>database-username</replaceable> </synopsis> Comments, whitespace and line continuations are handled in the same way as in @@ -849,6 +902,11 @@ local db1,db2,@demodbs all md5 database user name. The same <replaceable>map-name</replaceable> can be used repeatedly to specify multiple user-mappings within a single map. </para> + <para> + As for <filename>pg_hba.conf</filename>, the lines in this file can either + be inclusion directives or user name map records, and follow the same + rules. + </para> <para> There is no restriction regarding how many database users a given operating system user can correspond to, nor vice versa. Thus, entries diff --git a/doc/src/sgml/system-views.sgml b/doc/src/sgml/system-views.sgml index b4ee40c042..35ed69a64a 100644 --- a/doc/src/sgml/system-views.sgml +++ b/doc/src/sgml/system-views.sgml @@ -1003,12 +1003,21 @@ </para></entry> </row> + <row> + <entry role="catalog_table_entry"><para role="column_definition"> + <structfield>file_name</structfield> <type>text</type> + </para> + <para> + Name of the file containing this rule + </para></entry> + </row> + <row> <entry role="catalog_table_entry"><para role="column_definition"> <structfield>line_number</structfield> <type>int4</type> </para> <para> - Line number of this rule in <filename>pg_hba.conf</filename> + Line number of this rule the given <literal>file_name</literal> </para></entry> </row> @@ -1153,12 +1162,21 @@ </para></entry> </row> + <row> + <entry role="catalog_table_entry"><para role="column_definition"> + <structfield>file_name</structfield> <type>text</type> + </para> + <para> + Name of the file containing this mapping + </para></entry> + </row> + <row> <entry role="catalog_table_entry"><para role="column_definition"> <structfield>line_number</structfield> <type>int4</type> </para> <para> - Line number of this rule in <filename>pg_ident.conf</filename> + Line number of this mapping in the given <literal>file_name</literal> </para></entry> </row> diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c index deee05c197..814bfcf30d 100644 --- a/src/backend/libpq/hba.c +++ b/src/backend/libpq/hba.c @@ -21,6 +21,7 @@ #include <fcntl.h> #include <sys/param.h> #include <sys/socket.h> +#include <sys/stat.h> #include <netinet/in.h> #include <arpa/inet.h> #include <unistd.h> @@ -68,6 +69,12 @@ typedef struct check_network_data #define token_is_keyword(t, k) (!t->quoted && strcmp(t->string, k) == 0) #define token_matches(t, k) (strcmp(t->string, k) == 0) +typedef enum HbaIncludeKind +{ + SecondaryAuthFile, + IncludedAuthFile +} HbaIncludeKind; + /* * pre-parsed content of HBA config file: list of HbaLine structs. * parsed_hba_context is the memory context where it lives. @@ -112,10 +119,22 @@ static const char *const UserAuthName[] = }; +static void tokenize_file_with_context(MemoryContext linecxt, + const char *filename, FILE *file, + List **tok_lines, int depth, + int elevel); static List *tokenize_inc_file(List *tokens, const char *outer_filename, - const char *inc_filename, int elevel, char **err_msg); + const char *inc_filename, int depth, int elevel, + char **err_msg); static bool parse_hba_auth_opt(char *name, char *val, HbaLine *hbaline, int elevel, char **err_msg); +static FILE *open_inc_file(HbaIncludeKind kind, const char *inc_filename, + bool strict, const char *outer_filename, int elevel, + char **err_msg, char **inc_fullname); +static char *process_included_authfile(const char *inc_filename, bool strict, + const char *outer_filename, int depth, + int elevel, MemoryContext linecxt, + List **tok_lines); /* @@ -302,7 +321,7 @@ copy_auth_token(AuthToken *in) */ static List * next_field_expand(const char *filename, char **lineptr, - int elevel, char **err_msg) + int depth, int elevel, char **err_msg) { char buf[MAX_TOKEN]; bool trailing_comma; @@ -318,7 +337,7 @@ next_field_expand(const char *filename, char **lineptr, /* Is this referencing a file? */ if (!initial_quote && buf[0] == '@' && buf[1] != '\0') - tokens = tokenize_inc_file(tokens, filename, buf + 1, + tokens = tokenize_inc_file(tokens, filename, buf + 1, depth + 1, elevel, err_msg); else tokens = lappend(tokens, make_auth_token(buf, initial_quote)); @@ -346,6 +365,7 @@ static List * tokenize_inc_file(List *tokens, const char *outer_filename, const char *inc_filename, + int depth, int elevel, char **err_msg) { @@ -355,39 +375,30 @@ tokenize_inc_file(List *tokens, ListCell *inc_line; MemoryContext linecxt; - if (is_absolute_path(inc_filename)) - { - /* absolute path is taken as-is */ - inc_fullname = pstrdup(inc_filename); - } - else + /* + * Reject too-deep include nesting depth. This is just a safety check to + * avoid dumping core due to stack overflow if an include file loops back + * to itself. The maximum nesting depth is pretty arbitrary. + */ + if (depth > 10) { - /* relative path is relative to dir of calling file */ - inc_fullname = (char *) palloc(strlen(outer_filename) + 1 + - strlen(inc_filename) + 1); - strcpy(inc_fullname, outer_filename); - get_parent_directory(inc_fullname); - join_path_components(inc_fullname, inc_fullname, inc_filename); - canonicalize_path(inc_fullname); + *err_msg = psprintf("could not open configuration file \"%s\": maximum nesting depth exceeded", + inc_filename); + ereport(elevel, + (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED), + errmsg("%s", *err_msg))); + return tokens; } - inc_file = AllocateFile(inc_fullname, "r"); - if (inc_file == NULL) - { - int save_errno = errno; + inc_file = open_inc_file(SecondaryAuthFile, inc_filename, true, + outer_filename, elevel, err_msg, &inc_fullname); - ereport(elevel, - (errcode_for_file_access(), - errmsg("could not open secondary authentication file \"@%s\" as \"%s\": %m", - inc_filename, inc_fullname))); - *err_msg = psprintf("could not open secondary authentication file \"@%s\" as \"%s\": %s", - inc_filename, inc_fullname, strerror(save_errno)); - pfree(inc_fullname); + if (inc_file == NULL) return tokens; - } /* There is possible recursion here if the file contains @ */ - linecxt = tokenize_auth_file(inc_fullname, inc_file, &inc_lines, elevel); + linecxt = tokenize_auth_file(inc_fullname, inc_file, &inc_lines, depth + 1, + elevel); FreeFile(inc_file); pfree(inc_fullname); @@ -425,11 +436,38 @@ tokenize_inc_file(List *tokens, /* * tokenize_auth_file - * Tokenize the given file. + * + * Wrapper around tokenize_file_with_context, creating a dedicated memory + * context. + * + * Return value is this memory context which contains all memory allocated by + * this function (it's a child of caller's context). + */ +MemoryContext +tokenize_auth_file(const char *filename, FILE *file, List **tok_lines, + int depth, int elevel) +{ + MemoryContext linecxt; + linecxt = AllocSetContextCreate(CurrentMemoryContext, + "tokenize_auth_file", + ALLOCSET_SMALL_SIZES); + + *tok_lines = NIL; + + tokenize_file_with_context(linecxt, filename, file, tok_lines, depth, + elevel); + + return linecxt; +} + +/* + * Tokenize the given file. * * The output is a list of TokenizedAuthLine structs; see the struct definition * in libpq/hba.h. * + * linecxt: memory context which must contain all memory allocated by the + * function * filename: the absolute path to the target file * file: the already-opened target file * tok_lines: receives output list @@ -438,30 +476,22 @@ tokenize_inc_file(List *tokens, * Errors are reported by logging messages at ereport level elevel and by * adding TokenizedAuthLine structs containing non-null err_msg fields to the * output list. - * - * Return value is a memory context which contains all memory allocated by - * this function (it's a child of caller's context). */ -MemoryContext -tokenize_auth_file(const char *filename, FILE *file, List **tok_lines, - int elevel) +static void +tokenize_file_with_context(MemoryContext linecxt, const char *filename, + FILE *file, List **tok_lines, int depth, int elevel) { - int line_number = 1; StringInfoData buf; - MemoryContext linecxt; + int line_number = 1; MemoryContext oldcxt; - linecxt = AllocSetContextCreate(CurrentMemoryContext, - "tokenize_auth_file", - ALLOCSET_SMALL_SIZES); oldcxt = MemoryContextSwitchTo(linecxt); initStringInfo(&buf); - *tok_lines = NIL; - while (!feof(file) && !ferror(file)) { + TokenizedAuthLine *tok_line; char *lineptr; List *current_line = NIL; char *err_msg = NULL; @@ -514,7 +544,7 @@ tokenize_auth_file(const char *filename, FILE *file, List **tok_lines, { List *current_field; - current_field = next_field_expand(filename, &lineptr, + current_field = next_field_expand(filename, &lineptr, depth, elevel, &err_msg); /* add field to line, unless we are at EOL or comment start */ if (current_field != NIL) @@ -522,29 +552,127 @@ tokenize_auth_file(const char *filename, FILE *file, List **tok_lines, } /* - * Reached EOL; emit line to TokenizedAuthLine list unless it's boring + * Reached EOL; no need to emit line to TokenizedAuthLine list if it's + * boring. */ - if (current_line != NIL || err_msg != NULL) + if (current_line == NIL && err_msg == NULL) + goto next_line; + + /* If the line is valid, check if that's an include directive */ + if (err_msg == NULL && list_length(current_line) == 2) { - TokenizedAuthLine *tok_line; + AuthToken *first, *second; + + first = linitial(linitial_node(List, current_line)); + second = linitial(lsecond_node(List, current_line)); + + if (strcmp(first->string, "include") == 0) + { + char *inc_filename; + + inc_filename = second->string; + + err_msg = process_included_authfile(inc_filename, true, + filename, depth + 1, elevel, linecxt, + tok_lines); + + if (!err_msg) + { + /* + * The line is fully processed, bypass the general + * TokenizedAuthLine processing. + */ + goto next_line; + } + } + else if (strcmp(first->string, "include_dir") == 0) + { + char **filenames; + char *dir_name = second->string; + int num_filenames; + StringInfoData err_buf; + + filenames = GetDirConfFiles(dir_name, filename, elevel, + &num_filenames, &err_msg); + + if (!filenames) + { + /* We have the error in err_msg, simply process it */ + goto process_line; + } + + initStringInfo(&err_buf); + for (int i = 0; i < num_filenames; i++) + { + /* + * err_msg is used here as a temp buffer, it will be + * overwritten at the end of the loop with the + * cumulated errors, if any. + */ + err_msg = process_included_authfile(filenames[i], true, + filename, depth + 1, elevel, + linecxt, tok_lines); + + /* Cumulate errors if any. */ + if (err_msg) + { + if (err_buf.len > 0) + appendStringInfoChar(&err_buf, '\n'); + appendStringInfoString(&err_buf, err_msg); + } + } + + /* + * If there were no errors, the line is fully processed, bypass + * the general TokenizedAuthLine processing. + */ + if (err_buf.len == 0) + goto next_line; + + /* Otherwise, process the cumulated errors, if any. */ + err_msg = err_buf.data; + } + else if (strcmp(first->string, "include_if_exists") == 0) + { + char *inc_filename; - tok_line = (TokenizedAuthLine *) palloc(sizeof(TokenizedAuthLine)); - tok_line->fields = current_line; - tok_line->line_num = line_number; - tok_line->raw_line = pstrdup(buf.data); - tok_line->err_msg = err_msg; - *tok_lines = lappend(*tok_lines, tok_line); + inc_filename = second->string; + + err_msg = process_included_authfile(inc_filename, false, + filename, depth + 1, elevel, linecxt, + tok_lines); + + if (!err_msg) + { + /* + * The line is fully processed, bypass the general + * TokenizedAuthLine processing. + */ + goto next_line; + } + } } +process_line: + /* + * General processing: report the error if any and emit line to the + * TokenizedAuthLine + */ + tok_line = (TokenizedAuthLine *) palloc(sizeof(TokenizedAuthLine)); + tok_line->fields = current_line; + tok_line->file_name = pstrdup(filename); + tok_line->line_num = line_number; + tok_line->raw_line = pstrdup(buf.data); + tok_line->err_msg = err_msg; + *tok_lines = lappend(*tok_lines, tok_line); + +next_line: line_number += continuations + 1; } MemoryContextSwitchTo(oldcxt); - - return linecxt; } - /* * Does user belong to role? * @@ -859,7 +987,7 @@ do { \ errmsg("authentication option \"%s\" is only valid for authentication methods %s", \ optname, _(validmethods)), \ errcontext("line %d of configuration file \"%s\"", \ - line_num, HbaFileName))); \ + line_num, file_name))); \ *err_msg = psprintf("authentication option \"%s\" is only valid for authentication methods %s", \ optname, validmethods); \ return false; \ @@ -879,7 +1007,7 @@ do { \ errmsg("authentication method \"%s\" requires argument \"%s\" to be set", \ authname, argname), \ errcontext("line %d of configuration file \"%s\"", \ - line_num, HbaFileName))); \ + line_num, file_name))); \ *err_msg = psprintf("authentication method \"%s\" requires argument \"%s\" to be set", \ authname, argname); \ return NULL; \ @@ -902,7 +1030,7 @@ do { \ (errcode(ERRCODE_CONFIG_FILE_ERROR), \ errmsg("missing entry at end of line"), \ errcontext("line %d of configuration file \"%s\"", \ - line_num, IdentFileName))); \ + line_num, file_name))); \ *err_msg = psprintf("missing entry at end of line"); \ return NULL; \ } \ @@ -915,7 +1043,7 @@ do { \ (errcode(ERRCODE_CONFIG_FILE_ERROR), \ errmsg("multiple values in ident field"), \ errcontext("line %d of configuration file \"%s\"", \ - line_num, IdentFileName))); \ + line_num, file_name))); \ *err_msg = psprintf("multiple values in ident field"); \ return NULL; \ } \ @@ -938,6 +1066,7 @@ HbaLine * parse_hba_line(TokenizedAuthLine *tok_line, int elevel) { int line_num = tok_line->line_num; + char *file_name = tok_line->file_name; char **err_msg = &tok_line->err_msg; char *str; struct addrinfo *gai_result; @@ -952,6 +1081,7 @@ parse_hba_line(TokenizedAuthLine *tok_line, int elevel) HbaLine *parsedline; parsedline = palloc0(sizeof(HbaLine)); + parsedline->sourcefile = pstrdup(file_name); parsedline->linenumber = line_num; parsedline->rawline = pstrdup(tok_line->raw_line); @@ -966,7 +1096,7 @@ parse_hba_line(TokenizedAuthLine *tok_line, int elevel) errmsg("multiple values specified for connection type"), errhint("Specify exactly one connection type per line."), errcontext("line %d of configuration file \"%s\"", - line_num, HbaFileName))); + line_num, file_name))); *err_msg = "multiple values specified for connection type"; return NULL; } @@ -980,7 +1110,7 @@ parse_hba_line(TokenizedAuthLine *tok_line, int elevel) (errcode(ERRCODE_CONFIG_FILE_ERROR), errmsg("local connections are not supported by this build"), errcontext("line %d of configuration file \"%s\"", - line_num, HbaFileName))); + line_num, file_name))); *err_msg = "local connections are not supported by this build"; return NULL; #endif @@ -1004,7 +1134,7 @@ parse_hba_line(TokenizedAuthLine *tok_line, int elevel) errmsg("hostssl record cannot match because SSL is disabled"), errhint("Set ssl = on in postgresql.conf."), errcontext("line %d of configuration file \"%s\"", - line_num, HbaFileName))); + line_num, file_name))); *err_msg = "hostssl record cannot match because SSL is disabled"; } #else @@ -1012,7 +1142,7 @@ parse_hba_line(TokenizedAuthLine *tok_line, int elevel) (errcode(ERRCODE_CONFIG_FILE_ERROR), errmsg("hostssl record cannot match because SSL is not supported by this build"), errcontext("line %d of configuration file \"%s\"", - line_num, HbaFileName))); + line_num, file_name))); *err_msg = "hostssl record cannot match because SSL is not supported by this build"; #endif } @@ -1024,7 +1154,7 @@ parse_hba_line(TokenizedAuthLine *tok_line, int elevel) (errcode(ERRCODE_CONFIG_FILE_ERROR), errmsg("hostgssenc record cannot match because GSSAPI is not supported by this build"), errcontext("line %d of configuration file \"%s\"", - line_num, HbaFileName))); + line_num, file_name))); *err_msg = "hostgssenc record cannot match because GSSAPI is not supported by this build"; #endif } @@ -1045,7 +1175,7 @@ parse_hba_line(TokenizedAuthLine *tok_line, int elevel) errmsg("invalid connection type \"%s\"", token->string), errcontext("line %d of configuration file \"%s\"", - line_num, HbaFileName))); + line_num, file_name))); *err_msg = psprintf("invalid connection type \"%s\"", token->string); return NULL; } @@ -1058,7 +1188,7 @@ parse_hba_line(TokenizedAuthLine *tok_line, int elevel) (errcode(ERRCODE_CONFIG_FILE_ERROR), errmsg("end-of-line before database specification"), errcontext("line %d of configuration file \"%s\"", - line_num, HbaFileName))); + line_num, file_name))); *err_msg = "end-of-line before database specification"; return NULL; } @@ -1078,7 +1208,7 @@ parse_hba_line(TokenizedAuthLine *tok_line, int elevel) (errcode(ERRCODE_CONFIG_FILE_ERROR), errmsg("end-of-line before role specification"), errcontext("line %d of configuration file \"%s\"", - line_num, HbaFileName))); + line_num, file_name))); *err_msg = "end-of-line before role specification"; return NULL; } @@ -1100,7 +1230,7 @@ parse_hba_line(TokenizedAuthLine *tok_line, int elevel) (errcode(ERRCODE_CONFIG_FILE_ERROR), errmsg("end-of-line before IP address specification"), errcontext("line %d of configuration file \"%s\"", - line_num, HbaFileName))); + line_num, file_name))); *err_msg = "end-of-line before IP address specification"; return NULL; } @@ -1112,7 +1242,7 @@ parse_hba_line(TokenizedAuthLine *tok_line, int elevel) errmsg("multiple values specified for host address"), errhint("Specify one address range per line."), errcontext("line %d of configuration file \"%s\"", - line_num, HbaFileName))); + line_num, file_name))); *err_msg = "multiple values specified for host address"; return NULL; } @@ -1171,7 +1301,7 @@ parse_hba_line(TokenizedAuthLine *tok_line, int elevel) errmsg("invalid IP address \"%s\": %s", str, gai_strerror(ret)), errcontext("line %d of configuration file \"%s\"", - line_num, HbaFileName))); + line_num, file_name))); *err_msg = psprintf("invalid IP address \"%s\": %s", str, gai_strerror(ret)); if (gai_result) @@ -1191,7 +1321,7 @@ parse_hba_line(TokenizedAuthLine *tok_line, int elevel) errmsg("specifying both host name and CIDR mask is invalid: \"%s\"", token->string), errcontext("line %d of configuration file \"%s\"", - line_num, HbaFileName))); + line_num, file_name))); *err_msg = psprintf("specifying both host name and CIDR mask is invalid: \"%s\"", token->string); return NULL; @@ -1205,7 +1335,7 @@ parse_hba_line(TokenizedAuthLine *tok_line, int elevel) errmsg("invalid CIDR mask in address \"%s\"", token->string), errcontext("line %d of configuration file \"%s\"", - line_num, HbaFileName))); + line_num, file_name))); *err_msg = psprintf("invalid CIDR mask in address \"%s\"", token->string); return NULL; @@ -1225,7 +1355,7 @@ parse_hba_line(TokenizedAuthLine *tok_line, int elevel) errmsg("end-of-line before netmask specification"), errhint("Specify an address range in CIDR notation, or provide a separate netmask."), errcontext("line %d of configuration file \"%s\"", - line_num, HbaFileName))); + line_num, file_name))); *err_msg = "end-of-line before netmask specification"; return NULL; } @@ -1236,7 +1366,7 @@ parse_hba_line(TokenizedAuthLine *tok_line, int elevel) (errcode(ERRCODE_CONFIG_FILE_ERROR), errmsg("multiple values specified for netmask"), errcontext("line %d of configuration file \"%s\"", - line_num, HbaFileName))); + line_num, file_name))); *err_msg = "multiple values specified for netmask"; return NULL; } @@ -1251,7 +1381,7 @@ parse_hba_line(TokenizedAuthLine *tok_line, int elevel) errmsg("invalid IP mask \"%s\": %s", token->string, gai_strerror(ret)), errcontext("line %d of configuration file \"%s\"", - line_num, HbaFileName))); + line_num, file_name))); *err_msg = psprintf("invalid IP mask \"%s\": %s", token->string, gai_strerror(ret)); if (gai_result) @@ -1270,7 +1400,7 @@ parse_hba_line(TokenizedAuthLine *tok_line, int elevel) (errcode(ERRCODE_CONFIG_FILE_ERROR), errmsg("IP address and mask do not match"), errcontext("line %d of configuration file \"%s\"", - line_num, HbaFileName))); + line_num, file_name))); *err_msg = "IP address and mask do not match"; return NULL; } @@ -1286,7 +1416,7 @@ parse_hba_line(TokenizedAuthLine *tok_line, int elevel) (errcode(ERRCODE_CONFIG_FILE_ERROR), errmsg("end-of-line before authentication method"), errcontext("line %d of configuration file \"%s\"", - line_num, HbaFileName))); + line_num, file_name))); *err_msg = "end-of-line before authentication method"; return NULL; } @@ -1298,7 +1428,7 @@ parse_hba_line(TokenizedAuthLine *tok_line, int elevel) errmsg("multiple values specified for authentication type"), errhint("Specify exactly one authentication type per line."), errcontext("line %d of configuration file \"%s\"", - line_num, HbaFileName))); + line_num, file_name))); *err_msg = "multiple values specified for authentication type"; return NULL; } @@ -1335,7 +1465,7 @@ parse_hba_line(TokenizedAuthLine *tok_line, int elevel) (errcode(ERRCODE_CONFIG_FILE_ERROR), errmsg("MD5 authentication is not supported when \"db_user_namespace\" is enabled"), errcontext("line %d of configuration file \"%s\"", - line_num, HbaFileName))); + line_num, file_name))); *err_msg = "MD5 authentication is not supported when \"db_user_namespace\" is enabled"; return NULL; } @@ -1376,7 +1506,7 @@ parse_hba_line(TokenizedAuthLine *tok_line, int elevel) errmsg("invalid authentication method \"%s\"", token->string), errcontext("line %d of configuration file \"%s\"", - line_num, HbaFileName))); + line_num, file_name))); *err_msg = psprintf("invalid authentication method \"%s\"", token->string); return NULL; @@ -1389,7 +1519,7 @@ parse_hba_line(TokenizedAuthLine *tok_line, int elevel) errmsg("invalid authentication method \"%s\": not supported by this build", token->string), errcontext("line %d of configuration file \"%s\"", - line_num, HbaFileName))); + line_num, file_name))); *err_msg = psprintf("invalid authentication method \"%s\": not supported by this build", token->string); return NULL; @@ -1411,7 +1541,7 @@ parse_hba_line(TokenizedAuthLine *tok_line, int elevel) (errcode(ERRCODE_CONFIG_FILE_ERROR), errmsg("gssapi authentication is not supported on local sockets"), errcontext("line %d of configuration file \"%s\"", - line_num, HbaFileName))); + line_num, file_name))); *err_msg = "gssapi authentication is not supported on local sockets"; return NULL; } @@ -1423,7 +1553,7 @@ parse_hba_line(TokenizedAuthLine *tok_line, int elevel) (errcode(ERRCODE_CONFIG_FILE_ERROR), errmsg("peer authentication is only supported on local sockets"), errcontext("line %d of configuration file \"%s\"", - line_num, HbaFileName))); + line_num, file_name))); *err_msg = "peer authentication is only supported on local sockets"; return NULL; } @@ -1441,7 +1571,7 @@ parse_hba_line(TokenizedAuthLine *tok_line, int elevel) (errcode(ERRCODE_CONFIG_FILE_ERROR), errmsg("cert authentication is only supported on hostssl connections"), errcontext("line %d of configuration file \"%s\"", - line_num, HbaFileName))); + line_num, file_name))); *err_msg = "cert authentication is only supported on hostssl connections"; return NULL; } @@ -1491,7 +1621,7 @@ parse_hba_line(TokenizedAuthLine *tok_line, int elevel) (errcode(ERRCODE_CONFIG_FILE_ERROR), errmsg("authentication option not in name=value format: %s", token->string), errcontext("line %d of configuration file \"%s\"", - line_num, HbaFileName))); + line_num, file_name))); *err_msg = psprintf("authentication option not in name=value format: %s", token->string); return NULL; @@ -1535,7 +1665,7 @@ parse_hba_line(TokenizedAuthLine *tok_line, int elevel) (errcode(ERRCODE_CONFIG_FILE_ERROR), errmsg("cannot use ldapbasedn, ldapbinddn, ldapbindpasswd, ldapsearchattribute, ldapsearchfilter, or ldapurl together with ldapprefix"), errcontext("line %d of configuration file \"%s\"", - line_num, HbaFileName))); + line_num, file_name))); *err_msg = "cannot use ldapbasedn, ldapbinddn, ldapbindpasswd, ldapsearchattribute, ldapsearchfilter, or ldapurl together with ldapprefix"; return NULL; } @@ -1546,7 +1676,7 @@ parse_hba_line(TokenizedAuthLine *tok_line, int elevel) (errcode(ERRCODE_CONFIG_FILE_ERROR), errmsg("authentication method \"ldap\" requires argument \"ldapbasedn\", \"ldapprefix\", or \"ldapsuffix\" to be set"), errcontext("line %d of configuration file \"%s\"", - line_num, HbaFileName))); + line_num, file_name))); *err_msg = "authentication method \"ldap\" requires argument \"ldapbasedn\", \"ldapprefix\", or \"ldapsuffix\" to be set"; return NULL; } @@ -1562,7 +1692,7 @@ parse_hba_line(TokenizedAuthLine *tok_line, int elevel) (errcode(ERRCODE_CONFIG_FILE_ERROR), errmsg("cannot use ldapsearchattribute together with ldapsearchfilter"), errcontext("line %d of configuration file \"%s\"", - line_num, HbaFileName))); + line_num, file_name))); *err_msg = "cannot use ldapsearchattribute together with ldapsearchfilter"; return NULL; } @@ -1579,7 +1709,7 @@ parse_hba_line(TokenizedAuthLine *tok_line, int elevel) (errcode(ERRCODE_CONFIG_FILE_ERROR), errmsg("list of RADIUS servers cannot be empty"), errcontext("line %d of configuration file \"%s\"", - line_num, HbaFileName))); + line_num, file_name))); *err_msg = "list of RADIUS servers cannot be empty"; return NULL; } @@ -1590,7 +1720,7 @@ parse_hba_line(TokenizedAuthLine *tok_line, int elevel) (errcode(ERRCODE_CONFIG_FILE_ERROR), errmsg("list of RADIUS secrets cannot be empty"), errcontext("line %d of configuration file \"%s\"", - line_num, HbaFileName))); + line_num, file_name))); *err_msg = "list of RADIUS secrets cannot be empty"; return NULL; } @@ -1609,7 +1739,7 @@ parse_hba_line(TokenizedAuthLine *tok_line, int elevel) list_length(parsedline->radiussecrets), list_length(parsedline->radiusservers)), errcontext("line %d of configuration file \"%s\"", - line_num, HbaFileName))); + line_num, file_name))); *err_msg = psprintf("the number of RADIUS secrets (%d) must be 1 or the same as the number of RADIUS servers (%d)", list_length(parsedline->radiussecrets), list_length(parsedline->radiusservers)); @@ -1625,7 +1755,7 @@ parse_hba_line(TokenizedAuthLine *tok_line, int elevel) list_length(parsedline->radiusports), list_length(parsedline->radiusservers)), errcontext("line %d of configuration file \"%s\"", - line_num, HbaFileName))); + line_num, file_name))); *err_msg = psprintf("the number of RADIUS ports (%d) must be 1 or the same as the number of RADIUS servers (%d)", list_length(parsedline->radiusports), list_length(parsedline->radiusservers)); @@ -1641,7 +1771,7 @@ parse_hba_line(TokenizedAuthLine *tok_line, int elevel) list_length(parsedline->radiusidentifiers), list_length(parsedline->radiusservers)), errcontext("line %d of configuration file \"%s\"", - line_num, HbaFileName))); + line_num, file_name))); *err_msg = psprintf("the number of RADIUS identifiers (%d) must be 1 or the same as the number of RADIUS servers (%d)", list_length(parsedline->radiusidentifiers), list_length(parsedline->radiusservers)); @@ -1676,6 +1806,7 @@ parse_hba_auth_opt(char *name, char *val, HbaLine *hbaline, int elevel, char **err_msg) { int line_num = hbaline->linenumber; + char *file_name = hbaline->sourcefile; #ifdef USE_LDAP hbaline->ldapscope = LDAP_SCOPE_SUBTREE; @@ -1699,7 +1830,7 @@ parse_hba_auth_opt(char *name, char *val, HbaLine *hbaline, (errcode(ERRCODE_CONFIG_FILE_ERROR), errmsg("clientcert can only be configured for \"hostssl\" rows"), errcontext("line %d of configuration file \"%s\"", - line_num, HbaFileName))); + line_num, file_name))); *err_msg = "clientcert can only be configured for \"hostssl\" rows"; return false; } @@ -1716,7 +1847,7 @@ parse_hba_auth_opt(char *name, char *val, HbaLine *hbaline, (errcode(ERRCODE_CONFIG_FILE_ERROR), errmsg("clientcert only accepts \"verify-full\" when using \"cert\" authentication"), errcontext("line %d of configuration file \"%s\"", - line_num, HbaFileName))); + line_num, file_name))); *err_msg = "clientcert can only be set to \"verify-full\" when using \"cert\" authentication"; return false; } @@ -1729,7 +1860,7 @@ parse_hba_auth_opt(char *name, char *val, HbaLine *hbaline, (errcode(ERRCODE_CONFIG_FILE_ERROR), errmsg("invalid value for clientcert: \"%s\"", val), errcontext("line %d of configuration file \"%s\"", - line_num, HbaFileName))); + line_num, file_name))); return false; } } @@ -1741,7 +1872,7 @@ parse_hba_auth_opt(char *name, char *val, HbaLine *hbaline, (errcode(ERRCODE_CONFIG_FILE_ERROR), errmsg("clientname can only be configured for \"hostssl\" rows"), errcontext("line %d of configuration file \"%s\"", - line_num, HbaFileName))); + line_num, file_name))); *err_msg = "clientname can only be configured for \"hostssl\" rows"; return false; } @@ -1760,7 +1891,7 @@ parse_hba_auth_opt(char *name, char *val, HbaLine *hbaline, (errcode(ERRCODE_CONFIG_FILE_ERROR), errmsg("invalid value for clientname: \"%s\"", val), errcontext("line %d of configuration file \"%s\"", - line_num, HbaFileName))); + line_num, file_name))); return false; } } @@ -1846,7 +1977,7 @@ parse_hba_auth_opt(char *name, char *val, HbaLine *hbaline, (errcode(ERRCODE_CONFIG_FILE_ERROR), errmsg("invalid ldapscheme value: \"%s\"", val), errcontext("line %d of configuration file \"%s\"", - line_num, HbaFileName))); + line_num, file_name))); hbaline->ldapscheme = pstrdup(val); } else if (strcmp(name, "ldapserver") == 0) @@ -1864,7 +1995,7 @@ parse_hba_auth_opt(char *name, char *val, HbaLine *hbaline, (errcode(ERRCODE_CONFIG_FILE_ERROR), errmsg("invalid LDAP port number: \"%s\"", val), errcontext("line %d of configuration file \"%s\"", - line_num, HbaFileName))); + line_num, file_name))); *err_msg = psprintf("invalid LDAP port number: \"%s\"", val); return false; } @@ -1958,7 +2089,7 @@ parse_hba_auth_opt(char *name, char *val, HbaLine *hbaline, errmsg("could not parse RADIUS server list \"%s\"", val), errcontext("line %d of configuration file \"%s\"", - line_num, HbaFileName))); + line_num, file_name))); return false; } @@ -1977,7 +2108,7 @@ parse_hba_auth_opt(char *name, char *val, HbaLine *hbaline, errmsg("could not translate RADIUS server name \"%s\" to address: %s", (char *) lfirst(l), gai_strerror(ret)), errcontext("line %d of configuration file \"%s\"", - line_num, HbaFileName))); + line_num, file_name))); if (gai_result) pg_freeaddrinfo_all(hints.ai_family, gai_result); @@ -2006,7 +2137,7 @@ parse_hba_auth_opt(char *name, char *val, HbaLine *hbaline, errmsg("could not parse RADIUS port list \"%s\"", val), errcontext("line %d of configuration file \"%s\"", - line_num, HbaFileName))); + line_num, file_name))); *err_msg = psprintf("invalid RADIUS port number: \"%s\"", val); return false; } @@ -2019,7 +2150,7 @@ parse_hba_auth_opt(char *name, char *val, HbaLine *hbaline, (errcode(ERRCODE_CONFIG_FILE_ERROR), errmsg("invalid RADIUS port number: \"%s\"", val), errcontext("line %d of configuration file \"%s\"", - line_num, HbaFileName))); + line_num, file_name))); return false; } @@ -2042,7 +2173,7 @@ parse_hba_auth_opt(char *name, char *val, HbaLine *hbaline, errmsg("could not parse RADIUS secret list \"%s\"", val), errcontext("line %d of configuration file \"%s\"", - line_num, HbaFileName))); + line_num, file_name))); return false; } @@ -2064,7 +2195,7 @@ parse_hba_auth_opt(char *name, char *val, HbaLine *hbaline, errmsg("could not parse RADIUS identifiers list \"%s\"", val), errcontext("line %d of configuration file \"%s\"", - line_num, HbaFileName))); + line_num, file_name))); return false; } @@ -2078,7 +2209,7 @@ parse_hba_auth_opt(char *name, char *val, HbaLine *hbaline, errmsg("unrecognized authentication option name: \"%s\"", name), errcontext("line %d of configuration file \"%s\"", - line_num, HbaFileName))); + line_num, file_name))); *err_msg = psprintf("unrecognized authentication option name: \"%s\"", name); return false; @@ -2226,7 +2357,7 @@ load_hba(void) return false; } - linecxt = tokenize_auth_file(HbaFileName, file, &hba_lines, LOG); + linecxt = tokenize_auth_file(HbaFileName, file, &hba_lines, 0, LOG); FreeFile(file); /* Now parse all the lines */ @@ -2297,6 +2428,137 @@ load_hba(void) return true; } +/* + * Open the given file for inclusion in an authentication file, whether + * secondary or included. + */ +static FILE * +open_inc_file(HbaIncludeKind kind, const char *inc_filename, bool strict, + const char *outer_filename, int elevel, char **err_msg, + char **inc_fullname) +{ + FILE *inc_file; + + if (is_absolute_path(inc_filename)) + { + /* absolute path is taken as-is */ + *inc_fullname = pstrdup(inc_filename); + } + else + { + /* relative path is relative to dir of calling file */ + *inc_fullname = (char *) palloc(strlen(outer_filename) + 1 + + strlen(inc_filename) + 1); + strcpy(*inc_fullname, outer_filename); + get_parent_directory(*inc_fullname); + join_path_components(*inc_fullname, *inc_fullname, inc_filename); + canonicalize_path(*inc_fullname); + } + + inc_file = AllocateFile(*inc_fullname, "r"); + if (inc_file == NULL) + { + int save_errno = errno; + const char *msglog; + const char *msgview; + + if (strict) + { + switch (kind) + { + case SecondaryAuthFile: + msglog = "could not open secondary authentication file \"@%s\" as \"%s\": %m"; + msgview = "could not open secondary authentication file \"@%s\" as \"%s\": %s"; + break; + case IncludedAuthFile: + msglog = "could not open included authentication file \"%s\" as \"%s\": %m"; + msgview = "could not open included authentication file \"%s\" as \"%s\": %s"; + break; + default: + elog(ERROR, "unknown HbaIncludeKind: %d", kind); + break; + } + + ereport(elevel, + (errcode_for_file_access(), + errmsg(msglog, inc_filename, *inc_fullname))); + *err_msg = psprintf(msgview, inc_filename, *inc_fullname, + strerror(save_errno)); + } + else + { + Assert(kind == IncludedAuthFile); + ereport(LOG, + (errmsg("skipping missing authentication file \"%s\"", + *inc_fullname))); + } + + pfree(*inc_fullname); + *inc_fullname = NULL; + return NULL; + } + + return inc_file; +} + +/* + * Try to open an included file, and tokenize it using the given context. + * Returns NULL if no error happens during tokenization, otherwise the error. + */ +static char * +process_included_authfile(const char *inc_filename, bool strict, + const char *outer_filename, int depth, int elevel, + MemoryContext linecxt, List **tok_lines) +{ + char *inc_fullname; + FILE *inc_file; + char *err_msg = NULL; + + /* + * Reject too-deep include nesting depth. This is just a safety check to + * avoid dumping core due to stack overflow if an include file loops back + * to itself. The maximum nesting depth is pretty arbitrary. + */ + if (depth > 10) + { + char *err_msg; + + err_msg = psprintf("could not open configuration file \"%s\": maximum nesting depth exceeded", + inc_filename); + ereport(elevel, + (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED), + errmsg("%s", err_msg))); + return err_msg; + } + + inc_file = open_inc_file(IncludedAuthFile, inc_filename, strict, + outer_filename, elevel, &err_msg, &inc_fullname); + + if (inc_file == NULL) + { + if (strict) + { + /* open_inc_file should have reported an error. */ + Assert(err_msg != NULL); + return err_msg; + } + else + return NULL; + } + else + { + /* No error message should have been reported. */ + Assert(err_msg == NULL); + } + + tokenize_file_with_context(linecxt, inc_fullname, inc_file, + tok_lines, depth, elevel); + + FreeFile(inc_file); + pfree(inc_fullname); + + return NULL; +} /* * Parse one tokenised line from the ident config file and store the result in @@ -2315,6 +2577,7 @@ load_hba(void) IdentLine * parse_ident_line(TokenizedAuthLine *tok_line, int elevel) { + char *file_name = tok_line->file_name; int line_num = tok_line->line_num; char **err_msg = &tok_line->err_msg; ListCell *field; @@ -2375,7 +2638,7 @@ parse_ident_line(TokenizedAuthLine *tok_line, int elevel) errmsg("invalid regular expression \"%s\": %s", parsedline->ident_user + 1, errstr), errcontext("line %d of configuration file \"%s\"", - line_num, IdentFileName))); + line_num, file_name))); *err_msg = psprintf("invalid regular expression \"%s\": %s", parsedline->ident_user + 1, errstr); @@ -2610,7 +2873,7 @@ load_ident(void) return false; } - linecxt = tokenize_auth_file(IdentFileName, file, &ident_lines, LOG); + linecxt = tokenize_auth_file(IdentFileName, file, &ident_lines, 0, LOG); FreeFile(file); /* Now parse all the lines */ diff --git a/src/backend/libpq/pg_hba.conf.sample b/src/backend/libpq/pg_hba.conf.sample index 5f3f63eb0c..7433050112 100644 --- a/src/backend/libpq/pg_hba.conf.sample +++ b/src/backend/libpq/pg_hba.conf.sample @@ -9,16 +9,27 @@ # are authenticated, which PostgreSQL user names they can use, which # databases they can access. Records take one of these forms: # -# local DATABASE USER METHOD [OPTIONS] -# host DATABASE USER ADDRESS METHOD [OPTIONS] -# hostssl DATABASE USER ADDRESS METHOD [OPTIONS] -# hostnossl DATABASE USER ADDRESS METHOD [OPTIONS] -# hostgssenc DATABASE USER ADDRESS METHOD [OPTIONS] -# hostnogssenc DATABASE USER ADDRESS METHOD [OPTIONS] +# include FILE +# include_if_exists FILE +# include_dir DIRECTORY +# local DATABASE USER METHOD [OPTIONS] +# host DATABASE USER ADDRESS METHOD [OPTIONS] +# hostssl DATABASE USER ADDRESS METHOD [OPTIONS] +# hostnossl DATABASE USER ADDRESS METHOD [OPTIONS] +# hostgssenc DATABASE USER ADDRESS METHOD [OPTIONS] +# hostnogssenc DATABASE USER ADDRESS METHOD [OPTIONS] # # (The uppercase items must be replaced by actual values.) # -# The first field is the connection type: +# If the first field is "include", "include_if_exists" or "include_dir", it's +# not a mapping record but a directive to include records from respectively +# another file, another file if it exists or all the files in the given +# directory ending in '.conf'. FILE is the file name to include, and +# DIR is the directory name containing the file(s) to include. FILE and +# DIRECTORY can be specified with a relative or absolute path, and can be +# double quoted if they contains spaces. +# +# Otherwise the first field is the connection type: # - "local" is a Unix-domain socket # - "host" is a TCP/IP socket (encrypted or not) # - "hostssl" is a TCP/IP socket that is SSL-encrypted diff --git a/src/backend/libpq/pg_ident.conf.sample b/src/backend/libpq/pg_ident.conf.sample index a5870e6448..8e3fa29135 100644 --- a/src/backend/libpq/pg_ident.conf.sample +++ b/src/backend/libpq/pg_ident.conf.sample @@ -7,12 +7,23 @@ # # This file controls PostgreSQL user name mapping. It maps external # user names to their corresponding PostgreSQL user names. Records -# are of the form: +# are one of these forms: # -# MAPNAME SYSTEM-USERNAME PG-USERNAME +# include FILE +# include_if_exists FILE +# include_dir DIRECTORY +# MAPNAME SYSTEM-USERNAME PG-USERNAME # # (The uppercase quantities must be replaced by actual values.) # +# If the first field is "include", "include_if_exists" or "include_dir", it's +# not a mapping record but a directive to include records from respectively +# another file, another file if it exists or all the files in the given +# directory ending in '.conf'. FILE is the file name to include, and +# DIR is the directory name containing the file(s) to include. FILE and +# DIRECTORY can be specified with a relative or absolute path, and can be +# double quoted if they contains spaces. +# # MAPNAME is the (otherwise freely chosen) map name that was used in # pg_hba.conf. SYSTEM-USERNAME is the detected user name of the # client. PG-USERNAME is the requested PostgreSQL user name. The diff --git a/src/backend/utils/adt/hbafuncs.c b/src/backend/utils/adt/hbafuncs.c index c9be4bff1f..15326a01e2 100644 --- a/src/backend/utils/adt/hbafuncs.c +++ b/src/backend/utils/adt/hbafuncs.c @@ -26,12 +26,12 @@ static ArrayType *get_hba_options(HbaLine *hba); static void fill_hba_line(Tuplestorestate *tuple_store, TupleDesc tupdesc, - int rule_number, int lineno, HbaLine *hba, - const char *err_msg); + int rule_number, const char *filename, int lineno, + HbaLine *hba, const char *err_msg); static void fill_hba_view(Tuplestorestate *tuple_store, TupleDesc tupdesc); static void fill_ident_line(Tuplestorestate *tuple_store, TupleDesc tupdesc, - int mapping_number, int lineno, IdentLine *ident, - const char *err_msg); + int mapping_number, const char *filename, + int lineno, IdentLine *ident, const char *err_msg); static void fill_ident_view(Tuplestorestate *tuple_store, TupleDesc tupdesc); @@ -159,7 +159,7 @@ get_hba_options(HbaLine *hba) } /* Number of columns in pg_hba_file_rules view */ -#define NUM_PG_HBA_FILE_RULES_ATTS 10 +#define NUM_PG_HBA_FILE_RULES_ATTS 11 /* * fill_hba_line @@ -168,7 +168,8 @@ get_hba_options(HbaLine *hba) * tuple_store: where to store data * tupdesc: tuple descriptor for the view * rule_number: unique rule identifier among all valid rules - * lineno: pg_hba.conf line number (must always be valid) + * filename: name of the file containing that line + * lineno: line number in that file (must always be valid) * hba: parsed line data (can be NULL, in which case err_msg should be set) * err_msg: error message (NULL if none) * @@ -177,7 +178,7 @@ get_hba_options(HbaLine *hba) */ static void fill_hba_line(Tuplestorestate *tuple_store, TupleDesc tupdesc, - int rule_number, int lineno, HbaLine *hba, + int rule_number, const char *filename, int lineno, HbaLine *hba, const char *err_msg) { Datum values[NUM_PG_HBA_FILE_RULES_ATTS]; @@ -202,6 +203,8 @@ fill_hba_line(Tuplestorestate *tuple_store, TupleDesc tupdesc, nulls[index++] = true; else values[index++] = Int32GetDatum(rule_number); + /* file_name */ + values[index++] = CStringGetTextDatum(filename); /* line_number */ values[index++] = Int32GetDatum(lineno); @@ -345,7 +348,7 @@ fill_hba_line(Tuplestorestate *tuple_store, TupleDesc tupdesc, else { /* no parsing result, so set relevant fields to nulls */ - memset(&nulls[2], true, (NUM_PG_HBA_FILE_RULES_ATTS - 3) * sizeof(bool)); + memset(&nulls[3], true, (NUM_PG_HBA_FILE_RULES_ATTS - 4) * sizeof(bool)); } /* error */ @@ -386,7 +389,7 @@ fill_hba_view(Tuplestorestate *tuple_store, TupleDesc tupdesc) errmsg("could not open configuration file \"%s\": %m", HbaFileName))); - linecxt = tokenize_auth_file(HbaFileName, file, &hba_lines, DEBUG3); + linecxt = tokenize_auth_file(HbaFileName, file, &hba_lines, 0, DEBUG3); FreeFile(file); /* Now parse all the lines */ @@ -407,8 +410,8 @@ fill_hba_view(Tuplestorestate *tuple_store, TupleDesc tupdesc) if (tok_line->err_msg == NULL) rule_number++; - fill_hba_line(tuple_store, tupdesc, rule_number, tok_line->line_num, - hbaline, tok_line->err_msg); + fill_hba_line(tuple_store, tupdesc, rule_number, tok_line->file_name, + tok_line->line_num, hbaline, tok_line->err_msg); } /* Free tokenizer memory */ @@ -445,7 +448,7 @@ pg_hba_file_rules(PG_FUNCTION_ARGS) } /* Number of columns in pg_hba_file_mappings view */ -#define NUM_PG_IDENT_FILE_MAPPINGS_ATTS 6 +#define NUM_PG_IDENT_FILE_MAPPINGS_ATTS 7 /* * fill_ident_line: build one row of pg_ident_file_mappings view, add it to @@ -454,7 +457,8 @@ pg_hba_file_rules(PG_FUNCTION_ARGS) * tuple_store: where to store data * tupdesc: tuple descriptor for the view * mapping_number: unique rule identifier among all valid rules - * lineno: pg_ident.conf line number (must always be valid) + * filename: name of the file containing that line + * lineno: line number in that file (must always be valid) * ident: parsed line data (can be NULL, in which case err_msg should be set) * err_msg: error message (NULL if none) * @@ -463,8 +467,8 @@ pg_hba_file_rules(PG_FUNCTION_ARGS) */ static void fill_ident_line(Tuplestorestate *tuple_store, TupleDesc tupdesc, - int mapping_number, int lineno, IdentLine *ident, - const char *err_msg) + int mapping_number, const char *filename, int lineno, + IdentLine *ident, const char *err_msg) { Datum values[NUM_PG_IDENT_FILE_MAPPINGS_ATTS]; bool nulls[NUM_PG_IDENT_FILE_MAPPINGS_ATTS]; @@ -482,6 +486,8 @@ fill_ident_line(Tuplestorestate *tuple_store, TupleDesc tupdesc, nulls[index++] = true; else values[index++] = Int32GetDatum(mapping_number); + /* file_name */ + values[index++] = CStringGetTextDatum(filename); /* line_number */ values[index++] = Int32GetDatum(lineno); @@ -494,7 +500,7 @@ fill_ident_line(Tuplestorestate *tuple_store, TupleDesc tupdesc, else { /* no parsing result, so set relevant fields to nulls */ - memset(&nulls[2], true, (NUM_PG_IDENT_FILE_MAPPINGS_ATTS - 3) * sizeof(bool)); + memset(&nulls[3], true, (NUM_PG_IDENT_FILE_MAPPINGS_ATTS - 4) * sizeof(bool)); } /* error */ @@ -534,7 +540,7 @@ fill_ident_view(Tuplestorestate *tuple_store, TupleDesc tupdesc) errmsg("could not open usermap file \"%s\": %m", IdentFileName))); - linecxt = tokenize_auth_file(IdentFileName, file, &ident_lines, DEBUG3); + linecxt = tokenize_auth_file(IdentFileName, file, &ident_lines, 0, DEBUG3); FreeFile(file); /* Now parse all the lines */ @@ -556,7 +562,8 @@ fill_ident_view(Tuplestorestate *tuple_store, TupleDesc tupdesc) mapping_number++; fill_ident_line(tuple_store, tupdesc, mapping_number, - tok_line->line_num, identline, tok_line->err_msg); + tok_line->file_name, tok_line->line_num, identline, + tok_line->err_msg); } /* Free tokenizer memory */ diff --git a/src/backend/utils/misc/guc-file.l b/src/backend/utils/misc/guc-file.l index ce5633844c..b3e18e48cf 100644 --- a/src/backend/utils/misc/guc-file.l +++ b/src/backend/utils/misc/guc-file.l @@ -700,6 +700,122 @@ GUC_flex_fatal(const char *msg) return 0; /* keep compiler quiet */ } +/* + * Returns the list of config files located in a directory, in alphabetical + * order. + * + * We don't check for recursion or too-deep nesting depth here, its up to the + * caller to take care of that. + */ +char ** +GetDirConfFiles(const char *includedir, const char *calling_file, int elevel, + int *num_filenames, char **err_msg) +{ + char *directory; + DIR *d; + struct dirent *de; + char **filenames; + int size_filenames; + + /* + * Reject directory name that is all-blank (including empty), as that + * leads to confusion --- we'd read the containing directory, typically + * resulting in recursive inclusion of the same file(s). + */ + if (strspn(includedir, " \t\r\n") == strlen(includedir)) + { + ereport(elevel, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("empty configuration directory name: \"%s\"", + includedir))); + *err_msg = "empty configuration directory name"; + return NULL; + } + + directory = AbsoluteConfigLocation(includedir, calling_file); + d = AllocateDir(directory); + if (d == NULL) + { + ereport(elevel, + (errcode_for_file_access(), + errmsg("could not open configuration directory \"%s\": %m", + directory))); + *err_msg = psprintf("could not open directory \"%s\"", directory); + filenames = NULL; + goto cleanup; + } + + /* + * Read the directory and put the filenames in an array, so we can sort + * them prior to caller processing the contents. + */ + size_filenames = 32; + filenames = (char **) palloc(size_filenames * sizeof(char *)); + *num_filenames = 0; + + while ((de = ReadDir(d, directory)) != NULL) + { + struct stat st; + char filename[MAXPGPATH]; + + /* + * Only parse files with names ending in ".conf". Explicitly reject + * files starting with ".". This excludes things like "." and "..", + * as well as typical hidden files, backup files, and editor debris. + */ + if (strlen(de->d_name) < 6) + continue; + if (de->d_name[0] == '.') + continue; + if (strcmp(de->d_name + strlen(de->d_name) - 5, ".conf") != 0) + continue; + + join_path_components(filename, directory, de->d_name); + canonicalize_path(filename); + if (stat(filename, &st) == 0) + { + /* Ignore directories. */ + if (S_ISDIR(st.st_mode)) + continue; + + /* Add file to array, increasing its size in blocks of 32 */ + if (*num_filenames >= size_filenames) + { + size_filenames += 32; + filenames = (char **) repalloc(filenames, + size_filenames * sizeof(char *)); + } + filenames[*num_filenames] = pstrdup(filename); + (*num_filenames)++; + } + else + { + /* + * stat does not care about permissions, so the most likely reason + * a file can't be accessed now is if it was removed between the + * directory listing and now. + */ + ereport(elevel, + (errcode_for_file_access(), + errmsg("could not stat file \"%s\": %m", + filename))); + *err_msg = psprintf("could not stat file \"%s\"", filename); + pfree(filenames); + filenames = NULL; + goto cleanup; + } + } + + if (*num_filenames > 0) + qsort(filenames, *num_filenames, sizeof(char *), pg_qsort_strcmp); + +cleanup: + if (d) + FreeDir(d); + pfree(directory); + return filenames; +} + /* * Read and parse a single configuration file. This function recurses * to handle "include" directives. @@ -961,138 +1077,32 @@ ParseConfigDirectory(const char *includedir, ConfigVariable **head_p, ConfigVariable **tail_p) { - char *directory; - DIR *d; - struct dirent *de; + char *err_msg; char **filenames; int num_filenames; - int size_filenames; - bool status; - /* - * Reject directory name that is all-blank (including empty), as that - * leads to confusion --- we'd read the containing directory, typically - * resulting in recursive inclusion of the same file(s). - */ - if (strspn(includedir, " \t\r\n") == strlen(includedir)) - { - ereport(elevel, - (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("empty configuration directory name: \"%s\"", - includedir))); - record_config_file_error("empty configuration directory name", - calling_file, calling_lineno, - head_p, tail_p); - return false; - } + filenames = GetDirConfFiles(includedir, calling_file, elevel, + &num_filenames, &err_msg); - /* - * We don't check for recursion or too-deep nesting depth here; the - * subsequent calls to ParseConfigFile will take care of that. - */ - - directory = AbsoluteConfigLocation(includedir, calling_file); - d = AllocateDir(directory); - if (d == NULL) + if (!filenames) { - ereport(elevel, - (errcode_for_file_access(), - errmsg("could not open configuration directory \"%s\": %m", - directory))); - record_config_file_error(psprintf("could not open directory \"%s\"", - directory), - calling_file, calling_lineno, - head_p, tail_p); - status = false; - goto cleanup; - } - - /* - * Read the directory and put the filenames in an array, so we can sort - * them prior to processing the contents. - */ - size_filenames = 32; - filenames = (char **) palloc(size_filenames * sizeof(char *)); - num_filenames = 0; - - while ((de = ReadDir(d, directory)) != NULL) - { - struct stat st; - char filename[MAXPGPATH]; - - /* - * Only parse files with names ending in ".conf". Explicitly reject - * files starting with ".". This excludes things like "." and "..", - * as well as typical hidden files, backup files, and editor debris. - */ - if (strlen(de->d_name) < 6) - continue; - if (de->d_name[0] == '.') - continue; - if (strcmp(de->d_name + strlen(de->d_name) - 5, ".conf") != 0) - continue; - - join_path_components(filename, directory, de->d_name); - canonicalize_path(filename); - if (stat(filename, &st) == 0) - { - if (!S_ISDIR(st.st_mode)) - { - /* Add file to array, increasing its size in blocks of 32 */ - if (num_filenames >= size_filenames) - { - size_filenames += 32; - filenames = (char **) repalloc(filenames, - size_filenames * sizeof(char *)); - } - filenames[num_filenames] = pstrdup(filename); - num_filenames++; - } - } - else - { - /* - * stat does not care about permissions, so the most likely reason - * a file can't be accessed now is if it was removed between the - * directory listing and now. - */ - ereport(elevel, - (errcode_for_file_access(), - errmsg("could not stat file \"%s\": %m", - filename))); - record_config_file_error(psprintf("could not stat file \"%s\"", - filename), - calling_file, calling_lineno, - head_p, tail_p); - status = false; - goto cleanup; - } + record_config_file_error(err_msg, calling_file, calling_lineno, head_p, + tail_p); + return false; } - if (num_filenames > 0) + for (int i = 0; i < num_filenames; i++) { - int i; - - qsort(filenames, num_filenames, sizeof(char *), pg_qsort_strcmp); - for (i = 0; i < num_filenames; i++) + if (!ParseConfigFile(filenames[i], true, + calling_file, calling_lineno, + depth, elevel, + head_p, tail_p)) { - if (!ParseConfigFile(filenames[i], true, - calling_file, calling_lineno, - depth, elevel, - head_p, tail_p)) - { - status = false; - goto cleanup; - } + return false; } } - status = true; -cleanup: - if (d) - FreeDir(d); - pfree(directory); - return status; + return true; } /* diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat index e544f9f758..d66b2443a4 100644 --- a/src/include/catalog/pg_proc.dat +++ b/src/include/catalog/pg_proc.dat @@ -6128,16 +6128,16 @@ { oid => '3401', descr => 'show pg_hba.conf rules', proname => 'pg_hba_file_rules', prorows => '1000', proretset => 't', provolatile => 'v', prorettype => 'record', proargtypes => '', - proallargtypes => '{int4,int4,text,_text,_text,text,text,text,_text,text}', - proargmodes => '{o,o,o,o,o,o,o,o,o,o}', - proargnames => '{rule_number,line_number,type,database,user_name,address,netmask,auth_method,options,error}', + proallargtypes => '{int4,text,int4,text,_text,_text,text,text,text,_text,text}', + proargmodes => '{o,o,o,o,o,o,o,o,o,o,o}', + proargnames => '{rule_number,file_name,line_number,type,database,user_name,address,netmask,auth_method,options,error}', prosrc => 'pg_hba_file_rules' }, { oid => '6250', descr => 'show pg_ident.conf mappings', proname => 'pg_ident_file_mappings', prorows => '1000', proretset => 't', provolatile => 'v', prorettype => 'record', proargtypes => '', - proallargtypes => '{int4,int4,text,text,text,text}', - proargmodes => '{o,o,o,o,o,o}', - proargnames => '{mapping_number,line_number,map_name,sys_name,pg_username,error}', + proallargtypes => '{int4,text,int4,text,text,text,text}', + proargmodes => '{o,o,o,o,o,o,o}', + proargnames => '{mapping_number,file_name,line_number,map_name,sys_name,pg_username,error}', prosrc => 'pg_ident_file_mappings' }, { oid => '1371', descr => 'view system lock information', proname => 'pg_lock_status', prorows => '1000', proretset => 't', diff --git a/src/include/libpq/hba.h b/src/include/libpq/hba.h index 90036f7bcd..0ea100d1b8 100644 --- a/src/include/libpq/hba.h +++ b/src/include/libpq/hba.h @@ -79,6 +79,7 @@ typedef enum ClientCertName typedef struct HbaLine { + char *sourcefile; int linenumber; char *rawline; ConnType conntype; @@ -155,6 +156,7 @@ typedef struct AuthToken typedef struct TokenizedAuthLine { List *fields; /* List of lists of AuthTokens */ + char *file_name; /* File name */ int line_num; /* Line number */ char *raw_line; /* Raw line text */ char *err_msg; /* Error message if any */ @@ -174,6 +176,7 @@ extern HbaLine *parse_hba_line(TokenizedAuthLine *tok_line, int elevel); extern IdentLine *parse_ident_line(TokenizedAuthLine *tok_line, int elevel); extern bool pg_isblank(const char c); extern MemoryContext tokenize_auth_file(const char *filename, FILE *file, - List **tok_lines, int elevel); + List **tok_lines, int depth, + int elevel); #endif /* HBA_H */ diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h index e734493a48..1a3ab6306d 100644 --- a/src/include/utils/guc.h +++ b/src/include/utils/guc.h @@ -145,6 +145,8 @@ typedef struct ConfigVariable struct ConfigVariable *next; } ConfigVariable; +extern char **GetDirConfFiles(const char *includedir, const char *calling_file, + int elevel, int *num_filenames, char **err_msg); extern bool ParseConfigFile(const char *config_file, bool strict, const char *calling_file, int calling_lineno, int depth, int elevel, diff --git a/src/test/authentication/t/003_file_inclusion.pl b/src/test/authentication/t/003_file_inclusion.pl new file mode 100644 index 0000000000..8eae72b8d4 --- /dev/null +++ b/src/test/authentication/t/003_file_inclusion.pl @@ -0,0 +1,657 @@ + +# Copyright (c) 2021-2022, PostgreSQL Global Development Group + +# Set of tests for authentication and pg_hba.conf inclusion. +# This test can only run with Unix-domain sockets. + +use strict; +use warnings; +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; +use Test::More; +use Time::HiRes qw(usleep); +use IPC::Run qw(pump finish timer); +use Data::Dumper; + +if (!$use_unix_sockets) +{ + plan skip_all => + "authentication tests cannot run without Unix-domain sockets"; +} + +# stores the current line counter for each file. hba_rule and ident_rule are +# fake file names used for the global rule number for each auth view. +my %cur_line = ('hba_rule' => 1, 'ident_rule' => 1); + +my $hba_file = 'subdir1/pg_hba_custom.conf'; +my $ident_file = 'subdir2/pg_ident_custom.conf'; + +# Initialize primary node +my $node = PostgreSQL::Test::Cluster->new('primary'); +$node->init; +$node->start; + +my $data_dir = $node->data_dir; + +# Normalize the data directory for Windows +$data_dir =~ s/\/\.\//\//g; # reduce /./ to / +$data_dir =~ s/\/\//\//g; # reduce // to / +$data_dir =~ s/\/$//; # remove trailing / + + +# Add the given payload to the given relative HBA file of the given node. +# This function maintains the %cur_line metadata, so it has to be called in the +# expected inclusion evaluation order in order to keep it in sync. +# +# If the payload starts with "include" or "ignore", the function doesn't +# increase the general hba rule number. +# +# If an err_str is provided, it returns an arrayref containing the provided +# filename, the current line number in that file and the provided err_str. The +# err_str has to be a valid regex string. +# Otherwise it only returns the line number of the payload in the wanted file. +# This function has to be called in the expected inclusion evaluation order to +# keep the %cur_line information in sync. +sub add_hba_line +{ + my $node = shift; + my $filename = shift; + my $payload = shift; + my $err_str = shift; + my $globline; + my $fileline; + my @tokens; + my $line; + + # Append the payload to the given file + $node->append_conf($filename, $payload); + + # Get the current %cur_line counter for the file + if (not defined $cur_line{$filename}) + { + $cur_line{$filename} = 1; + } + $fileline = $cur_line{$filename}++; + + # Include directive, don't generate an underlying pg_hba_file_rules line + # but make sure we incremented the %cur_line counter. + # Also ignore line beginning with "ignore", for content of files that + # should not being included + if ($payload =~ qr/^(include|ignore)/) + { + if (defined $err_str) + { + return [$filename, $fileline, $err_str]; + } + else + { + return $fileline; + } + } + + # Get (and increment) the global rule number + $globline = $cur_line{'hba_rule'}++; + + # If caller provided an err_str, just returns the needed metadata + if (defined $err_str) + { + return [$filename, $fileline, $err_str]; + } + + # Otherwise, generate the expected pg_hba_file_rules line + @tokens = split(/ /, $payload); + $tokens[1] = '{' . $tokens[1] . '}'; # database + $tokens[2] = '{' . $tokens[2] . '}'; # user_name + + # add empty address and netmask betweed user_name and auth_method + splice @tokens, 3, 0, ''; + splice @tokens, 3, 0, ''; + + # append empty options and error + push @tokens, ''; + push @tokens, ''; + + # generate the expected final line + $line = ""; + $line .= "\n" if ($globline > 1); + $line .= "$globline|$data_dir/$filename|$fileline|"; + $line .= join('|', @tokens); + + return $line; +} + +# Add the given payload to the given relative ident file of the given node. +# Same as add_hba_line but for pg_ident files +sub add_ident_line +{ + my $node = shift; + my $filename = shift; + my $payload = shift; + my $err_str = shift; + my $globline; + my $fileline; + my @tokens; + my $line; + + # Append the payload to the given file + $node->append_conf($filename, $payload); + + # Get the current %cur_line counter for the file + if (not defined $cur_line{$filename}) + { + $cur_line{$filename} = 1; + } + $fileline = $cur_line{$filename}++; + + # Include directive, don't generate an underlying pg_hba_file_rules line + # but make sure we incremented the %cur_line counter. + # Also ignore line beginning with "ignore", for content of files that + # should not being included + if ($payload =~ qr/^(include|ignore)/) + { + if (defined $err_str) + { + return [$filename, $fileline, $err_str]; + } + else + { + return $fileline; + } + } + + # Get (and increment) the global rule number + $globline = $cur_line{'ident_rule'}++; + + # If caller provided an err_str, just returns the needed metadata + if (defined $err_str) + { + return [$filename, $fileline, $err_str]; + } + + # Otherwise, generate the expected pg_ident_file_mappings line + @tokens = split(/ /, $payload); + + # append empty error + push @tokens, ''; + + # generate the expected final line + $line = ""; + $line .= "\n" if ($globline > 1); + $line .= "$globline|$data_dir/$filename|$fileline|"; + $line .= join('|', @tokens); + + return $line; +} + +# Delete pg_hba.conf from the given node, add various entries to test the +# include infrastructure and then execute a reload to refresh it. +sub generate_valid_auth_files +{ + my $node = shift; + my $hba_expected = ''; + my $ident_expected = ''; + + # customise main auth file names + $node->safe_psql('postgres', "ALTER SYSTEM SET hba_file = '$data_dir/$hba_file'"); + $node->safe_psql('postgres', "ALTER SYSTEM SET ident_file = '$data_dir/$ident_file'"); + + # and make original ones invalid to be sure they're not used anywhere + $node->append_conf('pg_hba.conf', "some invalid line"); + $node->append_conf('pg_ident.conf', "some invalid line"); + + # pg_hba stuff + mkdir("$data_dir/subdir1"); + mkdir("$data_dir/hba_inc"); + mkdir("$data_dir/hba_inc_if"); + mkdir("$data_dir/hba_pos"); + + # Make sure we will still be able to connect + $hba_expected .= add_hba_line($node, "$hba_file", 'local all all trust'); + + # Add include data + add_hba_line($node, "$hba_file", "include ../pg_hba_pre.conf"); + $hba_expected .= add_hba_line($node, 'pg_hba_pre.conf', "local pre all reject"); + + $hba_expected .= add_hba_line($node, "$hba_file", "local all all reject"); + + add_hba_line($node, "$hba_file", "include ../hba_pos/pg_hba_pos.conf"); + $hba_expected .= add_hba_line($node, 'hba_pos/pg_hba_pos.conf', "local pos all reject"); + # include is relative to current path + add_hba_line($node, 'hba_pos/pg_hba_pos.conf', "include pg_hba_pos2.conf"); + $hba_expected .= add_hba_line($node, 'hba_pos/pg_hba_pos2.conf', "local pos2 all reject"); + + # include_if_exists data + add_hba_line($node, "$hba_file", "include_if_exists ../hba_inc_if/none"); + add_hba_line($node, "$hba_file", "include_if_exists ../hba_inc_if/some"); + $hba_expected .= add_hba_line($node, 'hba_inc_if/some', "local if_some all reject"); + + # include_dir data + add_hba_line($node, "$hba_file", "include_dir ../hba_inc"); + add_hba_line($node, 'hba_inc/garbageconf', "ignore - should not be included"); + $hba_expected .= add_hba_line($node, 'hba_inc/01_z.conf', "local dir_z all reject"); + $hba_expected .= add_hba_line($node, 'hba_inc/02_a.conf', "local dir_a all reject"); + + # secondary auth file + add_hba_line($node, $hba_file, 'local @../dbnames.conf all reject'); + $node->append_conf('dbnames.conf', "db1"); + $node->append_conf('dbnames.conf', "db3"); + $hba_expected .= "\n" . ($cur_line{'hba_rule'} - 1) + . "|$data_dir/$hba_file|" . ($cur_line{$hba_file} - 1) + . '|local|{db1,db3}|{all}|||reject||'; + + # pg_ident stuff + mkdir("$data_dir/subdir2"); + mkdir("$data_dir/ident_inc"); + mkdir("$data_dir/ident_inc_if"); + mkdir("$data_dir/ident_pos"); + + # Add include data + add_ident_line($node, "$ident_file", "include ../pg_ident_pre.conf"); + $ident_expected .= add_ident_line($node, 'pg_ident_pre.conf', "pre foo bar"); + + $ident_expected .= add_ident_line($node, "$ident_file", "test a b"); + + add_ident_line($node, "$ident_file", "include ../ident_pos/pg_ident_pos.conf"); + $ident_expected .= add_ident_line($node, 'ident_pos/pg_ident_pos.conf', "pos foo bar"); + # include is relative to current path + add_ident_line($node, 'ident_pos/pg_ident_pos.conf', "include pg_ident_pos2.conf"); + $ident_expected .= add_ident_line($node, 'ident_pos/pg_ident_pos2.conf', "pos2 foo bar"); + + # include_if_exists data + add_ident_line($node, "$ident_file", "include_if_exists ../ident_inc_if/none"); + add_ident_line($node, "$ident_file", "include_if_exists ../ident_inc_if/some"); + $ident_expected .= add_ident_line($node, 'ident_inc_if/some', "if_some foo bar"); + + # include_dir data + add_ident_line($node, "$ident_file", "include_dir ../ident_inc"); + add_ident_line($node, 'ident_inc/garbageconf', "ignore - should not be included"); + $ident_expected .= add_ident_line($node, 'ident_inc/01_z.conf', "dir_z foo bar"); + $ident_expected .= add_ident_line($node, 'ident_inc/02_a.conf', "dir_a foo bar"); + + $node->restart; + $node->connect_ok('dbname=postgres', + 'Connection ok after generating valid auth files'); + + return ($hba_expected, $ident_expected); +} + +# Delete pg_hba.conf and pg_ident.conf from the given node and add minimal +# entries to allow authentication. +sub reset_auth_files +{ + my $node = shift; + + unlink("$data_dir/$hba_file"); + unlink("$data_dir/$ident_file"); + + %cur_line = ('hba_rule' => 1, 'ident_rule' => 1); + + return add_hba_line($node, "$hba_file", 'local all all trust'); +} + +# Generate a list of expected error regex for the given array of error +# conditions, as generated by add_hba_line/add_ident_line with an err_str. +# +# 2 regex are generated per array entry: one for the given err_str, and one for +# the expected line in the specific file. Since all lines are independant, +# there's no guarantee that a specific failure regex and the per-line regex +# will match the same error. Calling code should add at least one test with a +# single error to make sure that the line number / file name is correct. +# +# On top of that, an extra line is generated for the general failure to process +# the main auth file. +sub generate_log_err_patterns +{ + my $node = shift; + my $raw_errors = shift; + my $is_hba_err = shift; + my @errors; + + foreach my $arr (@{$raw_errors}) + { + my $filename = @{$arr}[0]; + my $fileline = @{$arr}[1]; + my $err_str = @{$arr}[2]; + + push @errors, qr/$err_str/; + + # Context messages with the file / line location aren't always emitted + if ($err_str !~ /maximum nesting depth exceeded/ and + $err_str !~ /could not open secondary authentication file/) + { + push @errors, qr/line $fileline of configuration file "$data_dir\/$filename"/ + } + } + + push @errors, qr/could not load $data_dir\/$hba_file/ if ($is_hba_err); + + return \@errors; +} + +# Generate the expected output for the auth file view error reporting (file +# name, file line, error), for the given array of error conditions, as +# generated generated by add_hba_line/add_ident_line with an err_str. +sub generate_log_err_rows +{ + my $node = shift; + my $raw_errors = shift; + my $exp_rows = ''; + + foreach my $arr (@{$raw_errors}) + { + my $filename = @{$arr}[0]; + my $fileline = @{$arr}[1]; + my $err_str = @{$arr}[2]; + + $exp_rows .= "\n" if ($exp_rows ne ""); + + # Unescape regex patterns if any + $err_str =~ s/\\([\(\)])/$1/g; + $exp_rows .= "|$data_dir\/$filename|$fileline|$err_str" + } + + return $exp_rows; +} + +# Reset the main auth files, append the given payload to the given config file, +# and check that the instance cannot start, raising the expected error line(s). +sub start_errors_like +{ + my $node = shift; + my $file = shift; + my $payload = shift; + my $pattern = shift; + my $should_fail = shift; + + reset_auth_files($node); + $node->append_conf($file, $payload); + + unlink($node->logfile); + my $ret = + PostgreSQL::Test::Utils::system_log('pg_ctl', '-D', $data_dir, + '-l', $node->logfile, 'start'); + + if ($should_fail) + { + ok($ret != 0, "Cannot start postgres with faulty $file"); + } + else + { + ok($ret == 0, "postgres can start with faulty $file"); + } + + my $log_contents = slurp_file($node->logfile); + + foreach (@{$pattern}) + { + like($log_contents, + $_, + "Expected failure found in the logs"); + } + + if (not $should_fail) + { + # We can't simply call $node->stop here as the call is optimized out + # when the server isn't started with $node->start. + my $ret = + PostgreSQL::Test::Utils::system_log('pg_ctl', '-D', + $data_dir, 'stop', '-m', 'fast'); + ok($ret == 0, "Could stop postgres"); + } +} + +# We should be able to connect, and see an empty pg_ident.conf +is($node->psql( + 'postgres', 'SELECT count(*) FROM pg_ident_file_mappings'), + qq(0), + 'pg_ident.conf is empty'); + +############################################ +# part 1, test view reporting for valid data +############################################ +my ($exp_hba, $exp_ident) = generate_valid_auth_files($node); + +$node->connect_ok('dbname=postgres', 'Connection still ok'); + +is($node->safe_psql( + 'postgres', 'SELECT * FROM pg_hba_file_rules'), + qq($exp_hba), + 'pg_hba_file_rules content is expected'); + +is($node->safe_psql( + 'postgres', 'SELECT * FROM pg_ident_file_mappings'), + qq($exp_ident), + 'pg_ident_file_mappings content is expected'); + +############################################# +# part 2, test log reporting for invalid data +############################################# +reset_auth_files($node); +$node->restart('fast'); +$node->connect_ok('dbname=postgres', + 'Connection ok after resetting auth files'); + +$node->stop('fast'); + +start_errors_like($node, $hba_file, "include ../not_a_file", + [ + qr/could not open included authentication file "\.\.\/not_a_file" as "$data_dir\/not_a_file": No such file or directory/, + qr/could not load $data_dir\/$hba_file/ + ], 1); + +# include_dir, single included file +mkdir("$data_dir/hba_inc_fail"); +add_hba_line($node, "hba_inc_fail/inc_dir.conf", "local all all reject"); +add_hba_line($node, "hba_inc_fail/inc_dir.conf", "local all all reject"); +add_hba_line($node, "hba_inc_fail/inc_dir.conf", "local all all reject"); +add_hba_line($node, "hba_inc_fail/inc_dir.conf", "not_a_token"); +start_errors_like($node, $hba_file, "include_dir ../hba_inc_fail", + [ + qr/invalid connection type "not_a_token"/, + qr/line 4 of configuration file "$data_dir\/hba_inc_fail\/inc_dir\.conf"/, + qr/could not load $data_dir\/$hba_file/ + ], 1); + +# include_dir, single included file with nested inclusion +unlink("$data_dir/hba_inc_fail/inc_dir.conf"); +my @hba_raw_errors_step1; + +add_hba_line($node, "hba_inc_fail/inc_dir.conf", "include file1"); + +add_hba_line($node, "hba_inc_fail/file1", "include file2"); +add_hba_line($node, "hba_inc_fail/file2", "local all all reject"); +add_hba_line($node, "hba_inc_fail/file2", "include file3"); + +add_hba_line($node, "hba_inc_fail/file3", "local all all reject"); +add_hba_line($node, "hba_inc_fail/file3", "local all all reject"); +push @hba_raw_errors_step1, add_hba_line($node, "hba_inc_fail/file3", + "local all all zuul", + 'invalid authentication method "zuul"'); + +start_errors_like( + $node, $hba_file, "include_dir ../hba_inc_fail", + generate_log_err_patterns($node, \@hba_raw_errors_step1, 1), 1); + +# start_errors_like will reset the main auth files, so the previous error won't +# occur again. We keep it around as we will put back both bogus inclusions for +# the tests at step 3. +my @hba_raw_errors_step2; + +# include_if_exists, with various problems +push @hba_raw_errors_step2, add_hba_line($node, "hba_if_exists.conf", + "local", + "end-of-line before database specification"); +push @hba_raw_errors_step2, add_hba_line($node, "hba_if_exists.conf", + "local,host", + "multiple values specified for connection type"); +push @hba_raw_errors_step2, add_hba_line($node, "hba_if_exists.conf", + "local all", + "end-of-line before role specification"); +push @hba_raw_errors_step2, add_hba_line($node, "hba_if_exists.conf", + "local all all", + "end-of-line before authentication method"); +push @hba_raw_errors_step2, add_hba_line($node, "hba_if_exists.conf", + "host all all test/42", + 'specifying both host name and CIDR mask is invalid: "test/42"'); +push @hba_raw_errors_step2, add_hba_line($node, "hba_if_exists.conf", + 'local @dbnames_fails.conf all reject', + "could not open secondary authentication file \"\@dbnames_fails.conf\" as \"$data_dir/dbnames_fails.conf\": No such file or directory"); + +add_hba_line($node, "hba_if_exists.conf", "include recurse.conf"); +push @hba_raw_errors_step2, add_hba_line($node, "recurse.conf", + "include recurse.conf", + 'could not open configuration file "recurse.conf": maximum nesting depth exceeded'); + +# Generate the regex for the expected errors in the logs. There's no guarantee +# that the generated "line X of file..." will be emitted for the expected line, +# but previous tests already ensured that the correct line number / file name +# was emitted, so ensuring that there's an error in all expected lines is +# enough here. +my $expected_errors = generate_log_err_patterns($node, \@hba_raw_errors_step2, + 1); + +# Not an error, but it should raise a message in the logs. Manually add an +# extra log message to detect +add_hba_line($node, "hba_if_exists.conf", "include_if_exists if_exists_none"); +push @{$expected_errors}, + qr/skipping missing authentication file "$data_dir\/if_exists_none"/; + +start_errors_like( + $node, $hba_file, "include_if_exists ../hba_if_exists.conf", + $expected_errors, 1); + +# Mostly the same, but for ident files +reset_auth_files($node); + +my @ident_raw_errors_step1; + +# include_dir, single included file with nested inclusion +mkdir("$data_dir/ident_inc_fail"); +add_ident_line($node, "ident_inc_fail/inc_dir.conf", "include file1"); + +add_ident_line($node, "ident_inc_fail/file1", "include file2"); +add_ident_line($node, "ident_inc_fail/file2", "ok ok ok"); +add_ident_line($node, "ident_inc_fail/file2", "include file3"); + +add_ident_line($node, "ident_inc_fail/file3", "ok ok ok"); +add_ident_line($node, "ident_inc_fail/file3", "ok ok ok"); +push @ident_raw_errors_step1, add_ident_line($node, "ident_inc_fail/file3", + "failmap /(fail postgres", + 'invalid regular expression "\(fail": parentheses \(\) not balanced'); + +start_errors_like( + $node, $ident_file, "include_dir ../ident_inc_fail", + generate_log_err_patterns($node, \@ident_raw_errors_step1, 0), + 0); + +# start_errors_like will reset the main auth files, so the previous error won't +# occur again. We keep it around as we will put back both bogus inclusions for +# the tests at step 3. +my @ident_raw_errors_step2; + +# include_if_exists, with various problems +push @ident_raw_errors_step2, add_ident_line($node, "ident_if_exists.conf", "map", + "missing entry at end of line"); +push @ident_raw_errors_step2, add_ident_line($node, "ident_if_exists.conf", "map1,map2", + "multiple values in ident field"); +push @ident_raw_errors_step2, add_ident_line($node, "ident_if_exists.conf", + 'map @osnames_fails.conf postgres', + "could not open secondary authentication file \"\@osnames_fails.conf\" as \"$data_dir/osnames_fails.conf\": No such file or directory"); + +add_ident_line($node, "ident_if_exists.conf", "include ident_recurse.conf"); +push @ident_raw_errors_step2, add_ident_line($node, "ident_recurse.conf", "include ident_recurse.conf", + 'could not open configuration file "ident_recurse.conf": maximum nesting depth exceeded'); + +start_errors_like( + $node, $ident_file, "include_if_exists ../ident_if_exists.conf", + # There's no guarantee that the generated "line X of file..." will be + # emitted for the expected line, but previous tests already ensured that + # the correct line number / file name was emitted, so ensuring that there's + # an error in all expected lines is enough here. + generate_log_err_patterns($node, \@ident_raw_errors_step2, 0), + 0); + +##################################################### +# part 3, test reporting of various error scenario +# NOTE: this will be bypassed -DEXEC_BACKEND or win32 +##################################################### +reset_auth_files($node); + +$node->start; +$node->connect_ok('dbname=postgres', 'Can connect after an auth file reset'); + +is($node->safe_psql( + 'postgres', + 'SELECT count(*) FROM pg_hba_file_rules WHERE error IS NOT NULL'), + qq(0), + 'No error expected in pg_hba_file_rules'); + +add_ident_line($node, $ident_file, ''); +is($node->safe_psql( + 'postgres', + 'SELECT count(*) FROM pg_ident_file_mappings WHERE error IS NOT NULL'), + qq(0), + 'No error expected in pg_ident_file_mappings'); + +# The instance could be restarted and no error is detected. Now check if the +# build is compatible with the view error reporting (EXEC_BACKEND / win32 will +# fail when trying to connect as they always rely on the current auth files +# content) +my @hba_raw_errors; + +push @hba_raw_errors, add_hba_line($node, $hba_file, "include ../not_a_file", + "could not open included authentication file \"../not_a_file\" as \"$data_dir/not_a_file\": No such file or directory"); + +my ($stdout, $stderr); +my $cmdret = $node->psql('postgres', 'SELECT 1', + stdout => \$stdout, stderr => \$stderr); + +if ($cmdret != 0) +{ + # Connection failed. Bail out, but make sure to raise a failure if it + # didn't fail for the expected hba file modification. + like($stderr, + qr/connection to server.* failed: FATAL: could not load $data_dir\/$hba_file/, + "Connection failed due to loading an invalid hba file"); + + done_testing(); + diag("Build not compatible with auth file view error reporting, bail out.\n"); + exit; +} + +# Combine errors generated at step 2, in the same order. +$node->append_conf($hba_file, "include_dir ../hba_inc_fail"); +push @hba_raw_errors, @hba_raw_errors_step1; + +$node->append_conf($hba_file, "include_if_exists ../hba_if_exists.conf"); +push @hba_raw_errors, @hba_raw_errors_step2; + +my $hba_expected = generate_log_err_rows($node, \@hba_raw_errors); +is($node->safe_psql( + 'postgres', + 'SELECT rule_number, file_name, line_number, error FROM pg_hba_file_rules' + . ' WHERE error IS NOT NULL ORDER BY rule_number'), + qq($hba_expected), + 'Detected all error in hba file'); + +# and do the same for pg_ident +my @ident_raw_errors; + +push @ident_raw_errors, add_ident_line($node, $ident_file, "include ../not_a_file", + "could not open included authentication file \"../not_a_file\" as \"$data_dir/not_a_file\": No such file or directory"); + +$node->append_conf($ident_file, "include_dir ../ident_inc_fail"); +push @ident_raw_errors, @ident_raw_errors_step1; + +$node->append_conf($ident_file, "include_if_exists ../ident_if_exists.conf"); +push @ident_raw_errors, @ident_raw_errors_step2; + +my $ident_expected = generate_log_err_rows($node, \@ident_raw_errors); +is($node->safe_psql( + 'postgres', + 'SELECT mapping_number, file_name, line_number, error FROM pg_ident_file_mappings' + . ' WHERE error IS NOT NULL ORDER BY mapping_number'), + qq($ident_expected), + 'Detected all error in ident file'); + +done_testing(); diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out index 79408710e0..5ed2fe3704 100644 --- a/src/test/regress/expected/rules.out +++ b/src/test/regress/expected/rules.out @@ -1338,6 +1338,7 @@ pg_group| SELECT pg_authid.rolname AS groname, FROM pg_authid WHERE (NOT pg_authid.rolcanlogin); pg_hba_file_rules| SELECT a.rule_number, + a.file_name, a.line_number, a.type, a.database, @@ -1347,14 +1348,15 @@ pg_hba_file_rules| SELECT a.rule_number, a.auth_method, a.options, a.error - FROM pg_hba_file_rules() a(rule_number, line_number, type, database, user_name, address, netmask, auth_method, options, error); + FROM pg_hba_file_rules() a(rule_number, file_name, line_number, type, database, user_name, address, netmask, auth_method, options, error); pg_ident_file_mappings| SELECT a.mapping_number, + a.file_name, a.line_number, a.map_name, a.sys_name, a.pg_username, a.error - FROM pg_ident_file_mappings() a(mapping_number, line_number, map_name, sys_name, pg_username, error); + FROM pg_ident_file_mappings() a(mapping_number, file_name, line_number, map_name, sys_name, pg_username, error); pg_indexes| SELECT n.nspname AS schemaname, c.relname AS tablename, i.relname AS indexname, -- 2.37.0
>From 60df23e03e07397054fe71a224f8302524d52741 Mon Sep 17 00:00:00 2001 From: Julien Rouhaud <julien.rouh...@free.fr> Date: Tue, 22 Feb 2022 21:34:54 +0800 Subject: [PATCH v8 6/6] POC: Add a pg_hba_matches() function. Catversion is bumped. Author: Julien Rouhaud Reviewed-by: FIXME Discussion: https://postgr.es/m/20220223045959.35ipdsvbxcstrhya%40jrouhaud --- src/backend/catalog/system_functions.sql | 9 ++ src/backend/libpq/hba.c | 138 +++++++++++++++++++++++ src/include/catalog/pg_proc.dat | 7 ++ 3 files changed, 154 insertions(+) diff --git a/src/backend/catalog/system_functions.sql b/src/backend/catalog/system_functions.sql index 73da687d5d..bfee72f705 100644 --- a/src/backend/catalog/system_functions.sql +++ b/src/backend/catalog/system_functions.sql @@ -594,6 +594,15 @@ LANGUAGE internal STRICT IMMUTABLE PARALLEL SAFE AS 'unicode_is_normalized'; +CREATE OR REPLACE FUNCTION + pg_hba_matches( + IN address inet, IN role text, IN ssl bool DEFAULT false, + OUT file_name text, OUT line_num int4, OUT raw_line text) +RETURNS RECORD +LANGUAGE INTERNAL +VOLATILE +AS 'pg_hba_matches'; + -- -- The default permissions for functions mean that anyone can execute them. -- A number of functions shouldn't be executable by just anyone, but rather diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c index 814bfcf30d..70e7c871a8 100644 --- a/src/backend/libpq/hba.c +++ b/src/backend/libpq/hba.c @@ -27,6 +27,7 @@ #include <unistd.h> #include "access/htup_details.h" +#include "catalog/pg_authid.h" #include "catalog/pg_collation.h" #include "catalog/pg_type.h" #include "common/ip.h" @@ -42,6 +43,7 @@ #include "utils/acl.h" #include "utils/builtins.h" #include "utils/guc.h" +#include "utils/inet.h" #include "utils/lsyscache.h" #include "utils/memutils.h" #include "utils/varlena.h" @@ -2983,3 +2985,139 @@ hba_authname(UserAuth auth_method) return UserAuthName[auth_method]; } + +#define PG_HBA_MATCHES_ATTS 3 + +/* + * SQL-accessible SRF to return the entries that match the given connection + * info, if any. + */ +Datum pg_hba_matches(PG_FUNCTION_ARGS) +{ + MemoryContext ctxt; + inet *address = NULL; + bool ssl_in_use = false; + hbaPort *port = palloc0(sizeof(hbaPort)); + TupleDesc tupdesc; + Datum values[PG_HBA_MATCHES_ATTS]; + bool isnull[PG_HBA_MATCHES_ATTS]; + + if (!is_member_of_role(GetUserId(), ROLE_PG_READ_SERVER_FILES)) + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + errmsg("only superuser or a member of the pg_read_server_files role may call this function"))); + + if (PG_ARGISNULL(0)) + port->raddr.addr.ss_family = AF_UNIX; + else + { + int bits; + char *ptr; + char tmp[sizeof("xxxx:xxxx:xxxx:xxxx:xxxx:xxxx:255.255.255.255/128")]; + + address = PG_GETARG_INET_PP(0); + + bits = ip_maxbits(address) - ip_bits(address); + if (bits != 0) + { + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("Invalid address"))); + } + + /* force display of max bits, regardless of masklen... */ + if (pg_inet_net_ntop(ip_family(address), ip_addr(address), + ip_maxbits(address), tmp, sizeof(tmp)) == NULL) + ereport(ERROR, + (errcode(ERRCODE_INVALID_BINARY_REPRESENTATION), + errmsg("could not format inet value: %m"))); + + /* Suppress /n if present (shouldn't happen now) */ + if ((ptr = strchr(tmp, '/')) != NULL) + *ptr = '\0'; + + switch (ip_family(address)) + { + case PGSQL_AF_INET: + { + struct sockaddr_in *dst; + + dst = (struct sockaddr_in *) &port->raddr.addr; + dst->sin_family = AF_INET; + + /* ip_addr(address) always contains network representation */ + memcpy(&dst->sin_addr, &ip_addr(address), sizeof(dst->sin_addr)); + + break; + } + /* See pg_inet_net_ntop() for details about those constants */ + case PGSQL_AF_INET6: +#if defined(AF_INET6) && AF_INET6 != PGSQL_AF_INET6 + case AF_INET6: +#endif + { + struct sockaddr_in6 *dst; + + dst = (struct sockaddr_in6 *) &port->raddr.addr; + dst->sin6_family = AF_INET6; + + /* ip_addr(address) always contains network representation */ + memcpy(&dst->sin6_addr, &ip_addr(address), sizeof(dst->sin6_addr)); + + break; + } + default: + elog(ERROR, "unexpected ip_family: %d", ip_family(address)); + break; + } + } + + if (PG_ARGISNULL(1)) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("parameter role is mandatory"))); + port->user_name = text_to_cstring(PG_GETARG_TEXT_PP(1)); + + if (!PG_ARGISNULL(2)) + ssl_in_use = PG_GETARG_BOOL(2); + + port->ssl_in_use = ssl_in_use; + + tupdesc = CreateTemplateTupleDesc(PG_HBA_MATCHES_ATTS); + TupleDescInitEntry(tupdesc, (AttrNumber) 1, "file_name", + TEXTOID, -1, 0); + TupleDescInitEntry(tupdesc, (AttrNumber) 2, "line_num", + INT4OID, -1, 0); + TupleDescInitEntry(tupdesc, (AttrNumber) 3, "raw_line", + TEXTOID, -1, 0); + + BlessTupleDesc(tupdesc); + + memset(isnull, 0, sizeof(isnull)); + + /* FIXME rework API to not rely on PostmasterContext */ + ctxt = AllocSetContextCreate(CurrentMemoryContext, "load_hba", + ALLOCSET_DEFAULT_SIZES); + PostmasterContext = AllocSetContextCreate(ctxt, + "Postmaster", + ALLOCSET_DEFAULT_SIZES); + parsed_hba_context = NULL; + if (!load_hba()) + ereport(ERROR, + (errcode(ERRCODE_CONFIG_FILE_ERROR), + errmsg("Invalidation auth configuration file"))); + + check_hba(port); + + if (port->hba->auth_method == uaImplicitReject) + PG_RETURN_NULL(); + + values[0] = CStringGetTextDatum(port->hba->sourcefile); + values[1] = Int32GetDatum(port->hba->linenumber); + values[2] = CStringGetTextDatum(port->hba->rawline); + + MemoryContextDelete(PostmasterContext); + PostmasterContext = NULL; + + return HeapTupleGetDatum(heap_form_tuple(tupdesc, values, isnull)); +} diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat index d66b2443a4..f6609a4a5f 100644 --- a/src/include/catalog/pg_proc.dat +++ b/src/include/catalog/pg_proc.dat @@ -6139,6 +6139,13 @@ proargmodes => '{o,o,o,o,o,o,o}', proargnames => '{mapping_number,file_name,line_number,map_name,sys_name,pg_username,error}', prosrc => 'pg_ident_file_mappings' }, +{ oid => '9557', descr => 'show wether the given connection would match an hba line', + proname => 'pg_hba_matches', provolatile => 'v', prorettype => 'record', + proargtypes => 'inet text bool', proisstrict => 'f', + proallargtypes => '{inet,text,bool,text,int4,text}', + proargmodes => '{i,i,i,o,o,o}', + proargnames => '{address,role,ssl,file_name,line_num,raw_line}', + prosrc => 'pg_hba_matches' }, { oid => '1371', descr => 'view system lock information', proname => 'pg_lock_status', prorows => '1000', proretset => 't', provolatile => 'v', prorettype => 'record', proargtypes => '', -- 2.37.0