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 <[email protected]>
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 <[email protected]>
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
