From: guido@trentalancia.com (Guido Trentalancia)
Date: Tue, 07 Aug 2012 22:20:40 +0200
Subject: [refpolicy] [PATCH v4]: mcelog module initial rewrite
In-Reply-To: <1344368916.2306.14.camel@d30.localdomain>
References: <201208061519.q76FJcDp011962@vivaldi31.register.it>
	<1344267046.29329.57.camel@d30.localdomain>
	<50201053.9000506@trentalancia.com>
	<1344282251.29329.73.camel@d30.localdomain>
	<50215188.7040900@trentalancia.com>
	<1344361404.2306.5.camel@d30.localdomain>
	<50216DFF.1050309@trentalancia.com>
	<1344368916.2306.14.camel@d30.localdomain>
Message-ID: <50217898.1000106@trentalancia.com>
To: refpolicy@oss.tresys.com
List-Id: refpolicy.oss.tresys.com

On 07/08/2012 21:48, Dominick Grift wrote:
>
>
> On Tue, 2012-08-07 at 21:35 +0200, Guido Trentalancia wrote:
>> Hello Dominick.
>>
>> On 07/08/2012 19:43, Dominick Grift wrote:
>>>
>>>>
>>>> It's needed for the (untested) client mode.
>>>>
>>>> There is a boolean for that (and for the server mode, as one might want
>>>> to write another client for example).
>>>>
>>>
>>> Its already allowed... I will explain it one more time:
>>>
>>> allow mcelog_t self:unix_stream_socket create_stream_socket_perms;
>>> manage_sock_files_pattern(mcelog_t, mcelog_var_run_t, mcelog_var_run_t)
>>>
>>> is what allows this already. Its already there and therefore the
>>> stream_connect_pattern() is reduntant.
>>
>> I have triple-checked now, so at least you could double-check...
>>
>
> whoops you are right but in that case just do something like this
> instead:
>
> allow mcslog_t self:unix_stream_socket { create_socket_perms
> connectto; }

Yes, I think it can be done, but I would need to check. But is this not 
going to be considered overengineering ?

I don't like such term very much in this context anyway, as usually 
there is always an advantage in terms of maintanability.

> or show me the avc denials related to mcelog_t operating on mcelog_t
> unix stream sockets so that we can figure out the exact permissions.
>
> but using stream_connect_pattern() is not the way to go here

Initially you had suggested that pattern, so I went for that.

>> stream_connect_pattern() is needed for "connectto" (client-mode)
>>
>> removal of manage_sock_files_pattern would prevent sock_file creation:
>> it's the physical entry in the filesystem, not the logical socket
>> created by create_stream_socket_perms !
>
> I am not saying that you should remove it. I am saying that you should
> use the stream_connect_pattern()

>>> However i won't review it anymore because i have made my points already
>>> in previous reviews. No need for repeating myself.
>>
>> As already explained, all your revision have been introduced as applicable.
>>
>> My advice is to apply it as it is now and then you can submit further
>> patches as needed, which also seems much more efficient.
>
> It's not my call. I am just reviewing.
>
>> But I would strongly recommend you to also carry out some testing,
>> because otherwise, no matter how skilled you are, things similar to the
>> above might happen.
>
> It should be tested indeed
>
>> Regards,
>>
>> Guido