tighten up diod configuration and initialization #133
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Problem: diod caches the result of geteuid (), which requires a reader of the code to mentally track whether the euid changed since it was cached. There is likely no benefit in avoiding repeated calls to geteuid (). Just do that.
Problem: when diod starts, it may not be clear whether it is running as root or not. Also the code for making the transition is a over-complicated. Log a message after the transition. To simplify - use getpwnam() and getpwuid() instead of thread-safe versions - short-circuit no-op transition from root to root - drop an unused parameter from the internal function. - improve inline comments.
Problem: when diod drops root and loads the credentials of another user, it calls SYS_setgroups (per-thread) instead of setgroups() (per-process) on Linux, but this call should be for the whole process. Use setgroups().
Problem: when diod starts, it may not be clear which access policy has been enacted. Log it.
Problem: the --enable-impersonation=TYPE arguments and platform defaults are a little complex for this niche case. Replace with two binary options: --disable-multiuser to disable multi-user support --with-ganesha-kmod to use FreeBSD nfs-ganesha-kmod for multi-user support At this point FreeBSD users making a server-only multi-user build would use configure --disable-diodmount --with-ganesha-kmod
Problem: diod's three distinct modes of operation (allsquash, runasuser, and multiuser) are hard to discern in diod's startup logic. Refactor this code so that initialization occurs in distinct sections.
Problem: if multi-user mode is selected and diod is configured with --disable-multiuser or if ganesha initialization fails, the server still starts, which could lead to privilege escalation for clients. Make those errors fatal.
Problem: If diod is configured to require authentication, but was built without munge support, it still runs. Make that a fatal error. Log a message if users will be allowed to connect without authentication.
Problem: if munge is not found by configure, diod is silently built without authentication support. Make that a hard error unless configured with --disable-auth. Also, use pkg-config to find munge. Drop a GPL_LICENSED declaration in front of the munge.h include directive. The munge license has changed to LGPL and this is no longer necessary.
Problem: if libcap is not found by configure but multi-user is enabled, diod runs without it, leading to test failures noted in chaos#103. Make that a hard error unless configured with --disable-multiuser. Also, use pkg-config to find libcap. Don't build the libnpfs/capability unit test if multi-user is disabled.
|
Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This tidies up the initialization code in the diod mainline and causes some of its critical choices to be logged so it's more obvious how it will be operating. For example,
systemd status diodshows the following on my system:Versus when I start diod manually as a regular user:
In addition, the handling of some
configureoutcomes was a bit loose. For example, if munge wasn't found, authentication was silently disabled. This fixes those issues.