Created attachment 1973 [details] Force file permissions for sftp-server Hello, I have found that I require more control over file permissions for incoming files via sftp-server/internal-sftp than the -u <umask> parameter can provide. Please see the attached patch. It adds yet another option to sftp-server (-m) that will force file permissions and will ignore permissions specified by the client. The numeric permissions following the -m parameter are bounds checked by the same method now used for the -u parameter and can only range from 0 - 0777. Implementation in sshd_config would obviously be something like: ----------------------------------------------- Match Group sftponly ChrootDirectory /home/chroot-%u ForceCommand internal-sftp -m 660 ----------------------------------------------- or ---------------------------------------------------- Subsystem sftp /path/to/sftp-server -m 600 ---------------------------------------------------- I have tested extensively on several Linux distributions and have been using the changes in our production sftp-server environment. Note that the attached patch updates sftp-server.8 version 1.19 and sftp-server.c version 1.93. Please consider including this change or something similar in the next release. Thanks! -Rob Candland
I strongly support this patch or alternatively the sftpfilecontrol one (http://sftpfilecontrol.sourceforge.net/). Usage scenario: Client opens sftp connection to server, browses to a setgid 'workgroup' directory (he's in the corresponding group) and creates a new file. In order for other users in this group to be able to edit the file, it should have ...rw-... permissions. By using the -u flag in sshd_confg: Subsystem sftp /usr/lib/openssh/sftp-server -u002 The client's umask gets shadowed, but not overridden. That is, if the client has 022 for his umask (as most do), the -u flag can't achieve g+w on new files (it does however, for example, correctly flatten the group permissions with -u070). This should be independent of wildly varying client setups, so asking every user to change his local umask is not a practicable way. The patch in this report would allow setting a -m flag in sshd_config, the sftpfilecontrol patch mentioned above would allow a SftpUmask option also in sshd_config. Any of which would be highly useful for the described setup. Thanks and best wishes Donjan Rodic PS: Rob, does your patch handle directories as well?
My apologies for not responding more quickly. Due to lack of support from the OpenSSH team I have not released further patches but with the existing patch you should be able to patch newer versions of the sftp-server.c file and even the man page. The patch isn't a very elegant solution but it gets the job done and IMO makes the sftp server even more useful than it already is.
Created attachment 2547 [details] Patch for current upstream version First of all I would like to thank Rob for this patch. After few years I would like to take this question back to the discussion. Or try to start this discussion with upstream. We can agree that only using umask we can get into troubles and to files that are not accessible by anyone (when uploading from non-homogeneous user environment), as stated in comment #1. This is pretty inconvenient when processing files on sftp file server using scripts. Our customer is using another script to fix modes as a workaround and would be glad to see this feature in openssh. This implementation using command-line argument makes it easy to configure sftp file server instead of the other mentioned patch. I'm appending patch that is applicable to upstream version and I would like to hear some feedback.
Created attachment 2872 [details] patch with proper umask restore adding patch with proper umask restore, which I missed before.
Created attachment 3096 [details] Force file and directory permissions for internal-sftp with parameter -m Tested on: * Debian openssh-server 1:7.4p1-10+deb9u1 * Debian openssh-server 1:7.6p1-2 Changelog, authors: 2010-12-10 - Rob Candland - v1 - public domain - Force file permissions for sftp-server 2016-09-09 - Rob Candland, Jakub Jelen - v2 - public domain - patch with proper umask restore 2017-11-24 - Rob Candland, Jakub Jelen, Dr. Nagy Elemér Károly - v3 - public domain - Force directory permissions as well
Created attachment 3098 [details] Force file and directory permissions for internal-sftp with parameter -m Fixed documentation, no functional change
Checking back in here with a new email address/account. Even after seven years(!) I still believe this is a worthy addition to the sftp-server functionality and appreciate that Dr. Nagy Elemér Károly and Jakub Jelen have contributed their patches to help get this into the codebase.
Please do add this feature!
It is 2020 and this is still required. I have surfed the web to see tons of people looking for this feature, just to find it is not possible. please apply the patch.
Instead of patching in yet another complicating workaround, it could be preferable and possible to solve the root of the problem. The "specification" for the current default behavior seems to have been a never completed draft, of a working group that has been canceled. === SPECIFICATION PROBLEMS === https://tools.ietf.org/html/draft-ietf-secsh-filexfer-13#section-7.6 > Implementations MUST NOT send bits that are not defined. > > The server SHOULD NOT apply a 'umask' to the mode bits; but should > set the mode bits as specified by the client. The client MUST apply > an appropriate 'umask' to the mode bits before sending them. This part of the specification is very badly worded. 1) The talking about server and client is actually really doing wrong, it is not possible to specify correct permission handling with these wrong terms. That is because correct permission handling depends on the file sender and receiver (data transfer direction), not client and server. 2) It is also a problem that "defined bits" can be misunderstood as referring to the permission bits of the original file that is to be copied, instead of only referring to bits that were explicitly defined by the command (for which using a --preserve-permissions flag would be an example). So misconception 2) may really have broken SFTP for multi-user system group directories. (Assuming the umask in effect for the SFTP server process actually is 0002 on the server.) === PROPOSED SOLUTION === There is no point in applying a too strict nor too permissive umasks from a file sending system on the file receiving system (for example one may use User-Private-Groups with a default umask 0002). Nor should any file permissions be copied verbatim by default. Instead, as with all file creations, the default umask on the receiving side needs to be responsible to ensure safe default file permissions at the target location. Thus, a more sensible specification could be: 1) By default, the file sending side MUST NOT send permission bits. * This lets the receiver's default umask policy take effect by default. 2) The permission bits MUST be sent only if explicitly requested by the command (e.g. with an explicit --preserve-permissions option). 3) If explicit permission bits have been requested and set by the sender, then the receiver SHOULD NOT apply an 'umask' to the mode bits; but should set the mode bits as specified by the sender. * This is to allow overriding the receiver's default umask policy, by allowing the user to manually grant more or less permissions. The receiver's default settings always declare the permissions that are safe at the particular location on the receivers side, i.e. also for especially configured group directories, but there may be occasions to manually grant more or less permissions.) * The "should" still allows a receiver (e.g. a server) to apply further rules, if the use-case requires this.) --- Overall, this could eliminate most, if not all, reasons for the special override patches that have been proposed here.
checking in to add a vote for this :) I am inclined to disagree with comment #10 referring to the unfinished IETF spec because the IETF of course favours rough consensus and running code... and openBSD implemented it as-is many years ago and Fedora / RHEL have shipped this patch for 6 years now. :-) [1] As a result there is lots of documentation out there referring to "-m" workaround. To now suggest updating the specification to introduce this new idea of senders and receivers "applying rules" and also muddying what the existing umask flag does... is not the solution to the use case mentioned in this ticket. Infact it's not the solution to any real world problem as far as I can tell. The use case is just for an sftp server to ignore incoming permissions. Not to reinterpret the sent umask, not to apply rules. Just to set its own unilaterally. The patch does it, it has been running in production on RHEL for a long time and it is documented widely online. At this point, merging it at this point seems to me like a bit of a trivial decision. [1] https://src.fedoraproject.org/rpms/openssh/blob/f22/f/openssh-6.7p1-sftp-force-permission.patch
Hi, I wasn't aware that fedora has been shipping the -m override. I agree that it certainly makes sense to be able to override the permissions server-side. So, I'd second to merge the force-permission -m option. Though, proper umask handling at the target side is by no means a negligible problem, I think. As far as I know SFTP is the only remote filesystem (access) that transfers and creates files without proper use of the umask in effect at their target location. Already created a separate issue to fix the broken default behavior: https://bugzilla.mindrot.org/show_bug.cgi?id=3252 (file transfers break (mangle) secure/useful file permissions)
I think the -m option can actually be seen as the most straightforward of the useful "custom rules" for a server (mentioned in https://bugzilla.mindrot.org/show_bug.cgi?id=3252). The only other rules that I can think of at the moment would be forcing minimal and maximal umasks, to limit the file permissions that the client can create. But IMHO a simple file copy should just always have the appropriate permissions, by default. Just like with all other copy operations.