https://bugzilla.redhat.com/show_bug.cgi?id=844013 --- Comment #10 from Michael Scherer <misc@xxxxxxxx> --- * I still think the permission on all file except logs should not be such as apache could write them. If there is a security issue in apache ( like a php application gone crazy and letting anyone upload in any file, installed on the same server ), that mean the attacker would be able to write ruby code there, and then execute it by doing request on openshift, thus elevating his privileges. * I can see why the boolean are turned on, except for httpd_enable_homedirs, who permit to read from ~/public_html/, and that's curious that openshift would rely on this. I also see a boolean httpd_run_stickshift in man pages, maybe this would be helpful to turn it on ( not sure, please check with a real selinux specialist ) * The boolean are reset on each upgrade, thus erasing the settings changed by admin. That do not sound like a great idea IMHO. * I am also not sure that replacing the policy of passenger is the right thing to do. What if I deployed another application using it on the same server ? According to readme, this is because mod_passenger from phusion site ship a policy that do not play well with openshift. Wouldn't it be better to fix the policy of mod_passenger to not requires such hacks ( especially now the rpm is in fedora ) * %config(noreplace) %{brokerdir}/config/environments/production.rb I would try to put them in /etc, so someone doing a backup of /etc would get them ( principle of least surprise ). Of course, you need to a do a link somewhere. * that's redundant, since 0644 is already the default %attr(0644,-,-) %{_unitdir}/%{name}.service %attr(0644,-,-) %config(noreplace) %{_sysconfdir}/sysconfig/%{name} * %attr(0664,-,-) %ghost %{brokerdir}/log/production.log do we want any process in apache group be able to write there ? * after compiling the selinux policy , /usr/share/selinux/packages/stickshift-broker/stickshift-broker.pp end as being unowned ( and untracked ). this should be written as %ghost IMHO. There is also no filecontext ( file .fc ) to make sure file are of the proper type, so I would recommend to add that in the policy instead of adding it with semanage fcontext in %post * as a side note, the selinux policy still speak of oddjob, despites being obsoleted. And the policy speak of stickshift_t and stickthisft_exec_t type, but nothing is labeled with this in this rpm nor on openshift-origin-controller, so I think this should be revised or dropped, cause it doesn't seems to change anything ( but I am a novice in selinux policy ) * there is now macro for systemd : https://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Systemd * since openshift-broker is network facing, it should not be started by default https://fedoraproject.org/wiki/Starting_services_by_default ( ie, no chkconfig on ) * various cp invocation should use -p, to preserve the modification time https://fedoraproject.org/wiki/Packaging:Guidelines#Timestamps * those mkdir are not needed : mkdir -p %{buildroot}%{_sysconfdir}/oddjobd.conf.d mkdir -p %{buildroot}%{_sysconfdir}/dbus-1/system.d mkdir -p %{buildroot}%{_bindir} and this one is redundant (already done by all other thanks to -p ) : mkdir -p %{buildroot}%{brokerdir} theses one are also duplicated : mkdir -p %{buildroot}%{_var}/lib/stickshift mkdir -p %{buildroot}%{appdir} since appdir is /var/lib/stickshift. I guess you can remove appdir from the spec and keep var/lib/, more readable. ( and so remove the define too ) * better written as 1 line : ( ie directly move and rename with 1 mv ) mv %{buildroot}%{brokerdir}/init.d/* %{buildroot}%{_initddir} mv -f %{buildroot}%{_initddir}/stickshift-broker %{buildroot}%{_initddir}/%{name} -- You are receiving this mail because: You are on the CC list for the bug. _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review