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 a UserDetail::read_only method #503

Closed
wants to merge 2 commits into from
Closed

Conversation

asomers
Copy link
Contributor

@asomers asomers commented Mar 18, 2024

If it returns True, then the storage backend should not allow any destructive operations. Implement this method in the cap-ftpd example for unftp-sbe-fs.

Also, use Capsicum within unftp-sbe-fs, on FreeBSD. After authenticating a connection, limit the process's rights to mitigate any potential attacks.

If it returns True, then the storage backend should not allow any
destructive operations.

Implement this method in the cap-ftpd example for unftp-sbe-fs.
After authenticating a connection, limit the process's rights to
mitigate any potential attacks.
@hannesdejager
Copy link
Collaborator

Hi @asomers , terribly sorry for not answering you here. I saw this at some point but then forgot about it when life got busy.

I don't think this is the right approach. Instead of adding readonly capabilities to each storage back-end individually, we should do it only once. This is what I am doing here: https://github.com/hannesdejager/unftp-sbe-restrict

Can this be used for your purposes?

The crate is here: https://crates.io/crates/unftp-sbe-restrict

Let me know your thoughts.

@asomers
Copy link
Contributor Author

asomers commented May 8, 2024

Hi @asomers , terribly sorry for not answering you here. I saw this at some point but then forgot about it when life got busy.

I don't think this is the right approach. Instead of adding readonly capabilities to each storage back-end individually, we should do it only once. This is what I am doing here: https://github.com/hannesdejager/unftp-sbe-restrict

Can this be used for your purposes?

The crate is here: https://crates.io/crates/unftp-sbe-restrict

Let me know your thoughts.

Functionally, I think it would work. It wouldn't provide capability-based security, though. The nice thing about this PR is that Capsicum guarantees that, after StorageBackend::enter gets called, there's no way for the backend to modify any files even if we mess up some of the coding. Using unftp-sbe-restrict that wouldn't be possible, because the real storage backend wouldn't know about the user's permissions. Or is there another way to communicate that information?

@hannesdejager
Copy link
Collaborator

hannesdejager commented May 14, 2024

I think the capsicum code in enter is the right approach. What I don't think is, is adding the read_only method to UserDetail.

Maybe I don't understand where you're going with that but at least it seems the addition of read_only and checking for that in the storage back-end need not be coupled with the capsicum additions?

@asomers
Copy link
Contributor Author

asomers commented May 14, 2024

If UserDetail doesn't include read_only , then how would enter know whether it needs capsicum::Right::Write ?

@hannesdejager
Copy link
Collaborator

Right... I'm asleep after a long day. I have to think a bit on this... perhaps we need to bring the granularity of permissions that is implemented in unftp-sbe-restrict to libunftp itself i.e. have a permissions method on UserDetail and not just support readonly

@asomers
Copy link
Contributor Author

asomers commented May 14, 2024

Right... I'm asleep after a long day. I have to think a bit on this... perhaps we need to bring the granularity of permissions that is implemented in unftp-sbe-restrict to libunftp itself i.e. have a permissions method on UserDetail and not just support readonly

That sounds like a good idea.

@hannesdejager
Copy link
Collaborator

hannesdejager commented May 15, 2024

Something like this: #511

@robklg what do you think?

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