Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BREAKING: Selinux permissive type #183

Merged
merged 4 commits into from
Jan 31, 2017

Conversation

oranenj
Copy link
Contributor

@oranenj oranenj commented Jan 17, 2017

This is a rather simple provider which should fix #165.

This provider is even backwards-compatible, I think, if adding Puppet 4 types doesn't count.

There's a small corner case documented in the README where it's non-idempotent, but I didn't think it was worth complicating the parsing logic to fix.

@vinzent vinzent added the enhancement New feature or request label Jan 17, 2017
@vinzent vinzent added this to the 2.0.0 milestone Jan 17, 2017
Copy link
Contributor

@vinzent vinzent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should introduce this change to 1.0 - we don't have native types in it and thus no expirience how it will behave out there. the predecessor fryman/selinux 4mio downloads on puppet forge. I really like to introduce this in the next major version - so people already using have the clear visibility about the change.

mk_resource_methods

def self.instances
# currently this doesn't distinguish between built-in permissive types of which there are
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is because semange permissive just builds a custom selinux module and loads is if you set one type permissive. if you remove it it removes the custom module - which it can't when the domain is defined permissive in the system policy.

This whole semange tool has lots of what I would call bugs or inconsitencies. :-/

@@ -10,7 +10,8 @@
# }
#
define selinux::permissive (
$context,
String $context,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the param should default to $title if we're at it changing.

what about naming it seltype as in the other renewed types? I think this would be consistent.

@@ -19,10 +20,7 @@
Selinux::Permissive[$title] ->
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removing would need to happen before removing modules because one can not remove a module which has it set to permissive with semanage permissive.

@vinzent
Copy link
Contributor

vinzent commented Jan 17, 2017

added backwards-incomptabile flag because the data types are not puppet3 non-future parser compatible - and that is how most users use puppet3.

@oranenj
Copy link
Contributor Author

oranenj commented Jan 17, 2017

Something is weird with those tests I think.

I see now that it's running the context I added, but for some reason only the last bit, eg.

  on redhat-6-x86_64
    ensure selinux_permissive oddjob_mkhomedir_t is present
      should contain Selinux::Permissive[mycontextp] that comes before Anchor[selinux::end]
    ensure oddjob_mkhomedir_t is absent
      should contain Selinux::Permissive[mycontextp] that comes before Anchor[selinux::module pre]

or does that mean it does run them all but only prints out the last?

@vinzent
Copy link
Contributor

vinzent commented Jan 17, 2017 via email

@vinzent
Copy link
Contributor

vinzent commented Jan 17, 2017 via email

@oranenj
Copy link
Contributor Author

oranenj commented Jan 17, 2017

I suspect it has something to do with all the should expressions being inside one do block.

@vinzent
Copy link
Contributor

vinzent commented Jan 17, 2017 via email

@oranenj
Copy link
Contributor Author

oranenj commented Jan 19, 2017

my local rspec runs didn't give me those errors. Hm.

It's probably not kosher to reuse the let'd provider like that, but I'm not sure how to best fix it.

@oranenj oranenj force-pushed the selinux_permissive_type branch from dd2069f to b9130c3 Compare January 21, 2017 12:51
@oranenj
Copy link
Contributor Author

oranenj commented Jan 21, 2017

I wonder what's causing the rspec test to fail. They work on my local machine.

Maybe it's the ruby version? Hmh

@oranenj oranenj force-pushed the selinux_permissive_type branch from b9130c3 to 8c40a6b Compare January 21, 2017 18:03
@oranenj
Copy link
Contributor Author

oranenj commented Jan 21, 2017

Rebased and added some acceptance tests that pass on CentOS 6 at least. Still not sure if travis rspec will pass, though.

Warning: the acceptance tests are super slow because any operation with semanage seems to take ~20-30 seconds. They take about 6-7 minutes for the selinux::permissive spec

@oranenj oranenj force-pushed the selinux_permissive_type branch from c55c66e to 740e0ec Compare January 21, 2017 18:29
@oranenj
Copy link
Contributor Author

oranenj commented Jan 21, 2017

nil is the bane of my existence. It's not really fun to debug these test failures given that I only seem to be able to reproduce them via travis...

@vinzent
Copy link
Contributor

vinzent commented Jan 21, 2017 via email

@vinzent
Copy link
Contributor

vinzent commented Jan 30, 2017

tried to run with centos 5.11 :)

of course it failed. altough metadata.json states centos5 support i suspect at least selinux::module and selinux::permissive do not work at all. but this isn't related to this change.

  Info: /Stage[main]/Main/Selinux::Module[puppet_selinux_test_policy]/File[/usr/share/selinux/puppet_selinux_test_policy.te]: Scheduling refresh of Exec[/usr/share/selinux/puppet_selinux_test_policy.pp]
  Notice: /Stage[main]/Main/Selinux::Module[puppet_selinux_test_policy]/Exec[/usr/share/selinux/puppet_selinux_test_policy.pp]/returns: make: /usr/share/selinux/devel/Makefile: No such file or directory

or

      Failure/Error: shell('semanage permissive -a kernel_t')
      Beaker::Host::CommandFailure:
        Host 'centos-511-x64' exited with 1 running:
         semanage permissive -a kernel_t
        Last 10 lines of output were:
        		-M, --mask       Netmask
        		-P, --prefix     Prefix for home directory labeling
        		-L, --level      Default SELinux Level (MLS/MCS Systems only)
        		-R, --roles      SELinux Roles (ex: "sysadm_r staff_r")
        	
        		-s, --seuser     SELinux User Name
        		-t, --type       SELinux Type for the object
        		-r, --range      MLS/MCS Security Range (MLS/MCS Systems only)
        	
        	permissive not defined

@vinzent
Copy link
Contributor

vinzent commented Jan 30, 2017

added issue to remove centos 5 support: #190

@oranenj oranenj force-pushed the selinux_permissive_type branch from 127fc16 to 8094b16 Compare January 31, 2017 16:49
@oranenj
Copy link
Contributor Author

oranenj commented Jan 31, 2017

I fixed the beaker tests, I think.

Copy link
Contributor

@vinzent vinzent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

successfully ran acceptance tests on centos 6, centos 7, fedora 24 and fedora 25.

@vinzent vinzent merged commit 0554cd3 into voxpupuli:master Jan 31, 2017
@vinzent vinzent changed the title Selinux permissive type BREAKING: Selinux permissive type Mar 29, 2017
EmRowlands pushed a commit to EmRowlands/puppet-selinux that referenced this pull request Mar 29, 2023
cegeka-jenkins pushed a commit to cegeka/puppet-selinux that referenced this pull request Jan 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can't remove permissive domain
2 participants