openssh bugs - Jul 2024 - [Bug 3715] New: safely_chroot is a little too restrictive: noexec or nosuid should be enough

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

            Bug ID: 3715
           Summary: safely_chroot is a little too restrictive: noexec or
                    nosuid should be enough
           Product: Portable OpenSSH
           Version: 9.8p1
          Hardware: Other
                OS: Linux
            Status: NEW
          Severity: enhancement
          Priority: P5
         Component: sshd
          Assignee: unassigned-bugs at mindrot.org
          Reporter: joshudson at gmail.com

Created attachment 3828
  --> https://bugzilla.mindrot.org/attachment.cgi?id=3828&action=edit
most of a patch restoring old documented behavior -- needs autoconf
work
>From my perspective this is a regression, but it's a regression from a
far branch somewhere, not from a previous version of openssh. The
second blockquote here https://unix.stackexchange.com/a/396684 seems to
be quoting the manpage from the branch but I can't find its origin.

We have this working on our deployment today with a decode old
codebase; but in testing a replacement server closer to stock openssh
it doesn't work.

The general case is you want a lot of sftp users; each chrooted into
their own home directory, and each having write access to their own
home directory; where each can see no other.

Basically, there's no reason in particular this configuration shouldn't
work:

Subsystem sftp internal-sftp

Match Group filedrop
    ChrootDirectory %h
    AllowTCPForwarding no
    X11Forwarding no

where the home directories of all the users are under /u and /u is
mounted noexec.

I was pretty much able to just fix the problem on the first attempt
except for I got lost in autoconf; I have a patch available that still
needs to be edited so that autoconf knows what to do. (There's still
some systems lacking the statvfs() call.)

There's one weirdity I haven't been able to explain though. Why is
nodev (to be) required? It's not like an external filesystem; the user
can't *make* devices. But that's what the document says; so that's
what
I put back.

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

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

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |djm at mindrot.org
   Attachment #3828|0                           |1
           is patch|                            |
   Attachment #3828|application/octet-stream    |text/plain
          mime type|                            |

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

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

--- Comment #1 from Damien Miller <djm at mindrot.org> ---
fwiw that "documented behavior" you mentioned was never part of
OpenSSH, it may have been a patch added by a downstream distributor.

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

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

--- Comment #2 from Joshua Hudson <joshudson at gmail.com> ---
It may have been; but I jumped back a few versions and found the
original state was don't check permissions on the chroot directory.

I wonder if that was some other downstream distributor unbreaking
somebody else with a very similar setup.

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

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

Darren Tucker <dtucker at dtucker.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |dtucker at dtucker.net

--- Comment #3 from Darren Tucker <dtucker at dtucker.net> ---
(In reply to Joshua Hudson from comment #2)
> Why is nodev (to be) required? It's not like an external filesystem;
> the user can't *make* devices.
No, but they *can* create new hardlinks to existing devices on the same
filesystem (this example is OpenBSD, but I'd expect the same on most
systems where /dev is not a separate mount point):

$ id
uid=1000(builder) gid=1000(builder) groups=1000(builder)
$ mount 
/dev/sd0a on / type ffs (local, wxallowed)
$ ln /dev/null ~/null
$ ls -ld . ~/null
drwxr-xr-x  3 builder  builder       512 Jul 31 10:12 .
crw-rw-rw-  2 root     wheel      2,   2 Jul 31 10:10
/home/builder/null
$ echo foo >~/null
$

(In reply to Joshua Hudson from comment #2)
> It may have been; but I jumped back a few versions and found the
> original state was don't check permissions on the chroot directory.
Depending on how far back you went, but in the realm of "decade old"
you may have been reintroducing CVE-2009-2904
(https://bugzilla.redhat.com/show_bug.cgi?id=522141).

-- 
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=3715

--- Comment #4 from Damien Miller <djm at mindrot.org> ---
(In reply to Joshua Hudson from comment #2)
> It may have been; but I jumped back a few versions and found the
> original state was don't check permissions on the chroot directory.
> 
> I wonder if that was some other downstream distributor unbreaking
> somebody else with a very similar setup.
Redhat turned off the safely_chroot check entirely and gave themselves
this https://bugzilla.redhat.com/show_bug.cgi?id=CVE-2009-2904

-- 
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=3715

--- Comment #5 from Joshua Hudson <joshudson at gmail.com> ---
Well CVE-2009-2904 is exactly what would happen if you allowed
ChrootDirectory %h to a user-owned home directory on the root
filesystem.

Back in those days I would have called that a self-inflicted wound as I
would have thought the consequences of such a spectacular
misconfiguration obvious; but today I've seen too many system
administrators who just don't know what hard links are.

My plan is to mount the whole filesystem noexec *anyway* so there's
that. Even if you manage to link a suid binary into your own home
directory, you aren't running it.

Thus I write the check as the other guy described; I can reason out
that I could in theory allow uploading and running custom binaries and
there's no immediate breach of security; but my own decision is to not
trust there's not something else awful they can do (like say, kernel
exploit) and say no, no uploading binaries for you.

--

I tried the alternative of a custom shell to do the chroot process, and
even figured out how to get it to exec /usr/libexec/sftp-server from
outside the chroot while inside it, but it didn't actually work because
that binary isn't statically linked, and of course /lib will be
resolved inside the chroot. (glibc doesn't support static linkage for
anything other than emergency tools due to the way character sets
work...)

At this point I'm debating what's the least trouble for the maintenance
guy that doesn't involve getting access to hundreds of customer sites
all at once for the cutover.

-- 
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=3715

--- Comment #6 from Damien Miller <djm at mindrot.org> ---
One problem here is that statvfs() isn't specified to return ST_NOEXEC
and ST_NODEV https://pubs.opengroup.org/onlinepubs/9799919799/ and
indeed quite a few operating systems don't do it:

https://man.freebsd.org/cgi/man.cgi?query=statvfs
https://man.openbsd.org/statvfs.3
https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man3/statvfs.3.html
https://docs.oracle.com/cd/E86824_01/html/E54765/statvfs-2.html

I thought it might be better to use statfs() here, which is specified
to offer this, even though it isn't POSIX but that comes with its own
problems as the f_flags isn't really specified there at all, e.g. 

https://man.openbsd.org/statfs.2

Practically here, f_flags gets set to the mount flags as per
https://man.openbsd.org/mount.2 - most other operating systems document
this.

It's a pity that POSIX.1-2024  didn't improve statvfs(3)

-- 
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=3715

--- Comment #7 from Joshua Hudson <joshudson at gmail.com> ---
One problem here is that statvfs() isn't specified to return ST_NOEXEC
and ST_NODEV https://pubs.opengroup.org/onlinepubs/9799919799/ and
indeed quite a few operating systems don't do it:

Correct: in my imagination autoconf would notice the constants aren't
in the header file or the header file doesn't exist and it just reverts
to the target directory must indeed be owned by root.

If the constants exist in the header file the and the OS doesn't return
the flags; well it fails safe.

My own man page is telling me to not call statfs().

I suppose I could do a #elif HAVE_STATFS here (and rearrange the code a
bit).

-- 
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=3715

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #3828|0                           |1
        is obsolete|                            |

--- Comment #8 from Damien Miller <djm at mindrot.org> ---
Created attachment 3829
  --> https://bugzilla.mindrot.org/attachment.cgi?id=3829&action=edit
try statvfs and statfs

Something like this could work, though I think we'll need a few
opinions on whether this is safe at all.

Personally I'm not convinced - there's a fair bit of stuff in /etc that
can be referenced quite late in a program's execution and I don't trust
that libc has been sufficiently hardened against an adversarial /etc

-- 
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=3715

--- Comment #9 from Joshua Hudson <joshudson at gmail.com> ---
"there's a fair bit of stuff in /etc that can be referenced quite late
in a program's execution "

Hmmm; do you mean while the sshd process hasn't dropped privileges yet?

I went looking and found something very interesting:

1) setusercontext(..., LOGIN_SETUMASK) is almost certainly meant to be
applied outside the jail not inside it. However that's not a function
on my system so I could not detect the problem via testing.

2) In do_child we have this block:

if (s->authctxt->force_pwdchange) {
    do_setusercontext(pw, s-> authctxt->role);
    child_close_fds(ssh);
    do_pwdchange(s);
    exit(1);
}

There's no way that works correctly with any ChrootDirectory value
other than none, as do_pwdchange will be called inside the chroot jail
rather than outside of it. At least it has a safe failure mode. it
calls execl for passwd (no PATH search), which won't be there.

Any other failure modes must happen after uid has been set to the
user's value. While I'm pretty sure they can totally mess up the sftp
server with a hostile /etc the user's really only attacking themselves.

-- 
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=3715

--- Comment #10 from Joshua Hudson <joshudson at gmail.com> ---
On plotting an alternative pathway I discovered two very interesting
things.

1) sftp-server isn't very happy without an /etc/passwd; but
internal-sftp is fine with it.

2) When linking against musl libc, this chroot-ssh patch is definitely
safe so long as /proc isn't mounted in the chroot jail (which I have no
intention of doing). The number of things in musl libc that care about
late /etc or /usr/share access are so few that I can manually audit all
of them (result: all unreachable after chroot() is called).

-- 
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.