Hello Dominick.
>On Mon, 2012-08-06 at 14:56 +0200, Guido Trentalancia wrote:
>
>> >> diff -pruN refpolicy-04062012/policy/modules/contrib/mcelog.fc refpolicy-04062012-mcelog-support/policy/modules/contrib/mcelog.fc
>> >> --- refpolicy-04062012/policy/modules/contrib/mcelog.fc 2011-09-09 18:29:23.578610955 +0200
>> >> +++ refpolicy-04062012-mcelog-support/policy/modules/contrib/mcelog.fc 2012-08-05 23:36:37.355678527 +0200
>> >> @@ -1 +1,16 @@
>> >> +/etc/mcelog(/.*)? gen_context(system_u:object_r:mcelog_etc_t,s0)
>> >> +/etc/mcelog/.*-error-trigger -- gen_context(system_u:object_r:mcelog_exec_t,s0)
>> >> +/etc/mcelog/.*.local -- gen_context(system_u:object_r:mcelog_exec_t,s0)
>> >
>> >should probably be bin_t instead of mcelog_exec_t
>>
>> I'd like to confine the type of executable files that mcelog can reach, as that's safer...
>>
>> I have now added the missing can_exec() call, see below...
>>
>> Is there a specific reason for preferring bin_t ?
>
>Generally things like this add complexity with no real benefit. In this
>case since the files are in /etc anyways that argument may not apply.
>
>> >/etc/rc.d/initd/mcelog (escape the periods)
>
>hmm yes seems like your mail app screw it up.
>
>> I think the mail application or the copy+paste did something wrong on that...
>>
>> However the entry for mcelog.pid did not bring the escape sequence, so I've fixed it.
>
>it only seem tp apply on periods in directories. So the period in the
>mcelog.pid file should work without escaping it
>
>> But I think it's not mandatory, as it also works fine without it and there are other similar entries in current policy (if you want to amend them):
>> /var/run/lsassd.pid -- system_u:object_r:lsassd_var_run_t:s0
>> /var/run/lwregd.pid -- system_u:object_r:lwregd_var_run_t:s0
>
>For the /etc/rc.d/init.d/mcelog it is needed, alteast last time i
>tried.
>
>try it out, with and without escaping the periods and see if
>matchpathcon catches /etc/rc.d/init.d/mcelog
I trust it doesn't work, but at the moment, I have no other mail client.
>> >
>> >files_search_etc($1)
>>
>> files_search_etc() should be redundant with read_lnk_files_pattern().
>>
>> If we want to use files_search_etc(), then we should probably use a different equivalent pattern such as:
>>
>> files_search_etc($1)
>> read_files_pattern($1, etc_file_type_t, etc_file_type_t)
>> allow $1 etc_file_type_t:dir list_dir_perms;
>>
>> So, I have now moved to such pattern, which is probably also easier to read, but please let me know if you still believe it isn't right...
>
>I dont agree, i would just add files_search_etc($1) and get it over with
>
>> Client mode needs to { read connectto } the unix_stream_socket that the daemon eventually created using { write }.
>
>That should be allowed with allow mcelog_t self:unix_stream_socket
>create_socket_perms; i believe as ive suggested below.
I think create_socket_perms does not allow "connectto", which is what the client needs, but I'll double-check again.
>> Even introducing the stream_connect_pattern that you suggest, it should still need read permissions on it...
>
>There is no need for a mcelog_stream_connect_pattern() interface in the
>first place. Interfaces are not for internal use.
>
>
>> >
>> >The above isnt needed and wrong (mcelog_t is not a sock_file type), see
>> >below
>>
>> I think you mean it should be mcelog_var_run_t:sock_file and wrong description...
>
>Yes, but even so, its still not needed. interfaces shouldnt be used
>internally
>
>>
>> However, interfaces and their calls have now been redesigned mostly according to your feedback and that is now gone...
>
>> I have also added can_exec() now.
>>
>> Without them, it won't be able to execute the triggers, given they have been labelled mcelog_exec_t, rather than generic bin_t (see above).
>
>I still thinks its not needed but i may be wrong init_daemon_domain()
>makes mcelog_exec_t a application executable file automatically
I'll double-check that too.
>> Removed. As far as I remember, many other modules still keep it after init_daemon_domain(). If confirmed, we might want to do some maintenance cleaning ?
>
>Yes some.
Those redundancies (application_domain and role system_r) might keep getting propagated until they are removed everywhere.
>> >
>> >allow mcelog_t self:unix_stream_socket create_socket_perms;
>>
>> I have created an interface, as we started using them, if you don't mind...
>
>i do mind ;)
>
>interfaces shouldnt be used internally.
I'll remove them, but it will be less easier to read.
>mcelog_t already has full access to mcelog_var_run_t
>
>you just need to add:
>
>allow mcelog_t self:unix_stream_socket create_socket_perms; (or was it
>create_stream_socket_perms? i am not sure, have a look)
>
>>
>> >> miscfiles_read_localization(mcelog_t)
>> >> +
>> >> +# for /dev/mem access
>> >> +mls_file_read_all_levels(mcelog_t)
>> >> +
>> >> +term_use_all_ttys(mcelog_t)
>> >> diff -pruN refpolicy-04062012/policy/modules/kernel/corecommands.fc refpolicy-04062012-mcelog-support/policy/modules/kernel/corecommands.fc
>> >> --- refpolicy-04062012/policy/modules/kernel/corecommands.fc 2012-08-05 04:52:17.194005067 +0200
>> >> +++ refpolicy-04062012-mcelog-support/policy/modules/kernel/corecommands.fc 2012-08-05 17:49:05.594838788 +0200
>> >> @@ -72,12 +72,6 @@ ifdef(`distro_redhat',`
>> >> /etc/kde/shutdown(/.*)? gen_context(system_u:object_r:bin_t,s0)
>> >>
>> >> /etc/mail/make -- gen_context(system_u:object_r:bin_t,s0)
>> >> -/etc/mcelog/.*-error-trigger -- gen_context(system_u:object_r:bin_t,s0)
>> >> -/etc/mcelog/.*.local -- gen_context(system_u:object_r:bin_t,s0)
>>
>> If you leave them there, they would conflict as duplicates with the new private definitions !
>
>i would probably leave it bin_t. i dont have a compelling argument for
>it but i dont have a compelling argument for changing it to
>mcelog_exec_t either.
Do you believe it won't work that way ?
Regards,
Guido
On Mon, 2012-08-06 at 17:38 +0200, Guido Trentalancia wrote:
> I think create_socket_perms does not allow "connectto", which is what the client needs, but I'll double-check again.
Yes i was not sure. Use create_stream_socket_perms instead of
create_socket_perms. That should allow the connectto permission.
> Do you believe it won't work that way ?
It will work fine but that is not the point. The point is that it will
be a change from the usual and for not much benefit if any at all, at
least at the moment.
There is no compelling reason to use mcelog_exec_t.
>
> Regards,
>
> Guido
>