FTP server in 6.9.2 allows write access to shares it shouldn't.


IMTheNachoMan

Recommended Posts

From what I can tell, it looks like the FTP user permissions are not being honored. This is what I did (screenshots below).

 

  1. Created a new user
  2. In Settings > FTP Server I:
    1. Enabled
    2. Added the user
  3. In Users > the user:
    1. Gave the user read/write to  only one share

 

After that, when I connected to my unraid box using FTP using the user, I was able to navigate to other shares and create files.

 

Yes, I know it is advised not to use the built in FTP server but since I am not doing any port forwarding I figured it would work for me. And, even still, if the capability is there it should work as expected.

 

image.thumb.png.3382d3fa5129a82d7ac4bb0ab3117626.png

 

image.thumb.png.9a8a67fd8c9c8d6b8ddb99eb6fa8b84c.png

 

 

 

Link to comment
9 hours ago, trurl said:

Disable the builtin FTP server and use another from the Apps page.

 

Yes. I got that. But I still wanted to raise this issue cause I think it is a security issue worth addressing. I get for SSH there are no other user accounts and there is just the root account but if the FTP configuration allows to enter a user then there is an implied assumption the user's ACLs are enforced.

 

I see Unraid uses vsftpd. I think I can figure out the necessary vsftpd.conf setting to make this work as expected and maybe even jail the user to a specific share. I just can't find where Unraid stores the user name saved in the web UI for FTP. 

Link to comment

Since FTP uses plain text authentication, it's insecure no matter how you slice it.

 

You must not allow FTP access without constraining it externally.

 

Honestly the secure thing to do would be remove FTP from Unraid, and force people to use a docker container that can be sandboxed appropriately. However, I and probably many others use it to allow old equipment like office scanners and security cameras access, and it would be a pain to force everybody to implement proper security.

 

Keeping it disabled but available means those who want to take the risk can, but it's not automatically exposed.

 

What I'm saying is that it's wasted effort to modify what's currently there, the options are realistically to remove it, or just leave it disabled and use a proper solution.

 

40 minutes ago, IMTheNachoMan said:

Disabled by default is not a valid security control.

By that logic telnet should be removed as well.

Link to comment
3 minutes ago, JonathanM said:

Since FTP uses plain text authentication, it's insecure no matter how you slice it.

 

 

Sure, but there are legitimate use cases where you want to be able to FTP to a specific folder and no where else. I am okay with FTP, with its insecure nature, open on /some/random/path but not on /. So logically if I enable FTP on Unraid and limit it to user1 then user1 should only have access to the shares that user1 is given access to in Unraid. If user1 does not have access to a share on Unraid they should definitely not have access to the share via FTP. Now, if the FTP configuration in Unraid didn't ask for a user and worked just like SSH (only the root user) then that would be a different thing. But the Unraid UI lets me specify a user for FTP which means it should enforce the appropriate ACLs.

 

5 minutes ago, JonathanM said:

You must not allow FTP access without constraining it externally.

 

 

Of course. For example, I have a security camera that save videos to FTP. My FW rules restrict access between devices. I want my security camera to save videos by ftp to a specific share. But because of the misconfiguration in the FTP configuration in Unraid, the FTP user gets access to all shares, not just the one I've given it. 

 

My issue is not that the FTP system gives access to all shares. My issue is that the FTP configuration asks for a user which inherently implies the access is limited to just the shares the user has access to and yet the user can read/write from other shares.

 

If the FTP system with Unraid is going to give full access to all shares then it shouldn't let you specify a user and should default to the root account like the SSH system does.

Link to comment

This is a relatively easy fix. I have the vsftpd.conf that will limit/jail a user to a specific share as their root/home for FTP using vsftpd's virtual user feature.

 

I can make it work two ways but need help with both because I don't yet understand Unraid code:

  1. Mimic something ProFTPD where the "users" list in the FTP configuration for vsftpd in Unraid allows you to enter a specific syntax that, upon save, gets processed and dumped to a file. For example, entering "user1=/mnt/user/share1; user2=/mnt/user/share2" would result in some files being created in /etc/vsftpd that.
  2. Add a more robust UI to the configuration that lets you add users and select their share. Same result as above but a prettier front end.

While ProFTPD works, vsftpd is more lightweight and for Unraid purposes is perfect.

 

If someone can, at minimum, tell me where the Unraid code is that handles save/apply on the vsftpd FTP save UI, then I can try to modify it for the #1 option. For #2 I'd need someone's help to write the code for the UI. 

Link to comment

On your server the relevant code will be at /usr/local/emhttp/webGui/FTP.Page in a running system.

 

The GUI code is also available online at https://github.com/limetech/webgui/tree/master/plugins/dynamix so if you can make enhancements to FTP.Page that enhances it without breaking anything then you can submit a pull request to get it included in the standard Unraid release and I would expect the pull request is likely to be to be accepted. by Limetech.

Link to comment
3 hours ago, itimpi said:

On your server the relevant code will be at /usr/local/emhttp/webGui/FTP.Page in a running system.

 

The GUI code is also available online at https://github.com/limetech/webgui/tree/master/plugins/dynamix so if you can make enhancements to FTP.Page that enhances it without breaking anything then you can submit a pull request to get it included in the standard Unraid release and I would expect the pull request is likely to be to be accepted. by Limetech.

 

Can I ask one question? Assuming I don't do anything stupid crazy with the PHP code, making changes to /usr/local/emhttp/webGui/scripts a live, running system won't break anything, will it? I don't have capacity to run a VM for Unraid and I don't have a backup system so I was going to test changes on my live system. From what I can see it's a few lines added to the if block of that script and then some changes to the vsftpd.conf file. So pretty trivial but I don't know enough about Unraid's core to know if I'll ruin my system?

Link to comment
25 minutes ago, IMTheNachoMan said:

 

Can I ask one question? Assuming I don't do anything stupid crazy with the PHP code, making changes to /usr/local/emhttp/webGui/scripts a live, running system won't break anything, will it? I don't have capacity to run a VM for Unraid and I don't have a backup system so I was going to test changes on my live system. From what I can see it's a few lines added to the if block of that script and then some changes to the vsftpd.conf file. So pretty trivial but I don't know enough about Unraid's core to know if I'll ruin my system?


it should be safe to make the changes on a live system as even if you get them wrong you are only likely to affect the FTP component., and since that location is only in RAM a reboot would always revert back to the standard Released file.   Just make sure you backup any changes you make such as copy the changed file to /boot (the flash drive) if you intend to reboot and not lose your changes.   The change could well end up being quite trivial once you have worked out exactly what is needed - it is just a case of someone investing the effort to get it right.

  • Like 1
Link to comment
9 hours ago, itimpi said:


it should be safe to make the changes on a live system as even if you get them wrong you are only likely to affect the FTP component., and since that location is only in RAM a reboot would always revert back to the standard Released file.   Just make sure you backup any changes you make such as copy the changed file to /boot (the flash drive) if you intend to reboot and not lose your changes.   The change could well end up being quite trivial once you have worked out exactly what is needed - it is just a case of someone investing the effort to get it right.

 

So I got it done. I can't find where the `vsftpd.conf` file is in the repo. Do you know?

Link to comment
45 minutes ago, Xaero said:

Even if it is, enhancements to the functionality and security of core features (disabled by default or not) should be welcome. 
Thanks for taking the time to improve Unraid. Maybe someday I will be able to submit some PRs :D

 

Yup. I am almost done. Done with the core/backend stuff. I just gotta figure out the web UI stuff. Lack of documentation for the UI makes it difficult. Almost done.

Link to comment
  • 3 months later...

Give me some time and I'll report it to Limetech.  There is a lot of activity trying to get 6.10 released so it would probably be over looked right now.  PM me after 6.10 is released and I'll get it logged as a potential enhancement.  Can't guarantee anything.

Edited by dlandon
  • Thanks 1
Link to comment
17 hours ago, dlandon said:

Give me some time and I'll report it to Limetech.  There is a lot of activity trying to get 6.10 released so it would probably be over looked right now.  PM me after 6.10 is released and I'll get it logged as a potential enhancement.  Can't guarantee anything.

 

Got it. Thank you!

 

17 hours ago, bonienl said:

Just a general note about this PR on github. It requires some adaption when it comes to storing settings.

I guess it can be looked into after the release of 6.10.

 

 

Can you elaborate on what adaption? I can make changes if there is a different way I need to do something? I just started using Unraid and don't fully understand the code architecture. Happy to make changes as needed.

Link to comment

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.
Note: Your post will require moderator approval before it will be visible.

Guest
Reply to this topic...

×   Pasted as rich text.   Restore formatting

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.