Fixes #9126 - moved Katello policy to a separate repo #44
Conversation
|
@domcleal mind merging if that's sane so we can release theforeman/katello-selinux#3 Thank you. |
| @@ -7,11 +7,6 @@ set +e | |||
| for selinuxvariant in targeted | |||
| do | |||
| if /usr/sbin/semodule -s $selinuxvariant -l >/dev/null; then | |||
| # Remove all user defined ports (including the default one) | |||
There was a problem hiding this comment.
We need to remove this on upgrade somehow... maybe move it to the enable script?
There was a problem hiding this comment.
I was thinking to leave the port as is (defined) but good point. I am not sure if SELinux will like the fact the type is now defined in a different policy. Let me test.
There was a problem hiding this comment.
That's right. It will not upgrade:
Loading targeted modules: foreman-proxy foreman katello
libsepol.context_from_record: type elasticsearch_port_t is not defined (No such file or directory).
libsepol.context_from_record: could not create context structure (Invalid argument).
libsepol.port_from_record: could not create port structure for range 9200:9300 (tcp) (Invalid argument).
libsepol.sepol_port_modify: could not load port range 9200 - 9300 (tcp) (Invalid argument).
libsemanage.dbase_policydb_modify: could not modify record value (Invalid argument).
libsemanage.semanage_base_merge_components: could not merge local modifications into policy (Invalid argument).
/usr/sbin/semodule: Failed!
Therefore we need to remove the ports on upgrade.
lzap
force-pushed
the
remove-katello-9126
branch
from
a2471c0
to
9224c3e
Compare
|
So leaving it in enable script seems right, good plan. We can remove for 1.9 version (left a note there). Also fixed Makefile - now I use the remote-load for both policies and files were not being deleted. We need to require specific version of foreman-selinux from katello-selinux. Shell I bump a minor version or something? |
|
I mean I could bump release to 1: |
|
Wait, it works for the first upgrade, but then when katello module is loaded, we no longer want to remove the elasticsearch port. I am thinking of adding check for "katello" module presence (semodule list) and only remove when katello is not present. What you think? |
|
The issue we see here is actually quite ugly - a port and number assignement cannot be redefined. I think a solution would be to introduce special port type What you think of this? (See my mail to the list). This way we can solve this particular situation with I can make the changes there and just remove the elasticsearch_port_t here forever. No "katello" check is needed. Although this is not that relevant for this PR, we have a Docker PR opened for this repo and that would be relevant. I would like to introduce the special port for those "workarounds for RHEL6". Opinion? Thanks. |
|
I don't think the port issue particularly matters here, that's an issue for the docker PR and when you replace it in the katello-selinux PR. It's probably best to remove it when the katello module isn't present as you suggested earlier, then this allows you to put it back in the katello module if you wish. Typically you'd just add a dependency on >= 1.8.0 in a package, we don't ever attempt to version inside nightlies as they're in continuous development. |
lzap
force-pushed
the
remove-katello-9126
branch
2 times, most recently
from
4dc046c
to
b50c88e
Compare
|
Rebased, amended the change. Updated the upgrade The only drawback is that We could do this in |
|
Interesting problem! However I disagree that katello-selinux would always be installed first. If katello-selinux depends on foreman-selinux, then the package manager should upgrade foreman-selinux first. I tested the order of the scriptlets on EL7 and see that it runs pre/post for each package in turn, so this means foreman-selinux-enable will run before katello-selinux-enable (I presume), which means we might be able to remove the port declaration before the katello policy is loaded? |
|
So actually, re-reading your patch, I don't see why it wouldn't work right now, assuming katello-selinux depends on foreman-selinux. |
|
Yeah I was thinking bad - what you said seems to be correct. Therefore no additonal snippets are necessary and this should work as is. 👍 |
|
Can you link scratch builds? I think it'd be a good idea to test. |
lzap
force-pushed
the
remove-katello-9126
branch
from
b50c88e
to
fa1570f
Compare
|
Rebased because of tags. http://koji.katello.org/koji/taskinfo?taskID=230896 |
|
|
Sorry, I meant that it works with katello-selinux installation/upgrade too? |
|
Okay, now testing this setup:
|
Hmmmm |
|
Here is my plan: in the enable script, if katello module was loaded, then unload it, remove the port, load it back. |
|
I'm not entirely sure I follow, but questioning the setup you tested, does katello-selinux exist right now? And does it define elasticsearch_port_t? I'd expect you to either upgrade both katello-selinux and foreman-selinux in one transaction, so the port moves from one to the other, or to be upgrading foreman-selinux and installing katello-selinux for the first time. |
|
The problem is users will not be upgrading katello and foreman in one transaction. They will be upgrading foreman and installing (new package) for katello. |
|
This is the current tree (master -> nightly): https://github.com/Katello/katello-selinux |
|
Wouldn't an upgrade + install be in a single yum transaction? I'm not sure why it would be separate. |
|
Right let me retest with a single transaction, need to create local repo for that. |
|
Setup is not easy, made two mistakes, third try... :-P |
I should also suggest "yum shell" which lets you do "install /foo.rpm", "upgrade bar" and then "run". |
|
NOOOOOO I am setting things up for third time now... omfg |
|
Does not work, after transaction the port is missing completely (the type). RHEL6: Very same result on RHEL7. |
|
Does the katello policy you're testing redefine the elasticsearch_port_t type? It isn't in katello-selinux's master branch, but I don't know if you're testing locally with some other changes. https://github.com/theforeman/foreman-selinux/blob/1.8.0-RC1/foreman.te#L405 is the definition in our policy which you're correctly removing in this PR, but it will then need redefining in katello.te instead. |
|
Yeah I am testing that currently, it looks like we need a change in katello-selinux, hold on. |
|
Ok tested but one change is needed on the katello side. I haven't realized I actually do not create the type anymore there: #44 This is test from EL7, the very same result on EL6: |
| # If katello policy was not yet loaded and elasticsearch port is still defined, | ||
| # remove it (it was moved to katello policy). Katello policy will not load before | ||
| # we remove the port definition (because SELinux prevents that). | ||
| semodule -l | grep -q katello || \ |
There was a problem hiding this comment.
Is katello-selinux already released? It sounds like it has been, which could be an issue.
If the existing katello module is currently loaded, then this won't remove elasticsearch_port_t. You might need to handle it a bit more carefully, as if the package has been released, existing versions won't have added the port and you still need to remove it.
There was a problem hiding this comment.
It will work because released katello selinux (2.2) does not define any type. Only the fix which was merged recently introduces it.
Several nightly installation upgrades might be broken since the patch already made it to katello nightlies. I was hoping to shorten this window by merging this as well. Haven't checked github messages for few days now.
There was a problem hiding this comment.
Oh the patch theforeman/katello-selinux#6 was not yet merged, that's even better. Current katello-selinux implementation is harmless, it does not define the type, only requires it.
There was a problem hiding this comment.
Requiring it is a problem though. As the current released katello-selinux requires it without defining it, then this will happen I think:
- foreman-selinux-enable runs
- does not remove elasticsearch_port_t as 'katello' module is loaded
- loads new 'foreman' module which no longer defines it, this will throw an error as the 'katello' policy references it
While what should have happened, if the current released katello-selinux wasn't using it or didn't exist (the scenario you tested) is:
- foreman-selinux-enable runs
- removes elasticsearch port range as 'katello' isn't loaded
- loads new 'foreman' module which no longer defines it
- katello-selinux-enable runs
- loads new 'katello' module which defines it
- adds elasticsearch_port_t range
There was a problem hiding this comment.
The katello has all elasticsearch things in optional block, let me retest with the scenario you just described. I think it should work.
There was a problem hiding this comment.
It does not:
[root@fsix ~]# rpm -Uvh --force https://fedorapeople.org/groups/katello/releases/yum/nightly/katello/RHEL/6Server/x86_64/katello-selinux-2.2.1-1.el6.noarch.rpm
Retrieving https://fedorapeople.org/groups/katello/releases/yum/nightly/katello/RHEL/6Server/x86_64/katello-selinux-2.2.1-1.el6.noarch.rpm
Preparing... ########################################### [100%]
1:katello-selinux ########################################### [100%]
[root@fsix ~]# rpm -Uvh --force "http://koji.katello.org/koji/getfile?taskID=236410&name=foreman-selinux-1.9.0-0.develop.el6.noarch.rpm"
Retrieving http://koji.katello.org/koji/getfile?taskID=236410&name=foreman-selinux-1.9.0-0.develop.el6.noarch.rpm
Preparing... ########################################### [100%]
1:foreman-selinux ########################################### [100%]
libsepol.context_from_record: type elasticsearch_port_t is not defined (No such file or directory).
libsepol.context_from_record: could not create context structure (Invalid argument).
libsepol.port_from_record: could not create port structure for range 9200:9300 (tcp) (Invalid argument).
libsepol.sepol_port_modify: could not load port range 9200 - 9300 (tcp) (Invalid argument).
libsemanage.dbase_policydb_modify: could not modify record value (Invalid argument).
libsemanage.semanage_base_merge_components: could not merge local modifications into policy (Invalid argument).
/usr/sbin/semanage: Could not commit semanage transaction
There was a problem hiding this comment.
Ok is the right solution to remove katello module during the upgrade?
- foreman-selinux-enable runs
- remove katello module (katello-selinux-disable)
- removes elasticsearch port range
- loads new 'foreman' module which no longer defines it
- katello-selinux-enable runs
- loads new 'katello' module which defines it
- adds elasticsearch_port_t range
There was a problem hiding this comment.
It works:
[root@fsix ~]# /usr/sbin/katello-selinux-disable
[root@fsix ~]# rpm -Uvh --force "http://koji.katello.org/koji/getfile?taskID=236410&name=foreman-selinux-1.9.0-0.develop.el6.noarch.rpm"
Retrieving http://koji.katello.org/koji/getfile?taskID=236410&name=foreman-selinux-1.9.0-0.develop.el6.noarch.rpm
Preparing... ########################################### [100%]
1:foreman-selinux ########################################### [100%]
There was a problem hiding this comment.
It might work, but we'd have to enable it after too? (as katello-selinux may not be upgraded)
There was a problem hiding this comment.
What about not using elasticsearch_port_t in katello-selinux, so we remove all traces of it here, and then that package uses an entirely different name, like katello_allowed_ports_t.
|
Looks like elasticsearch was removed from katello nightly now, so we can proceed with this patch now. I will rebase it. |
lzap
force-pushed
the
remove-katello-9126
branch
from
fa1570f
to
8aabb0e
Compare
|
Rebased, things are much more simple. I added a block both here and in Katello to remove user-defined elasticsearch_port_t types, because we no longer need it. We need to have at least one release with this upgrade command, then we can get rid of it. |
| /usr/sbin/semanage port -E | \ | ||
| grep elasticsearch_port_t | \ | ||
| sed s/-a/-d/g | \ | ||
| /usr/sbin/semanage -S $selinuxvariant -i - |
There was a problem hiding this comment.
Nitpick, if you can only run semanage -i here if the port exists then it'll save a lot of installation time (on every single patch release for every user) as these calls are expensive. Probably changing the mktemp call to add -d, then save the port -E output to only call it when required.
You could even re-use the port -E output below, which should keep the speed the same.
There was a problem hiding this comment.
Sure, I directly added that to the semanage temp file which should work just fine.
lzap
force-pushed
the
remove-katello-9126
branch
from
8aabb0e
to
daa07a4
Compare
|
Amended, if you can do me a favor and only merge this after the openstack PR #45 since the other one is priority for me, there will be conflicts. Thanks! |
| # Load policy | ||
| /usr/sbin/semanage module -S $selinuxvariant \ | ||
| -a /usr/share/selinux/${selinuxvariant}/foreman.pp.bz2 | ||
|
|
||
| echo "boolean -m --on httpd_setrlimit" > $TMP | ||
|
|
||
| /usr/sbin/semanage port -E | grep -q elasticsearch_port_t || \ | ||
| echo "port -a -t elasticsearch_port_t -p tcp 9200-9300" >> $TMP | ||
|
|
||
| /usr/sbin/semanage port -E | grep -q docker_port_t || \ |
There was a problem hiding this comment.
Note to myself - I will add TMP_PORTS for all statements, including the one that will appear after I rebase this on top of develop.
|
Could you rebase this PR please? |
|
Sure thing. Done! |
|
|
||
| /usr/sbin/semanage port -E | grep -q docker_port_t || \ | ||
| echo "port -a -t docker_port_t -p tcp 2375-2376" >> $TMP | ||
| grep docker_port_t $TMP_PORTS | echo "port -a -t docker_port_t -p tcp 2375-2376" >> $TMP |
There was a problem hiding this comment.
Should | be || here, so as to create the port only when grep returns a bad error code? I don't think the output of grep needs piping. Also prefer grep -q.
| # Load policy | ||
| /usr/sbin/semanage module -S $selinuxvariant \ | ||
| -a /usr/share/selinux/${selinuxvariant}/foreman.pp.bz2 | ||
| /usr/sbin/semanage port -E > $TMP_PORTS |
There was a problem hiding this comment.
The redirection here doesn't seem to work on EL6 with policycoreutils-python-2.0.83-24.el6.
# rpm -Uvh foreman-selinux-1.12.0-0.develop.201605040716gitb1daca47.el6.noarch.rpm
Preparing... ########################################### [100%]
1:foreman-selinux ########################################### [100%]
libsepol.context_from_record: type elasticsearch_port_t is not defined (No such file or directory).
libsepol.context_from_record: could not create context structure (Invalid argument).
libsepol.port_from_record: could not create port structure for range 9200:9300 (tcp) (Invalid argument).
libsepol.sepol_port_modify: could not load port range 9200 - 9300 (tcp) (Invalid argument).
libsemanage.dbase_policydb_modify: could not modify record value (Invalid argument).
libsemanage.semanage_base_merge_components: could not merge local modifications into policy (Invalid argument).
/usr/sbin/semanage: Could not commit semanage transaction
/usr/sbin/semanage: Port tcp/2375-2376 already defined
warning: %post(foreman-selinux-1.12.0-0.develop.201605040716gitb1daca47.el6.noarch) scriptlet failed, exit status 1
[ edited it and added a cat, uncommented the trap ]
# bash -x /usr/sbin/foreman-selinux-enable
+ set +e
++ mktemp -t foreman-selinux-enable.XXXXX
+ TMP_EXEC_BEFORE=/tmp/foreman-selinux-enable.56Ryo
++ mktemp -t foreman-selinux-enable.XXXXX
+ TMP_EXEC_AFTER=/tmp/foreman-selinux-enable.BhL8L
++ mktemp -t foreman-selinux-enable.XXXXX
+ TMP_PORTS=/tmp/foreman-selinux-enable.9VQCl
+ for selinuxvariant in targeted
+ /usr/sbin/semodule -s targeted -l
+ /usr/sbin/semanage port -E
+ cat /tmp/foreman-selinux-enable.9VQCl
+ sed s/-a/-d/g
+ grep elasticsearch_port_t /tmp/foreman-selinux-enable.9VQCl
+ echo 'boolean -m --on httpd_setrlimit'
+ grep -q docker_port_t /tmp/foreman-selinux-enable.9VQCl
+ echo 'port -a -t docker_port_t -p tcp 2375-2376'
+ is_redhat_6
++ rpm -q --whatprovides redhat-release --qf '%{version}'
+ test x6 = x6
+ grep -q foreman_osapi_compute_port_t /tmp/foreman-selinux-enable.9VQCl
+ echo 'port -a -t foreman_osapi_compute_port_t -p tcp 8774'
+ test -s /tmp/foreman-selinux-enable.56Ryo
+ /usr/sbin/semanage module -S targeted -a /usr/share/selinux/targeted/foreman.pp.bz2
libsepol.context_from_record: type elasticsearch_port_t is not defined (No such file or directory).
libsepol.context_from_record: could not create context structure (Invalid argument).
libsepol.port_from_record: could not create port structure for range 9200:9300 (tcp) (Invalid argument).
libsepol.sepol_port_modify: could not load port range 9200 - 9300 (tcp) (Invalid argument).
libsemanage.dbase_policydb_modify: could not modify record value (Invalid argument).
libsemanage.semanage_base_merge_components: could not merge local modifications into policy (Invalid argument).
/usr/sbin/semanage: Could not commit semanage transaction
+ test -s /tmp/foreman-selinux-enable.BhL8L
+ /usr/sbin/semanage -S targeted -i /tmp/foreman-selinux-enable.BhL8L
/usr/sbin/semanage: Port tcp/2375-2376 already defined
# ll /tmp/foreman-selinux-enable.9VQCl
-rw-------. 1 root root 0 May 4 08:44 /tmp/foreman-selinux-enable.9VQCl
# semanage port -E
port -a -t docker_port_t -p tcp 2375-2376
port -a -t elasticsearch_port_t -p tcp 9200-9300
# semanage port -E > /tmp/a
# ls -l /tmp/a
-rw-r--r--. 1 root root 0 May 4 08:46 /tmp/a
# semanage port -E | tee /tmp/a
port -a -t docker_port_t -p tcp 2375-2376
port -a -t elasticsearch_port_t -p tcp 9200-9300
# ls -l /tmp/a
-rw-r--r--. 1 root root 91 May 4 08:47 /tmp/a
There was a problem hiding this comment.
I think the issue is simply SELinux with fd redirection - semanage runs in a context where it can't write to user_tmp_t. Switching to permissive allows it.
There was a problem hiding this comment.
I have just built on CentOS 6 (latest stable with updates) and I don't see any errors there. This was a clean install, therefore clean upgrade of the RPM package that I've built. No errors at all. policycoreutils-python-2.0.83-24.el6.x86_64
There was a problem hiding this comment.
Thanks for checking, it seems to have been added to the base policy between selinux-policy-targeted-3.7.19-195.el6 (EL6.4) and selinux-policy-targeted-3.7.19-231.el6 (EL6.5), so it's just my out of date installation. That's fine, we should just bump the min version in the RPM after this is merged.
No description provided.