[pgpool-hackers: 3534] Re: [PATCH] Feature: Add support for an user/password input file to pg_md5

[Muhammad Usama]

Hi Umar,

Here are some of the comments on your patch on top of Peng's comments.

1- The patch breaks the -f short option, by removing the : (colon) after f
from the optstring argument to getopt_long function

i.e
while ((opt = getopt_long(argc, argv, "hpmfi:u:", long_options, &optindex))
!= -1)
should be
while ((opt = getopt_long(argc, argv, "hpmf:i:u:", long_options,
&optindex)) != -1)

2- The input file has no option to specify escape characters. For example,
there is no way to specify a user name or password
in the input file that contains : (colon)

3- Specifying the input file disregards the "-m" "--md5auth" option

4- The patch is not considering the length of username and password buffers
while copying into them


+               strncpy(username, buf, pch-buf);
+               strncpy(password, pch+1, strlen(pch+1));


The len (3rd) argument of strncpy is intended to guard against the buffer
overflow of the destination string.

Thanks
Best regards
Muhammad Usama


On Wed, Mar 4, 2020 at 11:03 AM Umar Hayat <m.umarkiani at gmail.com> wrote:

> Hi Bo Peng,
> Please find updated patch with following changes.
> 1. Trailing whitespaces warning removed
> 2. Documentation added in "doc/src/sgml/ref/pg_md5.sgml" with options
> update and example usage.
>
> Regards,
> Umar Hayat
>
> On Wed, Mar 4, 2020 at 7:40 AM Bo Peng <pengbo at sraoss.co.jp> wrote:
>
>> hello,
>>
>> On Tue, 3 Mar 2020 14:13:33 +0500
>> Umar Hayat <m.umarkiani at gmail.com> wrote:
>>
>> > Hi Bo Peng,
>> > Any feedback, If this looks ok to you? I should send the same for pg_enc
>> > utility.
>>
>> Sorry for late response.
>>
>> First, when I apply this patch, I found some extra trailing spaces.
>>
>> -----------------
>> $ git apply pg_md5_input_file.diff
>> pg_md5_input_file.diff:85: trailing whitespace.
>> static void
>> warning: 1 line adds whitespace errors.
>> -----------------
>>
>> Could you add documentation in "doc/src/sgml/ref/pg_md5.sgml" to
>> explain how to use this option?
>>
>> >
>> > Regards
>> > Umar Hayat
>> > Principal Software Engineer
>> > EnterpriseDB: https://www.enterprisedb.com
>> >
>> > On Thu, Feb 13, 2020 at 9:54 AM Bo Peng <pengbo at sraoss.co.jp> wrote:
>> >
>> > > Thank you for your patch.
>> > > I will review your patch.
>> > >
>> > > On Tue, 11 Feb 2020 16:45:50 +0500
>> > > Umar Hayat <m.umarkiani at gmail.com> wrote:
>> > >
>> > > > Hello Hackers,
>> > > >
>> > > > I saw "Add support for an user/password input file to pg_md5"
>> enhancement
>> > > > in PgPool-II TODO list
>> > > > <
>> > >
>> https://pgpool.net/mediawiki/index.php/TODO#Add_support_for_an_user/password_input_file_to_pg_md5
>> > > >,
>> > > > (mentisbt-422 <https://www.pgpool.net/mantisbt/view.php?id=422>),
>> so I
>> > > > implemented this in pg_md5 utility. Please find attached patch for
>> > > > enhancement.
>> > > >
>> > > > As suggested, a new --input-file option added in pg_md5. Using this
>> > > option,
>> > > > pg_md5 will parse the *user:password* pairs from provide file and
>> will
>> > > > create *user:md5xxxxxx* value in pool_passwd file.
>> > > >
>> > > > Regards,
>> > > > Umar Hayat
>> > >
>> > >
>> > > --
>> > > Bo Peng <pengbo at sraoss.co.jp>
>> > > SRA OSS, Inc. Japan
>> > >
>>
>>
>> --
>> Bo Peng <pengbo at sraoss.co.jp>
>> SRA OSS, Inc. Japan
>>
> _______________________________________________
> pgpool-hackers mailing list
> pgpool-hackers at pgpool.net
> http://www.pgpool.net/mailman/listinfo/pgpool-hackers
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.sraoss.jp/pipermail/pgpool-hackers/attachments/20200304/b19040fc/attachment-0001.html>