[systemd-devel] [PATCH 0/12] kdbus: policy tests and fixes

[Djalal Harouni]

Hi Daniel,

(Really sorry for my late response, my university/research job...)

On Fri, Jun 20, 2014 at 07:21:25PM +0200, Daniel Mack wrote:
> Hi Djalal,
> 
> On 06/20/2014 06:49 PM, Djalal Harouni wrote:
> > This series adds the test-kdbus-policy test. The first patches are
> > prepration then you have the test.
> > 
> > Later there are several fixes and improvments, I've performed all the
> > tests with success.
> 
> Very nice, thanks a lot for doing this!
You are welcome! just reporting and fixing kdbus+systemd things that I
find...

> I'll comment on the individual patches.
Thank you for applying the series.


> > I still have another series which deals with the send access cache, will
> > send it soon, or perhaps tomorrow it should go on top of this.
> 
> Ok, great.
> 
> > Please Kay, Daniel allow me this question:
> > 
> > The policy holders are just connections that register policy entries!
> > They dont register names, so the registered policy entry wont take any
> > effect unless you acquire (register into database) its name !
> 
> That's correct. The idea here is to close the gap between name
> acquisition and the policy being applied, and the owner of a name should
> not be the same instance that decides who's allowed to own it, who may
> talk to it or see it.
Ok, I see that! that permits to reserve policies without races!
X registers (reserve name) the policy, but later Y is the real owner...

In the same time it will allow the *same* instance to own a name and
to decides who's allowed to own it, access it... IIUC since currently we
can do that.


> Likewise, a connection can only own a name on the system bus if there's
> a policy rule that allows just that, and the rule has to be added
> beforehand by the bus owner.
Ok, that's nice!

It also allow other general purpose use cases where you need kdbus
features and at the same time bus separation.

> > We need here two operations:
> > 1) register as a policy holder
> > 2) acquire the name to be able to send to that name and to activate
> > the policy rules.
> > 
> > Is this the intended behaviour ?
> 
> Yes, exactly, and installing a policy is a privileged operation. We
> thought a lot about the design here, and I think this is a good and
> clean solution. Did you understand that right away? Is there anything
Yes, installing a policy holder or an activator

For the activator that was easy, for the policy holder I think we need
to document this in the kdbus.h perhaps in the
'KDBUS_HELLO_POLICY_HOLDER' section add: "and to activate that name...
and to be able to send to it you must acquire it." ?

> illogical about the idea you're concerned about? We're open to
> suggestions. After all, the code is not yet in production :)
> 
Thank you for the explanation. Yes I thought that policy holders are
like activators, only activators are for systemd and policy holders are
for all other cases!

So I was wondering why I can't reach the policy holder and its name?
I misunderstood the concept at first, then I found we need to acquire
that name to activate it! and the acquire name operation will be subject
to the OWN access policy. So this is clear now, and it will for sure
prevent own names races! that's good design.

So perhaps just add small note into the 'KDBUS_HELLO_POLICY_HOLDER'
section ?


I was also worried about spoofing activators and policies but hopefully
it seems there is no chance for that.


Unrelated notes:
There is also some concern about LSM! I hope that kdbus will stay clean.
Ah and kdbus as it is now, can fit other general purpose use cases, not
only D-Bus... one example:
Send a *big* chunk of data using kdbus *features* + *metadata*, but
later receiver can just *drop* it if it does not fit for an X reason!
with kdbus this is a cheap operation...


I should also mention this:
There is also another hidden positive point for kdbus, by using
->ioctl() instead of ->read() or ->write() file operations you close
some windows against creds and privileges changes!

Currently there are checks against *current* creds at the time of the
ioctl() call!


However we all know that there are some setuid programs that
read()/write() from stdin/stdout... which can be dupped by unprivileged
parent, thus if the unprivileged parent is *able* to ->open() and *not*
able to ->read()/write() he will dup fd and pass it to the exec suid
program to pass privileges checks and do ->read()/write(). So some
kernel interfaces should worry about these cases! and the only protection
should be to do checks against *creds* that *opened* the file descriptor
not against *current* creds which may point to the privileged setuid
process.

In the other hand you dont find setuid programs that do ioclt() on
stdin/stdout... that would be stupid!

So being a char device with its own ioctl() just makes things simpler.

Thanks

> Thanks,
> Daniel
> 

-- 
Djalal Harouni
http://opendz.org