8000 Invent open_auth_file() in hba.c to refactor authentication file opening · postgres/postgres@783e8c6 · GitHub
[go: up one dir, main page]
Skip to content

Commit 783e8c6

Browse files
michaelpqmichaelpq
committed
Invent open_auth_file() in hba.c to refactor authentication file opening
This adds a check on the recursion depth when including authentication configuration files, something that has never been done when processing '@' files for database and user name lists in pg_hba.conf. On HEAD, this was leading to a rather confusing error, as of: FATAL: exceeded maxAllocatedDescs (NN) while trying to open file "/path/blah.conf" This refactors the code so as the error reported is now the following, which is the same as for GUCs: FATAL: could not open file "/path/blah.conf": maximum nesting depth exceeded This reduces a bit the verbosity of the error message used for files included in user and database lists, reporting only the file name of what's failing to load, without mentioning the relative or absolute path specified after '@' in a HBA file. The absolute path is built upon what '@' defines anyway, so there is no actual loss of information. This makes the future inclusion logic much simpler. A follow-up patch will add an error context to be able to track on which line of which file the inclusion is failing, to close the loop, providing all the information needed to know the full chain of events. This logic has been extracted from a larger patch written by Julien, rewritten by me to have a unique code path calling AllocateFile() on authentication files, and is useful on its own. This new interface will be used later for authentication files included with @include[_dir,_if_exists], in a follow-up patch. Author: Michael Paquier, Julien Rouhaud Discussion: https://www.postgresql.org/message-id/Y2xUBJ+S+Z0zbxRW@paquier.xyz
1 parent 45d5eca commit 783e8c6

File tree

3 files changed

+81
-45
lines changed
  • src
    • backend
  • include/libpq

    • 3 files changed

    +81
    -45
    lines changed

    ‎src/backend/libpq/hba.c

    Lines changed: 72 additions & 28 deletions
    Original file line numberDiff line numberDiff line change
    @@ -117,7 +117,8 @@ static const char *const UserAuthName[] =
    117117

    118118

    119119
    static List *tokenize_inc_file(List *tokens, const char *outer_filename,
    120-
    const char *inc_filename, int elevel, char **err_msg);
    120+
    const char *inc_filename, int elevel,
    121+
    int depth, char **err_msg);
    121122
    static bool parse_hba_auth_opt(char *name, char *val, HbaLine *hbaline,
    122123
    int elevel, char **err_msg);
    123124
    static int regcomp_auth_token(AuthToken *token, char *filename, int line_num,
    @@ -414,7 +415,7 @@ regexec_auth_token(const char *match, AuthToken *token, size_t nmatch,
    414415
    */
    415416
    static List *
    416417
    next_field_expand(const char *filename, char **lineptr,
    417-
    int elevel, char **err_msg)
    418+
    int elevel, int depth, char **err_msg)
    418419
    {
    419420
    char buf[MAX_TOKEN];
    420421
    bool trailing_comma;
    @@ -431,7 +432,7 @@ next_field_expand(const char *filename, char **lineptr,
    431432
    /* Is this referencing a file? */
    432433
    if (!initial_quote && buf[0] == '@' && buf[1] != '\0')
    433434
    tokens = tokenize_inc_file(tokens, filename, buf + 1,
    434-
    elevel, err_msg);
    435+
    elevel, depth + 1, err_msg);
    435436
    else
    436437
    tokens = lappend(tokens, make_auth_token(buf, initial_quote));
    437438
    } while (trailing_comma && (*err_msg == NULL));
    @@ -459,6 +460,7 @@ tokenize_inc_file(List *tokens,
    459460
    const char *outer_filename,
    460461
    const char *inc_filename,
    461462
    int elevel,
    463+
    int depth,
    462464
    char **err_msg)
    463465
    {
    464466
    char *inc_fullname;
    @@ -468,24 +470,18 @@ tokenize_inc_file(List *tokens,
    468470
    MemoryContext linecxt;
    469471

    470472
    inc_fullname = AbsoluteConfigLocation(inc_filename, outer_filename);
    473+
    inc_file = open_auth_file(inc_fullname, elevel, depth, err_msg);
    471474

    472-
    inc_file = AllocateFile(inc_fullname, "r");
    473475
    if (inc_file == NULL)
    474476
    {
    475-
    int save_errno = errno;
    476-
    477-
    ereport(elevel,
    478-
    (errcode_for_file_access(),
    479-
    errmsg("could not open secondary authentication file \"@%s\" as \"%s\": %m",
    480-
    inc_filename, inc_fullname)));
    481-
    *err_msg = psprintf("could not open secondary authentication file \"@%s\" as \"%s\": %s",
    482-
    inc_filename, inc_fullname, strerror(save_errno));
    477+
    /* error already logged */
    483478
    pfree(inc_fullname);
    484479
    return tokens;
    485480
    }
    486481

    487482
    /* There is possible recursion here if the file contains @ */
    488-
    linecxt = tokenize_auth_file(inc_fullname, inc_file, &inc_lines, elevel);
    483+
    linecxt = tokenize_auth_file(inc_fullname, inc_file, &inc_lines, elevel,
    484+
    depth);
    489485

    490486
    FreeFile(inc_file);
    491487
    pfree(inc_fullname);
    @@ -521,6 +517,59 @@ tokenize_inc_file(List *tokens,
    521517
    return tokens;
    522518
    }
    523519

    520+
    /*
    521+
    * open_auth_file
    522+
    * Open the given file.
    523+
    *
    524+
    * filename: the absolute path to the target file
    525+
    * elevel: message logging level
    526+
    * depth: recursion level when opening the file
    527+
    * err_msg: details about the error
    528+
    *
    529+
    * Return value is the opened file. On error, returns NULL with details
    530+
    * about the error stored in "err_msg".
    531+
    */
    532+
    FILE *
    533+
    open_auth_file(const char *filename, int elevel, int depth,
    534+
    char **err_msg)
    535+
    {
    536+
    FILE *file;
    537+
    538+
    /*
    539+
    * Reject too-deep include nesting depth. This is just a safety check to
    540+
    * avoid dumping core due to stack overflow if an include file loops back
    541+
    * to itself. The maximum nesting depth is pretty arbitrary.
    542+
    */
    543+
    if (depth > 10)
    544+
    {
    545+
    ereport(elevel,
    546+
    (errcode_for_file_access(),
    547+
    errmsg("could not open file \"%s\": maximum nesting depth exceeded",
    548+
    filename)));
    549+
    if (err_msg)
    550+
    *err_msg = psprintf("could not open file \"%s\": maximum nesting depth exceeded",
    551+
    filename);
    552+
    return NULL;
    553+
    }
    554+
    555+
    file = AllocateFile(filename, "r");
    556+
    if (file == NULL)
    557+
    {
    558+
    int save_errno = errno;
    559+
    560+
    ereport(elevel,
    561+
    (errcode_for_file_access(),
    562+
    errmsg("could not open file \"%s\": %m",
    563+
    filename)));
    564+
    if (err_msg)
    565+
    *err_msg = psprintf("could not open file \"%s\": %s",
    566+
    filename, strerror(save_errno));
    567+
    return NULL;
    568+
    }
    569+
    570+
    return file;
    571+
    }
    572+
    524573
    /*
    525574
    * tokenize_auth_file
    526575
    * Tokenize the given file.
    @@ -532,6 +581,7 @@ tokenize_inc_file(List *tokens,
    532581
    * file: the already-opened target file
    533582
    * tok_lines: receives output list
    534583
    * elevel: message logging level
    584+
    * depth: level of recursion when tokenizing the target file
    535585
    *
    536586
    * Errors are reported by logging messages at ereport level elevel and by
    537587
    * adding TokenizedAuthLine structs containing non-null err_msg fields to the
    @@ -542,7 +592,7 @@ tokenize_inc_file(List *tokens,
    542592
    */
    543593
    MemoryContext
    544594
    tokenize_auth_file(const char *filename, FILE *file, List **tok_lines,
    545-
    int elevel)
    595+
    int elevel, int depth)
    546596
    {
    547597
    int line_number = 1;
    548598
    StringInfoData buf;
    @@ -613,7 +663,7 @@ tokenize_auth_file(const char *filename, FILE *file, List **tok_lines,
    613663
    List *current_field;
    614664

    615665
    current_field = next_field_expand(filename, &lineptr,
    616-
    elevel, &err_msg);
    666+
    elevel, depth, &err_msg);
    617667
    /* add field to line, unless we are at EOL or comment start */
    618668
    if (current_field != NIL)
    619669
    current_line = lappend(current_line, current_field);
    @@ -2332,17 +2382,14 @@ load_hba(void)
    23322382
    MemoryContext oldcxt;
    23332383
    MemoryContext hbacxt;
    23342384

    2335-
    file = AllocateFile(HbaFileName, "r");
    2385+
    file = open_auth_file(HbaFileName, LOG, 0, NULL);
    23362386
    if (file == NULL)
    23372387
    {
    2338-
    ereport(LOG,
    2339-
    (errcode_for_file_access(),
    2340-
    errmsg("could not open configuration file \"%s\": %m",
    2341-
    HbaFileName)));
    2388+
    /* error already logged */
    23422389
    return false;
    23432390
    }
    23442391

    2345-
    linecxt = tokenize_auth_file(HbaFileName, file, &hba_lines, LOG);
    2392+
    linecxt = tokenize_auth_file(HbaFileName, file, &hba_lines, LOG, 0);
    23462393
    FreeFile(file);
    23472394

    23482395
    /* Now parse all the lines */
    @@ -2703,18 +2750,15 @@ load_ident(void)
    27032750
    MemoryContext ident_context;
    27042751
    IdentLine *newline;
    27052752

    2706-
    file = AllocateFile(IdentFileName, "r");
    2753+
    /* not FATAL ... we just won't do any special ident maps */
    2754+
    file = open_auth_file(IdentFileName, LOG, 0, NULL);
    27072755
    if (file == NULL)
    27082756
    {
    2709-
    /* not fatal ... we just won't do any special ident maps */
    2710-
    ereport(LOG,
    2711-
    (errcode_for_file_access(),
    2712-
    errmsg("could not open usermap file \"%s\": %m",
    2713-
    IdentFileName)));
    2757+
    /* error already logged */
    27142758
    return false;
    27152759
    }
    27162760

    2717-
    linecxt = tokenize_auth_file(IdentFileName, file, &ident_lines, LOG);
    2761+
    linecxt = tokenize_auth_file(IdentFileName, file, &ident_lines, LOG, 0);
    27182762
    FreeFile(file);
    27192763

    27202764
    /* Now parse all the lines */

    ‎src/backend/utils/adt/hbafuncs.c

    Lines changed: 6 additions & 16 deletions
    Original file line numberDiff line numberDiff line change
    @@ -380,14 +380,9 @@ fill_hba_view(Tuplestorestate *tuple_store, TupleDesc tupdesc)
    380380
    * (Most other error conditions should result in a message in a view
    381381
    * entry.)
    382382
    */
    383-
    file = AllocateFile(HbaFileName, "r");
    384-
    if (file == NULL)
    385-
    ereport(ERROR,
    386-
    (errcode_for_file_access(),
    387-
    errmsg("could not open configuration file \"%s\": %m",
    388-
    HbaFileName)));
    389-
    390-
    linecxt = tokenize_auth_file(HbaFileName, file, &hba_lines, DEBUG3);
    383+
    file = open_auth_file(HbaFileName, ERROR, 0, NULL);
    384+
    385+
    linecxt = tokenize_auth_file(HbaFileName, file, &hba_lines, DEBUG3, 0);
    391386
    FreeFile(file);
    392387

    393388
    /* Now parse all the lines */
    @@ -529,14 +524,9 @@ fill_ident_view(Tuplestorestate *tuple_store, TupleDesc tupdesc)
    529524
    * (Most other error conditions should result in a message in a view
    530525
    * entry.)
    531526
    */
    532-
    file = AllocateFile(IdentFileName, "r");
    533-
    if (file == NULL)
    534-
    ereport(ERROR,
    535-
    (errcode_for_file_access(),
    536-
    errmsg("could not open usermap file \"%s\": %m",
    537-
    IdentFileName)));
    538-
    539-
    linecxt = tokenize_auth_file(IdentFileName, file, &ident_lines, DEBUG3);
    527+
    file = open_auth_file(IdentFileName, ERROR, 0, NULL);
    528+
    529+
    linecxt = tokenize_auth_file(IdentFileName, file, &ident_lines, DEBUG3, 0);
    540530
    FreeFile(file);
    541531

    542532
    /* Now parse all the lines */

    ‎src/include/libpq/hba.h

    Lines changed: 3 additions & 1 deletion
    Original file line numberDiff line numberDiff line change
    @@ -177,7 +177,9 @@ extern int check_usermap(const char *usermap_name,
    177177
    extern HbaLine *parse_hba_line(TokenizedAuthLine *tok_line, int elevel);
    178178
    extern IdentLine *parse_ident_line(TokenizedAuthLine *tok_line, int elevel);
    179179
    extern bool pg_isblank(const char c);
    180+
    extern FILE *open_auth_file(const char *filename, int elevel, int depth,
    181+
    char **err_msg);
    180182
    extern MemoryContext tokenize_auth_file(const char *filename, FILE *file,
    181-
    List **tok_lines, int elevel);
    183+
    List **tok_lines, int elevel, int depth);
    182184

    183185
    #endif /* HBA_H */

    0 commit comments

    Comments
     (0)
    0