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

[Bo Peng]

Hello, 

On Wed, 18 Mar 2020 11:51:05 +0500
Umar Hayat <m.umarkiani at gmail.com> wrote:

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

I tested your patch and it works fine.

Usama,

If you don't have any feedback, I am going to update 
the patch based on Ishii-san's feedback and commit it.

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


-- 
Bo Peng <pengbo at sraoss.co.jp>
SRA OSS, Inc. Japan