From 050caf9657ceca7251d98476f325fc2d67f6787b Mon Sep 17 00:00:00 2001 From: Julien Rouhaud Date: Mon, 21 Feb 2022 17:38:34 +0800 Subject: [PATCH v3 2/4] Add a pg_ident_file_mappings view. This view is similar to pg_hba_file_rules view, and can be also helpful to help diagnosing configuration problems. A following commit will add the possibility to include files in pg_hba and pg_ident configuration files, which will then make this view even more useful. Author: Julien Rouhaud Reviewed-by: FIXME Discussion: https://postgr.es/m/20220223045959.35ipdsvbxcstrhya%40jrouhaud --- doc/src/sgml/catalogs.sgml | 108 ++++++++++++++++++++ doc/src/sgml/client-auth.sgml | 10 ++ doc/src/sgml/func.sgml | 5 +- src/backend/catalog/system_views.sql | 6 ++ src/backend/libpq/hba.c | 31 +++--- src/backend/utils/adt/hbafuncs.c | 136 +++++++++++++++++++++++++ src/include/catalog/pg_proc.dat | 7 ++ src/include/libpq/hba.h | 1 + src/test/regress/expected/rules.out | 6 ++ src/test/regress/expected/sysviews.out | 6 ++ src/test/regress/sql/sysviews.sql | 2 + 11 files changed, 302 insertions(+), 16 deletions(-) diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml index 7777d60514..e87f0543d6 100644 --- a/doc/src/sgml/catalogs.sgml +++ b/doc/src/sgml/catalogs.sgml @@ -9540,6 +9540,11 @@ SCRAM-SHA-256$<iteration count>:&l summary of client authentication configuration file contents + + pg_ident_file_mappings + summary of client user name mapping configuration file contents + + pg_indexes indexes @@ -10533,6 +10538,109 @@ SCRAM-SHA-256$<iteration count>:&l + + <structname>pg_ident_file_mappings</structname> + + + pg_ident_file_mappings + + + + The view pg_ident_file_mappings provides a summary + of the contents of the client user name mapping configuration file, + pg_ident.conf. + A row appears in this view for each + non-empty, non-comment line in the file, with annotations indicating + whether the rule could be applied successfully. + + + + This view can be helpful for checking whether planned changes in the + authentication configuration file will work, or for diagnosing a previous + failure. Note that this view reports on the current + contents of the file, not on what was last loaded by the server. + + + + By default, the pg_ident_file_mappings view can be + read only by superusers. + + + + <structname>pg_ident_file_mappings</structname> Columns + + + + Column Type + + + Description + + + + + + + + line_number int4 + + + Line number of this rule in pg_ident.conf + + + + + + map_name text + + + Name of the map + + + + + + sys_name text + + + Detected user name of the client + + + + + + pg_username text + + + Requested PostgreSQL user name + + + + + + error text + + + If not null, an error message indicating why this line could not be + processed + + + + +
+ + + Usually, a row reflecting an incorrect entry will have values for only + the line_number and error fields. + + + + See for more information about + client authentication configuration. + +
+ <structname>pg_indexes</structname> diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml index 02f0489112..142b0affcb 100644 --- a/doc/src/sgml/client-auth.sgml +++ b/doc/src/sgml/client-auth.sgml @@ -896,6 +896,16 @@ mymap /^(.*)@otherdomain\.com$ guest -HUP) to make it re-read the file. + + The system view + pg_ident_file_mappings + can be helpful for pre-testing changes to the + pg_ident.conf file, or for diagnosing problems if + loading of the file did not have the desired effects. Rows in the view with + non-null error fields indicate problems in the + corresponding lines of the file. + + A pg_ident.conf file that could be used in conjunction with the pg_hba.conf file in SIGHUP signal to the postmaster process, which in turn sends SIGHUP to each of its children.) You can use the - pg_file_settings and - pg_hba_file_rules views + pg_file_settings, + pg_hba_file_rules and + pg_ident_file_mappings views to check the configuration files for possible errors, before reloading.
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql index bb1ac30cd1..90011f2d68 100644 --- a/src/backend/catalog/system_views.sql +++ b/src/backend/catalog/system_views.sql @@ -607,6 +607,12 @@ CREATE VIEW pg_hba_file_rules AS REVOKE ALL ON pg_hba_file_rules FROM PUBLIC; REVOKE EXECUTE ON FUNCTION pg_hba_file_rules() FROM PUBLIC; +CREATE VIEW pg_ident_file_mappings AS + SELECT * FROM pg_ident_file_mappings() AS A; + +REVOKE ALL ON pg_ident_file_mappings FROM PUBLIC; +REVOKE EXECUTE ON FUNCTION pg_ident_file_mappings() FROM PUBLIC; + CREATE VIEW pg_timezone_abbrevs AS SELECT * FROM pg_timezone_abbrevs(); diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c index f9843a0b30..68136f6244 100644 --- a/src/backend/libpq/hba.c +++ b/src/backend/libpq/hba.c @@ -886,25 +886,22 @@ do { \ } while (0) /* - * Macros for handling pg_ident problems. - * Much as above, but currently the message level is hardwired as LOG - * and there is no provision for an err_msg string. + * Macros for handling pg_ident problems, similar as above. * * IDENT_FIELD_ABSENT: - * Log a message and exit the function if the given ident field ListCell is - * not populated. + * Reports when the given ident field ListCell is not populated. * * IDENT_MULTI_VALUE: - * Log a message and exit the function if the given ident token List has more - * than one element. + * Reports when the given ident token List has more than one element. */ #define IDENT_FIELD_ABSENT(field) \ do { \ if (!field) { \ - ereport(LOG, \ + ereport(elevel, \ (errcode(ERRCODE_CONFIG_FILE_ERROR), \ errmsg("missing entry in file \"%s\" at end of line %d", \ IdentFileName, line_num))); \ + *err_msg = psprintf("missing entry at end of line"); \ return NULL; \ } \ } while (0) @@ -912,11 +909,12 @@ do { \ #define IDENT_MULTI_VALUE(tokens) \ do { \ if (tokens->length > 1) { \ - ereport(LOG, \ + ereport(elevel, \ (errcode(ERRCODE_CONFIG_FILE_ERROR), \ errmsg("multiple values in ident field"), \ errcontext("line %d of configuration file \"%s\"", \ line_num, IdentFileName))); \ + *err_msg = psprintf("multiple values in ident field"); \ return NULL; \ } \ } while (0) @@ -2305,7 +2303,8 @@ load_hba(void) * Parse one tokenised line from the ident config file and store the result in * an IdentLine structure. * - * If parsing fails, log a message and return NULL. + * If parsing fails, log a message at ereport level elevel, store an error + * string in tok_line->err_msg and return NULL. * * If ident_user is a regular expression (ie. begins with a slash), it is * compiled and stored in IdentLine structure. @@ -2314,10 +2313,11 @@ load_hba(void) * to have set a memory context that will be reset if this function returns * NULL. */ -static IdentLine * -parse_ident_line(TokenizedAuthLine *tok_line) +IdentLine * +parse_ident_line(TokenizedAuthLine *tok_line, int elevel) { int line_num = tok_line->line_num; + char **err_msg = &tok_line->err_msg; ListCell *field; List *tokens; HbaToken *token; @@ -2371,11 +2371,14 @@ parse_ident_line(TokenizedAuthLine *tok_line) char errstr[100]; pg_regerror(r, &parsedline->re, errstr, sizeof(errstr)); - ereport(LOG, + ereport(elevel, (errcode(ERRCODE_INVALID_REGULAR_EXPRESSION), errmsg("invalid regular expression \"%s\": %s", parsedline->ident_user + 1, errstr))); + *err_msg = psprintf("invalid regular expression \"%s\": %s", + parsedline->ident_user + 1, errstr); + pfree(wstr); return NULL; } @@ -2626,7 +2629,7 @@ load_ident(void) continue; } - if ((newline = parse_ident_line(tok_line)) == NULL) + if ((newline = parse_ident_line(tok_line, LOG)) == NULL) { /* Parse error; remember there's trouble */ ok = false; diff --git a/src/backend/utils/adt/hbafuncs.c b/src/backend/utils/adt/hbafuncs.c index f230bad8b6..75e69383c2 100644 --- a/src/backend/utils/adt/hbafuncs.c +++ b/src/backend/utils/adt/hbafuncs.c @@ -28,6 +28,9 @@ static ArrayType *gethba_options(HbaLine *hba); static void fill_hba_line(Tuplestorestate *tuple_store, TupleDesc tupdesc, int lineno, HbaLine *hba, const char *err_msg); static void fill_hba_view(Tuplestorestate *tuple_store, TupleDesc tupdesc); +static void fill_ident_line(Tuplestorestate *tuple_store, TupleDesc tupdesc, + int lineno, IdentLine *ident, const char *err_msg); +static void fill_ident_view(Tuplestorestate *tuple_store, TupleDesc tupdesc); /* @@ -421,3 +424,136 @@ pg_hba_file_rules(PG_FUNCTION_ARGS) PG_RETURN_NULL(); } + +/* Number of columns in pg_hba_file_mappings view */ +#define NUM_PG_IDENT_FILE_MAPPINGS_ATTS 5 + +/* + * fill_ident_line: build one row of pg_ident_file_mappings view, add it to + * tuplestore + * + * tuple_store: where to store data + * tupdesc: tuple descriptor for the view + * lineno: pg_hba.conf line number (must always be valid) + * ident: parsed line data (can be NULL, in which case err_msg should be set) + * err_msg: error message (NULL if none) + * + * Note: leaks memory, but we don't care since this is run in a short-lived + * memory context. + */ +static void +fill_ident_line(Tuplestorestate *tuple_store, TupleDesc tupdesc, + int lineno, IdentLine *ident, const char *err_msg) +{ + Datum values[NUM_PG_IDENT_FILE_MAPPINGS_ATTS]; + bool nulls[NUM_PG_IDENT_FILE_MAPPINGS_ATTS]; + HeapTuple tuple; + int index; + + Assert(tupdesc->natts == NUM_PG_IDENT_FILE_MAPPINGS_ATTS); + + memset(values, 0, sizeof(values)); + memset(nulls, 0, sizeof(nulls)); + index = 0; + + /* line_number */ + values[index++] = Int32GetDatum(lineno); + + if (ident != NULL) + { + values[index++] = CStringGetTextDatum(ident->usermap); + values[index++] = CStringGetTextDatum(ident->ident_user); + values[index++] = CStringGetTextDatum(ident->pg_role); + } + else + { + /* no parsing result, so set relevant fields to nulls */ + memset(&nulls[1], true, (NUM_PG_IDENT_FILE_MAPPINGS_ATTS - 2) * sizeof(bool)); + } + + /* error */ + if (err_msg) + values[NUM_PG_IDENT_FILE_MAPPINGS_ATTS - 1] = CStringGetTextDatum(err_msg); + else + nulls[NUM_PG_IDENT_FILE_MAPPINGS_ATTS - 1] = true; + + tuple = heap_form_tuple(tupdesc, values, nulls); + tuplestore_puttuple(tuple_store, tuple); +} + +/* + * Read the pg_ident.conf file and fill the tuplestore with view records. + */ +static void +fill_ident_view(Tuplestorestate *tuple_store, TupleDesc tupdesc) +{ + FILE *file; + List *ident_lines = NIL; + ListCell *line; + MemoryContext linecxt; + MemoryContext identcxt; + MemoryContext oldcxt; + + /* + * In the unlikely event that we can't open pg_hba.conf, we throw an + * error, rather than trying to report it via some sort of view entry. + * (Most other error conditions should result in a message in a view + * entry.) + */ + file = AllocateFile(IdentFileName, "r"); + if (file == NULL) + ereport(ERROR, + (errcode_for_file_access(), + errmsg("could not open usermap file \"%s\": %m", + IdentFileName))); + + linecxt = tokenize_auth_file(HbaFileName, file, &ident_lines, DEBUG3); + FreeFile(file); + + /* Now parse all the lines */ + identcxt = AllocSetContextCreate(CurrentMemoryContext, + "ident parser context", + ALLOCSET_SMALL_SIZES); + oldcxt = MemoryContextSwitchTo(identcxt); + foreach(line, ident_lines) + { + TokenizedAuthLine *tok_line = (TokenizedAuthLine *) lfirst(line); + IdentLine *identline = NULL; + + /* don't parse lines that already have errors */ + if (tok_line->err_msg == NULL) + identline = parse_ident_line(tok_line, DEBUG3); + + fill_ident_line(tuple_store, tupdesc, tok_line->line_num, identline, + tok_line->err_msg); + } + + /* Free tokenizer memory */ + MemoryContextDelete(linecxt); + /* Free parse_hba_line memory */ + MemoryContextSwitchTo(oldcxt); + MemoryContextDelete(identcxt); +} + +/* + * SQL-accessible SRF to return all the entries in the pg_ident.conf file. + */ +Datum +pg_ident_file_mappings(PG_FUNCTION_ARGS) +{ + ReturnSetInfo *rsi; + + /* + * Build tuplestore to hold the result rows. We must use the Materialize + * mode to be safe against HBA file changes while the cursor is open. + * It's also more efficient than having to look up our current position in + * the parsed list every time. + */ + SetSingleFuncCall(fcinfo, 0); + + /* Fill the tuplestore */ + rsi = (ReturnSetInfo *) fcinfo->resultinfo; + fill_ident_view(rsi->setResult, rsi->setDesc); + + PG_RETURN_NULL(); +} diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat index d8e8715ed1..6ccbc9af4c 100644 --- a/src/include/catalog/pg_proc.dat +++ b/src/include/catalog/pg_proc.dat @@ -6115,6 +6115,13 @@ proargmodes => '{o,o,o,o,o,o,o,o,o}', proargnames => '{line_number,type,database,user_name,address,netmask,auth_method,options,error}', prosrc => 'pg_hba_file_rules' }, +{ oid => '9556', descr => 'show pg_ident.conf mappings', + proname => 'pg_ident_file_mappings', prorows => '1000', proretset => 't', + provolatile => 'v', prorettype => 'record', proargtypes => '', + proallargtypes => '{int4,text,text,text,text}', + proargmodes => '{o,o,o,o,o}', + proargnames => '{line_number,map_name,sys_name,pg_usernamee,error}', + prosrc => 'pg_ident_file_mappings' }, { oid => '1371', descr => 'view system lock information', proname => 'pg_lock_status', prorows => '1000', proretset => 't', provolatile => 'v', prorettype => 'record', proargtypes => '', diff --git a/src/include/libpq/hba.h b/src/include/libpq/hba.h index 19924dca67..fce7db248b 100644 --- a/src/include/libpq/hba.h +++ b/src/include/libpq/hba.h @@ -169,6 +169,7 @@ extern int check_usermap(const char *usermap_name, const char *pg_role, const char *auth_user, bool case_sensitive); extern HbaLine *parse_hba_line(TokenizedAuthLine *tok_line, int elevel); +extern IdentLine *parse_ident_line(TokenizedAuthLine *tok_line, int elevel); extern bool pg_isblank(const char c); extern MemoryContext tokenize_auth_file(const char *filename, FILE *file, List **tok_lines, int elevel); diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out index ac468568a1..76a209b717 100644 --- a/src/test/regress/expected/rules.out +++ b/src/test/regress/expected/rules.out @@ -1347,6 +1347,12 @@ pg_hba_file_rules| SELECT a.line_number, a.options, a.error FROM pg_hba_file_rules() a(line_number, type, database, user_name, address, netmask, auth_method, options, error); +pg_ident_file_mappings| SELECT a.line_number, + a.map_name, + a.sys_name, + a.pg_usernamee, + a.error + FROM pg_ident_file_mappings() a(line_number, map_name, sys_name, pg_usernamee, error); pg_indexes| SELECT n.nspname AS schemaname, c.relname AS tablename, i.relname AS indexname, diff --git a/src/test/regress/expected/sysviews.out b/src/test/regress/expected/sysviews.out index 442eeb1e3f..31ba549883 100644 --- a/src/test/regress/expected/sysviews.out +++ b/src/test/regress/expected/sysviews.out @@ -55,6 +55,12 @@ select count(*) > 0 as ok from pg_hba_file_rules; t (1 row) +select count(*) >= 0 as ok from pg_ident_file_mappings; + ok +---- + t +(1 row) + -- There will surely be at least one active lock select count(*) > 0 as ok from pg_locks; ok diff --git a/src/test/regress/sql/sysviews.sql b/src/test/regress/sql/sysviews.sql index 4980f07be2..1148014e47 100644 --- a/src/test/regress/sql/sysviews.sql +++ b/src/test/regress/sql/sysviews.sql @@ -28,6 +28,8 @@ select count(*) >= 0 as ok from pg_file_settings; -- There will surely be at least one rule select count(*) > 0 as ok from pg_hba_file_rules; +select count(*) >= 0 as ok from pg_ident_file_mappings; + -- There will surely be at least one active lock select count(*) > 0 as ok from pg_locks; -- 2.33.1