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

Yugo Nagata nagata at sraoss.co.jp
Thu Dec 24 19:01:35 JST 2015


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>


More information about the pgpool-hackers mailing list