Report pg_hba line number and contents when users fail to log in

Instead of just reporting which user failed to log in, log both the
line number in the active pg_hba.conf file (which may not match reality
in case the file has been edited and not reloaded) and the contents of
the matching line (which will always be correct), to make it easier
to debug incorrect pg_hba.conf files.

The message to the client remains unchanged and does not include this
information, to prevent leaking security sensitive information.

Reviewed by Tom Lane and Dean Rasheed
This commit is contained in:
Magnus Hagander 2013-03-10 15:54:37 +01:00
parent 96443d1420
commit 7f49a67f95
3 changed files with 81 additions and 48 deletions

View File

@ -297,9 +297,16 @@ auth_failed(Port *port, int status)
break; break;
} }
if (port->hba)
ereport(FATAL,
(errcode(errcode_return),
errmsg(errstr, port->user_name),
errdetail_log("Connection matched pg_hba.conf line %d: \"%s\"", port->hba->linenumber, port->hba->rawline)));
else
ereport(FATAL, ereport(FATAL,
(errcode(errcode_return), (errcode(errcode_return),
errmsg(errstr, port->user_name))); errmsg(errstr, port->user_name)));
/* doesn't return */ /* doesn't return */
} }

View File

@ -50,6 +50,7 @@
#define atoxid(x) ((TransactionId) strtoul((x), NULL, 10)) #define atoxid(x) ((TransactionId) strtoul((x), NULL, 10))
#define MAX_TOKEN 256 #define MAX_TOKEN 256
#define MAX_LINE 8192
/* callback data for check_network_callback */ /* callback data for check_network_callback */
typedef struct check_network_data typedef struct check_network_data
@ -93,7 +94,7 @@ static MemoryContext parsed_ident_context = NULL;
static MemoryContext tokenize_file(const char *filename, FILE *file, static MemoryContext tokenize_file(const char *filename, FILE *file,
List **lines, List **line_nums); List **lines, List **line_nums, List **raw_lines);
static List *tokenize_inc_file(List *tokens, const char *outer_filename, static List *tokenize_inc_file(List *tokens, const char *outer_filename,
const char *inc_filename); const char *inc_filename);
static bool parse_hba_auth_opt(char *name, char *val, HbaLine *hbaline, static bool parse_hba_auth_opt(char *name, char *val, HbaLine *hbaline,
@ -111,7 +112,8 @@ pg_isblank(const char c)
/* /*
* Grab one token out of fp. Tokens are strings of non-blank * Grab one token out of the string pointed to by lineptr.
* Tokens are strings of non-blank
* characters bounded by blank characters, commas, beginning of line, and * characters bounded by blank characters, commas, beginning of line, and
* end of line. Blank means space or tab. Tokens can be delimited by * end of line. Blank means space or tab. Tokens can be delimited by
* double quotes (this allows the inclusion of blanks, but not newlines). * double quotes (this allows the inclusion of blanks, but not newlines).
@ -134,7 +136,7 @@ pg_isblank(const char c)
* Handle comments. * Handle comments.
*/ */
static bool static bool
next_token(FILE *fp, char *buf, int bufsz, bool *initial_quote, next_token(char **lineptr, char *buf, int bufsz, bool *initial_quote,
bool *terminating_comma) bool *terminating_comma)
{ {
int c; int c;
@ -151,10 +153,10 @@ next_token(FILE *fp, char *buf, int bufsz, bool *initial_quote,
*terminating_comma = false; *terminating_comma = false;
/* Move over initial whitespace and commas */ /* Move over initial whitespace and commas */
while ((c = getc(fp)) != EOF && (pg_isblank(c) || c == ',')) while ((c = (*(*lineptr)++)) != '\0' && (pg_isblank(c) || c == ','))
; ;
if (c == EOF || c == '\n') if (c == '\0' || c == '\n')
{ {
*buf = '\0'; *buf = '\0';
return false; return false;
@ -164,17 +166,17 @@ next_token(FILE *fp, char *buf, int bufsz, bool *initial_quote,
* Build a token in buf of next characters up to EOF, EOL, unquoted comma, * Build a token in buf of next characters up to EOF, EOL, unquoted comma,
* or unquoted whitespace. * or unquoted whitespace.
*/ */
while (c != EOF && c != '\n' && while (c != '\0' && c != '\n' &&
(!pg_isblank(c) || in_quote)) (!pg_isblank(c) || in_quote))
{ {
/* skip comments to EOL */ /* skip comments to EOL */
if (c == '#' && !in_quote) if (c == '#' && !in_quote)
{ {
while ((c = getc(fp)) != EOF && c != '\n') while ((c = (*(*lineptr)++)) != '\0' && c != '\n')
; ;
/* If only comment, consume EOL too; return EOL */ /* If only comment, consume EOL too; return EOL */
if (c != EOF && buf == start_buf) if (c != '\0' && buf == start_buf)
c = getc(fp); (*lineptr)++;
break; break;
} }
@ -186,7 +188,7 @@ next_token(FILE *fp, char *buf, int bufsz, bool *initial_quote,
errmsg("authentication file token too long, skipping: \"%s\"", errmsg("authentication file token too long, skipping: \"%s\"",
start_buf))); start_buf)));
/* Discard remainder of line */ /* Discard remainder of line */
while ((c = getc(fp)) != EOF && c != '\n') while ((c = (*(*lineptr)++)) != '\0' && c != '\n')
; ;
break; break;
} }
@ -215,15 +217,14 @@ next_token(FILE *fp, char *buf, int bufsz, bool *initial_quote,
*initial_quote = true; *initial_quote = true;
} }
c = getc(fp); c = *(*lineptr)++;
} }
/* /*
* Put back the char right after the token (critical in case it is EOL, * Put back the char right after the token (critical in case it is EOL,
* since we need to detect end-of-line at next call). * since we need to detect end-of-line at next call).
*/ */
if (c != EOF) (*lineptr)--;
ungetc(c, fp);
*buf = '\0'; *buf = '\0';
@ -258,13 +259,13 @@ copy_hba_token(HbaToken *in)
/* /*
* Tokenize one HBA field from a file, handling file inclusion and comma lists. * Tokenize one HBA field from a line, handling file inclusion and comma lists.
* *
* The result is a List of HbaToken structs for each individual token, * The result is a List of HbaToken structs for each individual token,
* or NIL if we reached EOL. * or NIL if we reached EOL.
*/ */
static List * static List *
next_field_expand(const char *filename, FILE *file) next_field_expand(const char *filename, char **lineptr)
{ {
char buf[MAX_TOKEN]; char buf[MAX_TOKEN];
bool trailing_comma; bool trailing_comma;
@ -273,7 +274,7 @@ next_field_expand(const char *filename, FILE *file)
do do
{ {
if (!next_token(file, buf, sizeof(buf), &initial_quote, &trailing_comma)) if (!next_token(lineptr, buf, sizeof(buf), &initial_quote, &trailing_comma))
break; break;
/* Is this referencing a file? */ /* Is this referencing a file? */
@ -335,7 +336,7 @@ tokenize_inc_file(List *tokens,
} }
/* There is possible recursion here if the file contains @ */ /* There is possible recursion here if the file contains @ */
linecxt = tokenize_file(inc_fullname, inc_file, &inc_lines, &inc_line_nums); linecxt = tokenize_file(inc_fullname, inc_file, &inc_lines, &inc_line_nums, NULL);
FreeFile(inc_file); FreeFile(inc_file);
pfree(inc_fullname); pfree(inc_fullname);
@ -364,8 +365,8 @@ tokenize_inc_file(List *tokens,
} }
/* /*
* Tokenize the given file, storing the resulting data into two Lists: a * Tokenize the given file, storing the resulting data into three Lists: a
* List of lines, and a List of line numbers. * List of lines, a List of line numbers, and a List of raw line contents.
* *
* The list of lines is a triple-nested List structure. Each line is a List of * The list of lines is a triple-nested List structure. Each line is a List of
* fields, and each field is a List of HbaTokens. * fields, and each field is a List of HbaTokens.
@ -377,7 +378,7 @@ tokenize_inc_file(List *tokens,
*/ */
static MemoryContext static MemoryContext
tokenize_file(const char *filename, FILE *file, tokenize_file(const char *filename, FILE *file,
List **lines, List **line_nums) List **lines, List **line_nums, List **raw_lines)
{ {
List *current_line = NIL; List *current_line = NIL;
List *current_field = NIL; List *current_field = NIL;
@ -396,7 +397,28 @@ tokenize_file(const char *filename, FILE *file,
while (!feof(file) && !ferror(file)) while (!feof(file) && !ferror(file))
{ {
current_field = next_field_expand(filename, file); char rawline[MAX_LINE];
char *lineptr;
if (!fgets(rawline, sizeof(rawline), file))
break;
if (strlen(rawline) == MAX_LINE-1)
/* Line too long! */
ereport(ERROR,
(errcode(ERRCODE_CONFIG_FILE_ERROR),
errmsg("authentication file line too long"),
errcontext("line %d of configuration file \"%s\"",
line_number, filename)));
/* Strip trailing linebreak from rawline */
while (rawline[strlen(rawline)-1] == '\n' ||
rawline[strlen(rawline)-1] == '\r')
rawline[strlen(rawline)-1] = '\0';
lineptr = rawline;
while (strlen(lineptr) > 0)
{
current_field = next_field_expand(filename, &lineptr);
/* add tokens to list, unless we are at EOL or comment start */ /* add tokens to list, unless we are at EOL or comment start */
if (list_length(current_field) > 0) if (list_length(current_field) > 0)
@ -407,6 +429,8 @@ tokenize_file(const char *filename, FILE *file,
current_line = lappend(current_line, current_field); current_line = lappend(current_line, current_field);
*lines = lappend(*lines, current_line); *lines = lappend(*lines, current_line);
*line_nums = lappend_int(*line_nums, line_number); *line_nums = lappend_int(*line_nums, line_number);
if (raw_lines)
*raw_lines = lappend(*raw_lines, pstrdup(rawline));
} }
else else
{ {
@ -414,13 +438,11 @@ tokenize_file(const char *filename, FILE *file,
current_line = lappend(current_line, current_field); current_line = lappend(current_line, current_field);
} }
} }
else }
{
/* we are at real or logical EOL, so force a new line List */ /* we are at real or logical EOL, so force a new line List */
current_line = NIL; current_line = NIL;
line_number++; line_number++;
} }
}
MemoryContextSwitchTo(oldcxt); MemoryContextSwitchTo(oldcxt);
@ -815,7 +837,7 @@ check_same_host_or_net(SockAddr *raddr, IPCompareMethod method)
* NULL. * NULL.
*/ */
static HbaLine * static HbaLine *
parse_hba_line(List *line, int line_num) parse_hba_line(List *line, int line_num, char *raw_line)
{ {
char *str; char *str;
struct addrinfo *gai_result; struct addrinfo *gai_result;
@ -831,6 +853,7 @@ parse_hba_line(List *line, int line_num)
parsedline = palloc0(sizeof(HbaLine)); parsedline = palloc0(sizeof(HbaLine));
parsedline->linenumber = line_num; parsedline->linenumber = line_num;
parsedline->rawline = pstrdup(raw_line);
/* Check the record type. */ /* Check the record type. */
field = list_head(line); field = list_head(line);
@ -1761,8 +1784,10 @@ load_hba(void)
FILE *file; FILE *file;
List *hba_lines = NIL; List *hba_lines = NIL;
List *hba_line_nums = NIL; List *hba_line_nums = NIL;
List *hba_raw_lines = NIL;
ListCell *line, ListCell *line,
*line_num; *line_num,
*raw_line;
List *new_parsed_lines = NIL; List *new_parsed_lines = NIL;
bool ok = true; bool ok = true;
MemoryContext linecxt; MemoryContext linecxt;
@ -1779,7 +1804,7 @@ load_hba(void)
return false; return false;
} }
linecxt = tokenize_file(HbaFileName, file, &hba_lines, &hba_line_nums); linecxt = tokenize_file(HbaFileName, file, &hba_lines, &hba_line_nums, &hba_raw_lines);
FreeFile(file); FreeFile(file);
/* Now parse all the lines */ /* Now parse all the lines */
@ -1789,11 +1814,11 @@ load_hba(void)
ALLOCSET_DEFAULT_MINSIZE, ALLOCSET_DEFAULT_MINSIZE,
ALLOCSET_DEFAULT_MAXSIZE); ALLOCSET_DEFAULT_MAXSIZE);
oldcxt = MemoryContextSwitchTo(hbacxt); oldcxt = MemoryContextSwitchTo(hbacxt);
forboth(line, hba_lines, line_num, hba_line_nums) forthree(line, hba_lines, line_num, hba_line_nums, raw_line, hba_raw_lines)
{ {
HbaLine *newline; HbaLine *newline;
if ((newline = parse_hba_line(lfirst(line), lfirst_int(line_num))) == NULL) if ((newline = parse_hba_line(lfirst(line), lfirst_int(line_num), lfirst(raw_line))) == NULL)
{ {
/* /*
* Parse error in the file, so indicate there's a problem. NB: a * Parse error in the file, so indicate there's a problem. NB: a
@ -2153,7 +2178,7 @@ load_ident(void)
return false; return false;
} }
linecxt = tokenize_file(IdentFileName, file, &ident_lines, &ident_line_nums); linecxt = tokenize_file(IdentFileName, file, &ident_lines, &ident_line_nums, NULL);
FreeFile(file); FreeFile(file);
/* Now parse all the lines */ /* Now parse all the lines */

View File

@ -53,6 +53,7 @@ typedef enum ConnType
typedef struct HbaLine typedef struct HbaLine
{ {
int linenumber; int linenumber;
char *rawline;
ConnType conntype; ConnType conntype;
List *databases; List *databases;
List *roles; List *roles;