Hi, On Thu, Nov 24, 2022 at 02:37:23PM +0800, Julien Rouhaud wrote: > On Thu, Nov 24, 2022 at 02:07:21PM +0900, Michael Paquier wrote: > > On Wed, Nov 23, 2022 at 03:56:50PM +0800, Julien Rouhaud wrote: > > > The depth 0 is getting used quite a lot now, maybe we should have a > > > define for > > > it to make it easier to grep, like TOP_LEVEL_AUTH_FILE or something like > > > that? > > > And also add a define for the magical 10 for the max inclusion depth, for > > > both > > > auth files and GUC files while at it? > > > > Sounds like a good idea to me, and it seems to me that this had better > > be unified between the GUCs (see ParseConfigFp() that hardcodes a > > depth of 0) and hba.c. It looks like they could be added to > > conffiles.h, as of CONF_FILE_START_{LEVEL,DEPTH} and > > CONF_FILE_MAX_{LEVEL,DEPTH}. Would you like to send a patch?
So I went with CONF_FILE_START_DEPTH and CONF_FILE_MAX_DEPTH. Attached v22 that fixes it in all the places I found. > > Now, to the tests.. > > > > > Mmm, I haven't looked deeply so I'm not sure if the perl podules are > > > aware of > > > it or not, but maybe we could somehow detect the used delimiter at the > > > beginning after normalizing the directory, and use a $DELIM rather than a > > > plain > > > "/"? > > > > I am not sure. Could you have a look and see if you can get the CI > > back to green? The first thing I would test is to switch the error > > patterns to be regexps based on the basenames rather than the full > > paths (tweaking the queries on the system views to do htat), so as we > > avoid all this business with slash and backslash transformations. Apparently just making sure that the $node->data_dir consistently uses forward slashes is enough to make the CI happy, for VS 2019 [1] and MinGW64 [2], so done this way with an extra normalization step. [1] https://cirrus-ci.com/task/4944203946917888 [2] https://cirrus-ci.com/task/6070103853760512
>From 879cf469d00d9274b67b80eb5fe47dfccf03022d Mon Sep 17 00:00:00 2001 From: Julien Rouhaud <julien.rouh...@free.fr> Date: Thu, 24 Nov 2022 16:57:53 +0800 Subject: [PATCH v22 1/2] Introduce macros for initial/maximum depth levels for configuration files Author: Julien Rouhaud Reviewed-by: FIXME Discussion: https://postgr.es/m/20220223045959.35ipdsvbxcstrhya@jrouhaud --- src/backend/commands/extension.c | 4 +++- src/backend/libpq/hba.c | 8 ++++---- src/backend/utils/misc/guc-file.l | 5 +++-- src/backend/utils/misc/guc.c | 8 +++++--- src/include/utils/conffiles.h | 3 +++ 5 files changed, 18 insertions(+), 10 deletions(-) diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c index 806d6056ab..de01b792b9 100644 --- a/src/backend/commands/extension.c +++ b/src/backend/commands/extension.c @@ -60,6 +60,7 @@ #include "tcop/utility.h" #include "utils/acl.h" #include "utils/builtins.h" +#include "utils/conffiles.h" #include "utils/fmgroids.h" #include "utils/lsyscache.h" #include "utils/memutils.h" @@ -515,7 +516,8 @@ parse_extension_control_file(ExtensionControlFile *control, * Parse the file content, using GUC's file parsing code. We need not * check the return value since any errors will be thrown at ERROR level. */ - (void) ParseConfigFp(file, filename, 0, ERROR, &head, &tail); + (void) ParseConfigFp(file, filename, CONF_FILE_START_DEPTH, ERROR, &head, + &tail); FreeFile(file); diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c index 862ec18e91..8f1a0c4c73 100644 --- a/src/backend/libpq/hba.c +++ b/src/backend/libpq/hba.c @@ -620,7 +620,7 @@ free_auth_file(FILE *file, int depth) FreeFile(file); /* If this is the last cleanup, remove the tokenization context */ - if (depth == 0) + if (depth == CONF_FILE_START_DEPTH) { MemoryContextDelete(tokenize_context); tokenize_context = NULL; @@ -650,7 +650,7 @@ open_auth_file(const char *filename, int elevel, int depth, * 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) + if (depth > CONF_FILE_MAX_DEPTH) { ereport(elevel, (errcode_for_file_access(), @@ -684,7 +684,7 @@ open_auth_file(const char *filename, int elevel, int depth, * tokenization. This will be closed with this file when coming back to * this level of cleanup. */ - if (depth == 0) + if (depth == CONF_FILE_START_DEPTH) { /* * A context may be present, but assume that it has been eliminated @@ -762,7 +762,7 @@ tokenize_auth_file(const char *filename, FILE *file, List **tok_lines, initStringInfo(&buf); - if (depth == 0) + if (depth == CONF_FILE_START_DEPTH) *tok_lines = NIL; while (!feof(file) && !ferror(file)) diff --git a/src/backend/utils/misc/guc-file.l b/src/backend/utils/misc/guc-file.l index 88245475d1..f7e4457ded 100644 --- a/src/backend/utils/misc/guc-file.l +++ b/src/backend/utils/misc/guc-file.l @@ -202,7 +202,7 @@ ParseConfigFile(const char *config_file, bool strict, * 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) + if (depth > CONF_FILE_MAX_DEPTH) { ereport(elevel, (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED), @@ -321,7 +321,8 @@ GUC_flex_fatal(const char *msg) * Input parameters: * fp: file pointer from AllocateFile for the configuration file to parse * config_file: absolute or relative path name of the configuration file - * depth: recursion depth (should be 0 in the outermost call) + * depth: recursion depth (should be CONF_FILE_START_DEPTH in the outermost + * call) * elevel: error logging level to use * Input/Output parameters: * head_p, tail_p: head and tail of linked list of name/value pairs diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index 117a2d26a0..28313b3a94 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -44,6 +44,7 @@ #include "utils/acl.h" #include "utils/backend_status.h" #include "utils/builtins.h" +#include "utils/conffiles.h" #include "utils/float.h" #include "utils/guc_tables.h" #include "utils/memutils.h" @@ -287,7 +288,7 @@ ProcessConfigFileInternal(GucContext context, bool applySettings, int elevel) head = tail = NULL; if (!ParseConfigFile(ConfigFileName, true, - NULL, 0, 0, elevel, + NULL, 0, CONF_FILE_START_DEPTH, elevel, &head, &tail)) { /* Syntax error(s) detected in the file, so bail out */ @@ -304,7 +305,7 @@ ProcessConfigFileInternal(GucContext context, bool applySettings, int elevel) if (DataDir) { if (!ParseConfigFile(PG_AUTOCONF_FILENAME, false, - NULL, 0, 0, elevel, + NULL, 0, CONF_FILE_START_DEPTH, elevel, &head, &tail)) { /* Syntax error(s) detected in the file, so bail out */ @@ -4582,7 +4583,8 @@ AlterSystemSetConfigFile(AlterSystemStmt *altersysstmt) AutoConfFileName))); /* parse it */ - if (!ParseConfigFp(infile, AutoConfFileName, 0, LOG, &head, &tail)) + if (!ParseConfigFp(infile, AutoConfFileName, CONF_FILE_START_DEPTH, + LOG, &head, &tail)) ereport(ERROR, (errcode(ERRCODE_CONFIG_FILE_ERROR), errmsg("could not parse contents of file \"%s\"", diff --git a/src/include/utils/conffiles.h b/src/include/utils/conffiles.h index 3f23a2a011..294a88c854 100644 --- a/src/include/utils/conffiles.h +++ b/src/include/utils/conffiles.h @@ -13,6 +13,9 @@ #ifndef CONFFILES_H #define CONFFILES_H +#define CONF_FILE_START_DEPTH 0 +#define CONF_FILE_MAX_DEPTH 10 + extern char *AbsoluteConfigLocation(const char *location, const char *calling_file); extern char **GetConfFilesInDir(const char *includedir, -- 2.37.0
>From 5f7c7cc9a77cced1851a65758a7befde80d39f3e Mon Sep 17 00:00:00 2001 From: Julien Rouhaud <julien.rouh...@free.fr> Date: Thu, 24 Nov 2022 14:20:22 +0800 Subject: [PATCH v22 2/2] Add regression tests for file inclusion in HBA end ident configuration files Author: Julien Rouhaud Reviewed-by: FIXME Discussion: https://postgr.es/m/20220223045959.35ipdsvbxcstrhya@jrouhaud --- src/test/authentication/meson.build | 1 + .../authentication/t/004_file_inclusion.pl | 722 ++++++++++++++++++ 2 files changed, 723 insertions(+) create mode 100644 src/test/authentication/t/004_file_inclusion.pl diff --git a/src/test/authentication/meson.build b/src/test/authentication/meson.build index c2b48c43c9..cfc23fa213 100644 --- a/src/test/authentication/meson.build +++ b/src/test/authentication/meson.build @@ -7,6 +7,7 @@ tests += { 't/001_password.pl', 't/002_saslprep.pl', 't/003_peer.pl', + 't/004_file_inclusion.pl', ], }, } diff --git a/src/test/authentication/t/004_file_inclusion.pl b/src/test/authentication/t/004_file_inclusion.pl new file mode 100644 index 0000000000..1be807c0a2 --- /dev/null +++ b/src/test/authentication/t/004_file_inclusion.pl @@ -0,0 +1,722 @@ + +# 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 / +$data_dir =~ s/\\/\//g; # change \ to / + + +# 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 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 file "$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 file \"$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 file \"$data_dir/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 file \"$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 file \"$data_dir/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 file \"$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 file \"$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 map_number, file_name, line_number, error FROM pg_ident_file_mappings' + . ' WHERE error IS NOT NULL ORDER BY map_number'), + qq($ident_expected), + 'Detected all error in ident file'); + +done_testing(); -- 2.37.0