-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
Hello,
I have finished the first version of the SELinux model for OpenLMI [1].
Suggestions and comments are welcome.
[1] http://jsynacek.fedorapeople.org/openlmi/selinux/
Cheers, - -- Jan Synacek Software Engineer, Red Hat
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 04/11/2014 08:10 AM, Jan Synacek wrote:
Hello,
I have finished the first version of the SELinux model for OpenLMI [1].
Suggestions and comments are welcome.
I've just read through the design document and I don't see any obvious issues with the design. The only thing I might note are intentional non-goals of the provider. Specifically, it would be good to note that there is no intention to enable the modification of policy (except for enforcing/permissive/disabled and boolean-flipping).
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 04/11/2014 04:41 PM, Stephen Gallagher wrote:
I've just read through the design document and I don't see any obvious issues with the design. The only thing I might note are intentional non-goals of the provider. Specifically, it would be good to note that there is no intention to enable the modification of policy (except for enforcing/permissive/disabled and boolean-flipping).
Added to the Design section, thank you.
- -- Jan Synacek Software Engineer, Red Hat
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
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 04/14/2014 02:13 PM, Jan Safranek wrote:
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.
Thank you!
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/
Fixed.
- 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.
It already has a Description property (inherited from CIM_ManagedElement), but I didn't explicitly show it in Example class instances.
Fixed.
However, note that the description strings are pretty useless, they seem to be somehow generated, at least on my machine. For example:
httpd_can_network_connect (on , on) Allow httpd to can network connect
- 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.
I wanted the file class to fit into the whole model -- inherit from LMI_SELinuxElement. I know about the contexts in LMI_UnixFile and I wanted to remove them from there. The best solution would probably be if LMI_UnixFile inherited from LMI_SELinuxElement, which would be multiple inheritance and that is not allowed.
Since the path to a file is a primary key, it is common sense and the simplest solution to use paths to get references. InstanceID can be used for that. It may seem like a hack at first, and certainly not the CIM way, but, in my opinion, is the best solution.
As a bonus, if there is no SELinux on the managed machine, no SELinux* classes will be present and everything will fit.
I'll update the document to reflect the above.
- LMI_SELinuxService.State: it must have Disabled value.
Fixed.
- 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?
I'll go with 'SELinux' prefix for the states.
PolicyType sounds ok. I don't think it's necessary to prefix with 'SELinux' here.
- 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?
There are some commonly defined ones:
SUSE: Targeted, MLS Fedora/RHEL: Targeted, Minimum, MLS Gentoo: Targeted, Strict, MCS, MLS Debian: Targeted, Strict, MLS
I will just use a string for the PolicyType. It doesn't seem worth it to use an enumeration of common ones and OtherOptions for the rest in case there are any.
- suggestion: s/SELinux type/SELinux policy type/
Fixed.
- LMI_SELinuxService.PolicyVersion: is it really uint32? string might be
more appropriate.
Yes, see man security_policyvers(3).
- 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.
Well, you have to call it twice:) Your suggestion seems to be better...
...but, see below.
- 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.
You don't have to solve this kind of a problem with my original proposal, that is, to use SetState() to set only either of the states at once. That makes the operation atomic.
I'll leave it as it is for now.
- 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.
Added.
- LMI_SELinuxService.SetPortLabel: there should be a way how to remove a
label on a port, either in SetPortLabel or in separate method.
Added 'boolean Remove' parameter.
- LMI_SELinuxService.SetFileLabel: not sure if simple string would be
enough as Target... it's quite hard to get proper file reference.
I'll use paths instead of references to file objects, see above.
- LMI_SELinuxService.GetMislabeledFiles(): It's not clear how the method
returns list of files. Some LMI_SELinuxFileJob + its association to files is missing?
This is what LMI_AffectedSELinuxJobElement was added for. And yes, I forgot a job class.
Added LMI_SELinuxJob.
- 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?
Good point. Redesigned.
- 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'm trying to mimic what 'semanage port' does. However, I got it a bit wrong. AFAIK, you can't change the SELinux port definitions without changing policy. So, an SELinux port has a label defined by the policy, a protocol type and one or more actual network ports. One or more labels can be assigned to one or more actual ports. I don't think it's too low level, it's pretty much 1:1 to what 'semanage port' does.
- 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)
You will have both.
One LMI_SELinuxPort with InstanceID=="LMI:SELinuxPort:TCP:snmp_port_t" and Ports==["161-162", "199", "1161"]
and the other one with InstanceID=="LMI:SELinuxPort:TCP:xserver_port_t" and Ports==["1161", "6000-6020"]
- some note in the description would be appropriate
Added to the description of Ports[] property.
I also added the most important remarks to the Design section.
Cheers, - -- Jan Synacek Software Engineer, Red Hat
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 04/17/2014 10:23 AM, Jan Synacek wrote:
However, note that the description strings are pretty useless, they seem to be somehow generated, at least on my machine. For example:
httpd_can_network_connect (on , on) Allow httpd to can network connect
I just found out what was happening. I had to install selinux-policy-devel to get meaningful description, because they are pulled from /usr/share/selinux/devel/policy.xml. Who knew...
Now I get:
httpd_can_network_connect (on , on) Allow HTTPD scripts and modules to connect to the network using TCP.
- -- Jan Synacek Software Engineer, Red Hat
The second version looks production-ready, just two remarks:
On 04/17/2014 10:23 AM, Jan Synacek wrote:
- 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.
I wanted the file class to fit into the whole model -- inherit from LMI_SELinuxElement. I know about the contexts in LMI_UnixFile and I wanted to remove them from there. The best solution would probably be if LMI_UnixFile inherited from LMI_SELinuxElement, which would be multiple inheritance and that is not allowed.
We cannot easily remove stuff from already published LogicalFile provider, it would need to bump major version for that.
Stick to the attributes in LMI_UnixFile and don't create redundant ones in SELinux provider.
- 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?
Good point. Redesigned.
LMI_SELinuxService.ProcessTarget() is really bad name, all it can do is to check and optionally restore labels on mislabeled files, what about:
uint32 CheckFiles(string Target, uint16 Action('fix'/'report only'), boolean Recursively, [out] Job)
Jan
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 04/23/2014 03:31 PM, Jan Safranek wrote:
The second version looks production-ready, just two remarks:
On 04/17/2014 10:23 AM, Jan Synacek wrote:
- 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.
I wanted the file class to fit into the whole model -- inherit from LMI_SELinuxElement. I know about the contexts in LMI_UnixFile and I wanted to remove them from there. The best solution would probably be if LMI_UnixFile inherited from LMI_SELinuxElement, which would be multiple inheritance and that is not allowed.
We cannot easily remove stuff from already published LogicalFile provider, it would need to bump major version for that.
Stick to the attributes in LMI_UnixFile and don't create redundant ones in SELinux provider.
Fixed.
- 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?
Good point. Redesigned.
LMI_SELinuxService.ProcessTarget() is really bad name, all it can do is to check and optionally restore labels on mislabeled files, what about:
uint32 CheckFiles(string Target, uint16 Action('fix'/'report only'), boolean Recursively, [out] Job)
Ok, I changed the name to RestoreLabels, so it has "Restore" in the name, as "restorecon" does.
- -- Jan Synacek Software Engineer, Red Hat
On 04/24/2014 01:08 PM, Jan Synacek wrote:
On 04/23/2014 03:31 PM, Jan Safranek wrote:
The second version looks production-ready, just two remarks:
On 04/17/2014 10:23 AM, Jan Synacek wrote:
- 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.
I wanted the file class to fit into the whole model -- inherit from LMI_SELinuxElement. I know about the contexts in LMI_UnixFile and I wanted to remove them from there. The best solution would probably be if LMI_UnixFile inherited from LMI_SELinuxElement, which would be multiple inheritance and that is not allowed.
We cannot easily remove stuff from already published LogicalFile provider, it would need to bump major version for that.
Stick to the attributes in LMI_UnixFile and don't create redundant ones in SELinux provider.
Fixed.
- 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?
Good point. Redesigned.
LMI_SELinuxService.ProcessTarget() is really bad name, all it can do is to check and optionally restore labels on mislabeled files, what about:
uint32 CheckFiles(string Target, uint16 Action('fix'/'report only'), boolean Recursively, [out] Job)
Ok, I changed the name to RestoreLabels, so it has "Restore" in the name, as "restorecon" does.
Cool, thanks a lot for the design! I'm looking forward for implementation :-).
Jan
openlmi-devel@lists.stg.fedorahosted.org
-
Jan Safranek -
Jan Synacek -
Stephen Gallagher