OpenMediaVault-Plugin-Developers/openmediavault-sftp

The plugin doesn't wait for ZFS file system to be mounted

Eurbaix opened this issue · 11 comments

ZFS takes a while to mount after a reboot and sharedfolders on ZFS file systems may not be available yet when SFTP starts (are never available yet in my case).

Instead of waiting for the sharedfolders to become available, SFTP recreates the "missing" sharedfolders in the mount point, making the mount point invalid. Sharedfolders are not supposed to disappear after a reboot, so the SFTP plugin should never recreate "missing" sharedfolders directories silently when it starts, but throw an error after waiting, say, 60 to 120 seconds, to give time for all file systems to mount.

P.S.: This doesn't seem to affect "regular" file systems, probably because they mount quickly enough so the sharedfolders are already available when SFTP starts.

The plugin isn't doing anything magical. It is just creating fstab entries which means we expecting systemd to just do the right thing.

There are two things that might help.
systemctl edit omv-sftp.service

Add:
[Unit]
After=zfs-mount.service

If that doesn't work, then might have to add the fstab option for each sftp mountpoint to require the zfs mountpoint:
,x-systemd.requires=/path/to/zfs/mount

Let me know if either help.

I have tried both of your suggestions but none work.

What seems to work is to mount the /sharedfolders/SHARES instead of /path/to/filesystem/mount in fstab.

For example, my ZFS dataset is mounted as /mnt/DataFS and the sharedfolder is Backups
Currently, the SFTP create an fstab entry
/mnt/DataFS/Backups /sftp/user/Backups
which will create a directory in /mnt/DataFS and make it an invalid mount point if DataFS is not mounted yet.

If the SFTP plugin was creating this entry instead:
/sharedfolders/Backups /sftp/user/Backups
it would not make a mount point invalid and could mount /sharedfolder/Backups even if the file system is not mounted yet since /sharefolders sub-folders are always present.

What seems to work is to mount the /sharedfolders/SHARES instead of /path/to/filesystem/mount in fstab.

/sharedfolders are disabled by default now (change to OMV itself not the sftp plugin) and creating a bind mount to another bind mount is not a good idea.

For example, my ZFS dataset is mounted as /mnt/DataFS and the sharedfolder is Backups
Currently, the SFTP create an fstab entry
/mnt/DataFS/Backups /sftp/user/Backups
which will create a directory in /mnt/DataFS and make it an invalid mount point if DataFS is not mounted yet.

The sftp plugin doesn't create "Backups" folder. That is created when you create the sharedfolder in the OMV web interface. The sftp plugin does create /sftp/user/Backups directories to bind mount it to the original directory. If the "Backups" directory doesn't exist, something went wrong with the sharedfolder create or the parent directory isn't mounted (as you have found).

If the SFTP plugin was creating this entry instead:
/sharedfolders/Backups /sftp/user/Backups
it would not make a mount point invalid and could mount /sharedfolder/Backups even if the file system is not mounted yet since /sharefolders sub-folders are always present.

Strange that your /sharedfolder bind mounts are mounted but the sftp bind mounts aren't. They have been disabled by default because they are problematic.

The sftp plugin doesn't create "Backups" folder.

The sftp plugin may not create the "Backups" folder but it's implementation definitely is causing it: When there is no entries in SFTP access list, no folders are created in /mnt/DataFS and the ZFS dataset is able to mount properly. When there is an entry in SFTP access list for the Backups sharedfolder, a sub-folder is created in /mnt/DataFS and the ZFS do not mount because /mnt/DataFS is not an empty folder anymore.

I have checked how the SFTP plugin is implemented in config.xml. Apparently, the plugin use config.xml for all its config but not for the mount point. Why not use a mntent entry in config.xml instead of modifying directly fstab. For whatever reason, a mntent entry in config.xml doesn't create a sub-folder in the mount point before the file system is mounted. Maybe OMV check the chain of uuid to ensure they are processed hierarchically while fstab is just processing the entries directly.

The NFS plugin creates a mntent in config.xml instead of modifying fstab directly and I have never had any problem with sub-folders created in /mnt/DataFS before the dataset is mounted.

The sftp plugin may not create the "Backups" folder but it's implementation definitely is causing it: When there is no entries in SFTP access list, no folders are created in /mnt/DataFS and the ZFS dataset is able to mount properly. When there is an entry in SFTP access list for the Backups sharedfolder, a sub-folder is created in /mnt/DataFS and the ZFS do not mount because /mnt/DataFS is not an empty folder anymore.

No, this is incorrect. If you have not created a sharedfolder, then you cannot add a shared folder to the access list of the sftp plugin. And if you created a sharedfolder, then that process creates the folder on the filesystem. When you have sharedfolders enabled (OMV_SHAREDFOLDERS_DIR_ENABLED="YES" in /etc/default/openmediavault), a systemd bind mount unit file is created for each sharedfolder. systemd will create the folder if it does not exist because of the "Where" clause here - https://github.com/openmediavault/openmediavault/blob/master/deb/openmediavault/srv/salt/omv/deploy/systemd/10sharedfolders.sls#L61 and documented for systemd here - https://www.freedesktop.org/software/systemd/man/systemd.mount.html

I have checked how the SFTP plugin is implemented in config.xml. Apparently, the plugin use config.xml for all its config but not for the mount point.

In order to allow fine grain access to sharedfolders, the sftp uses shared folders and that is why the sharedfolderref is stored for each entry in the access list. The sharedfolder entry in config.xml has a reference to the mntent mount point entry. So, why would sftp store the same info again and duplicate data?

Why not use a mntent entry in config.xml instead of modifying directly fstab.

Because it needs the specific sharedfolder entry information to determine which users have access and what kind of access.

For whatever reason, a mntent entry in config.xml doesn't create a sub-folder in the mount point before the file system is mounted.

A mntent entry is a filesystem mount point which is why it stores the filesystem type. mntentry do create the initial mount point in /srv/. You don't see these because you are using zfs which doesn't not use fstab or systemd. zfs pools/datasets/filesystems are imported on each boot.

Maybe OMV check the chain of uuid to ensure they are processed hierarchically while fstab is just processing the entries directly.

OMV doesn't do any of this. systemd processes mount file and fstab entries to try to do the "right" thing. zfs screws a lot of this up since it isn't a native Linux filesystem and uses its own service.

The NFS plugin creates a mntent in config.xml instead of modifying fstab directly.

No, that is incorrect. Unless it has the hidden filed set to 1, all mntent entries create an fstab entry - https://github.com/openmediavault/openmediavault/tree/master/deb/openmediavault/srv/salt/omv/deploy/fstab. The nfs plugin creates a bind mount entry in fstab to /export/SHAREDFOLDERNAME.

have never had any problem with sub-folders created in /mnt/DataFS before the dataset is mounted.

Probably because the /sharedfolder bind mounts to your zfs filesystem wait for the zfs-mount service - https://github.com/openmediavault/openmediavault/blob/master/deb/openmediavault/srv/salt/omv/deploy/systemd/10sharedfolders.sls#L51 - which is why I mentioned trying it.

We have fought the zfs race condition for a long time with bind mounts. Unfortunately for the sftp plugin, they are required to create proper access in chroot. If you can find a solution that works, I will gladly accept a pull request.

No, this is incorrect. If you have not created a sharedfolder, then
I was talking about an entry in SFTP access list vs no entry in SFTP access list, the referenced sharedfolder being defined in both case. When it's not referenced by SFTP (no entry in SFTP access list), no sub-folders are created in the ZFS mount point, even though sub-folders of the ZFS dataset are referenced by other plugins (NFS, SMB/CIFS, Docker but only NFS mount a ZFS subfolder to another mount point).

The NFS plugin creates a mntent in config.xml instead of modifying fstab directly.

No, that is incorrect. Unless it has the hidden filed set to 1, all mntent entries create an fstab entry - https://github.com/openmediavault/openmediavault/tree/master/deb/openmediavault/srv/salt/omv/deploy/fstab. The nfs plugin creates a bind mount entry in fstab to /export/SHAREDFOLDERNAME.

NFS does create a mntent entry to /export/Sharedfoldername, and also a similar fstab entry, but do not create a sub-folders before the file system is mounted. Only SFTP access list entries do. I'm just trying to find what difference between the implementation of SFTP and NFS makes one not wait for the file system while the other don't, since both mount ZFS subfolders to another mount point.

If you can find a solution that works, I will gladly accept a pull request.
I will do some more tests when I can free a spare computer with enough RAM for NFS to use as a testing machine. Accidently "testing" this race condition on my main NAS creates a real mess that takes a while to clean up, so I will leave my main NAS alone for now ;-).

As for modifying the code myself, my programming days ended 25 years ago (I don't think js even existed), so I may do more damage than good. But if I find why SFTP fstab entry don't wait for the file system while NFS does, I will certainly let you know.

NFS does create a mntent entry to /export/Sharedfoldername, and also a similar fstab entry, but do not create a sub-folders before the file system is mounted. Only SFTP access list entries do. I'm just trying to find what difference between the implementation of SFTP and NFS makes one not wait for the file system while the other don't, since both mount ZFS subfolders to another mount point.

Sorry but have you looked at the commits for the sftp plugin? I think I know exactly what it does and it does not create folders. Creating the sharedfolder creates the subfolder on the source filesystem which means it exists on all bind mounts. NFS doesn't create a subfolder because the /export/sharedfoldername IS the sharedfolder.

Yes, the systemd bind mount unit files are probably better than fstab at waiting for zfs since they look for a lot of things. I just don't have the time to re-write the plugin to use bind mount unit files especially since this is a zfs-only issue.

Sorry but have you looked at the commits for the sftp plugin? I think I know exactly what it does and it does not create folders.

Once again, I was not saying the plugin create the subfolder, just that it's implementation cause a folder to be created, which is not the same thing. And I certainly never implied you don't know what a plugin you created do.

Yes, the systemd bind mount unit files are probably better than fstab at waiting for zfs since they look for a lot of things. I just don't have the time to re-write the plugin to use bind mount unit files especially since this is a zfs-only issue.

I may have a look at it. Not sure any good will come out of it, I'm really rusted. I love the realibility of ZFS and the key authentication of SFTP, so I'll add js on my derusting program.

You don't need the sftp plugin to use sftp since you can use the normal ssh server. It just allows you to create jails on a second sftp-only server.

Regular sftp gives access to the entire tree structure and you have to find your way to the sharedfolder you're looking for. It's fine for me, but really confusing for other, less-experienced, users of my NAS. Plus, my goal in using SFTP is to provide an entry point for remote backups, so I want limit the attack surface as much as possible.

That's why I liked your plugin: It provides a secure entry point for backups, with limited attack surface, and SFTP has decent performance for large backups. I don't know if your SFTP plugin is "battle-tested" but I was planning on testing it to make sure it would at least resist a script kiddie.

So far, I'm using a Webdav/NextCloud docker behind a reverse proxy as an entry point, which is fine for a reasonable amount of documents and pictures, but not for a remote user glued to his camera every minute his kids are around. Webdav is not really designed for large/fast uploads. Another alternative would be FTPS but I prefer the security of SFTP public keys with passphrase.

So, at the end, your plugin is really my preferred choice... if i can make it work with ZFS.

Or, hopefully, someone with a more relevant experience than me has the same problem and decide to give it a go.

Hello,
I solved this problem by adding this mount opt for each sftp mountpoint in the fstab:
x-systemd.after=zfs-share.service