On 04/11/2014 02:10 PM, Jan Synacek wrote:
Hello,
I have finished the first version of the SELinux model for OpenLMI [1].
Suggestions and comments are welcome.
Jan, it's a great start. There are some rough edges here and there, still it's the best CIM interface design OpenLMI has ever had.
Few comments:
- LMI_SELinuxBoolean: - suggestion: s/Currently set state/Current state/, - suggestion: s/State value that is set persistently across reboots/State on next system boot/
- LMI_SELinuxBoolean: It should have a Description property, as shown by 'semanage boolean -l', SELinux might have API for that somewhere. And if not, there is /usr/share/selinux/devel/policy.xml.
- LMI_SELinuxFile: we already have context and expected context as properties of LMI_UnixFile, it does not seem appropriate to provide them again in LMI_SELinuxFile (rendering LMI_SELinuxElementWithContext practically useless...) - also, enumeration of this class must be disabled, which makes creation of references pretty hard.
- LMI_SELinuxService.State: it must have Disabled value.
- LMI_SELinuxService: I would rename State and DefaultState to SELinuxState and SELinuxDefaultState, it looks confusing to have Enabled/EnabledDefault from CIM_Service and State and DefaultState for SELinux. - The same for LMI_SELinuxService.Type... PolicyType? SELinuxPolicyType?
- LMI_SELinuxService.Type: is it possible to have any other policy than Targeted/Minimum/MLS? Like a customer having its own policy, 'Unknown' value would be appropriate here. Better ask some SELinux guru. What policies do other distros ship? - suggestion: s/SELinux type/SELinux policy type/
- LMI_SELinuxService.PolicyVersion: is it really uint32? string might be more appropriate.
- LMI_SELinuxService.SetState: it's not possible to set state on the next boot without changing current SELinux state. E.g. this is not possible: I want Enforcing on next boot, while staying Permissive for now. It might be better to have SetState(uint16 NewState, uint16 NewDefaultState), where NULL would mean no change of the respective state.
- LMI_SELinuxService.SetState: What happens when e.g. runtime state setting succeeds, but setting the default state fails? Caller then does not know what's part of the operation succeeded/failed. Maybe have two separate functions? Or can we expect that the applications are smart enough to reload the actual state on failure? Some note in method description would be appropriate.
- LMI_SELinuxService.SetPortLabel: it should accept port number as string, in case someone wants to label a port, which is not yet in the policy and thus there is no LMI_SELinuxPort for it. And it should accept the string as range.
- LMI_SELinuxService.SetPortLabel: there should be a way how to remove a label on a port, either in SetPortLabel or in separate method.
- LMI_SELinuxService.SetFileLabel: not sure if simple string would be enough as Target... it's quite hard to get proper file reference.
- LMI_SELinuxService.GetMislabeledFiles(): It's not clear how the method returns list of files. Some LMI_SELinuxFileJob + its association to files is missing?
- LMI_SELinuxService.RestoreLabel: it should also export list of changes it did, in the very same way as GetMislabeledFiles() should do. Maybe there could be just one method with two modes - dry run / normal run?
- I am not sure about ports design. It's hard to say how a port will be labelled, it's all hidden in strings with ranges. If it's just generic overview of current policty then it's probably fine. I just have a gut feeling it's too much low level and we may hit some limitation in the future.
- I am also not sure about its behavior, having let's say snmp_port_t/tcp: 161-162, 199, 1161 and xserver_port_t/tcp: 6000-6020, what happens if I label 1161 with xserver_port_t? - will it (silently, no indications) remove the port from snmp_port_t and add it to xserver_port_t? - or it just adds 1161 to xserver_port_t, having the port with both labels? (semanage allows that) - some note in the description would be appropriate
Jan