change the default access mode to "client" and add SECURITY section to diod(8) #157
Conversation
This file contains hidden or 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
garlick
force-pushed
the
access_client
branch
from
de9f131 to
031be66
Compare
grondo
approved these changes
Oct 31, 2025
Member
grondo
left a comment
grondo
left a comment
There was a problem hiding this comment.
LGTM on a quick pass. Just saw one typo and one suggestion from a Claude review I wasn't sure about.
man/diod.8.in
Outdated
| the simplest and safest mode for \fBdiod\fR to operate in from a security | ||
| standpoint. | ||
| .LP | ||
| Muti-user support is intended to be paired with the Linux v9fs client |
Comment on lines
145
to
147
| Server worker threads handling a request on behalf of a non-root user on | ||
| a connection authenticated as root set \fBCAP_DAC_BYPASS\fR, \fBCAP_CHOWN\fR, | ||
| and \fBCAP_FOWNER\fR. The v9fs client is assumed to be performing access |
Member
There was a problem hiding this comment.
FYI - Claude code review called out that CAP_DAC_BYPASS may be a typo since CAP_DAC_OVERRIDE is more likely. I'm not about that, so thought I'd just share it for consideration.
Problem: the diod github wiki includes a security page that was out of date. Add a SECURITY section to the diod(8) man page so we can remove the wiki page.
Problem: mount.diod currently defaults to access=user, but access=client more closely describes what diod assumes v9fs will do. Both access=user and access=client enforce access controls at the client, but access=client was added specifically to underscore this behavior and is required when posixacl is also specified. Since diod's use of CAP_DAC_OVERRIDE assumes that the v9fs client is performing access checks, we'd better use the name that is documented to do it. Currently, other than posixacl requiring access=client, there do not appear to be any differences between the two modes. However, since a new mode was added, potentially someone thinks that could change. Change the default to access=client.
Problem: the mount options listed in mount.diod(8) are no longer accurate. Note that the default access mode is now "client". Cull descriptions of lesser used options and include a reference to the kernel documentation. Clarify descriptions of some options.
Problem: several tests specify access=user where access=client is more appropriate. Use access=client in the main v9fs multiuser test. Use mount.diod with defaults in the rsync test. Use mount.diod with defaults + -o posixacl in the ACL test.
garlick
force-pushed
the
access_client
branch
from
031be66 to
5a3ae78
Compare
Member
Author
|
Thanks! Fixes pushed and setting MWP. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Problem: the linux v9fs client has added a new access mode
clientwhich appears to be the same as our default ofuserbut 1) is documented to provide client side access checks (even thoughuseralso does) and 2) is required forposixacl.Even though there is no real change in functionality, it probably makes sense to change the default
mount.diodaccess option toclientgiven that diod relies on the client side checks. That way if the two options deviate in vf9s in the future, we'll have picked the right one. Also it means addingposixaclwill just work.In the process of reading about this and recalling diod's security design, and thinking of past user confusion and my crap memory, I wrote up a SECURITY section in the diod(8) man page to explain it all (likely to my future self!)