Re: Let people set host(no)ssl settings from initdb

From: David Fetter <david(at)fetter(dot)org>
To: Cary Huang <cary(dot)huang(at)highgo(dot)ca>
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Let people set host(no)ssl settings from initdb
Date: 2020-04-08 16:28:10
Message-ID: 20200408162810.GQ13804@fetter.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Apr 06, 2020 at 10:12:16PM +0000, Cary Huang wrote:
> 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'

Fixed.

> > * 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.

Fixed.

Thanks very much for the review!

Best,
David.
--
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

Attachment Content-Type Size
v4-0001-Enable-setting-pg_hba.conf-permissions-from-initd.patch text/x-diff 17.3 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Etsuro Fujita 2020-04-08 16:36:14 Re: warning in partioning code
Previous Message Erik Rijkers 2020-04-08 16:07:28 warning in partioning code