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

[Umar Hayat]

Hi,
Please updated patch based on feedback from Tatsu Ishii on pg_enc patch.

Regards
Umar Hayat

On Tue, Mar 10, 2020 at 3:39 PM Umar Hayat <m.umarkiani at gmail.com> wrote:

> Any further updated needed for this patch from my end?
>
> Regards
> Umar Hayat
>
> On Thu, Mar 5, 2020 at 1:45 AM Umar Hayat <m.umarkiani at gmail.com> wrote:
>
>> Hi Usama,
>> Thanks for feedback, updated patch is attached.
>> Please see comments inline.
>>
>> Regards
>> Umar Hayat
>>
>>
>> On Wed, Mar 4, 2020 at 8:05 PM Muhammad Usama <m.usama at gmail.com> wrote:
>>
>>> 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)
>>>
>> Fixed.
>>
>>> 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)
>>>
>> It does handle colon in password as it consider only left most colon as
>> separator. For example for pair like 'user:pass:word', user name will be
>> 'user' and password will be 'pass:word'
>> I am not sure user name with colon is valid scenario? If its true, we can
>> add a note otherwise IMO there could be ton of negative scenarios.
>>
>>> 3- Specifying the input file disregards the "-m" "--md5auth" option
>>>
>>> Fixed. That was purposefully disregarded  previously as mention
>> in mentisbt-422, input-file mode as only for md5auth.
>> Now It honour -m flag and in absence of this it will print md5 value
>> stdout.
>>
>>> 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.
>>>
>>> Fixed.
>>
>>> 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/20200318/63014a61/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: pg_md5_input_file_18mar20.diff
Type: application/octet-stream
Size: 6495 bytes
Desc: not available
URL: <http://www.sraoss.jp/pipermail/pgpool-hackers/attachments/20200318/63014a61/attachment-0001.obj>