Re: [REVIEW]: Password identifiers, protocol aging and SCRAM protocol

On 1 March 2016 at 06:34, Michael Paquier <michael(dot)paquier(at)gmail(dot)com> wrote:

> On Mon, Feb 29, 2016 at 8:43 PM, Valery Popov <v(dot)popov(at)postgrespro(dot)ru>
> wrote:
> > vpopov(at)vpopov-Ubuntu:~/Projects/pwdtest/postgresql$ git branch
>
> Thanks for the input!
>
> > 0001-Add-facility-to-store-multiple-password-verifiers.patch:2547:
> trailing
> > whitespace.
> > warning: 1 line adds whitespace errors.
> > 0003-Add-pg_auth_verifiers_sanitize.patch:87: indent with spaces.
> > if (!superuser())
> > warning: 1 line adds whitespace errors.
>
> Argh, yes. Those two ones have slipped though my successive rebases I
> think. Will fix in my tree, I don't think that it is worth sending
> again the whole series just for that though.
> --
> Michael
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>
>
Hi, Michael

Few questions about the documentation.

config.sgml:1200

> <listitem>
> <para>
> Specifies a comma-separated list of supported password formats by
> the server. Supported formats are currently <literal>plain</> and
> <literal>md5</>.
> </para>
>
> <para>
> When a password is specified in <xref linkend="sql-createuser"> or
> <xref linkend="sql-alterrole">, this parameter determines if the
> password specified is authorized to be stored or not, returning
> an error message to caller if it is not.
> </para>
>
> <para>
> The default is <literal>plain,md5,scram</>, meaning that
MD5-encrypted
> passwords, plain passwords, and SCRAM-encrypted passwords are
accepted.
> </para>
> </listitem>

The default value contains "scram". Shouldn't be here also:

> Specifies a comma-separated list of supported password formats by
> the server. Supported formats are currently <literal>plain</>,
> <literal>md5</> and <literal>scram</>.

Or I missed something?

And one more:

config.sgml:1284

> <para>
> <varname>db_user_namespace</> causes the client's and
> server's user name representation to differ.
> Authentication checks are always done with the server's user name
> so authentication methods must be configured for the
> server's user name, not the client's. Because
> <literal>md5</> uses the user name as salt on both the
> client and server, <literal>md5</> cannot be used with
> <varname>db_user_namespace</>.
> </para>

Looks like the same (pls, correct me if I'm wrong) is applicable for
"scram" as I see from the code below. Shouldn't be "scram" mentioned here
also? Here's the code:

> diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c
> index 28f9fb5..df0cc1d 100644
> --- a/src/backend/libpq/hba.c
> +++ b/src/backend/libpq/hba.c
> @@ -1184,6 +1184,19 @@ parse_hba_line(List *line, int line_num, char
*raw_line)
> }
> parsedline->auth_method = uaMD5;
> }
>+ else if (strcmp(token->string, "scram") == 0)
>+ {
>+ if (Db_user_namespace)
>+ {
>+ ereport(LOG,
>+ (errcode(ERRCODE_CONFIG_FILE_ERROR),
>+ errmsg("SCRAM authentication is not supported when \"db_user_namespace\"
is enabled"),
>+ errcontext("line %d of configuration file \"%s\"",
>+ line_num, HbaFileName)));
>+ return NULL;
>+ }
>+ parsedline->auth_method = uaSASL;
>+ }
> else if (strcmp(token->string, "pam") == 0)
> #ifdef USE_PAM
> parsedline->auth_method = uaPAM;