| From: | Cary Huang <cary(dot)huang(at)highgo(dot)ca> | 
|---|---|
| To: | pgsql-hackers(at)lists(dot)postgresql(dot)org | 
| Cc: | David Fetter <david(at)fetter(dot)org> | 
| Subject: | Re: Let people set host(no)ssl settings from initdb | 
| Date: | 2020-04-06 22:12:16 | 
| Message-ID: | 158621113633.772.4074542039539427130.pgcf@coridan.postgresql.org | 
| Views: | Whole Thread | Raw Message | Download mbox | Resend email | 
| Thread: | |
| Lists: | pgsql-hackers | 
The following review has been posted through the commitfest application:
make installcheck-world:  tested, passed
Implements feature:       tested, passed
Spec compliant:           tested, passed
Documentation:            tested, passed
Hi 
I applied the patch "v3-0001-Enable-setting-pg_hba.conf-permissions-from-initd.patch" and did some verification with it. The intended feature works overall and I think it is quite useful to support default auth methods for ssl and gss host types. I have also found some minor things in the patch and would like to share as below:
> +"# CAUTION: Configuring the system for \"trust\" authentication\n" \
> +"# allows any user who can reach the databse on the route specified\n" \
> +"# to connect as any PostgreSQL user, including the database\n" \
> +"# superuser.  If you do not trust all the users who could\n" \
> +"# reach the database on the route specified, use a more restrictive\n" \
> +"# authentication method.\n"
Found a typo: should be 'database' instead of 'databse'
>  * a sort of poor man's grep -v
>  */
> -#ifndef HAVE_UNIX_SOCKETS
>  static char **
>  filter_lines_with_token(char **lines, const char *token)
>  {
> @@ -461,7 +466,6 @@ filter_lines_with_token(char **lines, const char *token)
>  
>  	return result;
> }
> -#endif
I see that you have removed "#ifndef HAVE_UNIX_SOCKETS" around the filter_lines_with_token() function definition so it would be always available, which is used to remove the @tokens@ in case user does not specify a default auth method for the new hostssl, hostgss options. I think you should also remove the "#ifndef HAVE_UNIX_SOCKETS" around its declaration as well so both function definition and declaration would make sense.
#ifndef HAVE_UNIX_SOCKETS
static char **filter_lines_with_token(char **lines, const char *token);
#endif
Cary Huang
-------------
HighGo Software Inc. (Canada)
cary(dot)huang(at)highgo(dot)ca
www.highgo.ca
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Tomas Vondra | 2020-04-06 22:13:54 | Re: [PATCH] Incremental sort (was: PoC: Partial sort) | 
| Previous Message | Kartyshov Ivan | 2020-04-06 21:58:18 | Re: [HACKERS] make async slave to wait for lsn to be replayed |