[pgpool-hackers: 1348] Re: new watchdog IPC authentication mechanism

[Muhammad Usama]

On Thu, Jan 21, 2016 at 2:31 PM, Yugo Nagata <nagata at sraoss.co.jp> wrote:

> Hi Usama,
>
> I thought IPC_SHARED_KEY method was secure because the packet was
> sent using UNIX domain socket and nobody could read this directly.
> Hoever, I found the JSON data including IPC_SHARED_KEY was printed
> in the debug log messages, so anyone who watched debug messages is
> able to know the key and issue watchdog internal commands. Although
> the debug mode isn't used usually in product systems, it isn't secure
> so much.
>
> The log is printed at process_pgpool_replicate_command().
> I don't know whether there are ohter parts printing the json data.
>
> 4719     ereport(DEBUG1,
> 4720         (errmsg("processing pgpool replicate variable command"),
> 4721              errdetail("%s",pkt->data)));
>
> I think it takes times to rewrite the authentication method, so
> this should be left for the future, which might be minnorrelease.
> It would be good to resolve by removing the codes at this time.
>
>
Yes, good point. The shared key should not be printed in the log message.
But as far as an authentication method is concerned. I think it is good
enough. Although it is not bullet-proof, but since IPC packets are not
traveling on the wire and doesn't leave the system running the
pgpool-II server, so simple shared key matching can serve the purpose. As
if somehow the system itself is compromised so that an attacker is able to
sniff the shared key from IPC communication, He/she could also be able
access the pgpool-II config file that contains the wd_authkey, and could
use that to launch the attack

Thanks
Best regards
Muhammad Usama



> On Thu, 24 Dec 2015 19:01:35 +0900
> Yugo Nagata <nagata at sraoss.co.jp> wrote:
>
> > Hi Usama,
> >
> > I reviewed the patch and basically I think this is good to be commited.
> > Here are some comments:
> >
> > (1)
> > WD_IPC_AUTH_KEY and WD_IPC_SHARED_KEY are defined in watchdog.h. However,
> > other json keys (e.g. "FailoverCMDType", "SyncRequestType", and so on)
> are
> > hardcoded but I think it is better to define them similarly.
> >
> > (2)
> > In check_IPC_client_authetication(), json value of WD_IPC_SHARED_KEY
> > is checked and if it doesn't exist, watchdog says "invalid json data
> packet",
> > but I think this is valid even if it is the packet from external and has
> only
> > WD_IPC_AUTH_KEY. And WD_IPC_SHARED_KEY check is needed only when
> > internal_client_only is ture, so this check can be in
> > "if (internal_client_only) {...}" block.
> >
> > (3)
> > a very small typo:
> >
> > + * if internal_client_only is true than the JSON data must contain the
> > + * shared key present in the pgpool-II shared memory. This can be used
> >
> > than -> then
> >
> > On Wed, 23 Dec 2015 20:17:38 +0500
> > Muhammad Usama <m.usama at gmail.com> wrote:
> >
> > > Hi Yugo
> > >
> > > As you mentioned in your watchdog code review comments that there
> should be
> > > a mechanism to authenticate the watchdog IPC commands, Although this
> was
> > > also one of the TODO items in my list but somehow slipped out of radar.
> > >
> > > So please find the attached patch for the purpose.
> > >
> > > Basically the patch utilizes the *wd_authkey* configuration parameter
> to
> > > authenticate the watchdog IPC command clients. And since the IPC is
> meant
> > > for communication between internal pgpool-II processes and with
> external
> > > processes on the same machine, so instead of calculating the hash and
> > > inventing a more complicated process of IPC authentication, the simple
> > > wd_authkey matching is used to validate the clients. And when the
> > > wd_authkey is not provided by the user in pgpool.conf than the
> > > authentication is disabled.
> > >
> > > The patch also takes care of other authentication related problem in
> the
> > > watchdog IPC system. Since there are some watchdog functions visible
> by the
> > > IPC commands, and we want to restrict their access to the outer world.
> Like
> > > for example the interlocking command functions and failover related
> > > functions. So for those functions, the patch creates a shared key in
> the
> > > shared memory, and execution of these functions is only allowed if that
> > > shard key in that function data matches with the current shred key in
> > > shared memory. And when some external client tries to call these
> internal
> > > only functions, the authentication error will be returned to that
> since the
> > > key is secret to the pgpool-II only.
> > >
> > > So what are your thoughts on the mechanism and using the *wd_authkey*
> for
> > > the purpose.
> > >
> > > Thanks
> > > Best regards
> > > Muhammad Usama
> >
> >
> > --
> > Yugo Nagata <nagata at sraoss.co.jp>
> > _______________________________________________
> > pgpool-hackers mailing list
> > pgpool-hackers at pgpool.net
> > http://www.pgpool.net/mailman/listinfo/pgpool-hackers
>
>
> --
> Yugo Nagata <nagata at sraoss.co.jp>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.sraoss.jp/pipermail/pgpool-hackers/attachments/20160121/025d4c9a/attachment-0001.html>