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

Add support for mount export for fuse-nfs #140

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

Conversation

afbjorklund
Copy link

@afbjorklund afbjorklund commented Mar 15, 2025

mount: only root can use "--options" option

Led me to: https://github.com/sahlberg/fuse-nfs

It wants to list the exported file systems, for mount.

$ fuse-nfs -n "nfs://127.0.0.1/mount?nfsport=46335&mountport=46335" -m /tmp/nfs
... [ERROR] No handler for 100005.5
Failed to mount nfs share : zdr_replymsg failed in rpc_process_reply: libnfs_rpc_msg failed to decode REPLY, ret=0: libnfs_rpc_reply_body failed to decode ACCEPTED

But currently the nullauth ignores the dirpath anyway.

$ nfs-ls -D "nfs://127.0.0.1?nfsport=46335&mountport=46335"
nfs://127.0.0.1/mount
$ nfs-ls "nfs://127.0.0.1/mount?nfsport=46335&mountport=46335"
-rw-rw-rw-  1     0     0           11 hello.txt

Copy link
Owner

@willscott willscott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm okay with the general structure of allowing a custom auth handler to implement Export. I think the degraded case for null auth probably should be closer to 'not supporting it'

@afbjorklund
Copy link
Author

afbjorklund commented Mar 15, 2025

I couldn't figure out if ex_groups was allowed to be empty or not, so added "anonymous" (a synonym for *)

https://www.ietf.org/rfc/rfc1813.html#section-5.2.5

@afbjorklund
Copy link
Author

afbjorklund commented Mar 15, 2025

I used nfs-ls rather than showmount, because it was easier to add port support in libnfs.

@willscott
Copy link
Owner

My reading is that groups can be nil, since it's defined as a pointer.
can you try that and see if nfs-ls still works?

@afbjorklund
Copy link
Author

Sure, it still works even without the groups.

 // List of all exported file systems.
 func (h *NullAuthHandler) Export(context.Context) []nfs.Export {
-       groups := []nfs.Group{{Name: []byte("anonymous")}}
-       return []nfs.Export{{Dir: []byte("/mount"), Groups: groups}}
+       return []nfs.Export{{Dir: []byte("/mount")}}
 }
 

It wants to list the exported file systems, for mount.

But currently the nullauth ignores the dirpath anyway.

Signed-off-by: Anders F Björklund <[email protected]>
// Export contains an export node, with information about an exported filesystem.
type Export struct {
Dir []byte
Groups []Group
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this is *Group rather than []Group - in that it is either nil or the next group in a linked list, right?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is, but the Go wrapper "helpfully" dereferences all pointers - so we declare it as an array instead.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are documented as equivalent in XDR: https://datatracker.ietf.org/doc/html/rfc1014#section-3.18

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pattern in the rest of this library has been to write the individual fields with the extra 0 to signify the end of the list

e.g. https://github.com/willscott/go-nfs/blob/master/nfs_onreaddir.go#L116

while this is more fiddly, it makes the type 'logically correct', so that the caller doesn't have to understand the nuance.

Copy link
Owner

@willscott willscott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main consideration I've been trying to think through is whether this is a good 'core type' for ACL groups - since the rest of auth is not implemented here, I worry that a partial type would be hard for a separate implementation of an auth handler / auth to be able to integrate.

That makes me wonder if there's either a way to more generally stub out this part of mounting for implementation in a custom authentication handler, or if there's a complete but still simple authentication example that can do both this and use of the opaque group object.

I'm not asking for that from you, but I'm going to take a couple more days to go back to the spec to make sure the interface for the library is decent.

// Export contains an export node, with information about an exported filesystem.
type Export struct {
Dir []byte
Groups []Group
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pattern in the rest of this library has been to write the individual fields with the extra 0 to signify the end of the list

e.g. https://github.com/willscott/go-nfs/blob/master/nfs_onreaddir.go#L116

while this is more fiddly, it makes the type 'logically correct', so that the caller doesn't have to understand the nuance.

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.

2 participants