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

Introduce --without-procctl to explicitly disable procctl hardening #2128

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

rcmcdonald91
Copy link
Contributor

@rcmcdonald91 rcmcdonald91 commented Apr 10, 2023

This knob explicitly disables procctl hardening support on platforms that provide it (i.e. FreeBSD). This build option is necessary in order for child processes spawned by scripts to be reparented to the system default repear (init) instead of being explicitly killed. Under normal circumstances pkg should not be starting long-lived processes. This is a nice-to-have knob for projects that would like to use the pkg infrastructure and need a way to restart daemons, etc.

If this is accepted, we will also want to expose this knob in the FreeBSD port as well.

@kevans91
Copy link
Contributor

This ended up with an extra commit that I don't think you intended, the first just removes a line that the second re-adds. You should git rebase -i HEAD~2 this branch and squash the second into the first.

@rcmcdonald91
Copy link
Contributor Author

oooof thanks, fixed.

@bapt
Copy link
Member

bapt commented Apr 19, 2023

There is an option for pkg to restart the daemons: HANDLE_RC_SCRIPTS which imho should end up becoming a trigger that react on rc.d addition.

Isn't it enough?

If we ever end up removing procctl I would perfer a runtime option (aka something in pkg.conf) rather that a build time one.

@rcmcdonald91
Copy link
Contributor Author

There is an option for pkg to restart the daemons: HANDLE_RC_SCRIPTS which imho should end up becoming a trigger that react on rc.d addition.

Isn't it enough?

If we ever end up removing procctl I would perfer a runtime option (aka something in pkg.conf) rather that a build time one.

Hi,

No it isn't enough unfortunately, at least not without significant work.

I could implement this as a pkg.conf option and resubmit

@igalic
Copy link

igalic commented Jul 16, 2023

can you briefly explain what's currently missing from HANDLE_RC_SCRIPTS?

@rcmcdonald91
Copy link
Contributor Author

rcmcdonald91 commented Jul 16, 2023

pfSense uses pkg scripts to execute custom code which in turn restarts services during post-install. Yes, this is a bit unconventional, but it is what it is... pfSense is a very old codebase. Furthermore, pfSense uses a custom rc system so HANDLE_RC_SCRIPTS is insufficient for us. We carry the above patch internally to disable procctl, though having a runtime option would be useful

@bapt
Copy link
Member

bapt commented Jul 16, 2023

to be honnest the best and the safest it so have a trigger which executes when the whole transaction is over, a trigger that looks for rc.d modification.

It will work out of box, will be safer than the "in transaction" mode which could end up trying to restart daemon for which some plugins will be updated only later in the transaction "hello dovecot".

and it does not require any work on pkg.

oliveromahony pushed a commit to nginx/agent that referenced this pull request Nov 17, 2023
* fix: restart service on package upgrade on Debian/Ubuntu

Previously it was stopped during the executing of package scripts
due to incorrect behavior in prerm:

https://www.debian.org/doc/debian-policy/ch-maintainerscripts.html#summary-of-ways-maintainer-scripts-are-called

* fix: do not stop service on upgrade (RHEL-based distros)

* Avoid restarting nginx-agent service from pkg on FreeBSD during upgrades

This will end up with a new process being killed immediately by pkg:
https://github.com/freebsd/pkg/blob/1.19.1/libpkg/scripts.c#L100-L103
https://github.com/freebsd/pkg/blob/1.19.1/libpkg/scripts.c#L244-L261

See also:
freebsd/pkg#2128

* feat: OpenRC package scripts for Alpine Linux

* chore: add Alpine Linux instructions to README

While here, removed extra trailing spaces across the doc.

* chore: adjust service description, remove modelines

* chore: remove leading underscore from shell func names
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants