openssh bugs - Jul 2019 - [Bug 3036] New: Legacy SFTP rename (SSH2_FXP_RENAME) fails for files owned by others

https://bugzilla.mindrot.org/show_bug.cgi?id=3036

            Bug ID: 3036
           Summary: Legacy SFTP rename (SSH2_FXP_RENAME) fails for files
                    owned by others
           Product: Portable OpenSSH
           Version: 8.0p1
          Hardware: Other
                OS: Solaris
            Status: NEW
          Severity: normal
          Priority: P5
         Component: sftp-server
          Assignee: unassigned-bugs at mindrot.org
          Reporter: peter.harvey at oracle.com

Legacy SFTP clients are unable to rename (ie move) files owned by
others on Solaris and its derivatives.

Steps to reproduce using the undocumented "legacy" flag in SFTP's
rename:

-=-=-
Using Solaris 11.4
-=-=-

# pwd
/export/home/jack
# ls -l
total 34
drwxr-xr-x   2 jack     staff          2 Jul 16 11:23 bar
-rw-r--r--   1 root     staff          0 Jul 16 11:23 foo
# sftp jack at localhost
Password:
Connected to jack at localhost.
sftp> ls -l
drwxr-xr-x    2 jack     staff           2 Jul 16 11:23 bar
-rw-r--r--    1 root     staff           0 Jul 16 11:23 foo
sftp> rename foo bar/foo
sftp> rename bar/foo foo
sftp> rename -l foo bar/foo
Couldn't rename file "/export/home/jack/foo" to
"/export/home/jack/bar/foo": Permission denied
sftp>

Turning on verbose output we see:

sftp> rename -l foo bar/foo
debug3: Sent message SSH2_FXP_RENAME "/export/home/jack/foo" ->
"/export/home/jack/bar/foo"
debug3: SSH2_FXP_STATUS 3
Couldn't rename file "/export/home/jack/foo" to
"/export/home/jack/bar/foo": Permission denied
sftp> rename foo bar/foo
debug3: Sent message posix-rename at openssh.com
"/export/home/jack/foo"
-> "/export/home/jack/bar/foo"
debug3: SSH2_FXP_STATUS 0
sftp>

-=-=-

Older versions of SFTP use the pre-POSIX rename SSH2_FXP_RENAME or
SSH_FXP_RENAME. Examples include Solaris's SunSSH and WinSCP.

The problem is caused by the PRIV_FILE_LINK_ANY privilege being
dropped:

./sftp-server.c

int
sftp_server_main(int argc, char **argv, struct passwd *user_pw)
{
        [...]

        /* Drop any fine-grained privileges we don't need */
        platform_pledge_sftp_server();

./platform-pledge.c

/*
 * Drop any fine-grained privileges that are not needed for
post-startup
 * operation of sftp-server
 */
void
platform_pledge_sftp_server(void)
{
#ifdef USE_SOLARIS_PRIVS
        solaris_drop_privs_pinfo_net_fork_exec();
#endif
}

./openbsd-compat/port-solaris.c

void
solaris_drop_privs_pinfo_net_fork_exec(void)
{
        priv_set_t *pset = NULL, *npset = NULL;

        [...]

        if (priv_addset(npset, PRIV_FILE_CHOWN) != 0 ||
            priv_addset(npset, PRIV_FILE_DAC_READ) != 0 ||
            priv_addset(npset, PRIV_FILE_DAC_SEARCH) != 0 ||
            priv_addset(npset, PRIV_FILE_DAC_WRITE) != 0 ||
            priv_addset(npset, PRIV_FILE_OWNER) != 0)
                fatal("priv_addset: %s", strerror(errno));

        if (priv_delset(npset, PRIV_FILE_LINK_ANY) != 0 ||

The pre-POSIX rename operation uses link()/unlink(), the comment reads
"Race-free rename of regular files". Without PRIV_FILE_LINK_ANY the
calls to link() will fail if the effective user ID doesn't match the
owner of the file.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=3036

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |alex+mailinglists_openssh-d
                   |                            |ev at cooperi.net,
                   |                            |djm at mindrot.org

--- Comment #1 from Damien Miller <djm at mindrot.org> ---
The sandboxing code here was written by Alex Wilson (Cc'd) and I'd like
their opinion before restoring that particular privilege (I'm not
familiar with Solaris' privilege model myself)

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=3036

--- Comment #2 from Alex Wilson <alex+mailinglists_openssh-dev at
cooperi.net> ---
I think adding that particular privilege back in for sftp-server should
be fine. I guess I missed this particular semantic of older clients at
the time and took out a little too much.

The only user of solaris_drop_privs_pinfo_net_fork_exec() is
platform_pledge_sftp_server() at the moment, I believe, so simply
changing that function should be fine. The name doesn't make it clear
whether FILE_LINK_ANY is in there or not, so I think it'd also be fine
not to rename it.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=3036

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|NEW                         |RESOLVED
             Blocks|                            |2988

--- Comment #3 from Damien Miller <djm at mindrot.org> ---
Thanks, I've remove the delset line. This should be fixed in git master
and will be in the OpenSSH 8.1 release soon.


Referenced Bugs:

https://bugzilla.mindrot.org/show_bug.cgi?id=2988
[Bug 2988] Tracking bug for 8.1 release
-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=3036

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |CLOSED

--- Comment #4 from Damien Miller <djm at mindrot.org> ---
closing resolved bugs as of 8.6p1 release

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.