Implement the python side of SFTP access to all sharelinks #284
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
jonasbardino
added this to the
Sharelink improvements and capability extensions milestone
jonasbardino
added a commit
that referenced
this pull request
Jul 20, 2025
…n PR #284. None of them are all that important since we should already have prior checks in place e.g. in configuration init to bail out, but better safe than sorry.
jonasbardino
added a commit
that referenced
this pull request
Jul 20, 2025
Add a few variable initializations to address related `pylint` errors in PR #284. None of them are all that important since we _should_ already have prior checks in place e.g. in configuration init to bail out, but better safe than sorry.
jonasbardino
force-pushed
the
add/sftp-access-to-all-sharelinks
branch
2 times, most recently
from
85978ac to
a480cd3
Compare
…r-side access limitations on all operations when in read-only or write-only mode. This is work in progress and the PAM and NSS bits are still missing for sftpsubsys support.
…der to support access from sshd+sftpsubsys service, too. Simply adjusts the existing read-write sharelink test and auth to instead loop through the three access modes in turn and do the similar test for each. Minor adjustments to continue until one succeeds or all have been tried. The existing test+auth code is purposefully kept unindented in this version to make the diff simple to review. Will follow-up with a separate commit which only indents to fit loop nesting to keep that change simple as well.
…one byte string termination (`\0`). Dumb down the sharelink mode iterator to keep it simple. Adjust prefix for shared PAM and NSS log line helper to clarify that. Otherwise we get `pam_mig` log entries even for key login, which never hits PAM.
consistent client experience. Basically filter certain operations like `chattr` and `stat`/`lstat` more carefully in order to let read-only and write-only shares behave. For read-only shares it is necessary to allow non-changing `chattr` calls to avoid `sftp` failing in simple `get path` downloads. For write-only shares it's necessary to allow `lstat`/`stat` on given files and folders to avoid errors e.g. when sftp or sshfs mount `rm`s a file. Additional tweaks to allow non-changing `chmod`/`chown` in read-only shares are still under consideration.
jonasbardino
force-pushed
the
add/sftp-access-to-all-sharelinks
branch
from
bdf95c1 to
975cd24
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Implement SFTP access to all sharelinks with added server-side access limitations on all operations when in read-only or write-only mode.
This is still work in progress and the PAM and NSS bits are still missing for sftpsubsys support.We may still want to look into pointing all read-only sharelink access to a separate read-only (bind-) mount of the persistent storage file system to be extra protected against any missing access limitations.
As @aputtu pointed out after initial testing of the write-only sharelinks they felt somewhat crippled when used e.g. mounted from a file manager like
NautilusorThunarbecause we initially rejected all read operations includingstat/lstatcalls to query basic file and folder info. After some reconsideration that restriction was loosened up to allow explicitstat/lstatcalls on explicitly given paths to make most clients behave. Similarly it turned out that simple get downloads fromsftpon read-only shares would fail when emptychattrcalls were rejected. So such non-changingchattrcalls are now also allowed.We may still want to similarly allow non-changing
chown/chmodcalls and perhaps even overridelist_folderto always just return an empty list for write-only shares.