pcp
[Top] [All Lists]

Re: [pcp] user/group access control question

To: Dave Brolley <brolley@xxxxxxxxxx>, pcp@xxxxxxxxxxx
Subject: Re: [pcp] user/group access control question
From: Ken McDonell <kenj@xxxxxxxxxxxxxxxx>
Date: Wed, 29 Oct 2014 10:34:17 +1100
Delivered-to: pcp@xxxxxxxxxxx
In-reply-to: <544FD6DF.8060206@xxxxxxxxxx>
References: <001f01cff196$66cf3e00$346dba00$@internode.on.net> <544FD6DF.8060206@xxxxxxxxxx>
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.2.0
On 29/10/14 04:48, Dave Brolley wrote:
> ...
Looks like you made a good catch and your are definitely on the right
track. The non-secure server implementation of
__pmSecureServerHandshake() definitely needs to handle
PDU_FLAG_CREDS_REQD. However we need one additional bit from the secure
server implementation. Here is an updated patch for which
PDU_FLAG_CREDS_REQD is still not supported unless the connection is from
a unix domain socket.

Thanks Dave.

First I modified qa/944 to add another test to explicitly use a tcp connection, expecting this to pass without your refinement to the patch ... it fails with PM_ERR_PERMISSION. Hmm ... several iterations and a bunch of additional diagnostics later I discover that from DoCreds() we sail through __pmSecureServerHandshake() and then CheckAccountAccess() is passing in the unix domain socket case and failing in the tcp socket case because cp->attrs is not null in the first case, but is null in the second case ... more digging and I find that much earlier in CheckNewClient() we have this guard
        if (sts >= 0 && family == AF_UNIX)
in front of
        __pmServerSetLocalCreds(cp->fd, &cp->attrs)

So it is working, but that is kinda fortuitous.

I've added your check (to be sure, to be sure, and in case someone else ever calls __pmSecureServerHandshake() on a different code path, and the test is effectively the same test that was being done later in CheckAccountAccess()), but as expected (after the analysis) it does not change the behaviour ... 8^)>

I'll test it here, but would appreciate it if you could test it in your
failing environment.

Nathan, can you please also review the updated patch?

I have this change in the context of more diagnostics and the extensions to qa/944, so I'll push the commit if everyone is OK with that.

<Prev in Thread] Current Thread [Next in Thread>