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

[Yugo Nagata]

On Thu, 21 Jan 2016 17:11:44 +0500
Muhammad Usama <m.usama at gmail.com> wrote:

> 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

Yes, I also think the IPC authentication method is good, as long as
the key can't be leekd to the log messages.

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


-- 
Yugo Nagata <nagata at sraoss.co.jp>