From 1173e0badf1572816b30a251106a6d4feb8c070c Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Wed, 23 Nov 2022 14:47:17 +0900 Subject: [PATCH v21 2/2] 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. 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 --- src/include/catalog/pg_proc.dat | 12 +- src/backend/libpq/hba.c | 182 ++++- src/backend/libpq/pg_hba.conf.sample | 27 + src/backend/libpq/pg_ident.conf.sample | 34 + src/backend/utils/adt/hbafuncs.c | 39 +- src/test/authentication/meson.build | 1 + .../authentication/t/004_file_inclusion.pl | 721 ++++++++++++++++++ src/test/regress/expected/rules.out | 6 +- doc/src/sgml/client-auth.sgml | 85 ++- doc/src/sgml/system-views.sgml | 22 +- 10 files changed, 1075 insertions(+), 54 deletions(-) create mode 100644 src/test/authentication/t/004_file_inclusion.pl diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat index f15aa2dbb1..f9301b2627 100644 --- a/src/include/catalog/pg_proc.dat +++ b/src/include/catalog/pg_proc.dat @@ -6161,16 +6161,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 => '{map_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 => '{map_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/backend/libpq/hba.c b/src/backend/libpq/hba.c index 9064ad6714..3f94359c4f 100644 --- a/src/backend/libpq/hba.c +++ b/src/backend/libpq/hba.c @@ -463,6 +463,64 @@ next_field_expand(const char *filename, char **lineptr, return tokens; } +/* + * tokenize_inclusion_file + * Include a file from another file into an hba "field". + * + * Opens and tokenises a file included from another HBA config file with + * one of the inclusion records ("include", "include_if_exists" or + * "include_dir"), and assign all values found to an existing list of + * list of AuthTokens. + * + * All new tokens are allocated in the memory context dedicated to the + * tokenization. + * + * If missing_ok is true, ignore the missing files. + * + * In event of an error, log a message at ereport level elevel, and also + * set *err_msg to a string describing the error. Note that the result + * may be non-NIL anyway, so *err_msg must be tested to determine whether + * there was an error. + */ +static void +tokenize_inclusion_file(const char *outer_filename, + const char *inc_filename, + List **tok_lines, + int elevel, + int depth, + bool missing_ok, + char **err_msg) +{ + char *inc_fullname; + FILE *inc_file; + + inc_fullname = AbsoluteConfigLocation(inc_filename, outer_filename); + inc_file = open_auth_file(inc_fullname, elevel, depth, err_msg); + + if (!inc_file) + { + if (errno == ENOENT && missing_ok) + { + ereport(elevel, + (errmsg("skipping missing authentication file \"%s\"", + inc_fullname))); + *err_msg = NULL; + pfree(inc_fullname); + return; + } + + /* error in err_msg, so leave and report */ + pfree(inc_fullname); + Assert(err_msg); + return; + } + + tokenize_auth_file(inc_fullname, inc_file, tok_lines, elevel, + depth); + free_auth_file(inc_file, depth); + pfree(inc_fullname); +} + /* * tokenize_expand_file * Expand a file included from another file into an hba "field" @@ -471,7 +529,8 @@ next_field_expand(const char *filename, char **lineptr, * and returns all values found therein as a flat list of AuthTokens. If a * @-token is found, recursively expand it. The newly read tokens are * appended to "tokens" (so that foo,bar,@baz does what you expect). - * All new tokens are allocated in caller's memory context. + * All new tokens are allocated in the memory context dedicated to the + * tokenization. * * In event of an error, log a message at ereport level elevel, and also * set *err_msg to a string describing the error. Note that the result @@ -488,7 +547,7 @@ tokenize_expand_file(List *tokens, { char *inc_fullname; FILE *inc_file; - List *inc_lines; + List *inc_lines = NIL; ListCell *inc_line; inc_fullname = AbsoluteConfigLocation(inc_filename, outer_filename); @@ -705,6 +764,7 @@ tokenize_auth_file(const char *filename, FILE *file, List **tok_lines, while (!feof(file) && !ferror(file)) { + TokenizedAuthLine *tok_line; char *lineptr; List *current_line = NIL; char *err_msg = NULL; @@ -773,23 +833,115 @@ 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) - { - TokenizedAuthLine *tok_line; + if (current_line == NIL && err_msg == NULL) + goto next_line; - oldcxt = MemoryContextSwitchTo(tokenize_context); - 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 ? pstrdup(err_msg) : NULL; - *tok_lines = lappend(*tok_lines, tok_line); - MemoryContextSwitchTo(oldcxt); + /* If the line is valid, check if that's an include directive */ + if (err_msg == NULL && list_length(current_line) == 2) + { + AuthToken *first, + *second; + + first = linitial(linitial_node(List, current_line)); + second = linitial(lsecond_node(List, current_line)); + + if (strcmp(first->string, "include") == 0) + { + tokenize_inclusion_file(filename, second->string, tok_lines, + elevel, depth + 1, false, &err_msg); + + if (err_msg) + goto process_line; + + /* + * tokenize_auth_file() has taken care of creating the + * TokenizedAuthLines. + */ + 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 = GetConfFilesInDir(dir_name, filename, elevel, + &num_filenames, &err_msg); + + if (!filenames) + { + /* the error is in err_msg, so create an entry */ + goto process_line; + } + + initStringInfo(&err_buf); + for (int i = 0; i < num_filenames; i++) + { + tokenize_inclusion_file(filename, filenames[i], tok_lines, + elevel, depth + 1, false, &err_msg); + /* cumulate errors if any */ + if (err_msg) + { + if (err_buf.len > 0) + appendStringInfoChar(&err_buf, '\n'); + appendStringInfoString(&err_buf, err_msg); + } + } + + /* clean up things */ + for (int i = 0; i < num_filenames; i++) + pfree(filenames[i]); + pfree(filenames); + + /* + * 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; + goto process_line; + } + else if (strcmp(first->string, "include_if_exists") == 0) + { + + tokenize_inclusion_file(filename, second->string, tok_lines, + elevel, depth + 1, true, &err_msg); + if (err_msg) + goto process_line; + + /* + * tokenize_auth_file() has taken care of creating the + * TokenizedAuthLines. + */ + goto next_line; + } } +process_line: + + /* + * General processing: report the error if any and emit line to the + * TokenizedAuthLine. This is saved in the memory context dedicated + * to this list. + */ + oldcxt = MemoryContextSwitchTo(tokenize_context); + tok_line = (TokenizedAuthLine *) palloc0(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 ? pstrdup(err_msg) : NULL; + *tok_lines = lappend(*tok_lines, tok_line); + MemoryContextSwitchTo(oldcxt); + +next_line: line_number += continuations + 1; callback_arg.linenum = line_number; } diff --git a/src/backend/libpq/pg_hba.conf.sample b/src/backend/libpq/pg_hba.conf.sample index 5f3f63eb0c..f72f471d23 100644 --- a/src/backend/libpq/pg_hba.conf.sample +++ b/src/backend/libpq/pg_hba.conf.sample @@ -5,6 +5,10 @@ # documentation for a complete description of this file. A short # synopsis follows. # +# ------------------------------- +# Authentication records +# ------------------------------- +# # This file controls: which hosts are allowed to connect, how clients # are authenticated, which PostgreSQL user names they can use, which # databases they can access. Records take one of these forms: @@ -64,11 +68,34 @@ # its special character, and just match a database or username with # that name. # +# -------------------------------- +# Inclusion records +# --------------------------------- +# +# This files allow the inclusion of external files or directories holding +# extra authentication records, using the following keywords: +# +# include FILE +# include_if_exists FILE +# include_dir DIRECTORY +# +# FILE is the file name to include, and DIR is the directory name containing +# the file(s) to include. Any file in a directory will be loaded if suffixed +# with ".conf". The files of a directory are ordered by name. +# include_if_exists ignored missing files. FILE and DIRECTORY can be +# specified as a relative or absolute path, and can be double-quoted if they +# contain spaces. +# +# ------------------------------- +# Miscellaneous +# ------------------------------- +# # This file is read on server startup and when the server receives a # SIGHUP signal. If you edit the file on a running system, you have to # SIGHUP the server for the changes to take effect, run "pg_ctl reload", # or execute "SELECT pg_reload_conf()". # +# --------------------------------- # Put your actual configuration here # ---------------------------------- # diff --git a/src/backend/libpq/pg_ident.conf.sample b/src/backend/libpq/pg_ident.conf.sample index a5870e6448..59092bacf2 100644 --- a/src/backend/libpq/pg_ident.conf.sample +++ b/src/backend/libpq/pg_ident.conf.sample @@ -1,6 +1,10 @@ # PostgreSQL User Name Maps # ========================= # +# ------------------------------ +# Ident Records +# ------------------------------ +# # Refer to the PostgreSQL documentation, chapter "Client # Authentication" for a complete description. A short synopsis # follows. @@ -13,6 +17,14 @@ # # (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 @@ -31,6 +43,28 @@ # system user names and PostgreSQL user names are the same, you don't # need anything in this file. # +# ------------------------------ +# Inclusion records +# ------------------------------ +# +# This files allow the inclusion of external files or directories holding +# extra records, using the following keywords: +# +# include FILE +# include_if_exists FILE +# include_dir DIRECTORY +# +# FILE is the file name to include, and DIR is the directory name containing +# the file(s) to include. Any file in a directory will be loaded if suffixed +# with ".conf". The files of a directory are ordered by name. +# include_if_exists ignored missing files. FILE and DIRECTORY can be +# specified as a relative or absolute path, and can be double-quoted if they +# contain spaces. +# +# ------------------------------- +# Miscellaneous +# ------------------------------- +# # This file is read on server startup and when the postmaster receives # a SIGHUP signal. If you edit the file on a running system, you have # to SIGHUP the postmaster for the changes to take effect. You can diff --git a/src/backend/utils/adt/hbafuncs.c b/src/backend/utils/adt/hbafuncs.c index 87996da487..633eda30d3 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, 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 map_number, int lineno, IdentLine *ident, - const char *err_msg); + int map_number, 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 identifier among all valid rules - * lineno: pg_hba.conf line number (must always be valid) + * filename: configuration file name (must always be valid) + * lineno: line number of configuration 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, char *filename, int lineno, HbaLine *hba, const char *err_msg) { Datum values[NUM_PG_HBA_FILE_RULES_ATTS]; @@ -203,6 +204,9 @@ fill_hba_line(Tuplestorestate *tuple_store, TupleDesc tupdesc, else values[index++] = Int32GetDatum(rule_number); + /* file_name */ + values[index++] = CStringGetTextDatum(filename); + /* line_number */ values[index++] = Int32GetDatum(lineno); @@ -346,7 +350,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 */ @@ -402,7 +406,8 @@ fill_hba_view(Tuplestorestate *tuple_store, TupleDesc tupdesc) rule_number++; fill_hba_line(tuple_store, tupdesc, rule_number, - tok_line->line_num, hbaline, tok_line->err_msg); + tok_line->file_name, tok_line->line_num, hbaline, + tok_line->err_msg); } /* Free tokenizer memory */ @@ -439,7 +444,7 @@ pg_hba_file_rules(PG_FUNCTION_ARGS) } /* Number of columns in pg_ident_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 @@ -448,7 +453,8 @@ pg_hba_file_rules(PG_FUNCTION_ARGS) * tuple_store: where to store data * tupdesc: tuple descriptor for the view * map_number: unique identifier among all valid maps - * lineno: pg_ident.conf line number (must always be valid) + * filename: configuration file name (must always be valid) + * lineno: line number of configuration 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) * @@ -457,7 +463,7 @@ pg_hba_file_rules(PG_FUNCTION_ARGS) */ static void fill_ident_line(Tuplestorestate *tuple_store, TupleDesc tupdesc, - int map_number, int lineno, IdentLine *ident, + int map_number, char *filename, int lineno, IdentLine *ident, const char *err_msg) { Datum values[NUM_PG_IDENT_FILE_MAPPINGS_ATTS]; @@ -477,6 +483,9 @@ fill_ident_line(Tuplestorestate *tuple_store, TupleDesc tupdesc, else values[index++] = Int32GetDatum(map_number); + /* file_name */ + values[index++] = CStringGetTextDatum(filename); + /* line_number */ values[index++] = Int32GetDatum(lineno); @@ -489,7 +498,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 */ @@ -544,8 +553,8 @@ fill_ident_view(Tuplestorestate *tuple_store, TupleDesc tupdesc) map_number++; fill_ident_line(tuple_store, tupdesc, map_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/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..dbcdb3320f --- /dev/null +++ b/src/test/authentication/t/004_file_inclusion.pl @@ -0,0 +1,721 @@ + +# 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 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(); diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out index 7c7adbc004..37c1c86473 100644 --- a/src/test/regress/expected/rules.out +++ b/src/test/regress/expected/rules.out @@ -1339,6 +1339,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, @@ -1348,14 +1349,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.map_number, + a.file_name, a.line_number, a.map_name, a.sys_name, a.pg_username, a.error - FROM pg_ident_file_mappings() a(map_number, line_number, map_name, sys_name, pg_username, error); + FROM pg_ident_file_mappings() a(map_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, diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml index 32d5d45863..e5815a5390 100644 --- a/doc/src/sgml/client-auth.sgml +++ b/doc/src/sgml/client-auth.sgml @@ -89,8 +89,8 @@ - Each record specifies a connection type, a client IP address range - (if relevant for the connection type), a database name, a user name, + 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 @@ -100,20 +100,38 @@ access is denied. + + Each record can be an inclusion directive or an authentication record. + Inclusion directives specify files that can be included, that contain + additional records. The records will be inserted in place of the + inclusion records. Those records only contains two fields: + include, include_if_exists or + include_dir 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. For the include_dir form, all files + not starting with a . and ending with + .conf 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). + + A record can have several formats: -local database user auth-method auth-options -host database user address auth-method auth-options -hostssl database user address auth-method auth-options -hostnossl database user address auth-method auth-options -hostgssenc database user address auth-method auth-options -hostnogssenc database user address auth-method auth-options -host database user IP-address IP-mask auth-method auth-options -hostssl database user IP-address IP-mask auth-method auth-options -hostnossl database user IP-address IP-mask auth-method auth-options -hostgssenc database user IP-address IP-mask auth-method auth-options -hostnogssenc database user IP-address IP-mask auth-method auth-options +local database user auth-method auth-options +host database user address auth-method auth-options +hostssl database user address auth-method auth-options +hostnossl database user address auth-method auth-options +hostgssenc database user address auth-method auth-options +hostnogssenc database user address auth-method auth-options +host database user IP-address IP-mask auth-method auth-options +hostssl database user IP-address IP-mask auth-method auth-options +hostnossl database user IP-address IP-mask auth-method auth-options +hostgssenc database user IP-address IP-mask auth-method auth-options +hostnogssenc database user IP-address IP-mask auth-method auth-options +include file +include_if_exists file +include_dir directory The meaning of the fields is as follows: @@ -655,6 +673,39 @@ openssl x509 -in myclient.crt -noout --subject -nameopt RFC2253 | sed "s/^subjec + + + include + + + This line will be replaced by the contents of the given file. + + + + + + include_if_exists + + + 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. + + + + + + include_dir + + + This line will be replaced with the content of all the files found in + the directory, if they don't start with a . and end + with .conf, processed in file name order (according + to C locale rules, i.e., numbers before letters, and uppercase letters + before lowercase ones). + + + @@ -863,9 +914,11 @@ local db1,db2,@demodbs all md5 cluster's data directory. (It is possible to place the map file elsewhere, however; see the configuration parameter.) - The ident map file contains lines of the general form: + The ident map file contains lines of two general form: map-name system-username database-username +include file +include_dir directory Comments, whitespace and line continuations are handled in the same way as in pg_hba.conf. The @@ -875,6 +928,10 @@ local db1,db2,@demodbs all md5 database user name. The same map-name can be used repeatedly to specify multiple user-mappings within a single map. + + As for pg_hba.conf, the lines in this file can + be inclusion directives, following the same rules. + 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 7c716fe327..d38b42c5cd 100644 --- a/doc/src/sgml/system-views.sgml +++ b/doc/src/sgml/system-views.sgml @@ -1002,12 +1002,21 @@ + + + file_name text + + + Name of the file containing this rule + + + line_number int4 - Line number of this rule in pg_hba.conf + Line number of this rule in file_name @@ -1152,12 +1161,21 @@ + + + file_name text + + + Name of the file containing this map + + + line_number int4 - Line number of this map in pg_ident.conf + Line number of this map in file_name -- 2.38.1